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

[Mandy Chung]

> On Aug 30, 2016, at 7:58 PM, Steve Drach <steve.drach at oracle.com> wrote:
> 
>> 
>> 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.  

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

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

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.

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

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

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