# HG changeset patch # User sundar # Date 1481522499 -19800 # Node ID 86b1da05a4b02ed5ad138b2e198fb3934eef3a38 # Parent 814a5ddacddfd9c5cb9c6572eea15a95cd8784c0 8168925: MODULES property should be topologically ordered and space-separated list Reviewed-by: mchung, jlaskey diff -r 814a5ddacddf -r 86b1da05a4b0 jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/builder/DefaultImageBuilder.java --- 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 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 release = pool.findEntry("/java.base/release"); + if (release.isPresent()) { + try (InputStream is = release.get().content()) { + props.load(is); } } - } - - private void addModules(Properties props, Set 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 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> 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 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 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()); } diff -r 814a5ddacddf -r 86b1da05a4b0 jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImagePluginConfiguration.java --- 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 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); diff -r 814a5ddacddf -r 86b1da05a4b0 jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ModuleSorter.java --- /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 nodes = new LinkedList<>(); + private final Map> edges = new HashMap<>(); + private final Deque 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 visited = new LinkedList<>(); + Deque done = new LinkedList<>(); + ResourcePoolModule node; + while ((node = nodes.poll()) != null) { + if (!visited.contains(node)) { + visit(node, visited, done); + } + } + } + + public Stream sorted() { + build(); + return result.stream(); + } + + private void visit(ResourcePoolModule node, + Deque visited, + Deque 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); + } +} diff -r 814a5ddacddf -r 86b1da05a4b0 jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ResourcePoolManager.java --- 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); + } } /** diff -r 814a5ddacddf -r 86b1da05a4b0 jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ReleaseInfoPlugin.java --- 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 getState() { - return EnumSet.of(State.FUNCTIONAL); + return EnumSet.of(State.AUTO_ENABLED, State.FUNCTIONAL); } @Override @@ -87,6 +89,10 @@ @Override public void configure(Map 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 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); diff -r 814a5ddacddf -r 86b1da05a4b0 jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/plugin/Plugin.java --- 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. *
    *
  1. FILTER: Filter in/out resources or files.
    2. *
  2. TRANSFORMER: Transform resources or files(eg: refactoring, bytecode * manipulation).
    3. *
  3. MODULEINFO_TRANSFORMER: Transform only module-info.class
    4. *
  4. SORTER: Sort resources within the resource container.
    5. + *
  5. METAINFO_ADDER: Added meta info (like release, copyright etc.)
    6. *
  6. COMPRESSOR: Compress resource within the resouce containers.
    7. - *
  7. METAINFO_ADDER: Added meta info (like release, copyright etc.)
    8. *
  8. VERIFIER: Does some image verification.
    9. *
  9. PROCESSOR: Does some post processing on image.
    10. *
  10. PACKAGER: Final processing
    11. @@ -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"); diff -r 814a5ddacddf -r 86b1da05a4b0 jdk/test/tools/jlink/CustomPluginTest.java --- 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() diff -r 814a5ddacddf -r 86b1da05a4b0 jdk/test/tools/jlink/ModuleNamesOrderTest.java --- /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); + } + } +}