maven-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From schu...@apache.org
Subject [13/30] maven git commit: [MNG-6078] Undo the order reversal hack
Date Wed, 08 Mar 2017 16:56:29 GMT
[MNG-6078] Undo the order reversal hack

- ca4303031357a7decaee8de770b71fb2c2fedd28 used a hack to reverse the order of arguments
- The side effect of the hack is that the first named system property value on the CLI would
win
- The side-effect is causing a lot of integration test builds to fail and will likely have
other unintended consequences
- Correct fix is to put system properties at the end.
- If this change passes the integration tests then it will need to be augmented to correctly
round-trip the CLI options
  as there is the potential that somebody may legitimately be passing an arg parameter a value
that starts with -D
  for example 'mvn -ep -Dsecretpassword' or 'mvn -l -D.log' but given that this requires a
parse and unparse
  to handle the escaping, I want to get evidence that the integration tests pass first


Project: http://git-wip-us.apache.org/repos/asf/maven/repo
Commit: http://git-wip-us.apache.org/repos/asf/maven/commit/5885e70e
Tree: http://git-wip-us.apache.org/repos/asf/maven/tree/5885e70e
Diff: http://git-wip-us.apache.org/repos/asf/maven/diff/5885e70e

Branch: refs/heads/DEPMGMT-INCLUDE
Commit: 5885e70e24a69914da892eb343906658d5463bfa
Parents: 5cce371
Author: Stephen Connolly <stephen.alan.connolly@gmail.com>
Authored: Tue Feb 21 00:11:27 2017 +0000
Committer: Stephen Connolly <stephen.alan.connolly@gmail.com>
Committed: Thu Feb 23 12:44:34 2017 +0000

----------------------------------------------------------------------
 .../java/org/apache/maven/cli/MavenCli.java     | 26 ++++++++++++--------
 .../java/org/apache/maven/cli/MavenCliTest.java | 22 +++++++++++++++++
 2 files changed, 38 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/maven/blob/5885e70e/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java
----------------------------------------------------------------------
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 4e10b05..f788a5f 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
@@ -19,15 +19,12 @@ package org.apache.maven.cli;
  * under the License.
  */
 
-import static org.apache.maven.shared.utils.logging.MessageUtils.buffer;
-
 import com.google.common.base.Charsets;
 import com.google.common.io.Files;
 import com.google.inject.AbstractModule;
 import org.apache.commons.cli.CommandLine;
 import org.apache.commons.cli.ParseException;
 import org.apache.commons.cli.UnrecognizedOptionException;
-import org.apache.commons.lang3.ArrayUtils;
 import org.apache.maven.BuildAbort;
 import org.apache.maven.InternalErrorException;
 import org.apache.maven.Maven;
@@ -102,7 +99,6 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.io.PrintStream;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
@@ -115,6 +111,8 @@ import java.util.StringTokenizer;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import static org.apache.maven.shared.utils.logging.MessageUtils.buffer;
+
 // TODO push all common bits back to plexus cli and prepare for transition to Guice. We don't
need 50 ways to make CLIs
 
 /**
@@ -417,7 +415,20 @@ public class MavenCli
 
         try
         {
-            args.addAll( 0, Arrays.asList( cliRequest.args ) );
+            int index = 0;
+            for ( String arg : cliRequest.args )
+            {
+                if ( arg.startsWith( "-D" ) )
+                {
+                    // a property definition so needs to come last so that the last property
wins
+                    args.add( arg );
+                }
+                else
+                {
+                    // not a property definition so needs to come first to override maven.config
+                    args.add( index++, arg );
+                }
+            }
             cliRequest.commandLine = cliManager.parse( args.toArray( new String[args.size()]
) );
         }
         catch ( ParseException e )
@@ -1587,11 +1598,6 @@ public class MavenCli
             
             if ( defStrs != null )
             {
-                //The following is needed to get precedence
-                //of properties which are defined on command line
-                //over properties defined in the .mvn/maven.config. 
-                ArrayUtils.reverse( defStrs );
-                
                 for ( String defStr : defStrs )
                 {
                     setCliProperty( defStr, userProperties );

http://git-wip-us.apache.org/repos/asf/maven/blob/5885e70e/maven-embedder/src/test/java/org/apache/maven/cli/MavenCliTest.java
----------------------------------------------------------------------
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 d926624..66f247c 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
@@ -150,6 +150,28 @@ public class MavenCliTest
         
         String revision = System.getProperty( "revision" );
         assertEquals( "8.1.0", revision );
+    }
 
+    /**
+     * Read .mvn/maven.config with the following definitions:
+     * <pre>
+     *   -T 3
+     *   -Drevision=1.3.0
+     * </pre>
+     * and check if the {@code -Drevision-1.3.0} option can be overwritten via command line
+     * argument.
+     * @throws Exception
+     */
+    public void testMVNConfigurationCLIRepeatedPropertiesLastWins() throws Exception {
+        System.setProperty( MavenCli.MULTIMODULE_PROJECT_DIRECTORY, new File( "src/test/projects/mavenConfigProperties"
).getCanonicalPath() );
+        CliRequest request = new CliRequest( new String[]{ "-Drevision=8.1.0", "-Drevision=8.2.0"},
null );
+
+        cli.initialize( request );
+        // read .mvn/maven.config
+        cli.cli( request );
+        cli.properties( request );
+
+        String revision = System.getProperty( "revision" );
+        assertEquals( "8.2.0", revision );
     }
 }


Mime
View raw message