On Tue, 16 Jun 2026 15:46:20 GMT, Chen Liang <[email protected]> wrote:
>> Introduce a `PathResolution` to `ExplodedImage` to remove a lot of ad-hoc >> treatments of preview path discovery. Also some general comments and test >> additions. >> >> One interesting thing is the mainline jimage is not mapping >> `META-INF/preview/file` to `file` when preview is enabled; it does map >> `META-INF/preview/dir/file` to `dir/file`. Don't know if this is intended; >> exploded image performs the first mapping, which differs from jimage. >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > Chen Liang 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 three additional commits since > the last revision: > > - Review remarks > - Merge branch 'lworld' of https://github.com/openjdk/valhalla into > cleanup/lw-jrtfs > - Cleanup ExplodedImage and enhance SystemImageTest test/jdk/jdk/internal/jrtfs/SystemImageTest.java line 113: > 111: // Note: somehow this doesn't work for jimage if > file is in > 112: // root directory > 113: "!META-INF/preview/com/two.txt", The comment is very confusing. It's just a file that exists only when running with preview features enabled. My preference would be to not include tests that don't work with the run-time image, let's put them in the JBS issue to re-examine the jimage implementation. test/jdk/jdk/internal/jrtfs/SystemImageTest.java line 114: > 112: // root directory > 113: "!META-INF/preview/com/two.txt", > 114: // File cannot be discovered in any scenario Just say that this file is never found. test/jdk/jdk/internal/jrtfs/SystemImageTest.java line 247: > 245: assertDirContents(image, "/modules/modfoo/com/foo/bar", > "NormalBar.class"); > 246: > 247: // No preview activation The use of "activation" in the comments is very confusing. It's just checking that a file is not found. ------------- PR Review Comment: https://git.openjdk.org/valhalla/pull/2545#discussion_r3427463480 PR Review Comment: https://git.openjdk.org/valhalla/pull/2545#discussion_r3427465281 PR Review Comment: https://git.openjdk.org/valhalla/pull/2545#discussion_r3427474371
