8362598: [macos] Add tests for custom info plist files #27509

sashamatveev · 2025-09-26T02:54:32Z

Added test for custom info plist to cover app image including embedded runtime and runtime installer.
Fixed bug in writePList. It was missing writeStartDocument()/writeEndDocument() and DOCTYPE should be full xml string.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8362598: [macos] Add tests for custom info plist files (Enhancement - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/27509/head:pull/27509
$ git checkout pull/27509

Update a local copy of the PR:
$ git checkout pull/27509
$ git pull https://git.openjdk.org/jdk.git pull/27509/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 27509

View PR using the GUI difftool:
$ git pr show -t 27509

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/27509.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-09-26T02:56:22Z

👋 Welcome back almatvee! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

sashamatveev · 2025-09-26T02:56:39Z

@alexeysemenyukoracle Please review.

openjdk · 2025-09-26T02:57:42Z

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

openjdk · 2025-09-26T02:58:29Z

@sashamatveev The following label will be automatically applied to this pull request:

core-libs

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2025-09-26T03:01:50Z

Webrevs

alexeysemenyukoracle · 2025-09-26T11:22:48Z

src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/util/PListWriter.java

    public static void writePList(XMLStreamWriter xml, XmlConsumer content)
            throws XMLStreamException, IOException {
-        xml.writeDTD("plist PUBLIC \"-//Apple//DTD PLIST 1.0//EN\" \"https://www.apple.com/DTDs/PropertyList-1.0.dtd\"");
+        xml.writeStartDocument();


Why do we need writeStartDocument()/writeEndDocument() calls?

To write XML declaration to Info.plist file. Without it reading PList file fails.

alexeysemenyukoracle · 2025-09-26T11:30:01Z

test/jdk/tools/jpackage/helpers/jdk/jpackage/test/MacHelper.java

        return (cmd.hasArgument("--mac-signing-key-user-name") || cmd.hasArgument("--mac-app-image-sign-identity"));
    }

+    public static Path createInputRuntimeImage() throws IOException {


This is not mac-specific helper function. You'd rather move it to JPackageCommand class.

BTW, there is a bunch of createInputRuntimeImage() functions already:

jdk/test/jdk/tools/jpackage/macosx/SigningRuntimeImagePackageTest.java

Line 97 in 7bfdb01

private static Path createInputRuntimeImage() throws IOException {

jdk/test/jdk/tools/jpackage/share/RuntimePackageTest.java

Line 176 in 7bfdb01

private static Path createInputRuntimeImage() throws IOException {

jdk/test/jdk/tools/jpackage/share/RuntimeImageTest.java

Line 51 in 7bfdb01

if (JPackageCommand.DEFAULT_RUNTIME_IMAGE == null) {

Instead of adding another one, let's consolidate them in one.

alexeysemenyukoracle · 2025-09-26T11:41:44Z

test/jdk/tools/jpackage/macosx/CustomInfoPListTest.java

+    private static final String BUNDLE_NAME_RUNTIME = "CustomRuntimeName";
+
+    // We do not need full Info.plist for testing
+    private static String getInfoPListXML(String bundleName) {


Use jdk.jpackage.internal.util.XmlUtils.createXml() helper to create xml file. It will format the output xml and will also eliminate the need for the patch in PListWriter class.

I think using PListWriter is more convenient then jdk.jpackage.internal.util.XmlUtils.createXml(). I will prefer to use PListWriter.

I'm not suggesting replacing PListWriter with XmlUtils.createXml(). I'm suggesting replacing XMLOutputFactory.newInstance().createXMLStreamWriter(buf) with XmlUtils.createXml():

private static void savePList(String bundleName, Path plistFile) throws XMLStreamException, IOException { XmlUtils.createXml(plistFile, xml -> { writePList(xml, toXmlConsumer(() -> { writeDict(xml, toXmlConsumer(() -> { writeString(xml, "CFBundleName", bundleName); writeString(xml, "CFBundleIdentifier", "CustomInfoPListTest"); writeString(xml, "CFBundleVersion", "1.0"); })); })); }); }

alexeysemenyukoracle · 2025-09-26T11:44:18Z

test/jdk/tools/jpackage/macosx/CustomInfoPListTest.java

+        cmd.execute();
+
+        MacHelper.withExplodedDmg(cmd, dmgImage -> {
+            if (dmgImage.endsWith(cmd.name() + ".jdk")) {


Isn't this test redundant?

No. This function is called for all files in DMG such as /Volumes/foo/.background.

Ah, ok. I see this sort of check in 5 locations.

I guess if you replace dmgImage.endsWith(cmd.name() + ".jdk") with dmgImage.endsWith(cmd.appInstallationDirectory().getFileName()) it will become self-explanatory.

alexeysemenyukoracle · 2025-09-26T11:58:12Z

In the case of an app bundle, there is a single test case that replaces both the main and the runtime plist files. We need three test cases for better coverage:

custom main plist file
custom runtime plist file
custom main and runtime plist files

The tests don't cover pattern matching in custom plist files. E.g., jpackage will replace DEPLOY_LAUNCHER_NAME pattern in the main plist file with the main launcher name; jpackage will replace CF_BUNDLE_VERSION pattern in the runtime plist file with the app version. We need to test that jpackage replaces these patterns in custom plist files in the same way it does for the default plist files.

sashamatveev · 2025-10-02T02:22:38Z

8362598: [macos] Add tests for custom info plist files [v3]

Test updated to address all comments above.

alexeysemenyukoracle · 2025-10-03T14:57:21Z

test/jdk/tools/jpackage/macosx/SigningRuntimeImagePackageTest.java

 import java.util.function.Predicate;
 import java.util.stream.Stream;
+
+import base.SigningBase;


This should result in the following error:

open\test\jdk\tools\jpackage\macosx\SigningRuntimeImagePackageTest.java:29: error: package base does not exist import base.SigningBase; ^ 1 error

Removed. Not sure why it was added.

sashamatveev · 2025-10-04T00:57:51Z

8362598: [macos] Add tests for custom info plist files [v5]

Test updated after latest merge. It started failing since plistFile.queryValue did not able to find keys stored as value of CustomInfoPListFA. FA extension is xml stored as value of CustomInfoPListFA.

alexeysemenyukoracle · 2025-10-06T18:31:03Z

I took the liberty of using your patch as a baseline to improve testing of plist properties, including file associations. Here - #27658. Would you consider pulling it into your PR?

sashamatveev added 2 commits September 25, 2025 19:26

8362598: [macos] Add tests for custom info plist files

57082bc

8362598: [macos] Add tests for custom info plist files [v2]

50011da

openjdk bot added the core-libs [email protected] label Sep 26, 2025

openjdk bot added the rfr Pull request is ready for review label Sep 26, 2025

alexeysemenyukoracle reviewed Sep 26, 2025

View reviewed changes

8362598: [macos] Add tests for custom info plist files [v3]

f1e9913

alexeysemenyukoracle reviewed Oct 3, 2025

View reviewed changes

sashamatveev added 3 commits October 3, 2025 13:15

8362598: [macos] Add tests for custom info plist files [v4]

af103a6

Merge remote-tracking branch 'upstream/master' into JDK-8362598

267bd36

8362598: [macos] Add tests for custom info plist files [v5]

01e41f1

alexeysemenyukoracle mentioned this pull request Oct 6, 2025

8362598: [macos] Add tests for custom info plist files #27658

Open

3 tasks

8362598: [macos] Add tests for custom info plist files #27509

Are you sure you want to change the base?

8362598: [macos] Add tests for custom info plist files #27509

Conversation

sashamatveev commented Sep 26, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewing

Uh oh!

bridgekeeper bot commented Sep 26, 2025

Uh oh!

sashamatveev commented Sep 26, 2025

Uh oh!

openjdk bot commented Sep 26, 2025

Uh oh!

openjdk bot commented Sep 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mlbridge bot commented Sep 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexeysemenyukoracle Sep 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sashamatveev Sep 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexeysemenyukoracle commented Sep 26, 2025

Uh oh!

sashamatveev commented Oct 2, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sashamatveev commented Oct 4, 2025

Uh oh!

alexeysemenyukoracle commented Oct 6, 2025

Uh oh!

Uh oh!

sashamatveev commented Sep 26, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Sep 26, 2025 •

edited

Loading

mlbridge bot commented Sep 26, 2025 •

edited

Loading

alexeysemenyukoracle Sep 26, 2025 •

edited

Loading

sashamatveev Sep 29, 2025 •

edited

Loading