8350594: Misleading warning about install dir for DMG packaging

Alexey Semenyuk · Alexey Semenyuk · commit 6a31aaeb00b6 · 2025-03-04T16:22:28.000Z
Reviewed-by: almatvee
diff --git a/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacBaseInstallerBundler.java b/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacBaseInstallerBundler.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2025, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -102,16 +102,20 @@ public abstract class MacBaseInstallerBundler extends AbstractBundler {
      static String getInstallDir(
             Map<String, ? super Object>  params, boolean defaultOnly) {
         String returnValue = INSTALL_DIR.fetchFrom(params);
-        if (defaultOnly && returnValue != null) {
-            Log.info(I18N.getString("message.install-dir-ignored"));
+
+        final String defaultInstallDir;
+        if (StandardBundlerParam.isRuntimeInstaller(params)) {
+            defaultInstallDir = "/Library/Java/JavaVirtualMachines";
+        } else {
+            defaultInstallDir = "/Applications";
+        }
+
+        if (defaultOnly && returnValue != null && !Path.of(returnValue).equals(Path.of(defaultInstallDir))) {
+            Log.info(MessageFormat.format(I18N.getString("message.install-dir-ignored"), defaultInstallDir));
             returnValue = null;
         }
         if (returnValue == null) {
-            if (StandardBundlerParam.isRuntimeInstaller(params)) {
-                returnValue = "/Library/Java/JavaVirtualMachines";
-            } else {
-               returnValue = "/Applications";
-            }
+            returnValue = defaultInstallDir;
         }
         return returnValue;
     }
diff --git a/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/resources/MacResources.properties b/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/resources/MacResources.properties
@@ -1,5 +1,5 @@
 #
-# Copyright (c) 2017, 2024, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2017, 2025, Oracle and/or its affiliates. All rights reserved.
 # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
 #
 # This code is free software; you can redistribute it and/or modify it
@@ -91,7 +91,7 @@ message.preparing-scripts=Preparing package scripts.
 message.preparing-distribution-dist=Preparing distribution.dist: {0}.
 message.signing.pkg=Warning: For signing PKG, you might need to set "Always Trust" for your certificate using "Keychain Access" tool.
 message.setfile.dmg=Setting custom icon on DMG file skipped because 'SetFile' utility was not found. Installing Xcode with Command Line Tools should resolve this issue.
-message.install-dir-ignored=Warning: "--install-dir" is not supported by DMG and will be default to /Applications.
+message.install-dir-ignored=Warning: "--install-dir" option is ignored for DMG packaging. The installation directory will default to {0}.
 message.codesign.failed.reason.app.content="codesign" failed and additional application content was supplied via the "--app-content" parameter. Probably the additional content broke the integrity of the application bundle and caused the failure. Ensure content supplied via the "--app-content" parameter does not break the integrity of the application bundle, or add it in the post-processing step.
 message.codesign.failed.reason.xcode.tools=Possible reason for "codesign" failure is missing Xcode with command line developer tools. Install Xcode with command line developer tools to see if it resolves the problem.
 warning.unsigned.app.image=Warning: Using unsigned app-image to build signed {0}.
diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java
@@ -71,6 +71,7 @@ public JPackageCommand(JPackageCommand cmd) {
         ignoreDefaultRuntime = cmd.ignoreDefaultRuntime;
         ignoreDefaultVerbose = cmd.ignoreDefaultVerbose;
         immutable = cmd.immutable;
+        dmgInstallDir = cmd.dmgInstallDir;
         prerequisiteActions = new Actions(cmd.prerequisiteActions);
         verifyActions = new Actions(cmd.verifyActions);
         appLayoutAsserts = cmd.appLayoutAsserts;
@@ -497,7 +498,11 @@ public Path appInstallationDirectory() {
         }
 
         if (TKit.isOSX()) {
-            return MacHelper.getInstallationDirectory(this);
+            if (packageType() == PackageType.MAC_DMG && dmgInstallDir != null) {
+                return dmgInstallDir;
+            } else {
+                return MacHelper.getInstallationDirectory(this);
+            }
         }
 
         throw TKit.throwUnknownPlatformError();
@@ -868,6 +873,7 @@ private static JPackageCommand convertFromRuntime(JPackageCommand cmd) {
             var copy = new JPackageCommand(cmd);
             copy.immutable = false;
             copy.removeArgumentWithValue("--runtime-image");
+            copy.dmgInstallDir = cmd.appInstallationDirectory();
             return copy;
         }
 
@@ -1165,6 +1171,7 @@ public void run() {
     private boolean ignoreDefaultRuntime;
     private boolean ignoreDefaultVerbose;
     private boolean immutable;
+    private Path dmgInstallDir;
     private final Actions prerequisiteActions;
     private final Actions verifyActions;
     private Path executeInDirectory;
diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/MacHelper.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/MacHelper.java
@@ -22,34 +22,35 @@
  */
 package jdk.jpackage.test;
 
+import static java.util.stream.Collectors.toSet;
+
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.util.List;
 import java.util.ArrayList;
+import java.util.List;
 import java.util.Optional;
 import java.util.Set;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
-import static java.util.stream.Collectors.toSet;
 import java.util.stream.Stream;
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.parsers.ParserConfigurationException;
 import javax.xml.xpath.XPath;
 import javax.xml.xpath.XPathConstants;
 import javax.xml.xpath.XPathFactory;
+import jdk.jpackage.internal.RetryExecutor;
+import jdk.jpackage.internal.util.PathUtils;
 import jdk.jpackage.internal.util.function.ThrowingConsumer;
 import jdk.jpackage.internal.util.function.ThrowingSupplier;
 import jdk.jpackage.test.PackageTest.PackageHandlers;
-import jdk.jpackage.internal.RetryExecutor;
-import jdk.jpackage.internal.util.PathUtils;
-import org.xml.sax.SAXException;
 import org.w3c.dom.NodeList;
+import org.xml.sax.SAXException;
 
 public final class MacHelper {
 
@@ -298,9 +299,18 @@ static String getBundleName(JPackageCommand cmd) {
 
     static Path getInstallationDirectory(JPackageCommand cmd) {
         cmd.verifyIsOfType(PackageType.MAC);
-        return Path.of(cmd.getArgumentValue("--install-dir",
-                () -> cmd.isRuntime() ? "/Library/Java/JavaVirtualMachines" : "/Applications")).resolve(
-                        cmd.name() + (cmd.isRuntime() ? "" : ".app"));
+
+        final var defaultInstallLocation = Path.of(
+                cmd.isRuntime() ? "/Library/Java/JavaVirtualMachines" : "/Applications");
+
+        final Path installLocation;
+        if (cmd.packageType() == PackageType.MAC_DMG) {
+            installLocation = defaultInstallLocation;
+        } else {
+            installLocation = cmd.getArgumentValue("--install-dir", () -> defaultInstallLocation, Path::of);
+        }
+
+        return installLocation.resolve(cmd.name() + (cmd.isRuntime() ? "" : ".app"));
     }
 
     static Path getUninstallCommand(JPackageCommand cmd) {
diff --git a/test/jdk/tools/jpackage/share/InstallDirTest.java b/test/jdk/tools/jpackage/share/InstallDirTest.java
@@ -22,12 +22,20 @@
  */
 
 import java.nio.file.Path;
+import java.util.List;
+import java.util.Objects;
+import java.util.stream.Stream;
 import jdk.internal.util.OperatingSystem;
-import jdk.jpackage.test.TKit;
-import jdk.jpackage.test.PackageTest;
-import jdk.jpackage.test.PackageType;
 import jdk.jpackage.test.Annotations.Parameter;
+import jdk.jpackage.test.Annotations.ParameterSupplier;
 import jdk.jpackage.test.Annotations.Test;
+import jdk.jpackage.test.JPackageCommand;
+import jdk.jpackage.test.JPackageStringBundle;
+import jdk.jpackage.test.PackageTest;
+import jdk.jpackage.test.PackageType;
+import jdk.jpackage.test.RunnablePackageTest.Action;
+import jdk.jpackage.test.TKit;
+import jdk.jpackage.test.TKit.TextStreamVerifier;
 
 /**
  * Test --install-dir parameter. Output of the test should be
@@ -57,6 +65,7 @@
  * @key jpackagePlatformPackage
  * @build jdk.jpackage.test.*
  * @compile -Xlint:all -Werror InstallDirTest.java
+ * @requires (jpackage.test.SQETest != null)
  * @run main/othervm/timeout=540 -Xmx512m jdk.jpackage.test.Main
  *  --jpt-run=InstallDirTest.testCommon
  */
@@ -68,10 +77,9 @@
  * @key jpackagePlatformPackage
  * @build jdk.jpackage.test.*
  * @compile -Xlint:all -Werror InstallDirTest.java
- * @requires (os.family == "linux")
  * @requires (jpackage.test.SQETest == null)
- * @run main/othervm/timeout=360 -Xmx512m jdk.jpackage.test.Main
- *  --jpt-run=InstallDirTest.testLinuxInvalid
+ * @run main/othervm/timeout=720 -Xmx512m jdk.jpackage.test.Main
+ *  --jpt-run=InstallDirTest
  */
 public class InstallDirTest {
 
@@ -92,11 +100,6 @@ public static void testCommon(Path installDir) {
     @Parameter("foo")
     @Parameter("/opt/foo/.././.")
     public static void testLinuxInvalid(String installDir) {
-        testLinuxBad(installDir, "Invalid installation directory");
-    }
-
-    private static void testLinuxBad(String installDir,
-            String errorMessageSubstring) {
         new PackageTest().configureHelloApp()
         .setExpectedExitCode(1)
         .forTypes(PackageType.LINUX)
@@ -105,9 +108,120 @@ private static void testLinuxBad(String installDir,
             cmd.saveConsoleOutput(true);
         })
         .addBundleVerifier((cmd, result) -> {
-            TKit.assertTextStream(errorMessageSubstring).apply(
-                    result.getOutput().stream());
+            cmd.validateOutput(JPackageStringBundle.MAIN.cannedFormattedString("error.invalid-install-dir"));
         })
         .run();
     }
+
+    record DmgTestSpec(Path installDir, boolean installDirIgnored, boolean runtimeInstaller) {
+
+        DmgTestSpec {
+            Objects.requireNonNull(installDir);
+        }
+
+        static Builder build() {
+            return new Builder();
+        }
+
+        static final class Builder {
+
+            Builder ignoredInstallDir(String v) {
+                installDir = Path.of(v);
+                installDirIgnored = true;
+                return this;
+            }
+
+            Builder acceptedInstallDir(String v) {
+                installDir = Path.of(v);
+                installDirIgnored = false;
+                return this;
+            }
+
+            Builder runtimeInstaller() {
+                runtimeInstaller = true;
+                return this;
+            }
+
+            DmgTestSpec create() {
+                return new DmgTestSpec(installDir, installDirIgnored, runtimeInstaller);
+            }
+
+            private Path installDir;
+            private boolean installDirIgnored;
+            private boolean runtimeInstaller;
+        }
+
+        @Override
+        public String toString() {
+            final var sb = new StringBuilder();
+            sb.append(installDir);
+            if (installDirIgnored) {
+                sb.append(", ignore");
+            }
+            if (runtimeInstaller) {
+                sb.append(", runtime");
+            }
+            return sb.toString();
+        }
+
+        void run() {
+            final var test = new PackageTest().forTypes(PackageType.MAC_DMG).ignoreBundleOutputDir();
+            if (runtimeInstaller) {
+                test.addInitializer(cmd -> {
+                    cmd.removeArgumentWithValue("--input");
+                });
+            } else {
+                test.configureHelloApp();
+            }
+
+            test.addInitializer(JPackageCommand::setFakeRuntime).addInitializer(cmd -> {
+                cmd.addArguments("--install-dir", installDir);
+                cmd.validateOutput(createInstallDirWarningVerifier());
+            }).run(Action.CREATE_AND_UNPACK);
+        }
+
+        private TextStreamVerifier createInstallDirWarningVerifier() {
+            final var verifier = TKit.assertTextStream(
+                    JPackageStringBundle.MAIN.cannedFormattedString("message.install-dir-ignored", defaultDmgInstallDir()).getValue());
+            if (installDirIgnored) {
+                return verifier;
+            } else {
+                return verifier.negate();
+            }
+        }
+
+        private String defaultDmgInstallDir() {
+            if (runtimeInstaller) {
+                return "/Library/Java/JavaVirtualMachines";
+            } else {
+                return "/Applications";
+            }
+        }
+    }
+
+    @Test(ifOS = OperatingSystem.MACOS)
+    @ParameterSupplier
+    public static void testDmg(DmgTestSpec testSpec) {
+        testSpec.run();
+    }
+
+    public static List<Object[]> testDmg() {
+        return Stream.of(
+                DmgTestSpec.build().ignoredInstallDir("/foo"),
+                DmgTestSpec.build().ignoredInstallDir("/foo/bar"),
+                DmgTestSpec.build().ignoredInstallDir("/foo").runtimeInstaller(),
+                DmgTestSpec.build().ignoredInstallDir("/foo/bar").runtimeInstaller(),
+
+                DmgTestSpec.build().ignoredInstallDir("/Library/Java/JavaVirtualMachines"),
+                DmgTestSpec.build().ignoredInstallDir("/Applications").runtimeInstaller(),
+
+                DmgTestSpec.build().acceptedInstallDir("/Applications"),
+                DmgTestSpec.build().ignoredInstallDir("/Applications/foo/bar/buz"),
+
+                DmgTestSpec.build().runtimeInstaller().acceptedInstallDir("/Library/Java/JavaVirtualMachines"),
+                DmgTestSpec.build().runtimeInstaller().ignoredInstallDir("/Library/Java/JavaVirtualMachines/foo/bar/buz")
+        ).map(DmgTestSpec.Builder::create).map(testSpec -> {
+            return new Object[] { testSpec };
+        }).toList();
+    }
 }
