(maven) branch master updated: [MNG-8283] Parser must not alter global state (#1776)

[cs...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

cstamas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven.git


The following commit(s) were added to refs/heads/master by this push:
     new 20fe966770 [MNG-8283] Parser must not alter global state (#1776)
20fe966770 is described below

commit 20fe966770a87cab704e4bd311825750883235c2
Author: Tamas Cservenak <ta...@cservenak.net>
AuthorDate: Sat Oct 5 09:32:46 2024 +0200

    [MNG-8283] Parser must not alter global state (#1776)
    
    Parser must not (esp "partially") alter global state like
    setting Java System Properties. If invoker wants, invoker
    can set those (and cleanup).
    
    ---
    
    https://issues.apache.org/jira/browse/MNG-8283
---
 .../java/org/apache/maven/cling/invoker/BaseParser.java     | 12 ------------
 .../java/org/apache/maven/cling/invoker/LookupInvoker.java  | 13 +++++++++++++
 .../apache/maven/cling/invoker/mvn/DefaultMavenInvoker.java |  2 +-
 3 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/maven-cli/src/main/java/org/apache/maven/cling/invoker/BaseParser.java b/maven-cli/src/main/java/org/apache/maven/cling/invoker/BaseParser.java
index e6fde18337..7d1a8aaa78 100644
--- a/maven-cli/src/main/java/org/apache/maven/cling/invoker/BaseParser.java
+++ b/maven-cli/src/main/java/org/apache/maven/cling/invoker/BaseParser.java
@@ -34,7 +34,6 @@ import java.util.Map;
 import java.util.Objects;
 import java.util.Properties;
 import java.util.ServiceLoader;
-import java.util.Set;
 import java.util.function.Function;
 
 import org.apache.maven.api.Constants;
@@ -156,8 +155,6 @@ public abstract class BaseParser<O extends Options, R extends InvokerRequest<O>>
             }
             result = getCanonicalPath(Paths.get(mavenHome));
         }
-        // TODO: we still do this but would be cool if this becomes unneeded
-        System.setProperty(Constants.MAVEN_HOME, result.toString());
         return result;
     }
 
@@ -295,15 +292,6 @@ public abstract class BaseParser<O extends Options, R extends InvokerRequest<O>>
         Path propertiesFile = mavenConf.resolve("maven.properties");
         MavenPropertiesLoader.loadProperties(userProperties, propertiesFile, callback, false);
 
-        // ----------------------------------------------------------------------
-        // I'm leaving the setting of system properties here as not to break
-        // the SystemPropertyProfileActivator. This won't harm embedding. jvz.
-        // ----------------------------------------------------------------------
-        Set<String> sys = SystemProperties.getSystemProperties().stringPropertyNames();
-        userProperties.stringPropertyNames().stream()
-                .filter(k -> !sys.contains(k))
-                .forEach(k -> System.setProperty(k, userProperties.getProperty(k)));
-
         return toMap(userProperties);
     }
 
diff --git a/maven-cli/src/main/java/org/apache/maven/cling/invoker/LookupInvoker.java b/maven-cli/src/main/java/org/apache/maven/cling/invoker/LookupInvoker.java
index 3411c4604d..376e09f086 100644
--- a/maven-cli/src/main/java/org/apache/maven/cling/invoker/LookupInvoker.java
+++ b/maven-cli/src/main/java/org/apache/maven/cling/invoker/LookupInvoker.java
@@ -25,6 +25,7 @@ import java.io.PrintStream;
 import java.io.PrintWriter;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
@@ -153,7 +154,17 @@ public abstract class LookupInvoker<
         requireNonNull(invokerRequest);
 
         try (C context = createContext(invokerRequest)) {
+            Properties props = (Properties) System.getProperties().clone();
             try {
+                HashSet<String> sys =
+                        new HashSet<>(invokerRequest.systemProperties().keySet());
+                invokerRequest.userProperties().entrySet().stream()
+                        .filter(k -> !sys.contains(k.getKey()))
+                        .forEach(k -> System.setProperty(k.getKey(), k.getValue()));
+                System.setProperty(
+                        Constants.MAVEN_HOME,
+                        invokerRequest.installationDirectory().toString());
+
                 validate(context);
                 prepare(context);
                 configureLogging(context);
@@ -181,6 +192,8 @@ public abstract class LookupInvoker<
                 return execute(context);
             } catch (Exception e) {
                 throw handleException(context, e);
+            } finally {
+                System.setProperties(props);
             }
         }
     }
diff --git a/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/DefaultMavenInvoker.java b/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/DefaultMavenInvoker.java
index 58d9c388b7..3c8f6bb8f9 100644
--- a/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/DefaultMavenInvoker.java
+++ b/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/DefaultMavenInvoker.java
@@ -110,9 +110,9 @@ public abstract class DefaultMavenInvoker<
         DefaultMavenExecutionRequest mavenExecutionRequest = new DefaultMavenExecutionRequest();
         mavenExecutionRequest.setRepositoryCache(new DefaultRepositoryCache());
         mavenExecutionRequest.setInteractiveMode(true);
+        mavenExecutionRequest.setCacheTransferError(false);
         mavenExecutionRequest.setIgnoreInvalidArtifactDescriptor(true);
         mavenExecutionRequest.setIgnoreMissingArtifactDescriptor(true);
-        mavenExecutionRequest.setProjectPresent(true);
         mavenExecutionRequest.setRecursive(true);
         mavenExecutionRequest.setReactorFailureBehavior(MavenExecutionRequest.REACTOR_FAIL_FAST);
         mavenExecutionRequest.setStartTime(new Date());