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

Alan Bateman Alan.Bateman at oracle.com
Mon Feb 15 12:30:43 UTC 2016


On 10/02/2016 01:04, Steve Drach wrote:
> Hi,
>
> Yet another webrev, 
> http://cr.openjdk.java.net/~sdrach/8132734/webrev.06/ 
> <http://cr.openjdk.java.net/%7Esdrach/8132734/webrev.06/>, with a 
> change to JarEntryIterator to fix a problem discovered by performance 
> tests — calling hasNext() twice as often as needed.  I also removed 
> the @since 9 tags from the methods entries() and stream(), and added 
> an additional sentence to the spec for each of those methods that 
> clarifies what a base entry is (actually is not).
>
I went through the latest webrev and it looks quite good.

A few comments on the javadoc:

"... partitioned by the major version of Java platform releases" - this 
might be better as "... partitioned by the major version of the Java 
platform".

In JarFile.Release then it uses the phrase "top-most (base) directory". 
I thought we had purged "top-*" from the javadoc in previous iterations 
because it hints of classes or resources in the top most directory 
(which isn't the case with classes in a named package).

"... will not be accessible by this JarFile" hints of access control or 
even security manager. Would it clearer to re-word this to something 
like "will not be located by methods such as getEntry" ?

"returned depends whether" -> "returned depends on whether".

In the javadoc for entries() and stream() then it mentions "the 
constructor" many times. I would be tempted to replace many of these - 
for example "all entries are returned, regardless of the constructor" 
might be better as "all entries of returned, regards of how the JarFile 
is created".

A couple of nits on the implementation:

vze = JarFile.super.getEntry(META_INF_VERSIONS + i-- + sname);
- it would be more readable if you move the decrement to its own line. 
Also I assume that JarFile is not needed here.

L942-943 looks messy too, I assume that can be cleaned up.

JarFileFactory - "earl" will confuse readers, needs a comment or a 
better name.

I think this is all that I have for now.

-Alan.



More information about the core-libs-dev mailing list