RFR: JDK-8172432,jar cleanup/update for module and mrm jar

[Paul Sandoz]

Hi,

A nice cleanup.

At this time of year: usual review comment to update the years in the license.


Main
—

 987                 jentries.stream().forEach( je -> addPackageIfNamed(packages, je));

If you wish you can remove the “.stream()” and go straight to “.forEach(…)” on the Set.


1870     private static boolean isModuleInfoEntry(String name) {
1871         // root or versioned module-info.class
1872         return name.endsWith(MODULE_INFO) &&
1873             (name.length() == MODULE_INFO.length() || name.startsWith(VERSIONS_DIR));

Is this sufficient? For the versioned case do we need to check it is VERSIONS_DIR/{n}/MODULE_INFO ?


Validator
—

  56     private final int vdlen = VERSIONS_DIR.length();

Can be static.


ConcealedPackage
—

Should the class be renamed?

Paul.



> On 9 Jan 2017, at 10:21, Xueming Shen <xueming.shen at oracle.com> wrote:
> 
> Hi,
> 
> Please review the following proposed changes for jar tool
> 
> issue: https://bugs.openjdk.java.net/browse/JDK-8172432
> webrev: http://cr.openjdk.java.net/~sherman/8172432/webrev
>              http://cr.openjdk.java.net/~sherman/8172432/webrev_top/
> 
> (1) move the "packages" and "jarEntries" from "global" to "local", and only collect
> the  info when needed (if there is a module-info and we indeed need these info
> to update the module-info.class). The goal is to avoid/reduce any possible performance
> regression/impact to those"non-module" jar file creation and update operations.
> The proposed implementation now collects such info after "expand()" for "creation"
> if there is "module-info.class". For "update" it is done during the "updating"
> 
> (2) consolidate all "validation" related implementation into Validator.java. The
> "concealedPkgs" now is generated from the base 'module-info.class" from the
> resulting temporary jar file directly, instead of the "module-info.class" binary during
> the creation/update. Again, the reasoning is that the "validation" is only needed
> for the mr module jar (for now), it is not needed for a "normal" module jar file.
> A clear separation helps reducing the performance impact and improving the
> maintainability.
> 
> (3) remove the "checkModuleInfos" logic/implementation, which incorrectly enforces
> the restriction such as
>     a) there must always be, at least, a root module-info, when there is an entry that
>         has a name ended up with "module-info.class" in the jar file.
>     b) module-info.class file can only be at root or a versioned folder. (why I can't jar
>         a foo.jar that has a entry module-info.class at an arbitrary place?)
> 
> (4) consolidate and share the "updateModuleInfo()" for both creation and update.
> 
> (5) no longer always copy the "mainClass" and "version" attributes from the root
> module-info.class into the corresponding versioned module-info.class "silently"
> (in extendedInfoBytes()). Instead, the difference between the root module-info.class
> and its versioned copy is checked, jar fails if there is inconsistence.
> 
> (6) clean up the Entry class and the expand() implementation. It appears the Entry
> class might not be absolutely necessary, but I keep it as is for now.
> 
> (7) build the jar with -XDstringConcat=inline flag.
> 
> thanks,
> Sherman