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

Mandy Chung mchung at openjdk.org
Fri Dec 13 18:43:42 UTC 2024


On Fri, 13 Dec 2024 14:15:11 GMT, Severin Gehwolf <sgehwolf at openjdk.org> 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


More information about the core-libs-dev mailing list