8168925: MODULES property should be topologically ordered and space-separated list
Reviewed-by: mchung, jlaskey
--- a/jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/builder/DefaultImageBuilder.java Sun Dec 11 17:39:27 2016 -0800
+++ b/jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/builder/DefaultImageBuilder.java Mon Dec 12 11:31:39 2016 +0530
@@ -141,37 +141,41 @@
Files.createDirectories(mdir);
}
- private void storeFiles(Set<String> modules, Properties release) throws IOException {
- if (release != null) {
- addModules(release, modules);
- File r = new File(root.toFile(), "release");
- try (FileOutputStream fo = new FileOutputStream(r)) {
- release.store(fo, null);
+ private void storeRelease(ResourcePool pool) throws IOException {
+ Properties props = new Properties();
+ Optional<ResourcePoolEntry> release = pool.findEntry("/java.base/release");
+ if (release.isPresent()) {
+ try (InputStream is = release.get().content()) {
+ props.load(is);
}
}
- }
-
- private void addModules(Properties props, Set<String> modules) throws IOException {
- StringBuilder builder = new StringBuilder();
- int i = 0;
- for (String m : modules) {
- builder.append(m);
- if (i < modules.size() - 1) {
- builder.append(",");
- }
- i++;
+ File r = new File(root.toFile(), "release");
+ try (FileOutputStream fo = new FileOutputStream(r)) {
+ props.store(fo, null);
}
- props.setProperty("MODULES", quote(builder.toString()));
}
@Override
public void storeFiles(ResourcePool files) {
try {
- // populate release properties up-front. targetOsName
- // field is assigned from there and used elsewhere.
- Properties release = releaseProperties(files);
+ // populate targetOsName field up-front because it's used elsewhere.
+ Optional<ResourcePoolModule> javaBase = files.moduleView().findModule("java.base");
+ javaBase.ifPresent(mod -> {
+ // fill release information available from transformed "java.base" module!
+ ModuleDescriptor desc = mod.descriptor();
+ desc.osName().ifPresent(s -> {
+ this.targetOsName = s;
+ });
+ });
+
+ if (this.targetOsName == null) {
+ throw new PluginException("TargetPlatform attribute is missing for java.base module");
+ }
+
+ // store 'release' file
+ storeRelease(files);
+
Path bin = root.resolve("bin");
-
// check any duplicated resource files
Map<Path, Set<String>> duplicates = new HashMap<>();
files.entries()
@@ -209,8 +213,6 @@
}
});
- storeFiles(modules, release);
-
if (root.getFileSystem().supportedFileAttributeViews()
.contains("posix")) {
// launchers in the bin directory need execute permission.
@@ -241,52 +243,6 @@
}
}
- // Parse version string and return a string that includes only version part
- // leaving "pre", "build" information. See also: java.lang.Runtime.Version.
- private static String parseVersion(String str) {
- return Runtime.Version.parse(str).
- version().
- stream().
- map(Object::toString).
- collect(joining("."));
- }
-
- private static String quote(String str) {
- return "\"" + str + "\"";
- }
-
- private Properties releaseProperties(ResourcePool pool) throws IOException {
- Properties props = new Properties();
- Optional<ResourcePoolModule> javaBase = pool.moduleView().findModule("java.base");
- javaBase.ifPresent(mod -> {
- // fill release information available from transformed "java.base" module!
- ModuleDescriptor desc = mod.descriptor();
- desc.osName().ifPresent(s -> {
- props.setProperty("OS_NAME", quote(s));
- this.targetOsName = s;
- });
- desc.osVersion().ifPresent(s -> props.setProperty("OS_VERSION", quote(s)));
- desc.osArch().ifPresent(s -> props.setProperty("OS_ARCH", quote(s)));
- desc.version().ifPresent(s -> props.setProperty("JAVA_VERSION",
- quote(parseVersion(s.toString()))));
- desc.version().ifPresent(s -> props.setProperty("JAVA_FULL_VERSION",
- quote(s.toString())));
- });
-
- if (this.targetOsName == null) {
- throw new PluginException("TargetPlatform attribute is missing for java.base module");
- }
-
- Optional<ResourcePoolEntry> release = pool.findEntry("/java.base/release");
- if (release.isPresent()) {
- try (InputStream is = release.get().content()) {
- props.load(is);
- }
- }
-
- return props;
- }
-
/**
* Generates launcher scripts.
*
@@ -561,7 +517,7 @@
}
String mods = release.getProperty("MODULES");
if (mods != null) {
- String[] arr = mods.split(",");
+ String[] arr = mods.substring(1, mods.length() - 1).split(" ");
for (String m : arr) {
modules.add(m.trim());
}
--- a/jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImagePluginConfiguration.java Sun Dec 11 17:39:27 2016 -0800
+++ b/jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImagePluginConfiguration.java Mon Dec 12 11:31:39 2016 +0530
@@ -43,6 +43,9 @@
*/
public final class ImagePluginConfiguration {
+ // Order in which plugins are applied. Note that COMPRESSOR type plugins should come
+ // after any plugin that reads .class resources and operate on binary data.
+ // Plugin.Category enum element order matches this order for ease of read.
private static final List<Category> CATEGORIES_ORDER = new ArrayList<>();
static {
@@ -50,8 +53,8 @@
CATEGORIES_ORDER.add(Category.TRANSFORMER);
CATEGORIES_ORDER.add(Category.MODULEINFO_TRANSFORMER);
CATEGORIES_ORDER.add(Category.SORTER);
+ CATEGORIES_ORDER.add(Category.METAINFO_ADDER);
CATEGORIES_ORDER.add(Category.COMPRESSOR);
- CATEGORIES_ORDER.add(Category.METAINFO_ADDER);
CATEGORIES_ORDER.add(Category.VERIFIER);
CATEGORIES_ORDER.add(Category.PROCESSOR);
CATEGORIES_ORDER.add(Category.PACKAGER);
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ModuleSorter.java Mon Dec 12 11:31:39 2016 +0530
@@ -0,0 +1,110 @@
+/*
+ * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation. Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package jdk.tools.jlink.internal;
+
+import jdk.tools.jlink.plugin.PluginException;
+import jdk.tools.jlink.plugin.ResourcePoolModule;
+import jdk.tools.jlink.plugin.ResourcePoolModuleView;
+
+import java.lang.module.ModuleDescriptor;
+import java.util.Deque;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.LinkedList;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Stream;
+
+/**
+ * Helper class to sort modules in topological order
+ */
+public final class ModuleSorter {
+ private final Deque<ResourcePoolModule> nodes = new LinkedList<>();
+ private final Map<String, Set<ResourcePoolModule>> edges = new HashMap<>();
+ private final Deque<ResourcePoolModule> result = new LinkedList<>();
+
+ private final ResourcePoolModuleView moduleView;
+
+ public ModuleSorter(ResourcePoolModuleView moduleView) {
+ this.moduleView = moduleView;
+
+ moduleView.modules().forEach(this::addModule);
+ }
+
+ private ModuleSorter addModule(ResourcePoolModule module) {
+ ModuleDescriptor descriptor = module.descriptor();
+ addNode(module);
+ descriptor.requires().stream()
+ .forEach(req -> {
+ String dm = req.name();
+ ResourcePoolModule dep = moduleView.findModule(dm)
+ .orElseThrow(() -> new PluginException(dm + " not found"));
+ addNode(dep);
+ edges.get(module.name()).add(dep);
+ });
+ return this;
+ }
+
+ private void addNode(ResourcePoolModule module) {
+ nodes.add(module);
+ edges.computeIfAbsent(module.name(), _n -> new HashSet<>());
+ }
+
+ private synchronized void build() {
+ if (!result.isEmpty() || nodes.isEmpty())
+ return;
+
+ Deque<ResourcePoolModule> visited = new LinkedList<>();
+ Deque<ResourcePoolModule> done = new LinkedList<>();
+ ResourcePoolModule node;
+ while ((node = nodes.poll()) != null) {
+ if (!visited.contains(node)) {
+ visit(node, visited, done);
+ }
+ }
+ }
+
+ public Stream<ResourcePoolModule> sorted() {
+ build();
+ return result.stream();
+ }
+
+ private void visit(ResourcePoolModule node,
+ Deque<ResourcePoolModule> visited,
+ Deque<ResourcePoolModule> done) {
+ if (visited.contains(node)) {
+ if (!done.contains(node)) {
+ throw new IllegalArgumentException("Cyclic detected: " +
+ node + " " + edges.get(node.name()));
+ }
+ return;
+ }
+ visited.add(node);
+ edges.get(node.name()).stream()
+ .forEach(x -> visit(x, visited, done));
+ done.add(node);
+ result.addLast(node);
+ }
+}
--- a/jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ResourcePoolManager.java Sun Dec 11 17:39:27 2016 -0800
+++ b/jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ResourcePoolManager.java Mon Dec 12 11:31:39 2016 +0530
@@ -56,7 +56,11 @@
+ " module");
}
ByteBuffer bb = ByteBuffer.wrap(content.get().contentBytes());
- return ModuleDescriptor.read(bb);
+ try {
+ return ModuleDescriptor.read(bb);
+ } catch (RuntimeException re) {
+ throw new RuntimeException("module descriptor cannot be read for " + mod.name(), re);
+ }
}
/**
--- a/jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ReleaseInfoPlugin.java Sun Dec 11 17:39:27 2016 -0800
+++ b/jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ReleaseInfoPlugin.java Mon Dec 12 11:31:39 2016 +0530
@@ -36,6 +36,8 @@
import java.util.Properties;
import java.util.Set;
import java.util.function.Function;
+import java.util.stream.Collectors;
+import jdk.tools.jlink.internal.ModuleSorter;
import jdk.tools.jlink.internal.Utils;
import jdk.tools.jlink.plugin.ResourcePool;
import jdk.tools.jlink.plugin.ResourcePoolBuilder;
@@ -71,7 +73,7 @@
@Override
public Set<State> getState() {
- return EnumSet.of(State.FUNCTIONAL);
+ return EnumSet.of(State.AUTO_ENABLED, State.FUNCTIONAL);
}
@Override
@@ -87,6 +89,10 @@
@Override
public void configure(Map<String, String> config) {
String operation = config.get(NAME);
+ if (operation == null) {
+ return;
+ }
+
switch (operation) {
case "add": {
// leave it to open-ended! source, java_version, java_full_version
@@ -127,12 +133,46 @@
public ResourcePool transform(ResourcePool in, ResourcePoolBuilder out) {
in.transformAndCopy(Function.identity(), out);
+ Optional<ResourcePoolModule> javaBase = in.moduleView().findModule("java.base");
+ javaBase.ifPresent(mod -> {
+ // fill release information available from transformed "java.base" module!
+ ModuleDescriptor desc = mod.descriptor();
+ desc.osName().ifPresent(s -> {
+ release.put("OS_NAME", quote(s));
+ });
+ desc.osVersion().ifPresent(s -> release.put("OS_VERSION", quote(s)));
+ desc.osArch().ifPresent(s -> release.put("OS_ARCH", quote(s)));
+ desc.version().ifPresent(s -> release.put("JAVA_VERSION",
+ quote(parseVersion(s.toString()))));
+ desc.version().ifPresent(s -> release.put("JAVA_FULL_VERSION",
+ quote(s.toString())));
+ });
+
+ // put topological sorted module names separated by space
+ release.put("MODULES", new ModuleSorter(in.moduleView())
+ .sorted().map(ResourcePoolModule::name)
+ .collect(Collectors.joining(" ", "\"", "\"")));
+
// create a TOP level ResourcePoolEntry for "release" file.
out.add(ResourcePoolEntry.create("/java.base/release",
ResourcePoolEntry.Type.TOP, releaseFileContent()));
return out.build();
}
+ // Parse version string and return a string that includes only version part
+ // leaving "pre", "build" information. See also: java.lang.Runtime.Version.
+ private static String parseVersion(String str) {
+ return Runtime.Version.parse(str).
+ version().
+ stream().
+ map(Object::toString).
+ collect(Collectors.joining("."));
+ }
+
+ private static String quote(String str) {
+ return "\"" + str + "\"";
+ }
+
private byte[] releaseFileContent() {
Properties props = new Properties();
props.putAll(release);
--- a/jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/plugin/Plugin.java Sun Dec 11 17:39:27 2016 -0800
+++ b/jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/plugin/Plugin.java Mon Dec 12 11:31:39 2016 +0530
@@ -36,15 +36,15 @@
public interface Plugin {
/**
- * Order of categories:
+ * Order of categories matches the plugin sort order.
* <ol>
* <li>FILTER: Filter in/out resources or files.</li>
* <li>TRANSFORMER: Transform resources or files(eg: refactoring, bytecode
* manipulation).</li>
* <li>MODULEINFO_TRANSFORMER: Transform only module-info.class</li>
* <li>SORTER: Sort resources within the resource container.</li>
+ * <li>METAINFO_ADDER: Added meta info (like release, copyright etc.)</li>
* <li>COMPRESSOR: Compress resource within the resouce containers.</li>
- * <li>METAINFO_ADDER: Added meta info (like release, copyright etc.)</li>
* <li>VERIFIER: Does some image verification.</li>
* <li>PROCESSOR: Does some post processing on image.</li>
* <li>PACKAGER: Final processing</li>
@@ -55,8 +55,8 @@
TRANSFORMER("TRANSFORMER"),
MODULEINFO_TRANSFORMER("MODULEINFO_TRANSFORMER"),
SORTER("SORTER"),
+ METAINFO_ADDER("METAINFO_ADDER"),
COMPRESSOR("COMPRESSOR"),
- METAINFO_ADDER("METAINFO_ADDER"),
VERIFIER("VERIFIER"),
PROCESSOR("PROCESSOR"),
PACKAGER("PACKAGER");
--- a/jdk/test/tools/jlink/CustomPluginTest.java Sun Dec 11 17:39:27 2016 -0800
+++ b/jdk/test/tools/jlink/CustomPluginTest.java Mon Dec 12 11:31:39 2016 +0530
@@ -151,6 +151,8 @@
.pluginModulePath(pluginModulePath)
.output(helper.createNewImageDir(moduleName))
.addMods(moduleName)
+ .option("--disable-plugin")
+ .option("release-info")
.option("--rogue-filter")
.option("/foo/")
.call()
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/tools/jlink/ModuleNamesOrderTest.java Mon Dec 12 11:31:39 2016 +0530
@@ -0,0 +1,105 @@
+/*
+ * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+import java.io.File;
+import java.io.FileReader;
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.nio.file.Path;
+import java.util.Properties;
+import java.util.spi.ToolProvider;
+
+import tests.Helper;
+import tests.JImageGenerator;
+
+/*
+ * @test
+ * @bug 8168925
+ * @summary MODULES property should be topologically ordered and space-separated list
+ * @library ../lib
+ * @build tests.*
+ * @run main ModuleNamesOrderTest
+ */
+public class ModuleNamesOrderTest {
+ static final ToolProvider JLINK_TOOL = ToolProvider.findFirst("jlink")
+ .orElseThrow(() ->
+ new RuntimeException("jlink tool not found")
+ );
+
+ public static void main(String[] args) throws Exception {
+ Helper helper = Helper.newHelper();
+ if (helper == null) {
+ System.err.println("Test not run");
+ return;
+ }
+
+ Path outputDir = helper.createNewImageDir("image8168925");
+ JImageGenerator.getJLinkTask()
+ .modulePath(helper.defaultModulePath())
+ .output(outputDir)
+ .addMods("jdk.scripting.nashorn")
+ .call().assertSuccess();
+
+ File release = new File(outputDir.toString(), "release");
+ if (!release.exists()) {
+ throw new AssertionError("release not generated");
+ }
+
+ Properties props = new Properties();
+ try (FileReader reader = new FileReader(release)) {
+ props.load(reader);
+ }
+
+ String modules = props.getProperty("MODULES");
+ if (!modules.startsWith("\"java.base ")) {
+ throw new AssertionError("MODULES should start with 'java.base'");
+ }
+
+ if (!modules.endsWith(" jdk.scripting.nashorn\"")) {
+ throw new AssertionError("MODULES end with 'jdk.scripting.nashorn'");
+ }
+
+ checkDependency(modules, "java.logging", "java.base");
+ checkDependency(modules, "jdk.dynalink", "java.logging");
+ checkDependency(modules, "java.scripting", "java.base");
+ checkDependency(modules, "jdk.scripting.nashorn", "java.logging");
+ checkDependency(modules, "jdk.scripting.nashorn", "jdk.dynalink");
+ checkDependency(modules, "jdk.scripting.nashorn", "java.scripting");
+ }
+
+ private static void checkDependency(String modules, String fromMod, String toMod) {
+ int fromModIdx = modules.indexOf(fromMod);
+ if (fromModIdx == -1) {
+ throw new AssertionError(fromMod + " is missing in MODULES");
+ }
+ int toModIdx = modules.indexOf(toMod);
+ if (toModIdx == -1) {
+ throw new AssertionError(toMod + " is missing in MODULES");
+ }
+
+ if (toModIdx > fromModIdx) {
+ throw new AssertionError("in MODULES, " + fromMod + " should appear after " + toMod);
+ }
+ }
+}