On Mon, 2 Dec 2024 18:33:31 GMT, Mandy Chung <mch...@openjdk.org> wrote:
>> 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 15 additional >> commits since the last revision: >> >> - Merge branch 'master' into jdk-8343839-detect-patch-module >> - Use ModuleBootStrap for detecting patches >> - Revert "8344560: Add system property for patched runtime" >> >> This reverts commit 1d2395f39ee95a80937c63713e1f874ecc4ae76e. >> - Revert "Move and amend test" >> >> This reverts commit c27c874b4c722aa4cfa5f6c71f9231e92a30db0c. >> - Revert "Set the property to false for unpatched, expand tests" >> >> This reverts commit c635fdc0ad09c68ef652afb516c221b3c3a6299f. >> - Revert "Mention jdk.patched in System class" >> >> This reverts commit a11e26637d89d5ae840753eabcfe6deb2ac025be. >> - Merge branch 'jdk-8344560-jdk.patched-property' into >> jdk-8343839-detect-patch-module >> - Mention jdk.patched in System class >> - Set the property to false for unpatched, expand tests >> - Move and amend test >> - ... and 5 more: https://git.openjdk.org/jdk/compare/e9cac2e8...77695cea > > src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageFileCreator.java > line 226: > >> 224: } >> 225: String message = switch (e.getReason()) { >> 226: case PATCH_MODULE -> >> helper.getMessage("err.runtime.link.patched.module", e.getFile()); > > As `MODIFIED_FILE` is the only reason to throw RILE, the enum `Reason` is no > longer needed. This can be simplified. Yes, the updated patch has this removed. > src/jdk.jlink/share/classes/jdk/tools/jlink/internal/runtimelink/RuntimeImageLinkException.java > line 38: > >> 36: >> 37: public static enum Reason { >> 38: PATCH_MODULE, /* link exception due to patched module */ > > Given there is only one reason to throw `RuntimeImageLinkException`, I wonder > if `RuntimeImageLinkException` is still needed. Is RILE needed because > checking the mismatch is done in stream? Maybe > `JRTArchive::addNonClassResources` can return true/false to indicate if the > file is modified. The updated patch has RuntimeImageLinkException removed. UncheckedIOException is being used instead. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/22037#discussion_r1867818734 PR Review Comment: https://git.openjdk.org/jdk/pull/22037#discussion_r1867823637
