ppalaga commented on a change in pull request #580:
URL: https://github.com/apache/maven-mvnd/pull/580#discussion_r782206760



##########
File path: client/src/main/java/org/mvndaemon/mvnd/client/DefaultClient.java
##########
@@ -114,6 +115,12 @@ 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())) {
+            String jvmArgs = 
Files.lines(parameters.jvmConfigPath()).collect(Collectors.joining(" "));
+            parameters = parameters.withJvmArgs(" " + jvmArgs);

Review comment:
       So we do not care in any way to combine the content of `jvm.config` with 
`mvnd.jvmArgs`. If the user passes `-Xmx` via both, we pass `-Xmx` twice to 
`java` executable, possibly with two distinct values. I do not know whether 
`java` accepts that and whether it accepts the first or the last value. Anyway, 
it is might become a source of confusion for the users - "I am passing this 
value via jvm.config but it does not get effective.... Why oh why?!?! Oh, geez, 
I am also passing it via mvnd.jvmArgs and only that one is accepted!". I wonder 
whether we could somehow prevent that potential confusion? 

##########
File path: client/src/main/java/org/mvndaemon/mvnd/client/DefaultClient.java
##########
@@ -114,6 +115,12 @@ 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())) {
+            String jvmArgs = 
Files.lines(parameters.jvmConfigPath()).collect(Collectors.joining(" "));

Review comment:
       The stream returned by Files.lines() should be closed.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to