On Fri, 13 Dec 2024 14:15:11 GMT, Severin Gehwolf <sgehw...@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 258:

> 256:      * Verify linked modules using java --list-modules
> 257:      */
> 258:     private void verifyListModules(Path targetPath, List<String> 
> expected) throws Exception {

Suggestion:

    private void verifyListModules(Path targetPath, Set<String> expected) 
throws Exception {


Can simply use a Set to compare the result and  no need to sort `actual` from 
the output.

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

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

Reply via email to