> 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.

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/22644/files
  - new: https://git.openjdk.org/jdk/pull/22644/files/23114c18..1f4c6701

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=22644&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=22644&range=00-01

  Stats: 15390 lines in 2741 files changed: 7983 ins; 2513 del; 4894 mod
  Patch: https://git.openjdk.org/jdk/pull/22644.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/22644/head:pull/22644

PR: https://git.openjdk.org/jdk/pull/22644

Reply via email to