From 11cd639842b61952755ad83e88446c91237c19f5 Mon Sep 17 00:00:00 2001 From: Severin Gehwolf Date: Thu, 12 Dec 2024 17:58:05 +0000 Subject: [PATCH] 8345573: Module dependencies not resolved from run-time image when --limit-module is being used Reviewed-by: mchung --- .../jdk/tools/jlink/internal/JlinkTask.java | 282 ++++++++---------- test/jdk/tools/jlink/IntegrationTest.java | 6 +- .../jlink/bindservices/BindServices.java | 46 +-- .../jlink/bindservices/SuggestProviders.java | 53 ++-- .../AbstractLinkableRuntimeTest.java | 3 +- 5 files changed, 187 insertions(+), 203 deletions(-) diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java index 0eda0b5d455..6717d5517d1 100644 --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java @@ -49,7 +49,6 @@ import java.nio.file.SimpleFileVisitor; import java.nio.file.attribute.BasicFileAttributes; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; import java.util.Comparator; import java.util.Date; import java.util.HashMap; @@ -378,42 +377,60 @@ public class JlinkTask { // the token for "all modules on the module path" private static final String ALL_MODULE_PATH = "ALL-MODULE-PATH"; private JlinkConfiguration initJlinkConfig() throws BadArgs { + ModuleFinder appModuleFinder = newModuleFinder(options.modulePath); + ModuleFinder finder = appModuleFinder; + boolean isLinkFromRuntime = false; + if (!appModuleFinder.find("java.base").isPresent()) { + // If the application module finder doesn't contain the + // java.base module we have one of two cases: + // 1. A custom module is being linked into a runtime, but the JDK + // modules have not been provided on the module path. + // 2. We have a run-time image based link. + // + // Distinguish case 2 by adding the default 'jmods' folder and try + // the look-up again. For case 1 this will now find java.base, but + // not for case 2, since the jmods folder is not there or doesn't + // include the java.base module. + Path defModPath = getDefaultModulePath(); + if (defModPath != null) { + options.modulePath.add(defModPath); + finder = newModuleFinder(options.modulePath); + } + // We've just added the default module path ('jmods'). If we still + // don't find java.base, we must resolve JDK modules from the + // current run-time image. + if (!finder.find("java.base").isPresent()) { + // If we don't have a linkable run-time image this is an error + if (!LinkableRuntimeImage.isLinkableRuntime()) { + throw taskHelper.newBadArgs("err.runtime.link.not.linkable.runtime"); + } + isLinkFromRuntime = true; + // JDK modules come from the system module path + finder = ModuleFinder.compose(ModuleFinder.ofSystem(), appModuleFinder); + } + } + + // Sanity check version if we use JMODs + if (!isLinkFromRuntime) { + checkJavaBaseVersion(finder); + } + + // Determine the roots set Set roots = new HashSet<>(); for (String mod : options.addMods) { - if (mod.equals(ALL_MODULE_PATH) && options.modulePath.size() > 0) { - ModuleFinder finder = newModuleFinder(options.modulePath, options.limitMods, Set.of()); - // all observable modules are roots - finder.findAll() - .stream() - .map(ModuleReference::descriptor) - .map(ModuleDescriptor::name) - .forEach(mn -> roots.add(mn)); + if (mod.equals(ALL_MODULE_PATH)) { + ModuleFinder mf = newLimitedFinder(finder, options.limitMods, + Set.of()); + mf.findAll() + .stream() + .map(ModuleReference::descriptor) + .map(ModuleDescriptor::name) + .forEach(mn -> roots.add(mn)); } else { roots.add(mod); } } - - ModuleFinder finder = newModuleFinder(options.modulePath, options.limitMods, roots); - if (finder.find("java.base").isEmpty()) { - Path defModPath = getDefaultModulePath(); - if (defModPath != null) { - options.modulePath.add(defModPath); - } - finder = newModuleFinder(options.modulePath, options.limitMods, roots); - } - - boolean isLinkFromRuntime = options.modulePath.isEmpty(); - // In case of custom modules outside the JDK we may - // have a non-empty module path, which must not include - // java.base. If it did, we link using packaged modules from that - // module path. If the module path does not include java.base, we have - // the case where we link from the run-time image. In that case, we take - // the JDK modules from the run-time image (ModuleFinder.ofSystem()). - if (finder.find("java.base").isEmpty()) { - isLinkFromRuntime = true; - ModuleFinder runtimeImageFinder = ModuleFinder.ofSystem(); - finder = combinedFinders(runtimeImageFinder, finder, options.limitMods, roots); - } + finder = newLimitedFinder(finder, options.limitMods, roots); // --keep-packaged-modules doesn't make sense as we are not linking // from packaged modules to begin with. @@ -429,48 +446,13 @@ public class JlinkTask { options.generateLinkableRuntime); } - /** - * Creates a combined module finder of {@code finder} and - * {@code runtimeImageFinder} that first looks-up modules in the - * {@code runtimeImageFinder} and if not present in {@code finder}. - * - * @param runtimeImageFinder A system modules finder. - * @param finder A module finder based on packaged modules. - * @param limitMods The set of limited modules for the resulting - * finder (if any). - * @param roots All module roots. - * - * @return A combined finder, or the input finder, potentially applying - * module limits. + /* + * Creates a ModuleFinder for the given module paths. */ - private ModuleFinder combinedFinders(ModuleFinder runtimeImageFinder, - ModuleFinder finder, - Set limitMods, - Set roots) { - ModuleFinder combined = new ModuleFinder() { - - @Override - public Optional find(String name) { - Optional mref = runtimeImageFinder.find(name); - if (mref.isEmpty()) { - return finder.find(name); - } - return mref; - } - - @Override - public Set findAll() { - Set all = new HashSet<>(); - all.addAll(runtimeImageFinder.findAll()); - all.addAll(finder.findAll()); - return Collections.unmodifiableSet(all); - } - }; - // if limitmods is specified then limit the universe - if (limitMods != null && !limitMods.isEmpty()) { - return limitFinder(combined, limitMods, Objects.requireNonNull(roots)); - } - return combined; + public static ModuleFinder newModuleFinder(List paths) { + Runtime.Version version = Runtime.version(); + Path[] entries = paths.toArray(new Path[0]); + return ModulePath.of(version, true, entries); } private void createImage(JlinkConfiguration config) throws Exception { @@ -510,50 +492,83 @@ public class JlinkTask { } /* - * Returns a module finder of the given module path or the system modules - * if the module path is empty that limits the observable modules to those - * in the transitive closure of the modules specified in {@code limitMods} - * plus other modules specified in the {@code roots} set. - * - * @throws IllegalArgumentException if java.base module is present - * but its descriptor has no version + * Returns a module finder of the given module finder that limits the + * observable modules to those in the transitive closure of the modules + * specified in {@code limitMods} plus other modules specified in the + * {@code roots} set. */ - public static ModuleFinder newModuleFinder(List paths, - Set limitMods, - Set roots) - { - Runtime.Version version = Runtime.version(); - Path[] entries = paths.toArray(new Path[0]); - ModuleFinder finder = paths.isEmpty() ? ModuleFinder.ofSystem() - : ModulePath.of(version, true, entries); - if (finder.find("java.base").isPresent()) { - // use the version of java.base module, if present, as - // the release version for multi-release JAR files - ModuleDescriptor.Version v = finder.find("java.base").get() - .descriptor().version().orElseThrow(() -> - new IllegalArgumentException("No version in java.base descriptor") - ); - - // java.base version is different than the current runtime version - version = Runtime.Version.parse(v.toString()); - if (Runtime.version().feature() != version.feature() || - Runtime.version().interim() != version.interim()) - { - // jlink version and java.base version do not match. - // We do not (yet) support this mode. - throw new IllegalArgumentException(taskHelper.getMessage("err.jlink.version.mismatch", - Runtime.version().feature(), Runtime.version().interim(), - version.feature(), version.interim())); - } - } - - // if limitmods is specified then limit the universe + public static ModuleFinder newLimitedFinder(ModuleFinder finder, + Set limitMods, + Set roots) { + // if limitMods is specified then limit the universe if (limitMods != null && !limitMods.isEmpty()) { - finder = limitFinder(finder, limitMods, Objects.requireNonNull(roots)); + Objects.requireNonNull(roots); + // resolve all root modules + Configuration cf = Configuration.empty() + .resolve(finder, + ModuleFinder.of(), + limitMods); + + // module name -> reference + Map map = new HashMap<>(); + cf.modules().forEach(m -> { + ModuleReference mref = m.reference(); + map.put(mref.descriptor().name(), mref); + }); + + // add the other modules + roots.stream() + .map(finder::find) + .flatMap(Optional::stream) + .forEach(mref -> map.putIfAbsent(mref.descriptor().name(), mref)); + + // set of modules that are observable + Set mrefs = new HashSet<>(map.values()); + + return new ModuleFinder() { + @Override + public Optional find(String name) { + return Optional.ofNullable(map.get(name)); + } + + @Override + public Set findAll() { + return mrefs; + } + }; } return finder; } + /* + * Checks the version of the module descriptor of java.base for compatibility + * with the current runtime version. + * + * @throws IllegalArgumentException the descriptor of java.base has no + * version or the java.base version is not the same as the current runtime's + * version. + */ + private static void checkJavaBaseVersion(ModuleFinder finder) { + assert finder.find("java.base").isPresent(); + + // use the version of java.base module, if present, as + // the release version for multi-release JAR files + ModuleDescriptor.Version v = finder.find("java.base").get() + .descriptor().version().orElseThrow(() -> + new IllegalArgumentException("No version in java.base descriptor") + ); + + Runtime.Version version = Runtime.Version.parse(v.toString()); + if (Runtime.version().feature() != version.feature() || + Runtime.version().interim() != version.interim()) { + // jlink version and java.base version do not match. + // We do not (yet) support this mode. + throw new IllegalArgumentException(taskHelper.getMessage("err.jlink.version.mismatch", + Runtime.version().feature(), Runtime.version().interim(), + version.feature(), version.interim())); + } + } + private static void deleteDirectory(Path dir) throws IOException { Files.walkFileTree(dir, new SimpleFileVisitor() { @Override @@ -611,10 +626,6 @@ public class JlinkTask { // Perform some sanity checks for linking from the run-time image if (config.linkFromRuntimeImage()) { - if (!LinkableRuntimeImage.isLinkableRuntime()) { - String msg = taskHelper.getMessage("err.runtime.link.not.linkable.runtime"); - throw new IllegalArgumentException(msg); - } // Do not permit linking from run-time image and also including jdk.jlink module if (cf.findModule(JlinkTask.class.getModule().getName()).isPresent()) { String msg = taskHelper.getMessage("err.runtime.link.jdk.jlink.prohibited"); @@ -773,49 +784,6 @@ public class JlinkTask { } } - /* - * Returns a ModuleFinder that limits observability to the given root - * modules, their transitive dependences, plus a set of other modules. - */ - public static ModuleFinder limitFinder(ModuleFinder finder, - Set roots, - Set otherMods) { - - // resolve all root modules - Configuration cf = Configuration.empty() - .resolve(finder, - ModuleFinder.of(), - roots); - - // module name -> reference - Map map = new HashMap<>(); - cf.modules().forEach(m -> { - ModuleReference mref = m.reference(); - map.put(mref.descriptor().name(), mref); - }); - - // add the other modules - otherMods.stream() - .map(finder::find) - .flatMap(Optional::stream) - .forEach(mref -> map.putIfAbsent(mref.descriptor().name(), mref)); - - // set of modules that are observable - Set mrefs = new HashSet<>(map.values()); - - return new ModuleFinder() { - @Override - public Optional find(String name) { - return Optional.ofNullable(map.get(name)); - } - - @Override - public Set findAll() { - return mrefs; - } - }; - } - private static Platform targetPlatform(Configuration cf, Map modsPaths, boolean runtimeImageLink) throws IOException { diff --git a/test/jdk/tools/jlink/IntegrationTest.java b/test/jdk/tools/jlink/IntegrationTest.java index d79752f6d56..7f3dc223461 100644 --- a/test/jdk/tools/jlink/IntegrationTest.java +++ b/test/jdk/tools/jlink/IntegrationTest.java @@ -154,9 +154,13 @@ public class IntegrationTest { mods.add("java.management"); Set limits = new HashSet<>(); limits.add("java.management"); + boolean linkFromRuntime = false; JlinkConfiguration config = new Jlink.JlinkConfiguration(output, mods, - JlinkTask.newModuleFinder(modulePaths, limits, mods), false, false, false); + JlinkTask.newLimitedFinder(JlinkTask.newModuleFinder(modulePaths), limits, mods), + linkFromRuntime, + false /* ignore modified runtime */, + false /* generate run-time image */); List lst = new ArrayList<>(); diff --git a/test/jdk/tools/jlink/bindservices/BindServices.java b/test/jdk/tools/jlink/bindservices/BindServices.java index 02f8bfd52d2..89095631040 100644 --- a/test/jdk/tools/jlink/bindservices/BindServices.java +++ b/test/jdk/tools/jlink/bindservices/BindServices.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, 2018, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2017, 2024, 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 @@ -21,6 +21,9 @@ * questions. */ +import static jdk.test.lib.process.ProcessTools.executeProcess; +import static org.testng.Assert.assertTrue; + import java.io.File; import java.io.PrintWriter; import java.io.StringWriter; @@ -33,21 +36,20 @@ import java.util.spi.ToolProvider; import java.util.stream.Collectors; import java.util.stream.Stream; -import jdk.test.lib.compiler.CompilerUtils; -import static jdk.test.lib.process.ProcessTools.*; - import org.testng.annotations.BeforeTest; import org.testng.annotations.Test; -import static org.testng.Assert.*; + +import jdk.test.lib.compiler.CompilerUtils; +import jdk.tools.jlink.internal.LinkableRuntimeImage; /** * @test - * @bug 8174826 + * @bug 8174826 8345573 * @library /test/lib - * @modules jdk.compiler jdk.jlink + * @modules jdk.compiler jdk.jlink/jdk.tools.jlink.internal * @build BindServices jdk.test.lib.process.ProcessTools * jdk.test.lib.compiler.CompilerUtils - * @run testng BindServices + * @run testng/othervm BindServices */ public class BindServices { @@ -56,21 +58,23 @@ public class BindServices { private static final Path SRC_DIR = Paths.get(TEST_SRC, "src"); private static final Path MODS_DIR = Paths.get("mods"); + private static final boolean LINKABLE_RUNTIME = LinkableRuntimeImage.isLinkableRuntime(); + private static final boolean JMODS_EXIST = Files.exists(Paths.get(JAVA_HOME, "jmods")); - private static final String MODULE_PATH = - Paths.get(JAVA_HOME, "jmods").toString() + - File.pathSeparator + MODS_DIR.toString(); + private static final String MODULE_PATH = (JMODS_EXIST ? Paths.get(JAVA_HOME, "jmods").toString() + + File.pathSeparator : "") + + MODS_DIR.toString(); // the names of the modules in this test private static String[] modules = new String[] {"m1", "m2", "m3"}; - private static boolean hasJmods() { - if (!Files.exists(Paths.get(JAVA_HOME, "jmods"))) { - System.err.println("Test skipped. NO jmods directory"); - return false; + private static boolean isExplodedJDKImage() { + if (!JMODS_EXIST && !LINKABLE_RUNTIME) { + System.err.println("Test skipped. Not a linkable runtime and no JMODs"); + return true; } - return true; + return false; } /* @@ -78,7 +82,7 @@ public class BindServices { */ @BeforeTest public void compileAll() throws Throwable { - if (!hasJmods()) return; + if (isExplodedJDKImage()) return; for (String mn : modules) { Path msrc = SRC_DIR.resolve(mn); @@ -89,7 +93,7 @@ public class BindServices { @Test public void noServiceBinding() throws Throwable { - if (!hasJmods()) return; + if (isExplodedJDKImage()) return; Path dir = Paths.get("noServiceBinding"); @@ -103,7 +107,7 @@ public class BindServices { @Test public void fullServiceBinding() throws Throwable { - if (!hasJmods()) return; + if (isExplodedJDKImage()) return; Path dir = Paths.get("fullServiceBinding"); @@ -122,7 +126,7 @@ public class BindServices { @Test public void testVerbose() throws Throwable { - if (!hasJmods()) return; + if (isExplodedJDKImage()) return; Path dir = Paths.get("verbose"); @@ -153,7 +157,7 @@ public class BindServices { @Test public void testVerboseAndNoBindServices() throws Throwable { - if (!hasJmods()) return; + if (isExplodedJDKImage()) return; Path dir = Paths.get("verboseNoBind"); diff --git a/test/jdk/tools/jlink/bindservices/SuggestProviders.java b/test/jdk/tools/jlink/bindservices/SuggestProviders.java index 9685f927f8d..794d22fc570 100644 --- a/test/jdk/tools/jlink/bindservices/SuggestProviders.java +++ b/test/jdk/tools/jlink/bindservices/SuggestProviders.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2017, 2024, 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 @@ -21,6 +21,9 @@ * questions. */ +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertTrue; + import java.io.File; import java.io.PrintWriter; import java.io.StringWriter; @@ -32,17 +35,18 @@ import java.util.List; import java.util.spi.ToolProvider; import java.util.stream.Collectors; import java.util.stream.Stream; -import jdk.test.lib.compiler.CompilerUtils; import org.testng.annotations.BeforeTest; import org.testng.annotations.Test; -import static org.testng.Assert.*; + +import jdk.test.lib.compiler.CompilerUtils; +import jdk.tools.jlink.internal.LinkableRuntimeImage; /** * @test - * @bug 8174826 + * @bug 8174826 8345573 * @library /lib/testlibrary /test/lib - * @modules jdk.charsets jdk.compiler jdk.jlink + * @modules jdk.charsets jdk.compiler jdk.jlink/jdk.tools.jlink.internal * @build SuggestProviders jdk.test.lib.compiler.CompilerUtils * @run testng SuggestProviders */ @@ -54,20 +58,23 @@ public class SuggestProviders { private static final Path SRC_DIR = Paths.get(TEST_SRC, "src"); private static final Path MODS_DIR = Paths.get("mods"); - private static final String MODULE_PATH = - Paths.get(JAVA_HOME, "jmods").toString() + - File.pathSeparator + MODS_DIR.toString(); + private static final boolean LINKABLE_RUNTIME = LinkableRuntimeImage.isLinkableRuntime(); + private static final boolean JMODS_EXIST = Files.exists(Paths.get(JAVA_HOME, "jmods")); + + private static final String MODULE_PATH = (JMODS_EXIST ? Paths.get(JAVA_HOME, "jmods").toString() + + File.pathSeparator : "") + + MODS_DIR.toString(); // the names of the modules in this test private static String[] modules = new String[] {"m1", "m2", "m3"}; - private static boolean hasJmods() { - if (!Files.exists(Paths.get(JAVA_HOME, "jmods"))) { - System.err.println("Test skipped. NO jmods directory"); - return false; + private static boolean isExplodedJDKImage() { + if (!JMODS_EXIST && !LINKABLE_RUNTIME) { + System.err.println("Test skipped. Not a linkable runtime and no JMODs"); + return true; } - return true; + return false; } /* @@ -75,7 +82,7 @@ public class SuggestProviders { */ @BeforeTest public void compileAll() throws Throwable { - if (!hasJmods()) return; + if (isExplodedJDKImage()) return; for (String mn : modules) { Path msrc = SRC_DIR.resolve(mn); @@ -125,7 +132,7 @@ public class SuggestProviders { @Test public void suggestProviders() throws Throwable { - if (!hasJmods()) return; + if (isExplodedJDKImage()) return; List output = JLink.run("--module-path", MODULE_PATH, "--suggest-providers").output(); @@ -145,7 +152,7 @@ public class SuggestProviders { */ @Test public void observableModules() throws Throwable { - if (!hasJmods()) return; + if (isExplodedJDKImage()) return; List output = JLink.run("--module-path", MODULE_PATH, "--add-modules", "m1", @@ -165,7 +172,7 @@ public class SuggestProviders { */ @Test public void limitModules() throws Throwable { - if (!hasJmods()) return; + if (isExplodedJDKImage()) return; List output = JLink.run("--module-path", MODULE_PATH, "--limit-modules", "m1", @@ -184,7 +191,7 @@ public class SuggestProviders { @Test public void providersForServices() throws Throwable { - if (!hasJmods()) return; + if (isExplodedJDKImage()) return; List output = JLink.run("--module-path", MODULE_PATH, @@ -203,7 +210,7 @@ public class SuggestProviders { @Test public void unusedService() throws Throwable { - if (!hasJmods()) return; + if (isExplodedJDKImage()) return; List output = JLink.run("--module-path", MODULE_PATH, @@ -221,7 +228,7 @@ public class SuggestProviders { @Test public void nonExistentService() throws Throwable { - if (!hasJmods()) return; + if (isExplodedJDKImage()) return; List output = JLink.run("--module-path", MODULE_PATH, @@ -236,7 +243,7 @@ public class SuggestProviders { @Test public void noSuggestProviders() throws Throwable { - if (!hasJmods()) return; + if (isExplodedJDKImage()) return; List output = JLink.run("--module-path", MODULE_PATH, @@ -250,7 +257,7 @@ public class SuggestProviders { @Test public void suggestTypeNotRealProvider() throws Throwable { - if (!hasJmods()) return; + if (isExplodedJDKImage()) return; List output = JLink.run("--module-path", MODULE_PATH, @@ -268,7 +275,7 @@ public class SuggestProviders { @Test public void addNonObservableModule() throws Throwable { - if (!hasJmods()) return; + if (isExplodedJDKImage()) return; List output = JLink.run("--module-path", MODULE_PATH, diff --git a/test/jdk/tools/jlink/runtimeImage/AbstractLinkableRuntimeTest.java b/test/jdk/tools/jlink/runtimeImage/AbstractLinkableRuntimeTest.java index e7d5340e3b0..1df4455bc7d 100644 --- a/test/jdk/tools/jlink/runtimeImage/AbstractLinkableRuntimeTest.java +++ b/test/jdk/tools/jlink/runtimeImage/AbstractLinkableRuntimeTest.java @@ -192,7 +192,8 @@ public abstract class AbstractLinkableRuntimeTest { // if the exit checker failed, we expected the other outcome // i.e. fail for success and success for fail. boolean successExit = analyzer.getExitValue() == 0; - String msg = String.format("Expected jlink to %s given a jmodless image. Exit code was: %d", + String msg = String.format("Expected jlink to %s given a linkable run-time image. " + + "Exit code was: %d", (successExit ? "fail" : "pass"), analyzer.getExitValue()); throw new AssertionError(msg); }