RFR: 8351073: [macos] jpackage produces invalid Java runtime DMG bundles [v2]

[Alexey Semenyuk]

On Mon, 7 Jul 2025 23:20:08 GMT, Alexander Matveev <almatvee at openjdk.org> wrote:

>> Re-submission of https://github.com/openjdk/jdk/pull/25314 after merge conflict was resolved. Old PR will be closed.
>> 
>> All comments are addressed from old PR. Merge conflict was significant, so it is like new fix.
>> 
>> Fixed jpackage to produce valid Java runtimes based on description below:
>> 
>> Definitions:
>> 
>> - JDK bundle defined as bundle which contains "Contents/Home", "Contents/MacOS/libjli.dylib" and "Contents/Info.plist".
>> - Signed JDK bundle contains all files as JDK bundle + "Contents/_CodeSignature".
>> - JDK image defined as content of "Contents/Home".
>> - Signed JDK image does not exist, since it cannot be signed as bundle.
>> 
>> jpackage output based on input:
>> 
>> 1. "--runtime-image" points to unsigned JDK bundle and --mac-sign is not provided:
>> - jpackage will copy all files as is from provided path and run ad-hoc codesign.
>> 
>> 2. "--runtime-image" points to unsigned JDK bundle and --mac-sign is provided:
>> - jpackage will copy all files as is from provided path and run codesign with appropriate certificate based on same logic as we do for application image.
>>  
>> 3. "--runtime-image" points to signed JDK bundle and --mac-sign is not provided:
>> - jpackage will copy all files as is from provided path including "Contents/_CodeSignature" to preserve signing.
>> 
>> 4. "--runtime-image" points to signed JDK bundle and --mac-sign is provided:
>> - jpackage will copy all files as is from provided path including "Contents/_CodeSignature" and will re-sign bundle with appropriate certificate.
>> 
>> 5. "--runtime-image" points to JDK image and --mac-sign is not provided:
>>  - jpackage will check for libjli.dylib presence in "lib" folder.
>>  - Create JDK bundle by putting all files from provided path to "Contents/Home", libjli.dylib from "lib" to "Contents/MacOS/libjli.dylib" and create default "Contents/Info.plist" similar to what we do for runtime in application image.
>> - Ad-hoc signing will done.
>> 
>> 6. "--runtime-image" points to JDK image and --mac-sign is provided:
>> - 2 first steps from 5 and certificate signing will be done.
>> 
>> Additional changes:
>> - The bundle's top directory name will have the ".jdk" suffix.
>
> Alexander Matveev 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 two additional commits since the last revision:
> 
>  - Merge remote-tracking branch 'upstream/master' into JDK-8351073-2
>  - 8351073: [macos] jpackage produces invalid Java runtime DMG bundles

test/jdk/tools/jpackage/macosx/SigningRuntimeImagePackageTest.java line 134:

> 132:     }
> 133: 
> 134:     private static Path getRuntimeImagePath(boolean useJDKBundle,

The name of the function is misleading. It doesn't get the runtime image. It creates it.

test/jdk/tools/jpackage/macosx/SigningRuntimeImagePackageTest.java line 188:

> 186: 
> 187:             MacHelper.withExplodedDmg(dummyCMD, dmgImage -> {
> 188:                 if (dmgImage.endsWith(dummyCMD.name() + ".jdk")) {

Can this test ever return `false`?

test/jdk/tools/jpackage/macosx/SigningRuntimeImagePackageTest.java line 189:

> 187:             MacHelper.withExplodedDmg(dummyCMD, dmgImage -> {
> 188:                 if (dmgImage.endsWith(dummyCMD.name() + ".jdk")) {
> 189:                     Executor.of("cp", "-R")

Why noit use `FileUtils.copyRecursive()`?

test/jdk/tools/jpackage/macosx/SigningRuntimeImagePackageTest.java line 222:

> 220:                             SigningBase.CertIndex JDKBundleCert,
> 221:                             SigningBase.CertIndex signCert) throws Exception {
> 222:         final int JDKBundleCertIndex = JDKBundleCert.value();

`JDKBundleCertIndex` -> `jdkBundleCertIndex`?

test/jdk/tools/jpackage/macosx/SigningRuntimeImagePackageTest.java line 231:

> 229: 
> 230:         new PackageTest()
> 231:                 .forTypes(PackageType.MAC)

`.forTypes(PackageType.MAC)` is redundant. The test will only be executed on macOS

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26173#discussion_r2191253506
PR Review Comment: https://git.openjdk.org/jdk/pull/26173#discussion_r2191248838
PR Review Comment: https://git.openjdk.org/jdk/pull/26173#discussion_r2191250266
PR Review Comment: https://git.openjdk.org/jdk/pull/26173#discussion_r2191251656
PR Review Comment: https://git.openjdk.org/jdk/pull/26173#discussion_r2191252636