RFR: 8156499 Update jlink to support creating images with modules that are packaged as multi-release JARs

[Steve Drach]

>> I didn’t have it right ;-)  It turns out a JarFile stream of versioned entries was more interesting than I initially thought.  Here’s another webrev.  It’s not clear to me if I should include the change to JarFile in this changeset or if it should be in a stand alone changeset.  Advice appreciated.
>> 
>> webrev: http://cr.openjdk.java.net/~sdrach/8156499/webrev.02/ <http://cr.openjdk.java.net/~sdrach/8156499/webrev.02/>
> 
> 
> I think it’s time to enumerate several test cases of a multi-release Jar content, e.g. a MRJAR with a new concealed package in versioned entries but not in the base entry (I believe that’s allowed?? version 9 & 10 entries that makes sure versioned 10 entries are skipped.  It should also check the module-info.class with different requires and ensure that the image jlink created contains the module-info.class of the right version.  This would help the review of this change.

Okay, I can do that.

> 
> 
> I think ModularJarArchive is the right place to add the versioned entries support.

Okay.

>  JarArchive should probably be renamed to ZipArchive (that’s an existing code).

Just curious, why?  Note the processing of the module-info.class.  Isn’t that more appropriate for a JarArchive rather than a ZipArchive?  Seems to me we should just change the internal instances of ZipFile to JarFile and change the name zipFile to jarFile for consistency.

> 
> Version of JarFile
> 
> 221         jarFile = new JarFile(file.toFile(), true, ZipFile.OPEN_READ, JarFile.runtimeVersion());
> 
> JarFile::runtimeVersion is the version of the jlink tool.  You should not use the version of the jlink runtime.  Instead, this should use the version of java.base which will be the runtime version of the image being created.
> 
> ImageHelper::newArchive is one place where it creates JarArchive. You can find java.base module from the Configuration passed to ImageHelper constructor via Configuration.findModule(“java.base”) and from its module descriptor.  Jlink implementation is rather over engineering at the moment.  You will have to find if there are other places to pass the right versoin when creating ModularJarArchive.

As Alan noted, this is intended for phase 2 after he adds some apparently necessary code.

> 
> 
> 179         // a legal multi-release jar always has a base entry
> 
> I thought that a new class or a new concealed package can be added in a versioned entry in MRJAR.  If so, those cases will not be included in the finalNames.

If there is a class in a versioned directory that is not in the base directory, then that class must not be public or protected.  It’s not part of the public interface of the multi-release jar.  It exists purely because another class in the same versioned directory depends on it.  If we are creating a versionedStream for the version that the non-public class is in, it will be in finalNames, otherwise it won’t be.  I believe the code is correct here.

New concealed packages can be added in a versioned section of the jar file created by jar tool.  Should classes in concealed packages be added to finalNames or not?  Or stated differently, for jlink, should a versionedStream contain entries in concealed packages?

> 
> Have you considered adding a new method in JarFile to return the versioned entry name and/or the version?  If it’s the base version, it will return the same value as JarFile::getName.

There is a package-private method in JarFile that allows one to get the real name of a JarEntry.  It’s accessed externally by using SharedSecrets.  As far as i know it’s only used in two places.  The JEP-238 team decided not to make it a public method, although I think we could be persuaded to change our minds.

>  As we discussed the jdeps support for MRJAR offline, a tool would be interested in getting the base entry name, versioned entry name, or even version.

JarFile::getVersion exists.  And, as mentioned above, it possible to get a base name and a real name for a JarEntry.

> 
> Mandy