I believe the answer should be based on your viewpoint of what "is" a JAR.
Do you consider the JAR to be a dumb ZIP container or an artifact of the
Java runtime? If it's the former, then return everything because the
version folder is meaningless in this perspective. Otherwise, treat the JAR
according to how Java runtime would load it, which is the versioned
entries. Whatever your answer is, that should be the behavior for
entries()/stream().
Those who don't like the behavior should be given a second set of methods
to customize the return.
Cheers,
Paul
On Mon, Feb 1, 2016 at 2: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
> >>> */
>
>
