RFR: 8345185: Update jpackage to not include service bindings by default [v2]
Alexey Semenyuk
asemenyuk at openjdk.org
Tue Dec 17 03:05:47 UTC 2024
On Mon, 16 Dec 2024 20:21:06 GMT, Severin Gehwolf <sgehwolf at openjdk.org> wrote:
>> Please review these changes to jpackage in light of [JEP 493](https://openjdk.org/jeps/493). When this feature is enabled, then some of the `jpackage` tests fail. The failures fall into the following categories:
>>
>> - `ALL-DEFAULT` notion from `jpackage` which includes all modules that export an API, which includes `jdk.jlink`, which is prevented from being included when linking from the run-time image (see the [JEP 493](https://openjdk.org/jeps/493) restrictions). The proposal is to not include `jdk.jlink` and `jdk.jpackage` by default on a JDK build with JEP 493 enabled. A regular JDK build doesn't have this filtering. We could make this consistent across JDK builds by unconditionally filtering them, but I wasn't sure, so I've opted for the proposed solution for now.
>> - Don't issue a warning when there is no `jmods` folder in the JDK install and we have a JEP 493 enabled build. In that case issuing the warning isn't appropriate as it's the expected behaviour.
>> - `ALL-MODULE-PATH` changes: `BasicTest.java` verifies the `--add-modules` argument to `jpackage`. Using `ALL-MODULE-PATH` for JDK modules won't be supported for JEP 493-enabled builds. So I've changed this test to skip the test using `ALL-MODULE-PATH` when we have such an enabled build. Other tests, such as `RuntimeImageTest.java` and `RuntimeImageSymbolicLinksTest.java` tests verify something else not related to `ALL-MODULE-PATH` or `--add-modules`. It seems more appropriate to use the smaller set of modules to use for the runtime JDK image.
>> - `JLinkOptionsTest.java`: That test verifies options passed to `jlink` via the `ToolProvider` API. For some reason, it uses `--bind-services` extensively and that - in turn - and, when not limited with the `--limit-modules` option as well, will include `jdk.jlink` in the resulting image, again running afoul the JEP 493 restriction of not allowing `jdk.jlink` for now. I propose to use suitable options including `--limit-modules` which would then no longer include `jdk.jlink` in the runtime image and the link from a run-time image works as well. These changes depend on [JDK-8345573](https://bugs.openjdk.org/browse/JDK-8345573) for it to work fully.
>>
>> Testing:
>> - [ ] GHA
>> - [x] running tests in `test/jdk/tools/jpackage` on a JEP 493 enabled JDK. As far as I could see the failures that I was seeing weren't any more related to JEP 493 (some RPM requirements showing up that it didn't expect to).
>>
>> Thoughts? Opinions?
>
> Severin Gehwolf has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains seven additional commits since the last revision:
>
> - Refactor BasicTest.java according to review
> - Don't resolve service bindings by default
> - Revert "Handle ALL-DEFAULT in jpackage tool"
>
> This reverts commit ca506f85f67f495cd29e8f9ff1a7004c9888aaaf.
> - Merge branch 'master' into jdk-8345185-jpackage-all-default-fix
> - Adjust JLinkOptionsTest.java after JEP 493
> - Fix tests for JEP 493 enabled JDKs
> - Handle ALL-DEFAULT in jpackage tool
>
> When a JEP 493 enabled JDK is in use, the JMODs folder might not exist.
> Do some reasonable filtering based on that information. Otherwise, keep
> as-is.
test/jdk/tools/jpackage/share/BasicTest.java line 79:
> 77: boolean noJmods = Files.notExists(jmods);
> 78: if (LinkableRuntimeImage.isLinkableRuntime() && noJmods) {
> 79: System.out.println("ALL-MODULE-PATH test skipped for linkable run-time image");
Please use `TKit.trace()` instead of `System.out.println()`
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22644#discussion_r1887832827
More information about the core-libs-dev
mailing list