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

[Steve Drach]

> I think the following is a reasonable solution.
> 
> If the JarFile is constructed with any of the 5 constructors that do not contain a Release argument, then entries()/stream() returns the set of all entries in the jar file including those under the META-INF/versions/directory.  The base entries are “raw”, they are not aliases for versioned entries.
> 
> If the JarFile is constructed with the 1 constructor that does include a Release argument, then entries()/stream() returns the set of appropriately versioned entries that would result from invoking getEntry()/getJarEntry() with the name of each base entry.  The entries in the tree rooted at the directory META-INF/versions/ are not returned.

I created a new webrev, http://cr.openjdk.java.net/~sdrach/8132734/webrev.05/ <http://cr.openjdk.java.net/~sdrach/8132734/webrev.05/>, that implements what I outlined above.  In particular I enhanced the JarEntryIterator class and I added additional commentary to the entries() and stream() methods.  I also added a new test, MultiReleaseJarIterators, to test entries() and stream().

 

> 
>> On Feb 1, 2016, at 12:29 PM, Steve Drach <steve.drach at oracle.com> wrote:
>> 
>> I’m sorry, I didn’t look at the code close enough before I started talking ;-)  Right now entries()/stream() returns all entries and if the JarFile is constructed with a Release object != Release.BASE, the base entries that are returned are the versioned entries.  I think this behavior is a bit confusing and we should just return all entries without regard to versioning.  Then create the two new methods for specific versioned entries.
>> 
>>> On Feb 1, 2016, at 12:18 PM, Steve Drach <steve.drach at oracle.com> wrote:
>>> 
>>>>>>> Alan’s point is that traversing using entries()/stream() always returns the versioned entries (if any) rather than all entries, thus in a sense filters.
>>>>>>> 
>>>>>>> My assumption was the traversal should by default be consistent with a calls to getEntry, thus:
>>>>>>> 
>>>>>>> jarFile.stream().forEach(e ->  {
>>>>>>>  JarEntry je = jarFile.getJarEntry(e.getName());
>>>>>>>  assert e.equals(je);
>>>>>>> });
>>>>>>> 
>>>>>>> There might need to be another stream method that returns all entries.
>>>>>>> 
>>>>>> Right, I'm mostly just wondering if entries()/streams() should override the entries in the stream with versioned entries and filter out the META-INF/versions/ tree.
>>>>> I don’t think so.  That kind of behavior might be difficult to understand.  Returning all the entries provides some flexibility.  One can write code like this:
>>>>> 
>>>>> jarfile.stream().map(JarEntry::getName).filter(s ->  !s.startsWith(“META-INF”)).map(JarFile::getJarEntry).etc
>>>>> 
>>>>> to get the versioned results for any version you specify when constructing the JarFile.
>>>> 
>>>> The current specification treats those class files under meta-inf/releases like
>>>> kind of "metadata" of those base entries. Ideally those files should not even
>>>> be individual "files", but part of their corresponding entries. The consumer of
>>>> the MR-Jar should not need to be aware of these version-ed entries at all to use
>>>> this MR-jar file to load classes/resources. From this point of view, these entries
>>>> probably should be "invisible" from entries()/stream(), when the jar file is opened
>>>> with "version-enabled". And all returned entries should have all their "data"
>>>> (size, csize, timestamps, crc ...) pointed to the corresponding version-ed entries,
>>>> withe the only exception is the "name".
>>>> 
>>>> On the other hand it might be desired to keep JarFile.entries()/stream() asis to
>>>> match their "zip file" perspective, to return "all" raw entries. Then it might also
>>>> be desired to have an alternative "versioned streamVersion()" …
>>> 
>>> It seems to that we have two reasonable alternatives: (1) return all entries, and (2) return all entries except those under the “META-INF/versions/“ directory and for any entries returned, return their versioned equivalent if it exists.  If we choose alternative 2, we can still get alternative 1 by asking for JarFile.super.entries() and JarFile.super.stream().
>>> 
>>> Or we can do it both ways, leaving entries()/stream() as is and adding two new methods, versionedEntries() and versionedStream().
>>> 
>>>> 
>>>> something like
>>>> 
>>>> public Stream<JarEntry> stream(Release r); ?
>>> 
>>> We should not parametrize the methods with a Release, because what does it mean if we construct the JarFile with one Release but specify a different Release for the stream argument.  Parameterizing methods with a Release object feels like we’re starting to slide down a slippery slope.
>>> 
>>> I think adding the two new methods is the “right” solution, but I’d like some consensus here.
>>> 
>>>> 
>>>> -sherman
>>>> 
>>>> 
>>>> 
>>>> 
>>>>>> If I've gone to trouble of specifying the a Release then it seems the right thing to do. On the other hand, it comes at a cost and there will be use-cases like "get the names of all entries" that would be more efficient to just build on the current entries()/stream(). I'm loath to suggest this might need a new method but it might be one of the options to consider here. Minimally there is a javadoc to specify on how these methods behave when the JAR is multi-release and opened by specifying a release.
>>>>> How’s this?
>>>>> 
>>>>> diff -r 68867430065b src/java.base/share/classes/java/util/jar/JarFile.java
>>>>> --- a/src/java.base/share/classes/java/util/jar/JarFile.java	Fri Jan 29 12:34:44 2016 -0800
>>>>> +++ b/src/java.base/share/classes/java/util/jar/JarFile.java	Mon Feb 01 09:48:05 2016 -0800
>>>>> @@ -576,9 +576,11 @@
>>>>>   }
>>>>> 
>>>>>   /**
>>>>> -     * Returns an enumeration of the jar file entries.
>>>>> +     * Returns an enumeration of all the jar file entries.  Constructing this
>>>>> +     * JarFile with the {@link JarFile#JarFile(File, boolean, int, Release)}
>>>>> +     * constructor does not modify the behavior of this method.
>>>>>    *
>>>>> -     * @return an enumeration of the jar file entries
>>>>> +     * @return an enumeration of the all jar file entries
>>>>>    * @throws IllegalStateException
>>>>>    *         may be thrown if the jar file has been closed
>>>>>    */
>>>>> @@ -587,11 +589,13 @@
>>>>>   }
>>>>> 
>>>>>   /**
>>>>> -     * Returns an ordered {@code Stream} over the jar file entries.
>>>>> +     * Returns an ordered {@code Stream} over all the jar file entries.
>>>>>    * Entries appear in the {@code Stream} in the order they appear in
>>>>> -     * the central directory of the jar file.
>>>>> +     * the central directory of the jar file.  Constructing this
>>>>> +     * JarFile with the {@link JarFile#JarFile(File, boolean, int, Release)}
>>>>> +     * constructor does not modify the behavior of this method.
>>>>>    *
>>>>> -     * @return an ordered {@code Stream} of entries in this jar file
>>>>> +     * @return an ordered {@code Stream} of all entries in this jar file
>>>>>    * @throws IllegalStateException if the jar file has been closed
>>>>>    * @since 1.8
>>>>>    */
>> 
>