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

[Roger Riggs]

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

Copyright updates needed in: Archive, JImageTask, JRTArchive, JlinkTask, ResorucePoolTest, PackageModulesVsRuntimeImageLinkTest.

src/java.base/share/classes/jdk/internal/jimage/BasicImageReader.java line 365:

> 363:                 if (name.startsWith("/modules/") || name.startsWith("/packages/")) {
> 364:                     throw new IllegalArgumentException("Invalid entry name: " + name);
> 365:                 }

This can be covered by the == null case below. The distinguished exception isn't necessary.

src/java.base/share/classes/jdk/internal/jimage/ResourceEntries.java line 11:

> 9:  * <p>This API is designed only for use by the jlink classes, which manipulate
> 10:  * jimage files directly. For inspection of runtime resources, it is vital that
> 11:  * {@code previewMode} is correctly observed, making this API unsuitable.

"which manipulate jimage files directly" -> "read the raw jimage files". It should be clear that the directory structure is exactly that of the jimage file itself.

The 2nd part could be more clearly stated as use API xxx to access the classes and resources of a runtime depending on the preview mode.

src/java.base/share/classes/jdk/internal/jimage/ResourceEntries.java line 24:

> 22:      * Returns the full entry names for all resources in the given module, in
> 23:      * random order. Entry names will always be prefixed by the given module
> 24:      * name (e.g. "/<module-name/...").

Are these names useful with any ImageReader api other than ResourceEntries.sizeOf or open?

src/java.base/share/classes/jdk/internal/jimage/ResourceEntries.java line 26:

> 24:      * name (e.g. "/<module-name/...").
> 25:      */
> 26:     Stream<String> entryNamesIn(String module);

A cleaner name would be `entryNames`.

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

> 89:          * Returns the path of this entry.
> 90:          */
> 91:         public final String path() {return path;}

Typical style would expand to:

Suggestion:

        public final String path() {
            return path;
        }

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

> 303: 
> 304:         /**
> 305:          *  line: {@code <int>|<int>|<hashOrTarget>|<path>}

For internal doc/methods, javadoc markup makes the source harder to read.

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

> 459:                             // hashOrTarget field
> 460:                             if (symlink) {
> 461:                                 return Files.size(BASE.resolve(sha));

The hashes were present to ensure the integrity of the dependent resources. 
With them removed, what ensures the integrity of the dependencies?

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

> 1: /*

Will need a copyright update.

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

> 363:             ImagePluginStack stack = ImagePluginConfiguration.parseConfiguration(plugins);
> 364: 
> 365:             //Ask the stack to proceed;

Add a space after //

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

> 506:             stack.operate(imageProvider);
> 507:         }
> 508: 

Extra blank line.

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

> 1056:         public void close() throws IOException {
> 1057:             for (Archive archive : archives) {
> 1058:                 archive.close();

Some Archive implementations may throw; a try-catch is needed to make sure all archives are closed.
Extra points for accumulating an extra exceptions as suppressed exceptions on the first exception.
(Though it seems unlikely).

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

PR Review: https://git.openjdk.org/valhalla/pull/1719#pullrequestreview-3430787891
PR Review Comment: https://git.openjdk.org/valhalla/pull/1719#discussion_r2501053477
PR Review Comment: https://git.openjdk.org/valhalla/pull/1719#discussion_r2501068207
PR Review Comment: https://git.openjdk.org/valhalla/pull/1719#discussion_r2501070622
PR Review Comment: https://git.openjdk.org/valhalla/pull/1719#discussion_r2501071692
PR Review Comment: https://git.openjdk.org/valhalla/pull/1719#discussion_r2501076708
PR Review Comment: https://git.openjdk.org/valhalla/pull/1719#discussion_r2501101521
PR Review Comment: https://git.openjdk.org/valhalla/pull/1719#discussion_r2501123085
PR Review Comment: https://git.openjdk.org/valhalla/pull/1719#discussion_r2501082503
PR Review Comment: https://git.openjdk.org/valhalla/pull/1719#discussion_r2501086848
PR Review Comment: https://git.openjdk.org/valhalla/pull/1719#discussion_r2501089017
PR Review Comment: https://git.openjdk.org/valhalla/pull/1719#discussion_r2501095048