RFR: 8368475: [lworld] Add preview classes to jimage at make time [v3]

Tue Nov 4 18:00:32 UTC 2025

On Tue, 4 Nov 2025 17:42:24 GMT, David Beaumont <duke at openjdk.org> wrote:

>> Copies valuetype classes for each module into corresponding "/META-INF/preview/..." sub-directories to be pulled into jimage files and processed by the new preview mode handling code.
>> 
>> There might be a better way to do this in terms of Makefile semantics, but this seems to work well enough and doesn't prevent the value-class JAR files being generated for patching (which is still how everyone will get value classes until the rest of the work is plumbed in).
>> 
>> To enable the new preview mode work, set the "DISABLE_PREVIEW_PATCHING" system property to "true".
>
> David Beaumont has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision:
> 
>   Rollup of makefile changes and jlink fix (temp).
>   
>   * likely test fix
>   * Copy value classes into preview directories for inclusion in jimage

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Archive.java line 88:

> 86:         }
> 87: 
> 88:         /**

I'm not 100% sure why this method didn't exist before. This whole base class is a bit of a mystery really since some of its fields are only used by some subclasses and others by others. Feels like it would be just as good to make it an interface for all the value it has (and the lack of docs about what the fields actually do/are doesn't help).

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageFileCreator.java line 576:

> 574:     }
> 575: 
> 576:     /**

Unused now. There was once use of 'toPackage()' but that went away.

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JRTArchive.java line 197:

> 195:                                          .filter(rd -> rd.getKind() == ResourceDiff.Kind.REMOVED)
> 196:                                          .map(s -> {
> 197:                                                  int secondSlash = s.getName().indexOf("/", 1);

No need for any string munging now because the new API uses the same "full entry name" value everywhere.

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JRTArchive.java line 447:

> 445:                     @Override
> 446:                     public long size() {
> 447:                         return archive.imageResources.sizeOf(path());

*if* we didn't want a new 'path()' method in Entry, I could always capture 'resPath' here from the record, but using the actual value in the base class makes the Entry more self contained and avoid it being a hybrid of both "values from the base class" and "values from the outer class", which is just confusing.

Alternatively, make Entry an interface and just capture what's needed for each implementation (since you much have that available when Entry is made).

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JRTArchive.java line 456:

> 454:                 public long size() {
> 455:                     try {
> 456:                         if (resType != EntryType.CLASS_OR_RESOURCE) {

Having all this complexity inside the provided methods was never necessary. There are 3 types of behaviour to support, and each entry is always just 1 of them, determined by data available when it's constructed.

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java line 352:

> 350: 
> 351:         // First create the image provider
> 352:         try (ImageHelper imageProvider =

Wrap in a try-with-resources because this is where Archive lifetimes are bounded (by the ImageHelper instance).
Right now it's not doing much to improve things (Archive.close() is actually being called now to free a few things, but the resource entry API isn't closed because it's a singleton.
However, eventually, ImageHelper is likely the proper owner for a non-singleton resource API.

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ResourcePoolManager.java line 154:

> 152:          *     if no name could be inferred.
> 153:          */
> 154:         private static Optional<String> inferPackageName(ResourcePoolEntry res) {

This change is related to the need to reflect preview-only packages in the modules list.
It's different to the resource entry stuff however, but also needed. I'll figure out what PR this really goes in later.

test/jdk/tools/jlink/ResourcePoolTest.java line 166:

> 164:         Optional<ResourcePoolModule> modBase = manager.moduleView().findModule("java.base");
> 165:         assertTrue(modBase.isPresent());
> 166:         // Preview only package is included, and no packages start with 'META-INF'.

Test for the behaviour implemented in ResourcePoolManager.

test/jdk/tools/jlink/runtimeImage/PackagedModulesVsRuntimeImageLinkTest.java line 148:

> 146:     }
> 147: 
> 148:     // Helper to assert the content of two jimage files are the same and provide

Having the test fail with a one line error and no information was a bad experience, so I made it actually report the difference clearly. Seems worth submitting this.

test/jdk/tools/jlink/runtimeImage/PackagedModulesVsRuntimeImageLinkTest.java line 189:

> 187:     }
> 188: 
> 189:     private static boolean isTreeInfoResource(String path) {

This logic is used repeatedly around the modules code, and it's wrong.
You must account for paths starting `/modulesfoo/...` existing and *not* being an "info resource".

-------------

PR Review Comment: https://git.openjdk.org/valhalla/pull/1719#discussion_r2491503992
PR Review Comment: https://git.openjdk.org/valhalla/pull/1719#discussion_r2491506972
PR Review Comment: https://git.openjdk.org/valhalla/pull/1719#discussion_r2491512106
PR Review Comment: https://git.openjdk.org/valhalla/pull/1719#discussion_r2491535167
PR Review Comment: https://git.openjdk.org/valhalla/pull/1719#discussion_r2491540963
PR Review Comment: https://git.openjdk.org/valhalla/pull/1719#discussion_r2491551206
PR Review Comment: https://git.openjdk.org/valhalla/pull/1719#discussion_r2491560803
PR Review Comment: https://git.openjdk.org/valhalla/pull/1719#discussion_r2491565398
PR Review Comment: https://git.openjdk.org/valhalla/pull/1719#discussion_r2491569432
PR Review Comment: https://git.openjdk.org/valhalla/pull/1719#discussion_r2491573889