RFR - 8132734: java.util.jar.* changes to support multi-release jar files

Steve Drach steve.drach at oracle.com
Fri Jan 22 23:10:29 UTC 2016


Hi Alan, et. al.,

I’ve released a new webrev that addresses all the issues you raised.

http://cr.openjdk.java.net/~sdrach/8132734/webrev.03/index.html <http://cr.openjdk.java.net/~sdrach/8132734/webrev.03/index.html>

Specifically:

> For Release then I have to admit that I dislike _9 and wonder if other options were considered? javax.lang.model.SourceVersion uses the RELEASE_xx convention for example.

Changed to VERSION_9, i.e. Release.VERSION_9

> 
> Also I wonder about Release.ROOT and whether Release.UNVERSIONED was considered? In general the phrase "root entry" in the javadoc makes me think the root or top-most directory. An alternative that might be clearer is to say "unversioned entry" and define that term clearly in the class description.

Changed to BASE, i.e. Release.BASE

> 
> The javadoc for Release.RUNTIME looks like it will have a javadoc link to jdk.Version but that is a JDK-specific API. Could the text starting "The effective runtime ..." move to an @implNote?

Done

> 
> I assume @since will be added to Release before this is pushed.

Done

> 
> The new JarFile ctor doesn't seem to handle the case that version is null when multi release is forced. Also I assume it should specify @throws SecurityException.

Yes, both done and added more @throws clauses

> 
> Could the legacy JarFile ctor be changed to this(file, verify, mode, Release.ROOT) to avoid duplication?

Done

> 
> I don't have time to do a detailed pass over the updated tests but I wonder if SimpleHttpServer is really a candidate to put in the testlibrary tree. It looks like it is very specific to multi-release JARs and so I would expect to be co-located with those tests rather than being a hazard in the testlibrary tree.

Moved SimpleHttpServer into the test that uses it, MultiReleaseJarHttpProperties

> 
> A small comment is that it would be good to fix the really long lines before these changes are pushed. This will help future side-by-side reviews where it would be otherwise annoying to have to do horizontal scrolling to view diffs.

I think I fixed this




More information about the core-libs-dev mailing list