Daemon reuse ignores differences in .mvn/jvm.config, fixes #576 (#580)

apache · Jan 13, 2022 · 62d580b · 62d580b
1 parent 1de326c
commit 62d580b
Show file tree

Hide file tree

Showing 13 changed files with 127 additions and 38 deletions.
diff --git a/client/src/main/java/org/mvndaemon/mvnd/client/DaemonConnector.java b/client/src/main/java/org/mvndaemon/mvnd/client/DaemonConnector.java
@@ -36,8 +36,6 @@
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.stream.Collectors;
-import java.util.stream.Stream;
-import org.apache.maven.shared.utils.StringUtils;
 import org.mvndaemon.mvnd.common.DaemonCompatibilitySpec;
 import org.mvndaemon.mvnd.common.DaemonCompatibilitySpec.Result;
 import org.mvndaemon.mvnd.common.DaemonConnection;
@@ -348,15 +346,6 @@ private Process startDaemonProcess(String daemonId) {
                     }
                 }
             }
-            // .mvn/jvm.config
-            if (Files.isRegularFile(parameters.jvmConfigPath())) {
-                try (Stream<String> lines = Files.lines(parameters.jvmConfigPath())) {
-                    lines.flatMap(l -> Stream.of(l.split(" ")))
-                            .map(String::trim)
-                            .filter(StringUtils::isNotEmpty)
-                            .forEach(args::add);
-                }
-            }
             // memory
             String minHeapSize = parameters.minHeapSize();
             if (minHeapSize != null) {

diff --git a/client/src/main/java/org/mvndaemon/mvnd/client/DaemonParameters.java b/client/src/main/java/org/mvndaemon/mvnd/client/DaemonParameters.java
@@ -168,6 +168,11 @@ public Path suppliedPropertiesPath() {
                 .asPath();
     }
 
+    /**
+     * The content of the <code>.mvn/jvm.config</code> file will be read
+     * and used as arguments when starting a daemon JVM.
+     * See {@link Environment#MVND_JVM_ARGS}.
+     */
     public Path jvmConfigPath() {
         return multiModuleProjectDirectory().resolve(".mvn/jvm.config");
     }
@@ -311,9 +316,16 @@ public DaemonParameters cd(Path newUserDir) {
         return derive(b -> b.put(Environment.USER_DIR, newUserDir));
     }
 
-    public DaemonParameters withJdkJavaOpts(String opts) {
+    public DaemonParameters withJdkJavaOpts(String opts, boolean before) {
         String org = this.properties.getOrDefault(Environment.JDK_JAVA_OPTIONS.getProperty(), "");
-        return derive(b -> b.put(Environment.JDK_JAVA_OPTIONS, org + opts));
+        return derive(b -> b.put(Environment.JDK_JAVA_OPTIONS,
+                org.isEmpty() ? opts : before ? opts + " " + org : org + " " + opts));
+    }
+
+    public DaemonParameters withJvmArgs(String opts, boolean before) {
+        String org = this.properties.getOrDefault(Environment.MVND_JVM_ARGS.getProperty(), "");
+        return derive(b -> b.put(Environment.MVND_JVM_ARGS,
+                org.isEmpty() ? opts : before ? opts + " " + org : org + " " + opts));
     }
 
     protected DaemonParameters derive(Consumer<PropertiesBuilder> customizer) {

diff --git a/client/src/main/java/org/mvndaemon/mvnd/client/DefaultClient.java b/client/src/main/java/org/mvndaemon/mvnd/client/DefaultClient.java
@@ -33,6 +33,7 @@
 import java.util.List;
 import java.util.Optional;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import org.fusesource.jansi.Ansi;
 import org.fusesource.jansi.internal.CLibrary;
@@ -114,6 +115,14 @@ public static void main(String[] argv) throws Exception {
 
         System.setProperty(Environment.MVND_HOME.getProperty(), parameters.mvndHome().toString());
 
+        // .mvn/jvm.config
+        if (Files.isRegularFile(parameters.jvmConfigPath())) {
+            try (Stream<String> jvmArgs = Files.lines(parameters.jvmConfigPath())) {
+                String jvmArgsStr = jvmArgs.collect(Collectors.joining(" "));
+                parameters = parameters.withJvmArgs(jvmArgsStr, false);
+            }
+        }
+
         int exitCode = 0;
         boolean noBuffering = batchMode || parameters.noBuffering();
         try (TerminalOutput output = new TerminalOutput(noBuffering, parameters.rollingWindowSize(), logFile)) {
@@ -151,11 +160,12 @@ public static void setSystemPropertiesFromCommandLine(List<String> args) {
     public DefaultClient(DaemonParameters parameters) {
         // Those options are needed in order to be able to set the environment correctly
         this.parameters = parameters.withJdkJavaOpts(
-                " --add-opens java.base/java.io=ALL-UNNAMED"
-                        + " --add-opens java.base/java.lang=ALL-UNNAMED"
-                        + " --add-opens java.base/java.util=ALL-UNNAMED"
-                        + " --add-opens java.base/sun.net.www.protocol.jar=ALL-UNNAMED"
-                        + " --add-opens java.base/sun.nio.fs=ALL-UNNAMED");
+                "--add-opens java.base/java.io=ALL-UNNAMED "
+                        + "--add-opens java.base/java.lang=ALL-UNNAMED "
+                        + "--add-opens java.base/java.util=ALL-UNNAMED "
+                        + "--add-opens java.base/sun.net.www.protocol.jar=ALL-UNNAMED "
+                        + "--add-opens java.base/sun.nio.fs=ALL-UNNAMED",
+                true);
     }
 
     @Override

diff --git a/common/src/main/java/org/mvndaemon/mvnd/common/Environment.java b/common/src/main/java/org/mvndaemon/mvnd/common/Environment.java
@@ -185,35 +185,41 @@ public enum Environment {
      */
     MVND_THREADS("mvnd.threads", null, null, OptionType.STRING, Flags.NONE, "mvn:-T", "mvn:--threads"),
     /**
-     * The builder implementation the daemon should use
+     * The builder implementation the daemon should use.
      */
     MVND_BUILDER("mvnd.builder", null, "smart", OptionType.STRING, Flags.NONE, "mvn:-b", "mvn:--builder"),
     /**
-     * An ID for a newly started daemon
+     * An ID for a newly started daemon.
      */
     MVND_ID("mvnd.id", null, null, OptionType.STRING, Flags.INTERNAL),
     /**
-     * Internal option to specify the maven extension classpath
+     * Internal option to specify the maven extension classpath.
      */
     MVND_EXT_CLASSPATH("mvnd.extClasspath", null, null, OptionType.STRING, Flags.DISCRIMINATING | Flags.INTERNAL),
     /**
-     * Internal option to specify the list of maven extension to register
+     * Internal option to specify the list of maven extension to register.
      */
     MVND_CORE_EXTENSIONS("mvnd.coreExtensions", null, null, OptionType.STRING, Flags.DISCRIMINATING | Flags.INTERNAL),
     /**
-     * The <code>-Xms</code> value to pass to the daemon
+     * The <code>-Xms</code> value to pass to the daemon.
+     * This option takes precedence over options specified in {@link #MVND_JVM_ARGS}.
      */
     MVND_MIN_HEAP_SIZE("mvnd.minHeapSize", null, "128M", OptionType.MEMORY_SIZE, Flags.DISCRIMINATING),
     /**
-     * The <code>-Xmx</code> value to pass to the daemon
+     * The <code>-Xmx</code> value to pass to the daemon.
+     * This option takes precedence over options specified in {@link #MVND_JVM_ARGS}.
      */
     MVND_MAX_HEAP_SIZE("mvnd.maxHeapSize", null, "2G", OptionType.MEMORY_SIZE, Flags.DISCRIMINATING),
     /**
-     * The <code>-Xss</code> value to pass to the daemon
+     * The <code>-Xss</code> value to pass to the daemon.
+     * This option takes precedence over options specified in {@link #MVND_JVM_ARGS}.
      */
     MVND_THREAD_STACK_SIZE("mvnd.threadStackSize", null, "1M", OptionType.MEMORY_SIZE, Flags.DISCRIMINATING),
     /**
-     * Additional JVM args to pass to the daemon
+     * Additional JVM args to pass to the daemon.
+     * The content of the <code>.mvn/jvm.config</code> file will prepended (and thus with
+     * a lesser priority) to the user supplied value for this parameter before being used
+     * as startup options for the daemon JVM.
      */
     MVND_JVM_ARGS("mvnd.jvmArgs", null, null, OptionType.STRING, Flags.DISCRIMINATING | Flags.OPTIONAL),
     /**

diff --git a/daemon/src/main/java/org/apache/maven/cli/DaemonMavenCli.java b/daemon/src/main/java/org/apache/maven/cli/DaemonMavenCli.java
@@ -394,7 +394,7 @@ void logging(CliRequest cliRequest) {
 
             // redirect stdout and stderr to file
             try {
-                PrintStream ps = new PrintStream(new FileOutputStream(logFile));
+                PrintStream ps = new PrintStream(new FileOutputStream(logFile), true);
                 System.setOut(ps);
                 System.setErr(ps);
             } catch (FileNotFoundException e) {

diff --git a/daemon/src/main/java/org/mvndaemon/mvnd/logging/smart/LoggingOutputStream.java b/daemon/src/main/java/org/mvndaemon/mvnd/logging/smart/LoggingOutputStream.java
@@ -78,7 +78,7 @@ boolean isEol() {
 
     public static class LoggingPrintStream extends PrintStream {
         public LoggingPrintStream(LoggingOutputStream out) {
-            super(out);
+            super(out, true);
         }
 
         public void forceFlush() {

diff --git a/dist/src/main/distro/bin/mvnd.cmd b/dist/src/main/distro/bin/mvnd.cmd
@@ -160,15 +160,6 @@ cd "%EXEC_DIR%"
 
 :endDetectBaseDir
 
-set "jvmConfig=\.mvn\jvm.config"
-if not exist "%MAVEN_PROJECTBASEDIR%%jvmConfig%" goto endReadAdditionalConfig
-
-@setlocal EnableExtensions EnableDelayedExpansion
-for /F "usebackq delims=" %%a in ("%MAVEN_PROJECTBASEDIR%\.mvn\jvm.config") do set JVM_CONFIG_MAVEN_PROPS=!JVM_CONFIG_MAVEN_PROPS! %%a
-@endlocal & set JVM_CONFIG_MAVEN_PROPS=%JVM_CONFIG_MAVEN_PROPS%
-
-:endReadAdditionalConfig
-
 @setlocal EnableExtensions EnableDelayedExpansion
 for %%i in ("%MVND_HOME%"\mvn\boot\*.jar "%MVND_HOME%"\mvn\lib\ext\*.jar "%MVND_HOME%"\mvn\lib\*.jar) do set DAEMON_JAR=!DAEMON_JAR!;%%i
 @endlocal & set DAEMON_JAR="%DAEMON_JAR%"

diff --git a/dist/src/main/distro/bin/mvnd.sh b/dist/src/main/distro/bin/mvnd.sh
@@ -173,7 +173,6 @@ concat_lines() {
 }
 
 MAVEN_PROJECTBASEDIR="${MAVEN_BASEDIR:-`find_maven_basedir "$@"`}"
-MAVEN_OPTS="`concat_lines "$MAVEN_PROJECTBASEDIR/.mvn/jvm.config"` $MAVEN_OPTS"
 
 # For Cygwin, switch project base directory path to Windows format before
 # executing Maven otherwise this will cause Maven not to consider it.

diff --git a/integration-tests/src/test/java/org/mvndaemon/mvnd/it/JvmConfigNativeIT.java b/integration-tests/src/test/java/org/mvndaemon/mvnd/it/JvmConfigNativeIT.java
@@ -0,0 +1,50 @@
+/*
+ * Copyright 2019-2022 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.mvndaemon.mvnd.it;
+
+import java.io.IOException;
+import java.util.stream.Collectors;
+import javax.inject.Inject;
+import org.junit.jupiter.api.Test;
+import org.mvndaemon.mvnd.assertj.TestClientOutput;
+import org.mvndaemon.mvnd.client.Client;
+import org.mvndaemon.mvnd.client.DaemonParameters;
+import org.mvndaemon.mvnd.junit.MvndNativeTest;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+@MvndNativeTest(projectDir = "src/test/projects/jvm-config")
+public class JvmConfigNativeIT {
+
+    @Inject
+    Client client;
+
+    @Inject
+    DaemonParameters parameters;
+
+    @Test
+    void version() throws IOException, InterruptedException {
+        final TestClientOutput o = new TestClientOutput();
+        client.execute(o, "org.codehaus.gmaven:groovy-maven-plugin:2.1.1:execute",
+                "-Dsource=System.out.println(java.lang.management.ManagementFactory.getRuntimeMXBean().getInputArguments())")
+                .assertSuccess();
+        String xmx = "-Xmx512k";
+        assertTrue(o.getMessages().stream()
+                .anyMatch(m -> m.toString().contains(xmx)), "Output should contain " + xmx + " but is:\n"
+                        + o.getMessages().stream().map(Object::toString).collect(Collectors.joining("\n")));
+    }
+
+}
diff --git a/integration-tests/src/test/projects/jvm-config/.mvn/jvm.config b/integration-tests/src/test/projects/jvm-config/.mvn/jvm.config
@@ -0,0 +1 @@
+-Xmx512k
diff --git a/integration-tests/src/test/projects/jvm-config/.mvn/maven.config b/integration-tests/src/test/projects/jvm-config/.mvn/maven.config
@@ -0,0 +1,3 @@
+-Dmaven.wagon.httpconnectionManager.ttlSeconds=120
+-Dmaven.wagon.http.retryHandler.requestSentEnabled=true
+-Dmaven.wagon.http.retryHandler.count=10
diff --git a/integration-tests/src/test/projects/jvm-config/pom.xml b/integration-tests/src/test/projects/jvm-config/pom.xml
@@ -0,0 +1,27 @@
+<!--
+
+    Copyright 2021 the original author or authors.
+
+    Licensed under the Apache License, Version 2.0 (the "License");
+    you may not use this file except in compliance with the License.
+    You may obtain a copy of the License at
+
+         http://www.apache.org/licenses/LICENSE-2.0
+
+    Unless required by applicable law or agreed to in writing, software
+    distributed under the License is distributed on an "AS IS" BASIS,
+    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+    See the License for the specific language governing permissions and
+    limitations under the License.
+
+-->
+<project xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="http://maven.apache.org/POM/4.0.0"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+
+    <modelVersion>4.0.0</modelVersion>
+    <groupId>org.mvndaemon.mvnd.test.jvm-config</groupId>
+    <artifactId>jvm-config</artifactId>
+    <version>0.0.1-SNAPSHOT</version>
+    <packaging>pom</packaging>
+
+</project>
diff --git a/pom.xml b/pom.xml
@@ -309,6 +309,7 @@ limitations under the License.</inlineHeader>
                             <exclude>**/m2.conf</exclude>
                             <exclude>**/mvnd</exclude>
                             <exclude>**/.mvn/maven.config</exclude>
+                            <exclude>**/.mvn/jvm.config</exclude>
                             <exclude>.gitattributes/</exclude>
                             <exclude>.mvn/maven.config</exclude>
                             <exclude>.mvn/wrapper/</exclude>