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