On Wed, 5 Nov 2025 15:07:12 GMT, David Beaumont <[email protected]> wrote:
>> Adds support for writing preview related flags into jimage files.
>>
>> Preview mode is complex. It's not nearly as simple as "does something in
>> /modules/xxx/... have an entry in /modules/xxx/META-INF/preview/...".
>>
>> Specific issues include:
>>
>> Supporting preview-only resources without forcing a double lookup on
>> everything.
>> Changing the set of entries in /packages/xxx directories to account for
>> preview only packages in some modules.
>> Minimising the work done during image reader initialization to only need to
>> process the small number of preview resources (rather than scanning the
>> whole file to look for them).
>> The new flags added by this code address these issues, but calculating them
>> correctly with only minor adjustments to the existing code was not feasible,
>> it just became a lot larger and very complex.
>>
>> To address this, a new type (ModuleReference) is introduced to track and
>> then merge information about packages seen in each module. This allows a
>> much simpler inner loop for processing resource paths when building the node
>> tree, combined with a subsequent merging stage to produce the final package
>> information for each module.
>>
>> Not that since ModuleReference is needed during jimage reading, that class
>> is already present in the previous PR on which this is based, but it starts
>> to be used to calculate the module flags in this PR.
>>
>> This PR can also adds the ImageReader unit tests for preview mode, which
>> rely on being able to generate jimage files with preview mode flags in.
>
> David Beaumont has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Minor feedback changes
Changes requested by rriggs (Committer).
test/jdk/jdk/internal/jimage/ImageReaderTest.java line 251:
> 249:
> 250: // Preview version of classes either overwrite existing
> entries or are added to directories.
> 251: assertEquals("Preview: com.foo.HasPreviewVersion",
> loader.loadAndGetToString("modfoo", "com.foo.HasPreviewVersion"));
This test fails:
[12:00:51.621] STARTED ImageReaderTest::testPreviewResources_enabled
'testPreviewResources_enabled()'
org.opentest4j.AssertionFailedError: expected: <Preview:
com.foo.HasPreviewVersion> but was: <Class: com.foo.HasPreviewVersion>
at
org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
at
org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
at
org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
at
org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
at
org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:177)
at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1145)
at
ImageReaderTest.testPreviewResources_enabled(ImageReaderTest.java:251)
test/jdk/jdk/internal/jimage/ImageReaderTest.java line 310:
> 308: try (ImageReader reader = ImageReader.open(image,
> PreviewMode.ENABLED)) {
> 309: // In preview mode there is a new preview-only module
> visible.
> 310: assertDirContents(reader, "/packages/com.bar", "modbar",
> "modgus");
This test fails, not clear if its the impl or the test.
org.opentest4j.AssertionFailedError: Unexpected child names in directory
'/packages/com.bar' ==> expected: <[modbar, modgus]> but was: <[modbar]>
at
org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
at
org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
at
org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
at
org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1156)
at ImageReaderTest.assertDirContents(ImageReaderTest.java:362)
at
ImageReaderTest.testPreviewModeLinks_enabled(ImageReaderTest.java:310)
-------------
PR Review:
https://git.openjdk.org/valhalla/pull/1718#pullrequestreview-3423380755
PR Review Comment:
https://git.openjdk.org/valhalla/pull/1718#discussion_r2495429041
PR Review Comment:
https://git.openjdk.org/valhalla/pull/1718#discussion_r2495425964
