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.

> On Feb 1, 2016, at 12:29 PM, Steve Drach <steve.dr...@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.dr...@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
>>>>     */
> 

Reply via email to