640986 - abcdump should be better factored and dump more information about the abc

Status: RESOLVED FIXED

(Tamarin Graveyard :: Tools, enhancement)

Description

[Christopher A. Brichford]

Attachment: new library and abcdump

Most of the code in abcdump should be factored into a library and abc dump should print out all the information found in an abc.

Comment 1

[Christopher A. Brichford]

Most the credit for the work in the patch goes to alexmac.

Comment 2

[Christopher A. Brichford]

To try this tool out, apply the attached patch and build avmshel 64 bit release on mac.
You'll also need to have an asc.jar in the utils directory.  Then you should be able to go into utils/abc and type make.

Comment 3

[Edwin Smith]

Comment on attachment 518731 [details] [diff] [review]
new library and abcdump

Overall I like this tool very much
 - printing all the contents of abc is great
 - dot output is killer (we should add gml too)

minor: I'm not in love with the deep source code directory layout, especially
the 'com' -- src/adobe/whatever is fine.  lower case 'abc' package name
would be better too.

SR- for a logistical reasons only; really, its a nice tool.

 - new files need copyright headers, and preferrably modelines.

 - it adds a new abcdump, but doesn't remove the old one.  having two
   is a problem because it fragments our work; lets use one, and make
   improvements to the one that we use.  Does the old one have any
   features missing from the new one?

Other things I'd like to see done soon:

- the Makefile is hardcoded to mac.  Could we add a 'make utils' target 
to the toplevel makefile generated by configure.py?  then the utils/abc
makefile can get config variables from the make environment.  There are
existing build steps that need to know where asc.jar and avm are, so lets
use a single scheme.

- Documentation, please!  At least a skeliton that is easy to find and
make incremental improvements to.  Expanding the help output would be a
minimal first step.

Comment 4

[Christopher A. Brichford]

Thanks for the feedback.  R- was expected on the first patch.

(In reply to comment #3)
> Comment on attachment 518731 [details] [diff] [review]
> new library and abcdump
> 
> Overall I like this tool very much
>  - printing all the contents of abc is great
>  - dot output is killer (we should add gml too)
> 
> minor: I'm not in love with the deep source code directory layout, especially
> the 'com' -- src/adobe/whatever is fine.  lower case 'abc' package name
> would be better too.
> 

The deep source code layout is for flash builder compatibility.  For a time alexmac was able to edit and build this program in builder.  By the time he gave it to me that was broken, but maybe we should fix that.  Given the volume of code it would be nice to be able to use an IDE to edit this code.  If we can't do that in the tamarin tree, I can flatten the source layout.  Suggestions on what the source layout should be are welcome.


> SR- for a logistical reasons only; really, its a nice tool.
> 
>  - new files need copyright headers, and preferrably modelines.
> 

I assume I should just steal the copyright header from the old abcdump.  Ditto for the modelines.

>  - it adds a new abcdump, but doesn't remove the old one.  having two
>    is a problem because it fragments our work; lets use one, and make
>    improvements to the one that we use.  Does the old one have any
>    features missing from the new one?
> 

The old one prints out something approximating a class definition.  The new one does not.  The new one prints out all the pools ( the method bodies are thought of as a method body pool ).  A switch to trace out the old style format can be added if needed.  Is the old abcdump format a requirement?

> Other things I'd like to see done soon:
> 
> - the Makefile is hardcoded to mac.  Could we add a 'make utils' target 
> to the toplevel makefile generated by configure.py?  then the utils/abc
> makefile can get config variables from the make environment.  There are
> existing build steps that need to know where asc.jar and avm are, so lets
> use a single scheme.
> 

Fair enough.  I'll attempt to hook into the existing toplevel makefile.

> - Documentation, please!  At least a skeliton that is easy to find and
> make incremental improvements to.  Expanding the help output would be a
> minimal first step.

Fair enough. I'll add some meat to the help message.  I might also add a txt file to the docs directory if you think that make sense.

Comment 5

[Alex Macdonald]

(In reply to comment #4)
>  Is the old abcdump format a requirement?

The code is factored to allow you to write different backends, I chose the current style as it closely matches abcformat.txt which I thought was quite a cute feature.

It would be cool if someone wrote a small disassembler backend for this so it can print the abc out as compilable AS3 :)

Comment 6

[Edwin Smith]

> The deep source code layout is for flash builder compatibility.  For a time
> alexmac was able to edit and build this program in builder.  By the time he
> gave it to me that was broken, but maybe we should fix that.  Given the volume
> of code it would be nice to be able to use an IDE to edit this code.  If we
> can't do that in the tamarin tree, I can flatten the source layout. 
> Suggestions on what the source layout should be are welcome.

I suspected something like that.  An IDE friendly layout would be okay if IDE-editing were re-enabled soonish.  

More important: sketch a scheme we can live with and that other tools
can align with.  perhaps: packages-as-module names, and only use nesting
where it implies module containment.  then, directory layout to match
modules.  example modules would be 'abcdump', 'abc', 'swf', 'yourtool'.
(tweaks/mods welcome:  should the toplevel package be 'utils' to match
the utils folder?)

Less important: drop unnecessary nesting levels, like 'com'.  of course
adobe is a company, and must we really pay homage to java's (silly imho)
DNS-inspired module names?

> I assume I should just steal the copyright header from the old abcdump.  Ditto
> for the modelines.

Should be fine.  Maybe update the copyright date.

> The old one prints out something approximating a class definition.  The new one
> does not.  The new one prints out all the pools ( the method bodies are thought
> of as a method body pool ).  A switch to trace out the old style format can be
> added if needed.  Is the old abcdump format a requirement?

Not a strict one, no.  It did, however, inspire abcasm syntax, and we have
a desire to support abcasm/abcdump round-trip-editing.  (abcasm is limited
right now, but ideally at least abcasm->abcdump->abcasm should work, even
if abcdump->abcasm->abcdump is a long way off).

Suggestions?  I don't want to add a lot of work before this tool can land
but at the same time I want to avoid overlapping functionality if practical.

> Fair enough.  I'll attempt to hook into the existing toplevel makefile.

Jim Sudduth or Chris P might be able to help.

> Fair enough. I'll add some meat to the help message.  I might also add a txt
> file to the docs directory if you think that make sense.

Sure, and the help message could refer to it.  Again, not asking for a lot
of new work, just some bones we can add meat to over time.

Comment 7

[Christopher A. Brichford]

(In reply to comment #6)
> > The deep source code layout is for flash builder compatibility.  For a time
> > alexmac was able to edit and build this program in builder.  By the time he
> > gave it to me that was broken, but maybe we should fix that.  Given the volume
> > of code it would be nice to be able to use an IDE to edit this code.  If we
> > can't do that in the tamarin tree, I can flatten the source layout. 
> > Suggestions on what the source layout should be are welcome.
> 
> I suspected something like that.  An IDE friendly layout would be okay if
> IDE-editing were re-enabled soonish.  
> 
I fix the ide editing in the next patch.

> More important: sketch a scheme we can live with and that other tools
> can align with.  perhaps: packages-as-module names, and only use nesting
> where it implies module containment.  then, directory layout to match
> modules.  example modules would be 'abcdump', 'abc', 'swf', 'yourtool'.
> (tweaks/mods welcome:  should the toplevel package be 'utils' to match
> the utils folder?)

I think the source path entry should start at utils.  So no, utils would not be in the package name.

If we want to factor tools such that they each have their own directory with as3, python, etc I'd suggest we put most of this code in utils/lib/as3/... and tool specific as3 in utils/yourtool/as3.

> 
> Less important: drop unnecessary nesting levels, like 'com'.  of course
> adobe is a company, and must we really pay homage to java's (silly imho)
> DNS-inspired module names?

I'll drop com.adobe.

> 
> > I assume I should just steal the copyright header from the old abcdump.  Ditto
> > for the modelines.
> 
> Should be fine.  Maybe update the copyright date.
> 
> > The old one prints out something approximating a class definition.  The new one
> > does not.  The new one prints out all the pools ( the method bodies are thought
> > of as a method body pool ).  A switch to trace out the old style format can be
> > added if needed.  Is the old abcdump format a requirement?
> 
> Not a strict one, no.  It did, however, inspire abcasm syntax, and we have
> a desire to support abcasm/abcdump round-trip-editing.  (abcasm is limited
> right now, but ideally at least abcasm->abcdump->abcasm should work, even
> if abcdump->abcasm->abcdump is a long way off).
> 

I'll look into adding a backend that writes out something that abcasm can consume.

Comment 8

[Edwin Smith]

> > (abcasm is limited
> > right now, but ideally at least abcasm->abcdump->abcasm should work, even
> > if abcdump->abcasm->abcdump is a long way off).
> > 
> 
> I'll look into adding a backend that writes out something that abcasm can
> consume.

By the way, I didn't mean to imply that abcasm->abcdump->abcasm *does* work -- odds are its close, but not quite working.  but moving in that direction is
a good thing to do, thanks.

we can break this into multiple bugs/submissions if it helps; the idea is to
avoid long term overlapping functionality but its not a big deal for a little while.  (READMES can explain).

Comment 9

[Christopher A. Brichford]

(In reply to comment #8)
> 
> By the way, I didn't mean to imply that abcasm->abcdump->abcasm *does* work --
> odds are its close, but not quite working.  but moving in that direction is
> a good thing to do, thanks.
> 
> we can break this into multiple bugs/submissions if it helps; the idea is to
> avoid long term overlapping functionality but its not a big deal for a little
> while.  (READMES can explain).

I'm working on this in my own time, so there is a good chance its not going to get touched again until I or somebody else need it do something it does not currently do.  So I think I should clean up some of this stuff before checking in, lest the cleanups never get done.  If you can make this thing someone's day job, I'm happy to hand this off and then we can clean this stuff up over time.  It should not take me too many train rides to fix this up.

Comment 10

[Alex Macdonald]

Latest patch includes the following new features from Chris:
- filtering scripts with regular expressions
- supports SWCs
- implementation of the "abcdump classic" frontend on top of the new codebase
- abcasm roundtripping is almost correct (string escaping issues prevent it being correct (anything else missing Chris?)
- lots more small improvements to everything :)

Based on some of the previous feedback I've also stripped the adobe prefix from the package name, run it through 'fixtabs' and I've added the beginnings of a manifest.mk file to the utils directory.  I'll need help from a VM team person who understands the manifest files to get this hooked up for real.

to build you simply:

cd tamarin-redux/utils
curdir=/path/to/tamarin-redux/utils/ ASC=/path/to/asc.jar make -f manifest.mk utils

but with some integration help this should happen as part of the default build, or via an --enable-utils directive to the configure.py script.

Comment 11

[Alex Macdonald]

Attachment: new abcdump

Comment 12

[Christopher A. Brichford]

(In reply to Alex Macdonald from comment #10)
> - abcasm roundtripping is almost correct (string escaping issues prevent it
> being correct (anything else missing Chris?)

I don't know of anything else that is missing that is needed to round trip an abc file produced by abcasm.  Round tripping arbitrary abc is a different story.  abcasm needs support for classes, arbitrary qualified names, specifying pool order, etc.  Once abcasm has such support it should be easy to add the corresponding support in abcdump.

Comment 13

[Alex Macdonald]

Attachment: new abcdump

removed some unneeded files

Comment 14

[Alex Macdonald]

Ok, the patch is now ready, it doesn't have any known regressions vs the old abcdump and I think we'd like to propose it to be submitted. Hooking it up to the tamarin build scripts would be nice, but it wasn't before so hopefully thats not a blocker.

Comment 15

[Edwin Smith]

I'd like to land this as "abcdis" alongside the old abcdump so we can have a graceful migration.  Any strong feelings for or against?

Comment 16

[Alex Macdonald]

Do you mean land the new frontend as abcdis and name the old frontend (built on the new code) abcdump? that sounds fine to me

Comment 17

[Tamarin Bot]

changeset: 7225:0566c0b4e7ae
user:      Alex Macdonald <alexmac>
summary:   Bug 640986 - "abcdump should be better factored and dump more information about the abc" [r=alexmac]

http://hg.mozilla.org/tamarin-redux/rev/0566c0b4e7ae