RFR: 8304006: jlink should create the jimage file in the native endian for the target platform [v16]

[Alan Bateman]

On Tue, 28 Mar 2023 06:28:17 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> Can I please get a review for this change which proposes to fix the issue reported in https://bugs.openjdk.org/browse/JDK-8206890?
>> 
>> The `jlink` command allows a `--endian` option to specify the byte order in the generated image. Before this change, when such a image was being launched, the code would assume the byte order in the image to be the native order of the host where the image is being launched. That would result in failure to launch java, as noted in the linked issue.
>> 
>> The commit in this PR, changes relevant places to not assume native order and instead determine the byte order by reading the magic bytes in the image file's header content.
>> 
>> A new jtreg test has been added which reproduces the issue and verifies the fix.
>
> Jaikiran Pai has updated the pull request incrementally with one additional commit since the last revision:
> 
>   use test.jdk system property in test

test/jdk/tools/jlink/plugins/CDSPluginTest.java line 93:

> 91:            System.out.println("---- Test different platforms scenario ----");
> 92:            String jlinkPath = JDKToolFinder.getJDKTool("jlink");
> 93:            // copy over the java.base and java.logging module file to a temporary directory

This should say that it copies the packaged modules for java.base and java.logging.

test/jdk/tools/jlink/plugins/CDSPluginTest.java line 97:

> 95:            // separate --module-path will force the JLink task to read the ModuleTarget from
> 96:            // the java.base module-info.class to identify the target platform for the image
> 97:            // being generated.

I'm a bit uncomfortable with change the test for the CDS plugin as part of this change but can live with it.

test/jdk/tools/jlink/plugins/CDSPluginTest.java line 125:

> 123:     }
> 124: 
> 125:     private static void copyModuleFiles(Path jdkRoot, Path targetDir, String[] modules)

This method copies "packaged modules" so I think should be renamed to copyPackagedModules.

test/jdk/tools/jlink/plugins/CDSPluginTest.java line 136:

> 134:             if (!Files.exists(copy)) {
> 135:                 throw new AssertionError("Could not copy '" + module
> 136:                         + "' module file to directory: " + targetDir);

Files.copy throws if it fails so no need for the Files.exists check here.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/11943#discussion_r1150410251
PR Review Comment: https://git.openjdk.org/jdk/pull/11943#discussion_r1150411725
PR Review Comment: https://git.openjdk.org/jdk/pull/11943#discussion_r1150409630
PR Review Comment: https://git.openjdk.org/jdk/pull/11943#discussion_r1150409681