8347831: Re-examine version check when cross linking by slowhog · Pull Request #28155 · openjdk/jdk

slowhog · 2025-11-05T17:41:44Z

This PR include build changes from @magicus and jlink change to verify the build signature.

Tested with local builds for MacOS and Linux as below shows that cross linking with same build is working while linking with different build failed with error message.

❯ export JAVA_HOME=./build/macosx-x86_64-server-fastdebug/images/jdk-bundle/jdk-26.jdk/Contents/Home

❯ java --version
openjdk 26-internal 2026-03-17
OpenJDK Runtime Environment (fastdebug build 26-internal-adhoc.hjen.JDK-8347831)
OpenJDK 64-Bit Server VM (fastdebug build 26-internal-adhoc.hjen.JDK-8347831, mixed mode, sharing)

❯ jlink --version
26-internal

❯ jlink --module-path ./build/linux-x86_64-server-release/images/jdk/jmods --add-modules java.base --output linux

❯ jlink --add-modules java.base --output macos

❯ jlink --module-path ~/linux/jdk-25.0.1/jmods --add-modules java.base --output linux25
Error: jlink build N/A-26-internal-adhoc.hjen.JDK-8347831-2026-03-17 does not match target java.base build N/A

❯ jlink --module-path /Library/Java/JavaVirtualMachines/jdk-25.jdk/Contents/Home/jmods --add-modules java.base --output macos25
Error: jlink build N/A-26-internal-adhoc.hjen.JDK-8347831-2026-03-17 does not match target java.base build N/A

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
Change requires CSR request JDK-8371575 to be approved

Issues

JDK-8347831: Re-examine version check when cross linking (Enhancement - P3)
JDK-8371575: Re-examine version check when cross linking (CSR)

Reviewers

Erik Joelsson (@erikj79 - Reviewer)
Alan Bateman (@AlanBateman - Reviewer)

Contributors

Magnus Ihse Bursie <ihse@openjdk.org>

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 28155

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

Using diff file

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

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-11-05T17:43:11Z

👋 Welcome back henryjen! 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-11-05T17:43:23Z

@slowhog 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:

8347831: Re-examine version check when cross linking

Co-authored-by: Magnus Ihse Bursie <ihse@openjdk.org>
Reviewed-by: erikj, alanb

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 5 new commits pushed to the master branch:

5627ff2: 8370766: JVM crashes when running compiler/exceptions/TestAccessErrorInCatch.java fails with -XX:+VerifyStack
ac0e6af: 6185110: Undefined behaviour of SampleModel for width, height < 0
153c567: 8370905: Update vm.defmeth tests to use virtual threads
... and 2 more: https://git.openjdk.org/jdk/compare/ca4ae8063edddda36fafafd06b9b1a88ffbf9d2e...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-11-05T17:44:33Z

@slowhog The following labels will be automatically applied to this pull request:

build
core-libs

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

mlbridge · 2025-11-05T17:47:38Z

Webrevs

erikj79

I'm not thrilled about the code duplication between Gensrc.gmk in java.base and and jdk.jlink. I think this should be extracted out into a common file in some way. Having 2 copies of the template file also seems suboptimal.

make/modules/java.base/Gensrc.gmk

AlanBateman · 2025-11-06T11:07:21Z

I'm puzzled as to why the resource has to be saved in the jdk.jlink module. When cross linking, jlink should only need to check the company/version of java.base in the current image vs. company/version of the java.base found on the module path.

…rison

src/java.base/share/classes/jdk/internal/misc/resources/release.txt.template

AlanBateman · 2025-11-07T11:01:43Z

This PR include build changes from @magicus and jlink change to verify the build signature.

In that case, you'll need to list Magnus (/contributor command).

AlanBateman

The updated version is much simpler, thank you. Just a few small comments on the jlink changes.

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java

erikj79

Build changes look good now, pending Alan's comment about location of the output file.

slowhog · 2025-11-07T16:26:14Z

/contributor @magicus

openjdk · 2025-11-07T16:26:43Z

@slowhog Syntax: /contributor (add|remove) [@user | openjdk-user | Full Name <email@address>]. For example:

/contributor add @openjdk-bot
/contributor add duke
/contributor add J. Duke <duke@openjdk.org>

User names can only be used for users in the census associated with this repository. For other contributors you need to supply the full name and email address.

slowhog · 2025-11-07T18:47:45Z

/contributor add @magicus

openjdk · 2025-11-07T18:49:38Z

@slowhog
Contributor Magnus Ihse Bursie <ihse@openjdk.org> successfully added.

AlanBateman · 2025-11-08T07:10:22Z

/csr

openjdk · 2025-11-08T07:10:55Z

@AlanBateman has indicated that a compatibility and specification (CSR) request is needed for this pull request.

@slowhog please create a CSR request for issue JDK-8347831 with the correct fix version. This pull request cannot be integrated until the CSR request is approved.

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java

jerboaa

I think this can be tested akin to JLinkOptionsTest by implementing a simple plugin that transforms the release.txt resource and uses --keep-packaged-modules and then attempts to perform a link on the resulting image. Similarly we could cover the case of release.txt missing by using the --exclude-resources plugin.

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java

src/jdk.jlink/share/classes/jdk/tools/jlink/resources/jlink.properties

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java

jerboaa · 2025-11-19T12:47:14Z

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java

+            }
+
+            try (var r = new BufferedReader(new InputStreamReader(release.get()))) {
+                return Optional.of(r.readLine());


As suggested on the CSR we should read the entire file, not just the first line and treat the entire content as a string in a specific encoding. While I wasn't able to get the JDK build a vendor name with new lines (due to jrt-fs.jar putting COMPANYNAME in the MANIFEST.MF) it's conceivable that some release.txt file might have more than one line.

Alternatively we need to specify that the file must not contain line breaks or unusual encodings.

The proposal is to have a one line release signature set up by the build, briefly discussed the possibility to use a java properties file format as the current release file distributed with JDK, but then a simplified approach of one signature is preferred.

I'll update the PR once we come to conclusion with the CSR.

It might be useful to see what breaks if a JDK were built with a newline in the vendor name, I assume other things will break.

Overall I think the changes are looking good, the main thing is that the error message is clear.

It might be useful to see what breaks if a JDK were built with a newline in the vendor name, I assume other things will break.

Writhing the manifest entries for jrt-fs.jar break (header validation fails). Since the vendor populates to that jar file.

The CSR is approved, and the one line signature is in the text.
And since other things break with newline in the vendor name, I assume that vendor name is also limited, so are we good with current implementation?
Are we suggesting to add error message when there is more than one line? I think current implementation is fine.

I don't think we need to be concerned with this right now.

AlanBateman · 2025-12-02T16:20:49Z

I think this can be tested akin to JLinkOptionsTest by implementing a simple plugin that transforms the release.txt resource and uses --keep-packaged-modules and then attempts to perform a link on the resulting image. Similarly we could cover the case of release.txt missing by using the --exclude-resources plugin.

Maybe we should have a follow-up issue to attempt to create a test for this. However, the bigger topic of testing the cross-linking scenario isn't really something we can do in a jtreg test. Instead it is something for "special testing", like the manual tests, as it needs builds from different platforms.

jerboaa · 2025-12-02T18:12:39Z

I think this can be tested akin to JLinkOptionsTest by implementing a simple plugin that transforms the release.txt resource and uses --keep-packaged-modules and then attempts to perform a link on the resulting image. Similarly we could cover the case of release.txt missing by using the --exclude-resources plugin.

Maybe we should have a follow-up issue to attempt to create a test for this.

That's OK with me. Creating jtreg tests verifying that it fails if the file doesn't match/does not exist can still be done.

erikj79

Build change looks ok.

slowhog · 2025-12-02T22:08:27Z

/integrate

openjdk · 2025-12-02T22:11:49Z

Going to push as commit 8f0cb57.
Since your change was applied there have been 13 commits pushed to the master branch:

37cd8d6: 8371501: Change IAE to NPE in java.awt.image.Kernel when data is null
8a28a76: 8372937: Abbreviate list of supported releases
0fe1ffd: 8372940: Update symbol data script references
... and 10 more: https://git.openjdk.org/jdk/compare/ca4ae8063edddda36fafafd06b9b1a88ffbf9d2e...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-12-02T22:12:48Z

@slowhog Pushed as commit 8f0cb57.

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

magicus and others added 2 commits November 5, 2025 09:34

Build changes to generate release.txt for java.base and jdk.jlink.

71e2343

JLink to check java build information

c311f09

openjdk bot added build build-dev@openjdk.org core-libs core-libs-dev@openjdk.org labels Nov 5, 2025

openjdk bot added the rfr Pull request is ready for review label Nov 5, 2025

erikj79 reviewed Nov 5, 2025

View reviewed changes

make/modules/java.base/Gensrc.gmk Outdated Show resolved Hide resolved

make/modules/java.base/Gensrc.gmk Outdated Show resolved Hide resolved

make/modules/java.base/Gensrc.gmk Outdated Show resolved Hide resolved

Use release.txt from java.base module in current and target for compa…

7df6e15

…rison

AlanBateman reviewed Nov 7, 2025

View reviewed changes

src/java.base/share/classes/jdk/internal/misc/resources/release.txt.template Show resolved Hide resolved

AlanBateman reviewed Nov 7, 2025

View reviewed changes

erikj79 reviewed Nov 7, 2025

View reviewed changes

Address review feedbacks

8df7e80

openjdk bot added the csr Pull request needs approved CSR before integration label Nov 8, 2025

Refactoring to clarify version checking cases

68584d9

jerboaa reviewed Nov 11, 2025

View reviewed changes

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java Outdated Show resolved Hide resolved

AlanBateman reviewed Nov 11, 2025

View reviewed changes

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java Outdated Show resolved Hide resolved

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java Outdated Show resolved Hide resolved

Treat missing release.txt as emptry release signature

0f1c935

jerboaa reviewed Nov 11, 2025

View reviewed changes

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java Outdated Show resolved Hide resolved

Update the error message when not finding release signature

fa30f2c

remove the extra space

5fb443b

jerboaa reviewed Nov 14, 2025

View reviewed changes

adapt review feedbacks

a3edf9a

jerboaa reviewed Nov 19, 2025

View reviewed changes

openjdk bot removed the csr Pull request needs approved CSR before integration label Dec 2, 2025

Merge openjdk/master

2641504

AlanBateman approved these changes Dec 2, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Dec 2, 2025

erikj79 approved these changes Dec 2, 2025

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label Dec 2, 2025

openjdk bot closed this Dec 2, 2025

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

Conversation

slowhog commented Nov 5, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issues

Reviewers

Contributors

Reviewing

Uh oh!

bridgekeeper bot commented Nov 5, 2025

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Webrevs

Uh oh!

erikj79 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AlanBateman commented Nov 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

AlanBateman commented Nov 7, 2025

Uh oh!

AlanBateman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

erikj79 left a comment

Choose a reason for hiding this comment

Uh oh!

slowhog commented Nov 7, 2025

Uh oh!

openjdk bot commented Nov 7, 2025

Uh oh!

slowhog commented Nov 7, 2025

Uh oh!

openjdk bot commented Nov 7, 2025

Uh oh!

AlanBateman commented Nov 8, 2025

Uh oh!

openjdk bot commented Nov 8, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jerboaa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jerboaa Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

slowhog Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

AlanBateman Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

jerboaa Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

slowhog Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

slowhog commented Nov 5, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Nov 5, 2025 •

edited

Loading

openjdk bot commented Nov 5, 2025 •

edited

Loading

mlbridge bot commented Nov 5, 2025 •

edited

Loading

AlanBateman commented Nov 6, 2025 •

edited

Loading

slowhog Dec 2, 2025 •

edited

Loading