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