On Tue, 10 Dec 2024 23:33:08 GMT, Mandy Chung <mch...@openjdk.org> wrote:
>> Severin Gehwolf has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - Handle non-existent module-path with ALL-MODULE-PATH >> - Move test, more test fixes for JEP 493 enabled builds > > src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java line 417: > >> 415: allModsLimits = modsLimits; >> 416: } >> 417: ModuleFinder mf = newModuleFinder(rootsFinder, >> allModsLimits, Set.of(), isLinkFromRuntime); > > It took me some time to understand this change as it depends on > https://github.com/openjdk/jdk/pull/22609. I adjusted the code trying to > help the readers easier to understand. > > See what you think. Instead of the `determineLinkFromRuntime` method, it > does the check in this method that makes it explicit to the readers. > > > private JlinkConfiguration initJlinkConfig() throws BadArgs { > // Empty module path not allowed with ALL-MODULE-PATH in --add-modules > if (options.addMods.contains(ALL_MODULE_PATH) && > options.modulePath.isEmpty()) { > throw taskHelper.newBadArgs("err.all.module.path.empty.mod.path"); > } > > ModuleFinder appModulePathFinder = > createFinderFromPath(options.modulePath); > ModuleFinder finder = appModulePathFinder; > boolean isLinkFromRuntime = false; > // if packaged module for java.base is not found, either from > // the default module path or the run-time image > if (!appModulePathFinder.find("java.base").isPresent()) { > // Add the default module path if that exists > Path defModPath = getDefaultModulePath(); > if (defModPath != null) { > options.modulePath.add(defModPath); > finder = createFinderFromPath(options.modulePath); > } > if (!finder.find("java.base").isPresent()) { > isLinkFromRuntime = true; > // java.base is from the system module path > finder = ModuleFinder.compose(ModuleFinder.ofSystem(), > appModulePathFinder); > } > } > > // Determine the roots set > Set<String> roots = new HashSet<>(); > for (String mod : options.addMods) { > if (mod.equals(ALL_MODULE_PATH)) { > // all observable modules are roots > Set<String> initialRoots = appModulePathFinder.findAll() > .stream() > .map(ModuleReference::descriptor) > .map(ModuleDescriptor::name) > .collect(Collectors.toSet()); > > // Error if no module is found on module path > if (initialRoots.isEmpty()) { > throw taskHelper.... Is the `isLinkFromRuntime` parameter needed in the `newModuleFinder` method? The java.base version check does not have any issue for linking from run-time image, right? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/22494#discussion_r1879085895
