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 <[email protected]> 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
