8353185: Introduce the concept of upgradeable files in context of JEP 493 #24388

jerboaa · 2025-04-02T18:39:57Z

For JEP 493-enabled builds there are no JMODs. Certain files come from the installed JDK image when a user creates a custom run-time from it. This is problematic for example for files that often come from a different package (e.g. cacerts file for Linux distro builds of OpenJDK packaged as RPMs), or more generally when they get updated out-of-band of the JDK build itself like the tzupdater tool.

When that happens the hash sum recorded at JDK build time of those files no longer match, causing jlink to fail. I propose to allow for those files to get "upgraded" should this happen. The way this works is as follows:

The list of upgradeable files is configured by a resource file in jdk.jlink on a per module basis. Right now, only two files from the java.base module will be allowed to be upgraded with a link from the current run-time image.
For those files the hash sum checks are skipped.

Testing

GHA
jdk/tools/jlink jtreg tests (also on GHA)
Some manual tests with updated tzdb.dat and cacerts files.

Thoughts?

Progress

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

Issue

JDK-8353185: Introduce the concept of upgradeable files in context of JEP 493 (Enhancement - P3)

Reviewers

Christoph Langer (@RealCLanger - Reviewer)
Magnus Ihse Bursie (@magicus - Reviewer)
Alan Bateman (@AlanBateman - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 24388

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

Using diff file

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

Using Webrev

Link to Webrev Comment

… 493

bridgekeeper · 2025-04-02T18:40:25Z

👋 Welcome back sgehwolf! 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-04-02T18:41:09Z

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

8353185: Introduce the concept of upgradeable files in context of JEP 493

Reviewed-by: clanger, ihse, 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 162 new commits pushed to the master branch:

cf0308b: 8351152: x86: Remove code blocks that handle UseSSE < 2
d8bed69: 8354317: [XWayland] Problem list two tests crashing XWayland server
f955a8c: 8352687: Opensource few JInternalFrame and JTextField tests
... and 159 more: https://git.openjdk.org/jdk/compare/3ceabf0f647beb4943c06709aa8797f7511cd48e...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-04-02T18:41:39Z

@jerboaa 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-04-02T18:43:57Z

Webrevs

ecki · 2025-04-03T06:12:35Z

What about changes to conf files, especially Java.security (for hardening TLS settings) - or at least pointing to a include?

AlanBateman · 2025-04-03T06:31:20Z

What about changes to conf files, especially Java.security (for hardening TLS settings) - or at least pointing to a include?

The opposite is where weaker or insecure algorithms are enabled, wouldn't want that copied by jlink into another run-time image.

ecki · 2025-04-03T06:46:35Z

Well, shipping customized runtimes sounds like a valid usecase, but yes maybe it’s more important for jpackage.

jerboaa · 2025-04-03T08:44:08Z

What about changes to conf files, especially Java.security (for hardening TLS settings) - or at least pointing to a include?

You can still do that after creating the custom runtime as you do today when linking using JMODs. The point of this fix is to be able to have a way to still run jlink based on the run-time image when you're using cacerts file from an external location (outside the JDKs control) or have updated tzdata (also outside the JDK build). I wouldn't expect for this to be needed in many (most?) cases.

AlanBateman · 2025-04-03T09:46:38Z

Part of me is concerned that the hidden option is a bit of an attractive nuisance. What would you think about just having a fixed list in a properties file in the repo, thus a resource in the jlink module. This would avoid the list in JRTArchive too. It wouldn't rule out introducing something more in the future.

Right now the only examples we have are in java.base. In time it might be that some other modules might need something.

I've like to know more about cacerts. Are the distros swapping this to a link after the run-time image is created or is it that the sym link is there from the startup and the hashing has followed the link?

jerboaa · 2025-04-03T10:25:28Z

Part of me is concerned that the hidden option is a bit of an attractive nuisance. What would you think about just having a fixed list in a properties file in the repo, thus a resource in the jlink module. This would avoid the list in JRTArchive too. It wouldn't rule out introducing something more in the future.

I'm not entirely sure what you are suggesting. Is it keeping a list of "upgradeable" files in a properties file. Files listed in that properties file aren't checked for hash sums (i.e. even if it's not modified)? That is, the explicit opt-in is not needed? Fine by me, but it's a weaker check. If we don't need the explicit opt-in, the patch becomes simpler as well.

Right now the only examples we have are in java.base. In time it might be that some other modules might need something.

Sure. I've considered that. The java.base specific check can go away and it should work for any module as long as we have a list of what's considered an upgradable file (wherever it comes from).

I've like to know more about cacerts. Are the distros swapping this to a link after the run-time image is created or is it that the sym link is there from the startup and the hashing has followed the link?

Yes, that's one case. For example Fedora/RHEL distros have a ca-certificates package which also provides a cacerts file consumable from the JDK (/etc/pki/java/cacerts). This can be a build-time option, or replaced with a symlink after the JDK has been built.

Another use case is amending the cacerts store. For example with an enterprise internal CA, in a container. It's really not different to tzdb.dat. That too, can be a symlink to a system package or get updated with an external tool after a build.

The option of not following symlinks, isn't an option, though, as certain distro packaging standards require one to install, say man pages - in a specific directory. To keep the JDK image whole, that is usually fixed by placing a symlink back in the JDK image directory.

AlanBateman · 2025-04-04T07:55:20Z

I'm not entirely sure what you are suggesting. Is it keeping a list of "upgradeable" files in a properties file. Files listed in that properties file aren't checked for hash sums (i.e. even if it's not modified)? That is, the explicit opt-in is not needed? Fine by me, but it's a weaker check. If we don't need the explicit opt-in, the patch becomes simpler as well.

Yes, I think keep simple. We always want to allow tzdb.dat be upgraded by the TZ updater tool. I think treating cacerts the same way is okay. As you note, it has to be kept up to date too. I was thinking keytool import and wasn't sure if the Linux distros configure with -with-cacerts-file or did something else. Thanks for the clarification on this point.

Starting with a simple list of two files won't preclude us from re-visiting it again in the future.

jerboaa · 2025-04-04T08:21:52Z

I'm not entirely sure what you are suggesting. Is it keeping a list of "upgradeable" files in a properties file. Files listed in that properties file aren't checked for hash sums (i.e. even if it's not modified)? That is, the explicit opt-in is not needed? Fine by me, but it's a weaker check. If we don't need the explicit opt-in, the patch becomes simpler as well.

Yes, I think keep simple. We always want to allow tzdb.dat be upgraded by the TZ updater tool. I think treating cacerts the same way is okay. As you note, it has to be kept up to date too. I was thinking keytool import and wasn't sure if the Linux distros configure with -with-cacerts-file or did something else. Thanks for the clarification on this point.

Starting with a simple list of two files won't preclude us from re-visiting it again in the future.

OK. Thanks. I'll update it then.

…t of JEP 493" This reverts commit bfbfbcb.

jerboaa · 2025-04-04T15:24:51Z

@AlanBateman Updated the patch. Please take another look. Thanks!

jerboaa · 2025-04-04T15:25:20Z

/label add build-dev

openjdk · 2025-04-04T15:26:02Z

@jerboaa
The build label was successfully added.

jerboaa · 2025-04-04T15:32:02Z

It took me a while to find the magic trick to get the jlink resource file copied, so adding build-dev for their input too.

RealCLanger

Just a few fly-by findings, no full review.

I see that you're actively on the upgradeable files. What about #24190? (@AlanBateman, what are your thoughts, how could we progress there?)

make/modules/jdk.jlink/Java.gmk

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

test/jdk/tools/jlink/runtimeImage/UpgradeableFileCacertsTest.java

jerboaa · 2025-04-07T08:52:14Z

I see that you're actively on the upgradeable files. What about #24190?

Let's keep the discussion on #24190, please.

test/jdk/tools/jlink/runtimeImage/UpgradeableFileCacertsTest.java

AlanBateman · 2025-04-07T17:30:53Z

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

@@ -62,7 +68,38 @@ public static boolean isLinkableRuntime() {

    private static InputStream getDiffInputStream(String module) throws IOException {
        String resourceName = String.format(DIFF_PATTERN, module);
-        return LinkableRuntimeImage.class.getModule().getResourceAsStream(resourceName);
+        return JDK_JLINK_MOD.getResourceAsStream(resourceName);


FYI you can use LinkableRuntimeImage.class.getResourceAsStream here as the resource is in the current module.

It's very odd, but when I attempt this then the resource is not found. It seems to fail on the module name verification. For example: jlink --help | tail -n2 shows as disabled for an enabled linkable runtime image.

Without reproducing, I can't immediately say why Class.getResourceAsStream would fail here. This is a good API for locating resources in the caller's module.

I'll try to find a reproducer outside this PR and will let you know if I find anything.

make/modules/jdk.jlink/Java.gmk

magicus

Build changes look good now. Thanks!

/reviewers 2

openjdk · 2025-04-09T20:24:30Z

@magicus
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

jerboaa · 2025-04-10T08:42:58Z

@AlanBateman Any more thoughts on this? Thanks!

jerboaa · 2025-04-10T09:13:08Z

@RealCLanger Are you OK with the latest patch (as far as your comments are concerned)?

RealCLanger · 2025-04-11T07:25:07Z

@RealCLanger Are you OK with the latest patch (as far as your comments are concerned)?

Yes, looks good. 😄

jerboaa · 2025-04-15T10:15:15Z

/integrate

openjdk · 2025-04-15T10:16:35Z

Going to push as commit 4e24dc0.
Since your change was applied there have been 208 commits pushed to the master branch:

d7676c3: 8354508: JFR: Strengthen metadata checks for labels
81d4c80: 8354507: [ubsan] subnode.cpp:406:36: runtime error: negation of -9223372036854775808 cannot be represented in type 'long int'
24be888: 8351833: Unexpected increase in live nodes when splitting Phis through MergeMems in PhiNode::Ideal
... and 205 more: https://git.openjdk.org/jdk/compare/3ceabf0f647beb4943c06709aa8797f7511cd48e...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-04-15T10:16:48Z

@jerboaa Pushed as commit 4e24dc0.

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

8353185: Introduce the concept of upgradeable files in context of JEP…

bfbfbcb

… 493

openjdk bot added the rfr label Apr 2, 2025

openjdk bot added the core-libs label Apr 2, 2025

jerboaa changed the title ~~8353185: Introduce the concept of upgradeable files in context of JEP493~~ 8353185: Introduce the concept of upgradeable files in context of JEP 493 Apr 2, 2025

Merge branch 'master' into jdk-8353185-upgradable-files-jep493

f709c33

jerboaa added 2 commits April 4, 2025 10:50

Revert "8353185: Introduce the concept of upgradeable files in contex…

d944753

…t of JEP 493" This reverts commit bfbfbcb.

Reboot upgradeable files approach

74b6bfe

openjdk bot added the build label Apr 4, 2025

RealCLanger reviewed Apr 5, 2025

View reviewed changes

Review comments from Christoph

fe33cfd

RealCLanger reviewed Apr 7, 2025

View reviewed changes

test/jdk/tools/jlink/runtimeImage/UpgradeableFileCacertsTest.java Outdated Show resolved Hide resolved

Review v2

89d3886

AlanBateman reviewed Apr 7, 2025

View reviewed changes

magicus reviewed Apr 9, 2025

View reviewed changes

make/modules/jdk.jlink/Java.gmk Outdated Show resolved Hide resolved

Use *.conf for upgrade files

4434c78

magicus approved these changes Apr 9, 2025

View reviewed changes

openjdk bot added the ready label Apr 9, 2025

openjdk bot removed the ready label Apr 9, 2025

AlanBateman approved these changes Apr 10, 2025

View reviewed changes

openjdk bot added the ready label Apr 10, 2025

RealCLanger approved these changes Apr 11, 2025

View reviewed changes

openjdk bot added the integrated label Apr 15, 2025

openjdk bot closed this Apr 15, 2025

openjdk bot removed ready rfr labels Apr 15, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8353185: Introduce the concept of upgradeable files in context of JEP 493 #24388

8353185: Introduce the concept of upgradeable files in context of JEP 493 #24388

jerboaa commented Apr 2, 2025 •

edited by openjdk bot

Loading

bridgekeeper bot commented Apr 2, 2025

openjdk bot commented Apr 2, 2025 •

edited

Loading

openjdk bot commented Apr 2, 2025

mlbridge bot commented Apr 2, 2025 •

edited

Loading

ecki commented Apr 3, 2025

AlanBateman commented Apr 3, 2025

ecki commented Apr 3, 2025

jerboaa commented Apr 3, 2025

AlanBateman commented Apr 3, 2025

jerboaa commented Apr 3, 2025

AlanBateman commented Apr 4, 2025

jerboaa commented Apr 4, 2025

jerboaa commented Apr 4, 2025

jerboaa commented Apr 4, 2025

openjdk bot commented Apr 4, 2025

jerboaa commented Apr 4, 2025

RealCLanger left a comment

jerboaa commented Apr 7, 2025

AlanBateman Apr 7, 2025

jerboaa Apr 8, 2025

AlanBateman Apr 10, 2025

jerboaa Apr 10, 2025

magicus left a comment

openjdk bot commented Apr 9, 2025

jerboaa commented Apr 10, 2025

jerboaa commented Apr 10, 2025

RealCLanger commented Apr 11, 2025

jerboaa commented Apr 15, 2025

openjdk bot commented Apr 15, 2025

openjdk bot commented Apr 15, 2025

8353185: Introduce the concept of upgradeable files in context of JEP 493 #24388

8353185: Introduce the concept of upgradeable files in context of JEP 493 #24388

Conversation

jerboaa commented Apr 2, 2025 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

bridgekeeper bot commented Apr 2, 2025

openjdk bot commented Apr 2, 2025 • edited Loading

openjdk bot commented Apr 2, 2025

mlbridge bot commented Apr 2, 2025 • edited Loading

Webrevs

ecki commented Apr 3, 2025

AlanBateman commented Apr 3, 2025

ecki commented Apr 3, 2025

jerboaa commented Apr 3, 2025

AlanBateman commented Apr 3, 2025

jerboaa commented Apr 3, 2025

AlanBateman commented Apr 4, 2025

jerboaa commented Apr 4, 2025

jerboaa commented Apr 4, 2025

jerboaa commented Apr 4, 2025

openjdk bot commented Apr 4, 2025

jerboaa commented Apr 4, 2025

RealCLanger left a comment

Choose a reason for hiding this comment

jerboaa commented Apr 7, 2025

AlanBateman Apr 7, 2025

Choose a reason for hiding this comment

jerboaa Apr 8, 2025

Choose a reason for hiding this comment

AlanBateman Apr 10, 2025

Choose a reason for hiding this comment

jerboaa Apr 10, 2025

Choose a reason for hiding this comment

magicus left a comment

Choose a reason for hiding this comment

openjdk bot commented Apr 9, 2025

jerboaa commented Apr 10, 2025

jerboaa commented Apr 10, 2025

RealCLanger commented Apr 11, 2025

jerboaa commented Apr 15, 2025

openjdk bot commented Apr 15, 2025

openjdk bot commented Apr 15, 2025

jerboaa commented Apr 2, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Apr 2, 2025 •

edited

Loading

mlbridge bot commented Apr 2, 2025 •

edited

Loading