RFR: 8345259: Disallow ALL-MODULE-PATH without explicit --module-path [v5]

[Mandy Chung]

On Tue, 10 Dec 2024 23:33:08 GMT, Mandy Chung <mchung at 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