NativeImageMojo: Make additionalBuildArgs not override quarkus.native.additional-build-args property

[zakkak]

Description
When debugging a native-image -Dquarkus.native.additional-build-args can be very handy, since it allows the users to pass various flags to native-image.

For instance, -Dquarkus.native.additional-build-args='-H:Log=dwarf.*:3,-Dgraal.LogFile=log.txt' will enable verbose logging for dwarf symbols generation and store the output in log.txt.

However, in the case where the pom file of the application at hand already contains a non empty additionalBuildArgs entry in its configuration anything passed to quarkus.native.additional-build-args will be ignored.

For example ./mvnw clean compile package -Dquarkus.native.additional-build-args='-H:Log=dwarf.debug_lines:3,-Dgraal.LogFile=lineslog.out' -Dnative -pl integration-tests/rest-client does not pass -H:Log=dwarf.debug_lines:3,-Dgraal.LogFile=lineslog.out to native-image as shown in the log below:

./mvnw clean compile package -Dquarkus.native.additional-build-args='-H:Log=dwarf.debug_lines:3,-Dgraal.LogFile=lineslog.out' -Dnative -pl integration-tests/rest-client
...
[INFO] [io.quarkus.deployment.pkg.steps.JarResultBuildStep] Building native image source jar: /home/zakkak/code/quarkus/integration-tests/rest-client/target/quarkus-integration-test-rest-client-999-SNAPSHOT-native-image-source-jar/quarkus-integration-test-rest-client-999-SNAPSHOT-runner.jar
[INFO] [io.quarkus.deployment.pkg.steps.NativeImageBuildStep] Building native image from /home/zakkak/code/quarkus/integration-tests/rest-client/target/quarkus-integration-test-rest-client-999-SNAPSHOT-native-image-source-jar/quarkus-integration-test-rest-client-999-SNAPSHOT-runner.jar
[INFO] [io.quarkus.deployment.pkg.steps.NativeImageBuildStep] Running Quarkus native-image plugin on GraalVM Version 20.3.0-dev (Java Version null)
[INFO] [io.quarkus.deployment.pkg.steps.NativeImageBuildStep] /home/zakkak/Downloads/graalvm-ce-java11-20.3.0-dev/bin/native-image -J-Dsun.nio.ch.maxUpdateArraySize=100 -J-Djava.util.logging.manager=org.jboss.logmanager.LogManager -J-Dvertx.logger-delegate-factory-class-name=io.quarkus.vertx.core.runtime.VertxLogDelegateFactory -J-Dvertx.disableDnsResolver=true -J-Dio.netty.leakDetection.level=DISABLED -J-Dio.netty.allocator.maxOrder=1 -J-Duser.language=en -J-Dfile.encoding=UTF-8 -J-Djavax.net.ssl.trustStore=/home/zakkak/code/quarkus/integration-tests/rest-client/self-signed -J-Djavax.net.ssl.trustStorePassword=changeit --initialize-at-build-time= -H:InitialCollectionPolicy=com.oracle.svm.core.genscavenge.CollectionPolicy\$BySpaceAndTime -H:+JNI -jar quarkus-integration-test-rest-client-999-SNAPSHOT-runner.jar -H:FallbackThreshold=0 -H:+ReportExceptionStackTraces -H:-AddAllCharsets -H:EnableURLProtocols=http,https --enable-all-security-services -H:NativeLinkerOption=-no-pie --no-server -H:-UseServiceLoaderFeature -H:+StackTrace quarkus-integration-test-rest-client-999-SNAPSHOT-runner...

Implementation ideas
The override of the quarkus.native.additional-build-args property by additionalBuildArgs happens in

quarkus/devtools/maven/src/main/java/io/quarkus/maven/NativeImageMojo.java

Lines 268 to 273 in 9d5f822

	Map<String, String> config = createCustomConfig();
	Map<String, String> old = new HashMap<>();
	for (Map.Entry<String, String> e : config.entrySet()) {
	old.put(e.getKey(), System.getProperty(e.getKey()));
	System.setProperty(e.getKey(), e.getValue());
	}

Depending on how we want to handle this we can change

quarkus/devtools/maven/src/main/java/io/quarkus/maven/NativeImageMojo.java

Lines 318 to 324 in 9d5f822

	if (additionalBuildArgs != null && !additionalBuildArgs.isEmpty()) {
	configs.put("quarkus.native.additional-build-args",
	additionalBuildArgs.stream()
	.map(val -> val.replace("\\", "\\\\"))
	.map(val -> val.replace(",", "\\,"))
	.collect(joining(",")));
	}

to check if quarkus.native.additional-build-args is already defined and either ignore additionalBuildArgs (reverse behavior) or combine additionalBuildArgs with the existing quarkus.native.additional-build-args

[gsmet]

I don't think it's worth doing anything about it given additionalBuildArgs is deprecated and shouldn't be used.

[zakkak]

Uh interesting, I was not aware of that. So moving forward only quarkus.native.additional-build-args should be used for this functionality right?

In that case shall I open a PR replacing the use of additionalBuildArgs with quarkus.native.additional-build-args in the integration tests rest-client, redis-client, main, vertx-http, and smallrye-metrics?

BTW there is still a loose reference to additionalBuildArgs in https://github.com/quarkusio/quarkus/blob/master/docs/src/main/asciidoc/writing-native-applications-tips.adoc as well. I can fix that as well in the aforementioned PR.

@Karm FYI (you are using additionalBuildArgs in https://github.com/Karm/mandrel-integration-tests/blob/1c62a14d9f55e04eda78627037ccfb1b96553f2d/apps/quarkus-full-microprofile/pom.xml)

[geoand]

additionalBuildArgs needs to be removed from the maven plugin as this is not the first time it has caused confusion...

Uh interesting, I was not aware of that. So moving forward only quarkus.native.additional-build-args should be used for this functionality right?

exactly

[zakkak]

additionalBuildArgs needs to be removed from the maven plugin as this is not the first time it has caused confusion...

Or at least make the maven plugin print a warning that this is deprecated, it overrides quarkus.native.additional-build-args, and is going to be removed in the near future :)

[geoand]

I don't know if that is possible, but if it is, then it would surely make sense

[zakkak]

I think this should do it:

diff --git a/devtools/maven/src/main/java/io/quarkus/maven/NativeImageMojo.java b/devtools/maven/src/main/java/io/quarkus/maven/NativeImageMojo.java
index c3203c09ca..e3706f1391 100644
--- a/devtools/maven/src/main/java/io/quarkus/maven/NativeImageMojo.java
+++ b/devtools/maven/src/main/java/io/quarkus/maven/NativeImageMojo.java
@@ -316,6 +316,9 @@ public class NativeImageMojo extends AbstractMojo {
             configs.put("quarkus.native.add-all-charsets", addAllCharsets.toString());
         }
         if (additionalBuildArgs != null && !additionalBuildArgs.isEmpty()) {
+            getLog().warn("Your application is setting the deprecated 'additionalBuildArgs' Maven option."
+                    + " This option overrides any value passed to the quarkus.native.additional-build-args"
+                    + " property and will be removed in the future");
             configs.put("quarkus.native.additional-build-args",
                     additionalBuildArgs.stream()
                             .map(val -> val.replace("\\", "\\\\"))

Output:

[INFO] --- quarkus-maven-plugin:999-SNAPSHOT:native-image (native-image) @ quarkus-integration-test-rest-client ---
[WARNING] Your application is setting the deprecated 'additionalBuildArgs' Maven option. This option overrides any value passed to the quarkus.native.additional-build-args property and will be removed in the future
[INFO] [org.jboss.threads] JBoss Threads version 3.1.1.Final

I'll prepare a PR.