RFR: 8153654: Update jdeps to be multi-release jar aware

[Steve Drach]

The latest web revs.  Answers to questions in-line below:

http://cr.openjdk.java.net/~sdrach/8153654/webrev.10/ <http://cr.openjdk.java.net/~sdrach/8153654/webrev.10/>

http://cr.openjdk.java.net/~sdrach/8153654/webrev.jdk/ <http://cr.openjdk.java.net/~sdrach/8153654/webrev.jdk/>

>>> 
>>> This looks quite good.  JDK-8163798 and JDK-8164665 will define public APIs to get the versioned entries and real name which I think are useful for tools.  It’s fine to proceed with this change and update jdeps to use the public APIs when available.
>> 
>> The product team has decided not to move forward on those two issues for now, they will remain unresolved.
> 
> OK.  What about putting these helper methods in jdk.internal.jar package as jlink and jdeps both use them to would avoid duplicated code.

VersionedStream is now in jdk.internal.util.jar

>  
> 
>>> 
>>> This patch parse MR-JAR only if —-multi-release option is specified. It would also be useful to analyze all versioned entries for the default option (i.e. analyze direct dependencies from class files) that can be done in a separate RFE.
>>> 
>>> Comments to this patch:
>>> 
>>> ClassFileReader.java
>>> 
>>> 386                         if (Integer.parseInt(v) < 9) {
>>> 387                             String[] msg = {"err.multirelease.jar.malformed",
>>> 388                                     jarfile.getName(), rn
>>> 389                             };
>>> 390                             throw new MultiReleaseException(msg);
>>> 391                         }
>>> 
>>> To get here, I can think of the case when it’s not a MRJAR (so JarFile::stream returns all entries).
>> 
>> fixed 
>> 
>>> If it’s a MR-JAR, the JarFile will be opened with a valid version.  versionedStream should only return the proper versioned entries.  Maybe you should emit warning (add it to skippedEntries if this happens) or make it an assert.
>> 
>> I’m not sure what you mean.
> 
> My point is that JarFile::getEntry should not return such JarEntry - is that true?
> 
> In that case, how SharedSecrets.javaUtilJarAccess().getRealName(jarfile, e) would return META-INF/versions/$n  where n < 9?
> 
> This is not an expected error and so InternalError (or assert) is more appropriate than throwing MultiReleaseException.

I put an assert in.

> 
>> 
>>> 
>>> Can you add the following scenario in the new test and Bar and Gee are public and shows the result when -—multi-release 9 or 10 or base?
>>> p/Foo.class
>>> META-INF/versions/9/p/Foo.class
>>> META-INF/versions/9/q/Bar.class
>>> META-INF/versions/10/q/Bar.class
>>> META-INF/versions/10/q/Gee.class
>> 
>> One can not put new public classes in the versioned section, so that layout is not legal

If I make Bar and Gee non-public, I can build a jar with them in it.  See the second test I added in test.tools.jdep.MultireleaseJar

> 
> But such JAR file can be created.  What about adding a non-public class under:
>    META-INF/versions/9/q/Zee.class
>    META-INF/versions/10/q/Zee.class
> 
> Keeping public q/Bar.class is okay as JarFile::getName(“q/Bar.class”) should return null if not allowed for any Runtime.Version.

jar tool won’t validate a q.Bar as a public class.  

> 
>> 
>>> 
>>> JDK-8163798 would cover more scenarios in depth.  I’m okay to use the versionedStream you have and leave that to JDK-8163798.
>> 
>>> BTW the copyright header is missing in the new tests.
>> 
>> All the existing test input source files do not have the copyright header.  These little classes are just there to feed the test that does have the required copyright
> 
> I put the copyright headers in all source files, including tiny test classes.

Done.

> 
>> 
>>> 
>>> JdepsTask.java
>>> 401                             throw new BadArgs("err.multirelease.option.illegal", arg);
>>> 
>>> You can simply use err.invalid.arg.for.option which I think is simple and adequate.
>> 
>> That’s not an existing property, so I left it where it is with all the multi-release properties
> 
> err.invalid.arg.for.option is an existing property.

Okay.  Unfortunately we lose a better error message.

> 
>> 
>>> jdeps.properties: what about a shorter version:
>>> 
>>> --multi-release <version>         Specifies the version when processing
>>>                                  multi-release jar files. <version> can be
>>>> = 9 or base.
>> 
>> I did that but I think it’s more confusing
> 
> User guides, man page to include examples would help that.
> 
> Mandy
>