On Fri, 13 Dec 2024 14:15:11 GMT, Severin Gehwolf <[email protected]> wrote:
>> Please review this extension to #22609 which now disallows `ALL-MODULE-PATH`
>> without explicit `--module-path` option or a non-existent module path. In
>> addition, this fixes a bug mentioned in #22609 when `ALL-MODULE-PATH` and
>> `--limit-modules` are used in combination. It failed earlier and passes now
>> due to alignment of `ModuleFinder`s. With this patch JEP 493 enabled builds
>> and regular JDK builds behave the same in terms of `ALL-MODULE-PATH`.
>>
>> When an explicit module path is being added, there is no difference. All
>> modules on that path will be added as roots. Tests have been added for the
>> various cases and existing tests updated to allow for them to run on JEP 493
>> enabled builds. Thoughts?
>>
>> Testing:
>> - [x] GHA, `test/jdk/tools/jlink` (all pass)
>> - [x] Added jlink test.
>
> Severin Gehwolf has updated the pull request incrementally with two
> additional commits since the last revision:
>
> - Test cleanup
> - Only use the combined list when creating the finder
test/jdk/tools/jlink/basic/AllModulePath.java line 234:
> 232: assertTrue(allOut.stdout.isEmpty());
> 233: assertTrue(allOut.stderr.isEmpty());
> 234: List<String> expected = List.of("java.base", "jdk.jfr");
jlink --add-modules ALL-MODULE-PATH --limit-modules jdk.jfr --module-path jmods
--output all-mods-limit-mods.image
This PR changes to find the root modules from the paths specified in
`--module-path` only. So I would expect that `jdk.jfr` should not be included
in the resulting image because it's not on `--module-path`. This seems a bug
as it finds all modules using the module finder that includes `jdk.jfr`. The
module finder should be:
ModuleFinder mf = newLimitedFinder(finder,
options.limitMods.isEmpty()
? initialRoots : Set.of(),
Set.of());
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22494#discussion_r1884340758
