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

Alan Bateman Alan.Bateman at oracle.com
Thu Jan 21 16:12:10 UTC 2016


On 20/01/2016 17:56, Steve Drach wrote:
> Hi,
>
> This is a repeat of the RFR I sent last Wed (Jan 13).
>
> :
>
> Webrev: http://cr.openjdk.java.net/~sdrach/8132734/webrev.02/ <http://cr.openjdk.java.net/~sdrach/8132734/webrev.02/>
>
Overall I think the API looks much better.

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.

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.

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?

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

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.

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

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.

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.

-Alan.



More information about the core-libs-dev mailing list