On Fri, 4 Apr 2025 15:22:10 GMT, Severin Gehwolf <sgehw...@openjdk.org> wrote:
>> 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:
>>
>> 1. 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.
>> 2. For those files the hash sum checks are skipped.
>>
>> **Testing**
>>
>> - [x] GHA
>> - [x] `jdk/tools/jlink` jtreg tests
>> - [x] Some manual tests with updated `tzdb.dat` and `cacerts` files.
>>
>> Thoughts?
>
> 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 four additional
> commits since the last revision:
>
> - Reboot upgradeable files approach
> - Revert "8353185: Introduce the concept of upgradeable files in context of
> JEP 493"
>
> This reverts commit bfbfbcb8212ed0f9825549b02b4b52e930c379a7.
> - Merge branch 'master' into jdk-8353185-upgradable-files-jep493
> - 8353185: Introduce the concept of upgradeable files in context of JEP 493
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 line 29:
> 27:
> 28: COPY += \
> 29:
> $(TOPDIR)/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/runtimelink/upgrade_files_java.base
Wouldn't `COPY += upgrade_files_java.base` work here ?
src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JRTArchive.java line 250:
> 248:
> 249: /**
> 250: * Certain files in the a module are considered upgradeable. That is,
Suggestion:
* Certain files in a module are considered upgradeable. That is,
src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JRTArchive.java line 254:
> 252: *
> 253: * @param resPath The resource path of the file to check for
> upgradeability.
> 254: * @return {@code true} iff the file is upgradeable. {@code false}
> otherwise.
Suggestion:
* @return {@code true} if the file is upgradeable. {@code false} otherwise.
test/jdk/tools/jlink/runtimeImage/UpgradeableFileCacertsTest.java line 43:
> 41: * @test
> 42: * @summary Verify warnings are being produced when linking from the
> run-time
> 43: * image and files have been modified
I guess this summary has to be modified.
-------------
PR Review: https://git.openjdk.org/jdk/pull/24388#pullrequestreview-2744611775
PR Review Comment: https://git.openjdk.org/jdk/pull/24388#discussion_r2029696671
PR Review Comment: https://git.openjdk.org/jdk/pull/24388#discussion_r2029696836
PR Review Comment: https://git.openjdk.org/jdk/pull/24388#discussion_r2029696899
PR Review Comment: https://git.openjdk.org/jdk/pull/24388#discussion_r2029697243
