RFR: JDK-8152622: tools/pack200/Pack200Props.java timed out

Kumar Srinivasan kumar.x.srinivasan at oracle.com
Wed Apr 6 13:09:41 UTC 2016


Thanks for the review. Please see inline comments.

> Creating a zip file from the classes in the jrt file system make 
> sense, and should be fast.
> A few comments on Utils.JrtToZip:
> - the URI creation in the run() method isn't reliable, could you use 
> this instead:
>   URI uri = URI.create("jar:" + outFile.toURI());
> - In the toZipFs method then I assume you should use 
> URI.create("jrt:/"), best to stay away from URL.
> - Is Files.createDirectories needed? The walk is specified to be depth 
> first so you'll also visit parent directories first.

Yes, zipfs does not allow me to create the file without creating the 
enclosing directory
first, so if I were to do this in visitFile, then presumably I would 
have to add a
check, to prevent duplicate creation going into the provider, only to find a
directory already exists and return, not sure if this is what you want, 
preVisitDirectory does this conveniently.

> - I'm also curious about the REPLACE_EXISTING as I assume that isn't 
> needed.

Oh! while I was developing in NB as a discrete/stand alone project, I 
was reusing the
same zip/jar output file,  will leave it as-is, no harm, right ?.

> A minor comment is that "root" rather than "p" would be nicer for the 
> root directory. Personally I would avoid the really long lines as it 
> makes future side-by-side reviews harder. Also throws Exception seems 
> too broad but it's test code so not a big deal.


> -Alan.

More information about the core-libs-dev mailing list