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

[Severin Gehwolf]

On Thu, 12 Dec 2024 18:41:51 GMT, Mandy Chung <mchung at 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
>
> test/jdk/tools/jlink/basic/AllModulePath.java line 129:
> 
>> 127:             // java.base is a dependency of external modules
>> 128:             modules.add("java.base");
>> 129:         }
> 
> The difference in the expected list is because `$JDK/jmods` is added to the module path explicitly.
> 
> I suggest to change `createImage` not to include `jmods` in `--module-path` option to simplify it so this test case checks for the same expected result in different linking modes.

Makes sense. Done.

> test/jdk/tools/jlink/basic/AllModulePath.java line 184:
> 
>> 182:         assertTrue(rc != 0);
>> 183:         String actual = new String(baos.toByteArray()).trim();
>> 184:         assertEquals(actual, "Error: --module-path option must be specified with --add-modules ALL-MODULE-PATH");
> 
> The new test cases can be cleaned up a little bit to match the existing test style.
> 
> Suggest to refactor this that other test cases can use something like `String jlink(boolean succeed, String... options)` that returns the output of stdout and stderr.   This constrains the output/error checking but it will catch test failures if any. 
> 
> `createImage` can simply call `jlink(true, ....)` that concatenates `options` with `"--output", image.toString()`

Yes, agreed. It should be much nicer now.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/22494#discussion_r1883969543
PR Review Comment: https://git.openjdk.org/jdk/pull/22494#discussion_r1883970477