[GR-49996] Make -jar only implicitly set image name if not already defined previously.

olpaw · olpaw · commit abc1684f5d10 · 2024-01-22T08:51:27.000Z
PullRequest: graal/16638
diff --git a/docs/reference-manual/native-image/guides/build-native-executable-from-jar.md b/docs/reference-manual/native-image/guides/build-native-executable-from-jar.md
@@ -72,6 +72,8 @@ For other installation options, visit the [Downloads section](https://www.graalv
     ```
 
     It will produce a native executable in the project root directory.
+    The default name of the image will be the name of the JAR file (`App` in this case).
+    It can be customized by either providing a custom name as a last argument (for example, `native-image -jar App.jar imagename`), or by using `-o imagename` before or after `-jar jarfile`, for example: `native-image -jar App.jar -o imagename`.
 
 6. Run the native executable:
     ```shell
diff --git a/substratevm/CHANGELOG.md b/substratevm/CHANGELOG.md
@@ -4,6 +4,7 @@ This changelog summarizes major changes to GraalVM Native Image.
 
 ## GraalVM for JDK 23 (Internal Version 24.1.0)
 * (GR-51106) Fields that are accessed via a `VarHandle` or `MethodHandle` are no longer marked as "unsafe accessed" when the `VarHandle`/`MethodHandle` can be fully intrinsified.
+* (GR-49996) Ensure explicitly set image name (e.g., via `-o imagename`) is not accidentally overwritten by `-jar jarfile` option.
 
 ## GraalVM for JDK 22 (Internal Version 24.0.0)
 * (GR-48304) Red Hat added support for the JFR event ThreadAllocationStatistics.
diff --git a/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/DefaultOptionHandler.java b/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/DefaultOptionHandler.java
@@ -251,7 +251,7 @@ private void handleJarFileArg(Path jarFilePath) {
         }
         if (!jarFileNameBase.isEmpty()) {
             String origin = "manifest from " + jarFilePath.toUri();
-            nativeImage.addPlainImageBuilderArg(nativeImage.oHName + jarFileNameBase, origin);
+            nativeImage.addPlainImageBuilderArg(nativeImage.oHName + jarFileNameBase, origin, false);
         }
         Path finalFilePath = nativeImage.useBundle() ? nativeImage.bundleSupport.substituteClassPath(jarFilePath) : jarFilePath;
         if (!NativeImage.processJarManifestMainAttributes(finalFilePath, nativeImage::handleManifestFileAttributes)) {
diff --git a/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java b/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java
@@ -1091,7 +1091,7 @@ private int completeImageBuild() {
         addTargetArguments();
 
         String defaultLibC = OS.getCurrent() == OS.LINUX ? "glibc" : null;
-        targetLibC = getHostedOptionFinalArgument(imageBuilderArgs, oHUseLibC).map(ArgumentEntry::value).orElse(System.getProperty("substratevm.HostLibC", defaultLibC));
+        targetLibC = getHostedOptionArgument(imageBuilderArgs, oHUseLibC).map(ArgumentEntry::value).orElse(System.getProperty("substratevm.HostLibC", defaultLibC));
 
         String clibrariesBuilderArg = config.getBuilderCLibrariesPaths().stream()
                         .flatMap(this::resolveTargetSpecificPaths)
@@ -1166,7 +1166,7 @@ private int completeImageBuild() {
 
         imageBuilderJavaArgs.addAll(getAgentArguments());
 
-        mainClass = getHostedOptionFinalArgumentValue(imageBuilderArgs, oHClass);
+        mainClass = getHostedOptionArgumentValue(imageBuilderArgs, oHClass);
         buildExecutable = imageBuilderArgs.stream().noneMatch(arg -> arg.startsWith(oHEnableSharedLibraryFlagPrefix));
         staticExecutable = imageBuilderArgs.stream().anyMatch(arg -> arg.contains(oHEnableStaticExecutable));
         boolean listModules = imageBuilderArgs.stream().anyMatch(arg -> arg.contains(oH + "+" + "ListModules"));
@@ -1188,8 +1188,8 @@ private int completeImageBuild() {
 
             if (!jarOptionMode) {
                 /* Main-class from customImageBuilderArgs counts as explicitMainClass */
-                boolean explicitMainClass = getHostedOptionFinalArgumentValue(imageBuilderArgs, oHClass) != null;
-                mainClassModule = getHostedOptionFinalArgumentValue(imageBuilderArgs, oHModule);
+                boolean explicitMainClass = getHostedOptionArgumentValue(imageBuilderArgs, oHClass) != null;
+                mainClassModule = getHostedOptionArgumentValue(imageBuilderArgs, oHModule);
 
                 boolean hasMainClassModule = mainClassModule != null && !mainClassModule.isEmpty();
                 boolean hasMainClass = mainClass != null && !mainClass.isEmpty();
@@ -1207,11 +1207,11 @@ private int completeImageBuild() {
 
                 if (extraImageArgs.isEmpty()) {
                     /* No explicit image name, define image name by other means */
-                    if (getHostedOptionFinalArgumentValue(imageBuilderArgs, oHName) == null) {
+                    if (getHostedOptionArgumentValue(imageBuilderArgs, oHName) == null) {
                         /* Also no explicit image name given as customImageBuilderArgs */
                         if (explicitMainClass) {
                             imageBuilderArgs.add(oH(SubstrateOptions.Name, "main-class lower case as image name") + mainClass.toLowerCase());
-                        } else if (getHostedOptionFinalArgumentValue(imageBuilderArgs, oHName) == null) {
+                        } else if (getHostedOptionArgumentValue(imageBuilderArgs, oHName) == null) {
                             if (hasMainClassModule) {
                                 imageBuilderArgs.add(oH(SubstrateOptions.Name, "image-name from module-name") + mainClassModule.toLowerCase());
                             } else if (!listModules) {
@@ -1236,9 +1236,9 @@ private int completeImageBuild() {
             }
         }
 
-        ArgumentEntry imageNameEntry = getHostedOptionFinalArgument(imageBuilderArgs, oHName).orElseThrow();
+        ArgumentEntry imageNameEntry = getHostedOptionArgument(imageBuilderArgs, oHName).orElseThrow();
         imageName = imageNameEntry.value;
-        ArgumentEntry imagePathEntry = getHostedOptionFinalArgument(imageBuilderArgs, oHPath).orElseThrow();
+        ArgumentEntry imagePathEntry = getHostedOptionArgument(imageBuilderArgs, oHPath).orElseThrow();
         imagePath = Path.of(imagePathEntry.value);
         Path imageNamePath = Path.of(imageName);
         Path imageNamePathParent = imageNamePath.getParent();
@@ -1307,7 +1307,7 @@ private int completeImageBuild() {
         imageProvidedJars.forEach(this::processClasspathNativeImageMetaInf);
 
         if (!config.buildFallbackImage()) {
-            Optional<ArgumentEntry> fallbackThresholdEntry = getHostedOptionFinalArgument(imageBuilderArgs, oHFallbackThreshold);
+            Optional<ArgumentEntry> fallbackThresholdEntry = getHostedOptionArgument(imageBuilderArgs, oHFallbackThreshold);
             if (fallbackThresholdEntry.isPresent() && fallbackThresholdEntry.get().value.equals("" + SubstrateOptions.ForceFallback)) {
                 /* Bypass regular build and proceed with fallback image building */
                 return ExitStatus.FALLBACK_IMAGE.getValue();
@@ -1344,11 +1344,11 @@ private static String getLocationAgnosticArgPrefix(String argPrefix) {
         return "^" + argPrefix.substring(0, argPrefix.length() - 1) + "(@[^=]*)?=";
     }
 
-    private static String getHostedOptionFinalArgumentValue(List<String> args, String argPrefix) {
-        return getHostedOptionFinalArgument(args, argPrefix).map(entry -> entry.value).orElse(null);
+    private static String getHostedOptionArgumentValue(List<String> args, String argPrefix) {
+        return getHostedOptionArgument(args, argPrefix).map(entry -> entry.value).orElse(null);
     }
 
-    private static Optional<ArgumentEntry> getHostedOptionFinalArgument(List<String> args, String argPrefix) {
+    private static Optional<ArgumentEntry> getHostedOptionArgument(List<String> args, String argPrefix) {
         List<ArgumentEntry> values = getHostedOptionArgumentValues(args, argPrefix);
         return values.isEmpty() ? Optional.empty() : Optional.of(values.get(values.size() - 1));
     }
@@ -1371,7 +1371,7 @@ private static List<ArgumentEntry> getHostedOptionArgumentValues(List<String> ar
     private record ArgumentEntry(int index, String value) {
     }
 
-    private static Boolean getHostedOptionFinalBooleanArgumentValue(List<String> args, OptionKey<Boolean> option) {
+    private static Boolean getHostedOptionBooleanArgumentValue(List<String> args, OptionKey<Boolean> option) {
         String locationAgnosticBooleanPattern = "^" + oH + "[+-]" + option.getName() + "(@[^=]*)?$";
         Pattern pattern = Pattern.compile(locationAgnosticBooleanPattern);
         Boolean result = null;
@@ -1442,7 +1442,7 @@ private void addTargetArguments() {
          * process (see comments for NativeImageGenerator.getTargetPlatform), we are parsing the
          * --target argument here, and generating required internal arguments.
          */
-        targetPlatform = getHostedOptionFinalArgumentValue(imageBuilderArgs, oHTargetPlatform);
+        targetPlatform = getHostedOptionArgumentValue(imageBuilderArgs, oHTargetPlatform);
         if (targetPlatform == null) {
             return;
         }
@@ -2007,11 +2007,36 @@ List<String> apply(boolean strict) {
     }
 
     void addPlainImageBuilderArg(String plainArg, String origin) {
-        addPlainImageBuilderArg(injectHostedOptionOrigin(plainArg, origin));
+        addPlainImageBuilderArg(plainArg, origin, true);
+    }
+
+    void addPlainImageBuilderArg(String plainArg, String origin, boolean override) {
+        addPlainImageBuilderArg(injectHostedOptionOrigin(plainArg, origin), override);
     }
 
     void addPlainImageBuilderArg(String plainArg) {
+        addPlainImageBuilderArg(plainArg, true);
+    }
+
+    void addPlainImageBuilderArg(String plainArg, boolean override) {
         assert plainArg.startsWith(NativeImage.oH) || plainArg.startsWith(NativeImage.oR);
+        if (!override) {
+            int posValueSeparator = plainArg.indexOf('=');
+            if (posValueSeparator > 0) {
+                String argPrefix = plainArg.substring(0, posValueSeparator);
+                int posOriginSeparator = plainArg.indexOf('@');
+                if (posOriginSeparator > 0) {
+                    argPrefix = argPrefix.substring(0, posOriginSeparator);
+                }
+                String existingValue = getHostedOptionArgumentValue(imageBuilderArgs, argPrefix + '=');
+                if (existingValue != null) {
+                    /* Respect the existing value. Do not append overriding value. */
+                    return;
+                }
+            } else {
+                VMError.shouldNotReachHere("override=false currently only works for non-boolean options");
+            }
+        }
         imageBuilderArgs.add(plainArg);
     }
 
@@ -2344,7 +2369,7 @@ private static boolean logRedirectedToFile() {
 
     private boolean configureBuildOutput() {
         boolean useColorfulOutput = false;
-        String colorValue = getHostedOptionFinalArgumentValue(imageBuilderArgs, oHColor);
+        String colorValue = getHostedOptionArgumentValue(imageBuilderArgs, oHColor);
         if (colorValue != null) { // use value set by user
             if ("always".equals(colorValue)) {
                 useColorfulOutput = true;
@@ -2353,18 +2378,18 @@ private boolean configureBuildOutput() {
                 addPlainImageBuilderArg(oHColor + (useColorfulOutput ? "always" : "never"), OptionOrigin.originDriver);
             }
         } else {
-            Boolean buildOutputColorfulValue = getHostedOptionFinalBooleanArgumentValue(imageBuilderArgs, SubstrateOptions.BuildOutputColorful);
+            Boolean buildOutputColorfulValue = getHostedOptionBooleanArgumentValue(imageBuilderArgs, SubstrateOptions.BuildOutputColorful);
             if (buildOutputColorfulValue != null) {
                 useColorfulOutput = buildOutputColorfulValue; // use value set by user
             } else if (hasColorSupport()) {
                 useColorfulOutput = true;
                 addPlainImageBuilderArg(oHColor + "always", OptionOrigin.originDriver);
             }
         }
-        if (getHostedOptionFinalBooleanArgumentValue(imageBuilderArgs, SubstrateOptions.BuildOutputProgress) == null && hasProgressSupport(imageBuilderArgs)) {
+        if (getHostedOptionBooleanArgumentValue(imageBuilderArgs, SubstrateOptions.BuildOutputProgress) == null && hasProgressSupport(imageBuilderArgs)) {
             addPlainImageBuilderArg(oHEnableBuildOutputProgress);
         }
-        if (getHostedOptionFinalBooleanArgumentValue(imageBuilderArgs, SubstrateOptions.BuildOutputLinks) == null && (colorValue == null || "auto".equals(colorValue)) && useColorfulOutput) {
+        if (getHostedOptionBooleanArgumentValue(imageBuilderArgs, SubstrateOptions.BuildOutputLinks) == null && (colorValue == null || "auto".equals(colorValue)) && useColorfulOutput) {
             addPlainImageBuilderArg(oHEnableBuildOutputLinks);
         }
         return useColorfulOutput;

Original file line number	Diff line number	Diff line change
`@@ -251,7 +251,7 @@ private void handleJarFileArg(Path jarFilePath) {`
`251`	`251`	`}`
`252`	`252`	`if (!jarFileNameBase.isEmpty()) {`
`253`	`253`	`String origin = "manifest from " + jarFilePath.toUri();`
`254`		`- nativeImage.addPlainImageBuilderArg(nativeImage.oHName + jarFileNameBase, origin);`
	`254`	`+ nativeImage.addPlainImageBuilderArg(nativeImage.oHName + jarFileNameBase, origin, false);`
`255`	`255`	`}`
`256`	`256`	`Path finalFilePath = nativeImage.useBundle() ? nativeImage.bundleSupport.substituteClassPath(jarFilePath) : jarFilePath;`
`257`	`257`	`if (!NativeImage.processJarManifestMainAttributes(finalFilePath, nativeImage::handleManifestFileAttributes)) {`