RFR 8168925: MODULES property should be topologically ordered and space-separated list

Mandy Chung mandy.chung at oracle.com
Fri Dec 9 20:48:38 UTC 2016


> On Dec 9, 2016, at 12:49 AM, Sundararajan Athijegannathan <sundararajan.athijegannathan at oracle.com> wrote:
> 
> Please review http://cr.openjdk.java.net/~sundar/8168925/webrev.01/index.html for https://bugs.openjdk.java.net/browse/JDK-8168925
> 

Is the order of Plugin.Category enums significant?  You moved COMPRESSOR down - is it necessary?

Can you look at DefaultImageBuilder::releaseProperties which I think this should be moved to ReleaseInfoPlugin?  The content of `release` should be written when the new entry "/java.base/release” is added to the resource pool.  DefaultImageBuilder does not need to add any more properties to this `release` file as all properties in `release` are known once the graph is resolved.

 550                 String[] arr = mods.split(" ");
I think you will need to remove the double-quotes before doing the split.

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ResourcePoolManager.java
- is this change needed?

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ReleaseInfoPlugin.java
 137         release.put("MODULES", "\"" + new ModuleSorter(in.moduleView())
 138                 .sorted().map(ResourcePoolModule::name).collect(Collectors.joining(" "))
 139                 + "\"”);

Since there is always at least one module, you can use Collectors.joining(“ “, "\””, "\””) instead of the prepending and appending.

test/tools/jlink/CustomPluginTest.java
   It’s one option to disable the releaseinfo plugin.  An alternative
   way to fix this test is [2].

test/tools/jlink/ModuleNamesOrderTest.java

  57         String moduleName = "bug8168925”;

Is it the output dir name?  This is not a module name.  @bug has the bug id.  I suggest to rename this to “image” or some other name.

One suggestion to the validation is to remove the double quotes from MODULES property value and split it into an array.  That may be a better way to verify the dependences.


> PS. Mandy Chung wrote ModuleSorter for another fix (yet to be pushed).  I'm using it for this fix after discussion with her (private email).

ModuleSorter is part of the patch for JDK-8169925[1].  I will take it out if Sundar pushes this changeset before JDK-8169925.

Mandy
[1] 
http://mail.openjdk.java.net/pipermail/jigsaw-dev/2016-December/010416.html
[2] http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8169925/webrev.00/jdk/test/tools/jlink/CustomPluginTest.java.udiff.html


More information about the jigsaw-dev mailing list