8256475: Fix Behavior when Installer name differs from application name.

Reviewed-by: asemenyuk, almatvee, kizune
openjdk · Nov 24, 2020 · 303631e · 303631e · openjdk-notifier · Nov 24, 2020
1 parent fa3cfcd
commit 303631e
Show file tree

Hide file tree

Showing 18 changed files with 203 additions and 40 deletions.
diff --git a/src/jdk.jpackage/linux/classes/jdk/jpackage/internal/LinuxDebBundler.java b/src/jdk.jpackage/linux/classes/jdk/jpackage/internal/LinuxDebBundler.java
@@ -47,6 +47,7 @@
 import java.util.stream.Stream;
 import static jdk.jpackage.internal.OverridableResource.createResource;
 import static jdk.jpackage.internal.StandardBundlerParam.APP_NAME;
+import static jdk.jpackage.internal.StandardBundlerParam.INSTALLER_NAME;
 import static jdk.jpackage.internal.StandardBundlerParam.VERSION;
 import static jdk.jpackage.internal.StandardBundlerParam.RELEASE;
 import static jdk.jpackage.internal.StandardBundlerParam.VENDOR;
@@ -71,8 +72,7 @@ public class LinuxDebBundler extends LinuxPackageBundler {
             Arguments.CLIOptions.LINUX_BUNDLE_NAME.getId(),
             String.class,
             params -> {
-                String nm = APP_NAME.fetchFrom(params);
-
+                String nm = INSTALLER_NAME.fetchFrom(params);
                 if (nm == null) return null;
 
                 // make sure to lower case and spaces/underscores become dashes

diff --git a/src/jdk.jpackage/linux/classes/jdk/jpackage/internal/LinuxRpmBundler.java b/src/jdk.jpackage/linux/classes/jdk/jpackage/internal/LinuxRpmBundler.java
@@ -38,6 +38,7 @@
 import java.util.stream.Collectors;
 
 import static jdk.jpackage.internal.StandardBundlerParam.APP_NAME;
+import static jdk.jpackage.internal.StandardBundlerParam.INSTALLER_NAME;
 import static jdk.jpackage.internal.StandardBundlerParam.LICENSE_FILE;
 import static jdk.jpackage.internal.StandardBundlerParam.VERSION;
 import static jdk.jpackage.internal.StandardBundlerParam.RELEASE;
@@ -73,7 +74,7 @@ public class LinuxRpmBundler extends LinuxPackageBundler {
             Arguments.CLIOptions.LINUX_BUNDLE_NAME.getId(),
             String.class,
             params -> {
-                String nm = APP_NAME.fetchFrom(params);
+                String nm = INSTALLER_NAME.fetchFrom(params);
                 if (nm == null) return null;
 
                 // make sure to lower case and spaces become dashes

diff --git a/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacAppStoreBundler.java b/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacAppStoreBundler.java
@@ -143,7 +143,7 @@ public Path bundle(Map<String, ? super Object> params,
             ProcessBuilder pb;
 
             // create the final pkg file
-            Path finalPKG = outdir.resolve(INSTALLER_NAME.fetchFrom(params)
+            Path finalPKG = outdir.resolve(MAC_INSTALLER_NAME.fetchFrom(params)
                     + INSTALLER_SUFFIX.fetchFrom(params)
                     + ".pkg");
             Files.createDirectories(outdir);

diff --git a/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacBaseInstallerBundler.java b/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacBaseInstallerBundler.java
@@ -37,6 +37,7 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import static jdk.jpackage.internal.StandardBundlerParam.APP_NAME;
+import static jdk.jpackage.internal.StandardBundlerParam.INSTALLER_NAME;
 import static jdk.jpackage.internal.StandardBundlerParam.INSTALL_DIR;
 import static jdk.jpackage.internal.StandardBundlerParam.PREDEFINED_APP_IMAGE;
 import static jdk.jpackage.internal.StandardBundlerParam.VERSION;
@@ -75,12 +76,12 @@ public abstract class MacBaseInstallerBundler extends AbstractBundler {
             params -> "",
             null);
 
-    public static final BundlerParamInfo<String> INSTALLER_NAME =
+    public static final BundlerParamInfo<String> MAC_INSTALLER_NAME =
             new StandardBundlerParam<> (
             "mac.installerName",
             String.class,
             params -> {
-                String nm = APP_NAME.fetchFrom(params);
+                String nm = INSTALLER_NAME.fetchFrom(params);
                 if (nm == null) return null;
 
                 String version = VERSION.fetchFrom(params);

diff --git a/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacDmgBundler.java b/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacDmgBundler.java
@@ -270,7 +270,7 @@ private Path buildDMG( Map<String, ? super Object> params,
         }
 
         Path protoDMG = imagesRoot.resolve(APP_NAME.fetchFrom(params) +"-tmp.dmg");
-        Path finalDMG = outdir.resolve(INSTALLER_NAME.fetchFrom(params)
+        Path finalDMG = outdir.resolve(MAC_INSTALLER_NAME.fetchFrom(params)
                 + INSTALLER_SUFFIX.fetchFrom(params) + ".dmg");
 
         Path srcFolder = APP_IMAGE_TEMP_ROOT.fetchFrom(params);

diff --git a/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacPkgBundler.java b/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacPkgBundler.java
@@ -448,7 +448,7 @@ private Path createPKG(Map<String, ? super Object> params,
             IOUtils.exec(pb);
 
             // build final package
-            Path finalPKG = outdir.resolve(INSTALLER_NAME.fetchFrom(params)
+            Path finalPKG = outdir.resolve(MAC_INSTALLER_NAME.fetchFrom(params)
                     + INSTALLER_SUFFIX.fetchFrom(params)
                     + ".pkg");
             Files.createDirectories(outdir);

diff --git a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/AddLauncherArguments.java b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/AddLauncherArguments.java
@@ -31,6 +31,7 @@
 import java.util.List;
 import jdk.jpackage.internal.Arguments.CLIOptions;
 import static jdk.jpackage.internal.StandardBundlerParam.LAUNCHER_DATA;
+import static jdk.jpackage.internal.StandardBundlerParam.APP_NAME;
 
 /*
  * AddLauncherArguments
@@ -158,8 +159,10 @@ private String getOptionValue(CLIOptions option) {
         Map<String, ? super Object> tmp = new HashMap<>(original);
         List.of(exclude).forEach(tmp::remove);
 
-        // remove LauncherData from map so it will re-run the defaultValueFunction
+        // remove LauncherData from map so it will be re-computed
         tmp.remove(LAUNCHER_DATA.getID());
+        // remove "application-name" so it will be re-computed
+        tmp.remove(APP_NAME.getID());
 
         if (additional.containsKey(CLIOptions.MODULE.getId())) {
             tmp.remove(CLIOptions.MAIN_JAR.getId());

diff --git a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/AppImageFile.java b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/AppImageFile.java
@@ -233,6 +233,16 @@ static List<String> getLauncherNames(Path appImageDir,
         return launchers;
     }
 
+    public static String extractAppName(Path appImageDir) {
+        try {
+            return AppImageFile.load(appImageDir).getLauncherName();
+        } catch (IOException ioe) {
+            Log.verbose(MessageFormat.format(I18N.getString(
+                    "warning.foreign-app-image"), appImageDir));
+            return null;
+        }
+    }
+
     private static String xpathQueryNullable(XPath xPath, String xpathExpr,
             Document xml) throws XPathExpressionException {
         NodeList nodes = (NodeList) xPath.evaluate(xpathExpr, xml,

diff --git a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/StandardBundlerParam.java b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/StandardBundlerParam.java
@@ -128,25 +128,59 @@ class StandardBundlerParam<T> extends BundlerParamInfo<T> {
                     (s, p) -> Path.of(s)
             );
 
-    static final StandardBundlerParam<String> APP_NAME =
+    static final StandardBundlerParam<Path> PREDEFINED_APP_IMAGE =
+            new StandardBundlerParam<>(
+            Arguments.CLIOptions.PREDEFINED_APP_IMAGE.getId(),
+            Path.class,
+            params -> null,
+            (s, p) -> Path.of(s));
+
+    // this is the raw --app-name arg - used in APP_NAME and INSTALLER_NAME
+    static final StandardBundlerParam<String> NAME =
             new StandardBundlerParam<>(
                     Arguments.CLIOptions.NAME.getId(),
                     String.class,
+                    params -> null,
+                    (s, p) -> s
+            );
+
+    // this is the application name, either from the app-image (if given),
+    // the name (if given) derived from the main-class, or the runtime image
+    static final StandardBundlerParam<String> APP_NAME =
+            new StandardBundlerParam<>(
+                    "application-name",
+                    String.class,
                     params -> {
-                        String s = MAIN_CLASS.fetchFrom(params);
-                        if (s != null) {
-                            int idx = s.lastIndexOf(".");
-                            if (idx >= 0) {
-                                return s.substring(idx+1);
-                            }
-                            return s;
-                        } else if (isRuntimeInstaller(params)) {
-                            Path f = PREDEFINED_RUNTIME_IMAGE.fetchFrom(params);
-                            if (f != null) {
-                                return f.getFileName().toString();
+                        Path appImage = PREDEFINED_APP_IMAGE.fetchFrom(params);
+                        String appName = NAME.fetchFrom(params);
+                        if (appImage != null) {
+                            String name = AppImageFile.extractAppName(appImage);
+                            appName  = (name != null) ? name : appName;
+                        } else if (appName == null) {
+                            String s = MAIN_CLASS.fetchFrom(params);
+                            if (s != null) {
+                                int idx = s.lastIndexOf(".");
+                                appName = (idx < 0) ? s : s.substring(idx+1);
+                            } else if (isRuntimeInstaller(params)) {
+                                Path f = PREDEFINED_RUNTIME_IMAGE.fetchFrom(params);
+                                if (f != null) {
+                                    appName = f.getFileName().toString();
+                                }
                             }
                         }
-                        return null;
+                        return appName;
+                    },
+                    (s, p) -> s
+            );
+
+    static final StandardBundlerParam<String> INSTALLER_NAME =
+            new StandardBundlerParam<>(
+                    "installer-name",
+                    String.class,
+                    params -> {
+                        String installerName = NAME.fetchFrom(params);
+                        return (installerName != null) ? installerName :
+                                APP_NAME.fetchFrom(params);
                     },
                     (s, p) -> s
             );
@@ -285,12 +319,6 @@ class StandardBundlerParam<T> extends BundlerParamInfo<T> {
                     (s, p) -> s
     );
 
-    static final StandardBundlerParam<Path> PREDEFINED_APP_IMAGE =
-            new StandardBundlerParam<>(
-            Arguments.CLIOptions.PREDEFINED_APP_IMAGE.getId(),
-            Path.class,
-            params -> null,
-            (s, p) -> Path.of(s));
 
     @SuppressWarnings("unchecked")
     static final StandardBundlerParam<List<Map<String, ? super Object>>> ADD_LAUNCHERS =

diff --git a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/MainResources.properties b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/MainResources.properties
@@ -72,6 +72,7 @@ error.tool-old-version=Can not find {0} {1} or newer
 error.tool-old-version.advice=Please install {0} {1} or newer
 error.jlink.failed=jlink failed with: {0}
 error.blocked.option=jlink option [{0}] is not permitted in --jlink-options
+error.no.name=Name not specified with --name and cannot infer one from app-image
 
 warning.no.jdk.modules.found=Warning: No JDK Modules found
 warning.foreign-app-image=Warning: app-image dir ({0}) not generated by jpackage.

diff --git a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/MainResources_ja.properties b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/MainResources_ja.properties
@@ -72,6 +72,7 @@ error.tool-old-version={0} {1}\u4EE5\u964D\u304C\u898B\u3064\u304B\u308A\u307E\u
 error.tool-old-version.advice={0} {1}\u4EE5\u964D\u3092\u30A4\u30F3\u30B9\u30C8\u30FC\u30EB\u3057\u3066\u304F\u3060\u3055\u3044
 error.jlink.failed=jlink\u304C\u6B21\u3067\u5931\u6557\u3057\u307E\u3057\u305F: {0}
 error.blocked.option=jlink\u30AA\u30D7\u30B7\u30E7\u30F3[{0}]\u306F--jlink-options\u3067\u306F\u8A31\u53EF\u3055\u308C\u307E\u305B\u3093
+error.no.name=Name not specified with --name and cannot infer one from app-image
 
 warning.no.jdk.modules.found=\u8B66\u544A: JDK\u30E2\u30B8\u30E5\u30FC\u30EB\u304C\u898B\u3064\u304B\u308A\u307E\u305B\u3093
 warning.foreign-app-image=Warning: app-image dir ({0}) not generated by jpackage.

diff --git a/...jdk.jpackage/share/classes/jdk/jpackage/internal/resources/MainResources_zh_CN.properties b/...jdk.jpackage/share/classes/jdk/jpackage/internal/resources/MainResources_zh_CN.properties
@@ -72,6 +72,7 @@ error.tool-old-version=\u627E\u4E0D\u5230 {0} {1}\u6216\u66F4\u65B0\u7248\u672C
 error.tool-old-version.advice=\u8BF7\u5B89\u88C5 {0} {1}\u6216\u66F4\u65B0\u7248\u672C
 error.jlink.failed=jlink \u5931\u8D25\uFF0C\u51FA\u73B0 {0}
 error.blocked.option=\u4E0D\u5141\u8BB8\u5728 --jlink-options \u4E2D\u4F7F\u7528 jlink \u9009\u9879 [{0}]
+error.no.name=Name not specified with --name and cannot infer one from app-image
 
 warning.no.jdk.modules.found=\u8B66\u544A: \u672A\u627E\u5230 JDK \u6A21\u5757
 warning.foreign-app-image=Warning: app-image dir ({0}) not generated by jpackage.

diff --git a/src/jdk.jpackage/windows/classes/jdk/jpackage/internal/WinMsiBundler.java b/src/jdk.jpackage/windows/classes/jdk/jpackage/internal/WinMsiBundler.java
@@ -56,6 +56,7 @@
 
 import static jdk.jpackage.internal.OverridableResource.createResource;
 import static jdk.jpackage.internal.StandardBundlerParam.APP_NAME;
+import static jdk.jpackage.internal.StandardBundlerParam.INSTALLER_NAME;
 import static jdk.jpackage.internal.StandardBundlerParam.CONFIG_ROOT;
 import static jdk.jpackage.internal.StandardBundlerParam.DESCRIPTION;
 import static jdk.jpackage.internal.StandardBundlerParam.LICENSE_FILE;
@@ -171,7 +172,7 @@ public class WinMsiBundler  extends AbstractBundler {
             "win.installerName",
             String.class,
             params -> {
-                String nm = APP_NAME.fetchFrom(params);
+                String nm = INSTALLER_NAME.fetchFrom(params);
                 if (nm == null) return null;
 
                 String version = VERSION.fetchFrom(params);
@@ -305,23 +306,21 @@ public boolean validate(Map<String, ? super Object> params)
     private void prepareProto(Map<String, ? super Object> params)
                 throws PackagerException, IOException {
         Path appImage = StandardBundlerParam.getPredefinedAppImage(params);
+        String appName = APP_NAME.fetchFrom(params);
         Path appDir;
-        String appName;
+        if (appName == null) {
+            // Can happen when no name is given, and using a foreign app-image
+            throw new PackagerException("error.no.name");
+        }
 
         // we either have an application image or need to build one
         if (appImage != null) {
-            appDir = MSI_IMAGE_DIR.fetchFrom(params).resolve(APP_NAME.fetchFrom(params));
+            appDir = MSI_IMAGE_DIR.fetchFrom(params).resolve(appName);
             // copy everything from appImage dir into appDir/name
             IOUtils.copyRecursive(appImage, appDir);
-            try {
-                appName = AppImageFile.load(appDir).getLauncherName();
-            } catch (Exception e) {
-                appName = APP_NAME.fetchFrom(params);
-            }
         } else {
             appDir = appImageBundler.execute(params, MSI_IMAGE_DIR.fetchFrom(
                     params));
-            appName = APP_NAME.fetchFrom(params);
         }
 
         // Configure installer icon

diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java
@@ -209,9 +209,28 @@ public String version() {
     }
 
     public String name() {
+        String appImage = getArgumentValue("--app-image", () -> null);
+        if (appImage != null) {
+            String name =  AppImageFile.extractAppName(Path.of(appImage));
+            // can be null if using foreign app-image
+            return ((name != null) ? name : getArgumentValue("--name"));
+        }
         return getArgumentValue("--name", () -> getArgumentValue("--main-class"));
     }
 
+    public String installerName() {
+        verifyIsOfType(PackageType.NATIVE);
+        String installerName = getArgumentValue("--name",
+                () -> getArgumentValue("--main-class", () -> null));
+        if (installerName == null) {
+            String appImage = getArgumentValue("--app-image");
+            if (appImage != null) {
+                installerName = AppImageFile.extractAppName(Path.of(appImage));
+            }
+        }
+        return installerName;
+    }
+
     public boolean isRuntime() {
         return  hasArgument("--runtime-image")
                 && !hasArgument("--main-jar")

diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/LinuxHelper.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/LinuxHelper.java
@@ -51,7 +51,7 @@ private static String getRelease(JPackageCommand cmd) {
     public static String getPackageName(JPackageCommand cmd) {
         cmd.verifyIsOfType(PackageType.LINUX);
         return cmd.getArgumentValue("--linux-package-name",
-                () -> cmd.name().toLowerCase());
+                () -> cmd.installerName().toLowerCase());
     }
 
     public static Path getDesktopFile(JPackageCommand cmd) {

diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/MacHelper.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/MacHelper.java
@@ -194,7 +194,7 @@ static Path getInstallationDirectory(JPackageCommand cmd) {
 
     private static String getPackageName(JPackageCommand cmd) {
         return cmd.getArgumentValue("--mac-package-name",
-                () -> cmd.name());
+                () -> cmd.installerName());
     }
 
     public static final class PListWrapper {

diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/WindowsHelper.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/WindowsHelper.java
@@ -40,7 +40,7 @@ public class WindowsHelper {
 
     static String getBundleName(JPackageCommand cmd) {
         cmd.verifyIsOfType(PackageType.WINDOWS);
-        return String.format("%s-%s%s", cmd.name(), cmd.version(),
+        return String.format("%s-%s%s", cmd.installerName(), cmd.version(),
                 cmd.packageType().getSuffix());
     }