On Thu, 12 Dec 2024 17:52:45 GMT, Mandy Chung <mch...@openjdk.org> wrote:
>> Severin Gehwolf has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 20 commits: >> >> - More test clean-ups >> - Merge two AllModulePath tests >> - Better error message with no modules on mod-path and ALL-MODULE-PATH >> - Merge branch 'jdk-8345573-runtime-link-limit-mods' into >> jdk-8345259-all-module-path-fix >> - Mandy's feedback >> - Merge branch 'jdk-8345573-runtime-link-limit-mods' into >> jdk-8345259-all-module-path-fix >> - Handle non-existent module-path with ALL-MODULE-PATH >> - Move test, more test fixes for JEP 493 enabled builds >> - Fix JLinkTest.java >> - 8345259: Disallow ALL-MODULE-PATH without explicit --module-path >> - ... and 10 more: https://git.openjdk.org/jdk/compare/c9643d31...9ba004cc > > src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java line 376: > >> 374: throw taskHelper.newBadArgs("err.no.module.path"); >> 375: } >> 376: List<Path> originalModulePath = new >> ArrayList<>(options.modulePath); > > This is to keep the module path set in the command line for error message. > An alternative is to keep `options.modulePath` be the original value and do > not add the default module path to it. Instead: > > > Path defModPath = getDefaultModulePath(); > if (defModPath != null) { > finder = > newModuleFinder(Stream.concat(options.modulePath.stream(), > Stream.of(defModPath)).toList()); > } > > > Then the error message can just print `options.modulePath`. Correct. Nice idea. Will fix. > Is `-Duser.language=en -Duser.country=US` needed? I think so, since the test asserts the error message which is potentially localized. > Please add `@bug` to this test. Sure. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/22494#discussion_r1883724186 PR Review Comment: https://git.openjdk.org/jdk/pull/22494#discussion_r1883723275
