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
>>> */
