[modules-dev] Review request for 6581297

[Stanley M. Ho]

Andreas Sterbenz wrote:
> Some comments below:
> 
>   . please explain why it is a good idea to change the imports from 2 
> lines to 11 lines or undo that change

This makes it easier to figure out the exact type dependencies. The 
number of import statements will be larger, but this has helped me many 
times to catch unexpected dependencies during development.

>   . the tmp file is not deleted anywhere. There seems to be a call to 
> tfname.deleteOnExit() missing

The tmp file is created under tmpDir which is already registered for 
deleteOnExit(), so it will be deleted. I added the call anyway to avoid 
confusion.

>   . it would seem more reliable if the check at line 153 was for (tfname 
> == null) rather than duplicate the file name checking logic

Fixed.

>   . which raises an existing issue: the run() method should first set all 
> the member variables to null (superpackageName, fname, ...). Otherwise, a 
> 2nd call to run() may not behave correctly / accept invalid arguments.

Updated as suggested.

>   . shouldn't the required name be .jam.pack.gz rather than .pack.gz?

Yes, it should. Fixed.

>   . if we are trying to make assumption based on the file extension, then 
> that should always be checked. Otherwise someone could assume that 
> specifying foo.jam.pack as a name will get them a (non-gzipped) pack'ed 
> JAM file, which it does not. In other words, verify that the name of the 
> JAM file ends with either .jam, .jar, or .jam.pack.gz and give an error 
> otherwise.

This is an existing issue. Added checks as suggested.

New webrev:
http://javaweb.sfbay.sun.com/~stanleyh/webrev/jsr-277/6581297-4/

- Stanley