8349648: Test tools/jpackage/share/JLinkOptionsTest.java fails with --enable-linkable-runtime set after JDK-8346434 by RealCLanger · Pull Request #23514 · openjdk/jdk

RealCLanger · 2025-02-07T13:13:24Z

The change for JDK-8346434 added a new test case to tools/jpackage/share/JLinkOptionsTest.java which does not respect the constraint of the linkable runtime (JEP 493) that no jdk.jlink module can be part of the target image.
This can be circumvented by limiting the modules.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8349648: Test tools/jpackage/share/JLinkOptionsTest.java fails with --enable-linkable-runtime set after JDK-8346434 (Bug - P4)

Reviewers

Alexey Semenyuk (@alexeysemenyukoracle - Reviewer) Review applies to e606244c
Severin Gehwolf (@jerboaa - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/23514/head:pull/23514
$ git checkout pull/23514

Update a local copy of the PR:
$ git checkout pull/23514
$ git pull https://git.openjdk.org/jdk.git pull/23514/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 23514

View PR using the GUI difftool:
$ git pr show -t 23514

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/23514.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-02-07T13:13:59Z

👋 Welcome back clanger! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2025-02-07T13:14:04Z

@RealCLanger This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8349648: Test tools/jpackage/share/JLinkOptionsTest.java fails with --enable-linkable-runtime set after JDK-8346434

Reviewed-by: sgehwolf, asemenyuk

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 52 new commits pushed to the master branch:

7a0832f: 8349934: Wrong file regex for copyright header format check in .jcheck/conf
2a5d1da: 8349571: Remove JavaThreadFactory interface from SA
74e458b: 8349684: Remove SA core file tests from problem list for macosx-x64
ba28119: 8348427: DeferredLintHandler API should use JCTree instead of DiagnosticPosition
c5ac3c4: 8249831: Test sun/security/mscapi/nonUniqueAliases/NonUniqueAliases.java is marked with @ignore
336d0d8: 8349926: [BACKOUT] Support static JDK in libfontmanager/freetypeScaler.c
4b463ee: 8342103: C2 compiler support for Float16 type and associated scalar operations
332d87c: 8349859: Support static JDK in libfontmanager/freetypeScaler.c
73e1780: 8349836: G1: Improve group prediction log message
ed17c55: 8349145: Make Class.getProtectionDomain() non-native
... and 42 more: https://git.openjdk.org/jdk/compare/86cec4ea2c2c56f03b23be44caade49b922cd3c6...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

openjdk · 2025-02-07T13:14:29Z

@RealCLanger The following label will be automatically applied to this pull request:

core-libs

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2025-02-07T13:17:32Z

Webrevs

01: Full - Incremental (d13fa654)
00: Full (e606244c)

jerboaa · 2025-02-07T16:48:09Z

test/jdk/tools/jpackage/share/JLinkOptionsTest.java

+        final var defaultModules = getModulesInRuntime("--limit-modules java.smartcardio,jdk.crypto.cryptoki,java.desktop");
+        final var modulesWithBindServices = getModulesInRuntime("--bind-services --limit-modules java.smartcardio,jdk.crypto.cryptoki,java.desktop");


Pre-existing: It would be good to document which additional modules in the set get dragged in by --bind-services. That would allow future readers of that test to figure out why a test starts to fail (e.g. by a future change removing a provided service in a module).

As far as my testing is concerned it's only jdk.crypto.cryptoki which is different with --bind-services (as it provides a security provider as a service and doesn't export API).

This is the diff of modules between "jpackage without --bind-services" and "jpackage with --bind-services" before this PR:

[20:27:10.849] TRACE: assertNotEquals(, jdk.charsets,jdk.crypto.cryptoki,jdk.crypto.mscapi,jdk.editpad,jdk.jdeps,jdk.jlink,jdk.jpackage,jdk.jstatd,jdk.localedata,jdk.naming.dns,jdk.naming.rmi): Check '--bind-services' option adds modules

I added a comment about what is expected.

RealCLanger · 2025-02-13T10:02:13Z

Thanks for the reviews.

/integrate

openjdk · 2025-02-13T10:03:25Z

Going to push as commit 29202d1.
Since your change was applied there have been 57 commits pushed to the master branch:

24b7f81: 8349783: g1RemSetSummary.cpp:344:68: runtime error: member call on null pointer of type 'struct G1HeapRegion'
a637ccf: 8349851: RISC-V: Call VM leaf can use movptr2
adc3f53: 8349787: java/lang/Thread/virtual/ThreadPollOnYield.java#default passes unexpectedly without libVThreadPinner.so
55097dd: 8344802: Crash in StubRoutines::verify_mxcsr with -XX:+EnableX86ECoreOpts and -Xcheck:jni
b8576eb: 6562489: Font-Renderer should ignore invisible characters \u2062 and \u2063
7a0832f: 8349934: Wrong file regex for copyright header format check in .jcheck/conf
2a5d1da: 8349571: Remove JavaThreadFactory interface from SA
74e458b: 8349684: Remove SA core file tests from problem list for macosx-x64
ba28119: 8348427: DeferredLintHandler API should use JCTree instead of DiagnosticPosition
c5ac3c4: 8249831: Test sun/security/mscapi/nonUniqueAliases/NonUniqueAliases.java is marked with @ignore
... and 47 more: https://git.openjdk.org/jdk/compare/86cec4ea2c2c56f03b23be44caade49b922cd3c6...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-02-13T10:03:32Z

@RealCLanger Pushed as commit 29202d1.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

JDK-8349648

e606244

openjdk bot added the rfr Pull request is ready for review label Feb 7, 2025

openjdk bot added the core-libs core-libs-dev@openjdk.org label Feb 7, 2025

alexeysemenyukoracle approved these changes Feb 7, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Feb 7, 2025

jerboaa reviewed Feb 7, 2025

View reviewed changes

Add a comment to help understand the output

d13fa65

openjdk bot removed the ready Pull request is ready to be integrated label Feb 11, 2025

jerboaa approved these changes Feb 12, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Feb 12, 2025

openjdk bot added the integrated Pull request has been integrated label Feb 13, 2025

openjdk bot closed this Feb 13, 2025

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Feb 13, 2025

RealCLanger deleted the JDK-8349648 branch February 13, 2025 13:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8349648: Test tools/jpackage/share/JLinkOptionsTest.java fails with --enable-linkable-runtime set after JDK-8346434#23514

8349648: Test tools/jpackage/share/JLinkOptionsTest.java fails with --enable-linkable-runtime set after JDK-8346434#23514
RealCLanger wants to merge 2 commits intoopenjdk:masterfrom
RealCLanger:JDK-8349648

RealCLanger commented Feb 7, 2025 •

edited by openjdk bot

Loading

Uh oh!

bridgekeeper bot commented Feb 7, 2025

Uh oh!

openjdk bot commented Feb 7, 2025 •

edited

Loading

Uh oh!

openjdk bot commented Feb 7, 2025

Uh oh!

mlbridge bot commented Feb 7, 2025 •

edited

Loading

Uh oh!

jerboaa Feb 7, 2025

Uh oh!

jerboaa Feb 7, 2025

Uh oh!

alexeysemenyukoracle Feb 7, 2025

Uh oh!

RealCLanger Feb 11, 2025

Uh oh!

RealCLanger commented Feb 13, 2025

Uh oh!

openjdk bot commented Feb 13, 2025

Uh oh!

openjdk bot commented Feb 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

		final var defaultModules = getModulesInRuntime("--limit-modules java.smartcardio,jdk.crypto.cryptoki,java.desktop");
		final var modulesWithBindServices = getModulesInRuntime("--bind-services --limit-modules java.smartcardio,jdk.crypto.cryptoki,java.desktop");

Conversation

RealCLanger commented Feb 7, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Feb 7, 2025

Uh oh!

openjdk bot commented Feb 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk bot commented Feb 7, 2025

Uh oh!

mlbridge bot commented Feb 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

jerboaa Feb 7, 2025

Choose a reason for hiding this comment

Uh oh!

jerboaa Feb 7, 2025

Choose a reason for hiding this comment

Uh oh!

alexeysemenyukoracle Feb 7, 2025

Choose a reason for hiding this comment

Uh oh!

RealCLanger Feb 11, 2025

Choose a reason for hiding this comment

Uh oh!

RealCLanger commented Feb 13, 2025

Uh oh!

openjdk bot commented Feb 13, 2025

Uh oh!

openjdk bot commented Feb 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

RealCLanger commented Feb 7, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Feb 7, 2025 •

edited

Loading

mlbridge bot commented Feb 7, 2025 •

edited

Loading