You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by mi...@apache.org on 2022/12/26 19:48:03 UTC

[maven] branch MNG-7651 updated (3c0f98272 -> aae070bba)

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

michaelo pushed a change to branch MNG-7651
in repository https://gitbox.apache.org/repos/asf/maven.git


 discard 3c0f98272 [MNG-7651] Simplify and document merge of maven.config file and CLI args
     new aae070bba [MNG-7651] Simplify and document merge of maven.config file and CLI args

This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version.  This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:

 * -- * -- B -- O -- O -- O   (3c0f98272)
            \
             N -- N -- N   refs/heads/MNG-7651 (aae070bba)

You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.

Any revisions marked "omit" are not gone; other references still
refer to them.  Any revisions marked "discard" are gone forever.

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


[maven] 01/01: [MNG-7651] Simplify and document merge of maven.config file and CLI args

Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

michaelo pushed a commit to branch MNG-7651
in repository https://gitbox.apache.org/repos/asf/maven.git

commit aae070bbac562635877d72bcf781b1f71ba60c2a
Author: Michael Osipov <mi...@apache.org>
AuthorDate: Mon Dec 26 18:29:11 2022 +0100

    [MNG-7651] Simplify and document merge of maven.config file and CLI args
    
    This closes #939
---
 .../main/java/org/apache/maven/cli/MavenCli.java   | 44 ++++++++++++----------
 .../java/org/apache/maven/cli/MavenCliTest.java    |  6 +--
 2 files changed, 27 insertions(+), 23 deletions(-)

diff --git a/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java b/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java
index 509b02ede..b3dacf8e3 100644
--- a/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java
+++ b/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java
@@ -339,35 +339,33 @@ public class MavenCli {
 
         cliManager = new CLIManager();
 
-        List<String> args = new ArrayList<>();
         CommandLine mavenConfig = null;
         try {
             File configFile = new File(cliRequest.multiModuleProjectDirectory, MVN_MAVEN_CONFIG);
 
             if (configFile.isFile()) {
-                for (String arg : Files.readAllLines(configFile.toPath(), Charset.defaultCharset())) {
-                    if (!arg.isEmpty()) {
-                        args.add(arg);
-                    }
-                }
-
-                mavenConfig = cliManager.parse(args.toArray(new String[0]));
+                String[] args = Files.lines(configFile.toPath(), Charset.defaultCharset())
+                        .filter(arg -> !arg.isEmpty())
+                        .toArray(size -> new String[size]);
+                mavenConfig = cliManager.parse(args);
                 List<?> unrecognized = mavenConfig.getArgList();
                 if (!unrecognized.isEmpty()) {
-                    throw new ParseException("Unrecognized maven.config entries: " + unrecognized);
+                    // This file can only contain options, not args (goals or phases)
+                    throw new ParseException("Unrecognized maven.config file entries: " + unrecognized);
                 }
             }
         } catch (ParseException e) {
-            System.err.println("Unable to parse maven.config: " + e.getMessage());
+            System.err.println("Unable to parse maven.config file options: " + e.getMessage());
             cliManager.displayHelp(System.out);
             throw e;
         }
 
         try {
+            CommandLine mavenCli = cliManager.parse(cliRequest.args);
             if (mavenConfig == null) {
-                cliRequest.commandLine = cliManager.parse(cliRequest.args);
+                cliRequest.commandLine = mavenCli;
             } else {
-                cliRequest.commandLine = cliMerge(cliManager.parse(cliRequest.args), mavenConfig);
+                cliRequest.commandLine = cliMerge(mavenConfig, mavenCli);
             }
         } catch (ParseException e) {
             System.err.println("Unable to parse command line options: " + e.getMessage());
@@ -392,20 +390,26 @@ public class MavenCli {
         }
     }
 
-    private CommandLine cliMerge(CommandLine mavenArgs, CommandLine mavenConfig) {
+    private CommandLine cliMerge(CommandLine mavenConfig, CommandLine mavenCli) {
         CommandLine.Builder commandLineBuilder = new CommandLine.Builder();
 
-        // the args are easy, cli first then config file
-        for (String arg : mavenArgs.getArgs()) {
-            commandLineBuilder.addArg(arg);
-        }
-        for (String arg : mavenConfig.getArgs()) {
+        // the args are easy, CLI only since maven.config file can only contain options
+        for (String arg : mavenCli.getArgs()) {
             commandLineBuilder.addArg(arg);
         }
 
-        // now add all options, except for -D with cli first then config file
+        /* Although this looks wrong in terms of order Commons CLI stores the value of options in
+         * an array and when a value is potentionally overriden it is added to the array. The single
+         * arg option value is retrieved and instead of returning values[values.length-1] it returns
+         * values[0] which means that the original value instead of the overridden one is returned
+         * (first wins). With properties values are truely overriden since at the end a map is used
+         * to merge which means last wins.
+         *
+         * TODO Report this behavioral bug with Commons CLI
+         */
+        // now add all options, except for user properties with CLI first then maven.config file
         List<Option> setPropertyOptions = new ArrayList<>();
-        for (Option opt : mavenArgs.getOptions()) {
+        for (Option opt : mavenCli.getOptions()) {
             if (String.valueOf(CLIManager.SET_USER_PROPERTY).equals(opt.getOpt())) {
                 setPropertyOptions.add(opt);
             } else {
diff --git a/maven-embedder/src/test/java/org/apache/maven/cli/MavenCliTest.java b/maven-embedder/src/test/java/org/apache/maven/cli/MavenCliTest.java
index 1f1da2e7c..54e937b31 100644
--- a/maven-embedder/src/test/java/org/apache/maven/cli/MavenCliTest.java
+++ b/maven-embedder/src/test/java/org/apache/maven/cli/MavenCliTest.java
@@ -244,7 +244,7 @@ public class MavenCliTest {
         cli.cli(request);
         cli.properties(request);
 
-        String revision = System.getProperty("revision");
+        String revision = request.getUserProperties().getProperty("revision");
         assertEquals("8.1.0", revision);
     }
 
@@ -273,7 +273,7 @@ public class MavenCliTest {
         cli.cli(request);
         cli.properties(request);
 
-        String revision = System.getProperty("revision");
+        String revision = request.getUserProperties().getProperty("revision");
         assertEquals("8.2.0", revision);
     }
 
@@ -308,7 +308,7 @@ public class MavenCliTest {
 
         assertEquals("3", request.commandLine.getOptionValue(CLIManager.THREADS));
 
-        String revision = System.getProperty("revision");
+        String revision = request.getUserProperties().getProperty("revision");
         assertEquals("8.2.0", revision);
 
         assertEquals("bar ", request.getUserProperties().getProperty("foo"));