Mailing-List: contact commits-help@maven.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@maven.apache.org
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
From: schulte@apache.org
To: commits@maven.apache.org
Date: Wed, 08 Mar 2017 17:04:34 -0000
Message-Id: <9251f713e92b46ccbcba1f3e6da40d17@git.apache.org>
In-Reply-To: <5635eba896e245219ad9d10d6282e57d@git.apache.org>
References: <5635eba896e245219ad9d10d6282e57d@git.apache.org>
Subject: [34/45] maven git commit: [MNG-6078] Perform a proper merge of the
 two sources of command line arguments
archived-at: Wed, 08 Mar 2017 17:04:08 -0000

[MNG-6078] Perform a proper merge of the two sources of command line arguments

- Needed to extend Commons CLI's CommandLine just to perform the merged


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

Branch: refs/heads/MNG-6114
Commit: dc9c4db4494b62e2231bb67b39678decf6329852
Parents: 5885e70
Author: Stephen Connolly <stephen.alan.connolly@gmail.com>
Authored: Tue Feb 21 10:10:21 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     | 25 +++---
 .../org/apache/maven/cli/MergedCommandLine.java | 75 ++++++++++++++++++
 .../java/org/apache/maven/cli/MavenCliTest.java | 83 +++++++++++++++-----
 3 files changed, 149 insertions(+), 34 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/maven/blob/dc9c4db4/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 f788a5f..8d38ab0 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
@@ -383,7 +383,7 @@ public class MavenCli
         CLIManager cliManager = new CLIManager();
 
         List<String> args = new ArrayList<>();
-
+        CommandLine mavenConfig = null;
         try
         {
             File configFile = new File( cliRequest.multiModuleProjectDirectory, MVN_MAVEN_CONFIG );
@@ -398,8 +398,8 @@ public class MavenCli
                     }
                 }
 
-                CommandLine config = cliManager.parse( args.toArray( new String[args.size()] ) );
-                List<?> unrecongized = config.getArgList();
+                mavenConfig = cliManager.parse( args.toArray( new String[args.size()] ) );
+                List<?> unrecongized = mavenConfig.getArgList();
                 if ( !unrecongized.isEmpty() )
                 {
                     throw new ParseException( "Unrecognized maven.config entries: " + unrecongized );
@@ -415,21 +415,14 @@ public class MavenCli
 
         try
         {
-            int index = 0;
-            for ( String arg : cliRequest.args )
+            if ( mavenConfig == null )
             {
-                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( cliRequest.args );
+            }
+            else
+            {
+                cliRequest.commandLine = new MergedCommandLine( cliManager.parse( cliRequest.args ), mavenConfig );
             }
-            cliRequest.commandLine = cliManager.parse( args.toArray( new String[args.size()] ) );
         }
         catch ( ParseException e )
         {

http://git-wip-us.apache.org/repos/asf/maven/blob/dc9c4db4/maven-embedder/src/main/java/org/apache/maven/cli/MergedCommandLine.java
----------------------------------------------------------------------
diff --git a/maven-embedder/src/main/java/org/apache/maven/cli/MergedCommandLine.java b/maven-embedder/src/main/java/org/apache/maven/cli/MergedCommandLine.java
new file mode 100644
index 0000000..cb0a587
--- /dev/null
+++ b/maven-embedder/src/main/java/org/apache/maven/cli/MergedCommandLine.java
@@ -0,0 +1,75 @@
+package org.apache.maven.cli;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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.
+ */
+
+import org.apache.commons.cli.CommandLine;
+import org.apache.commons.cli.Option;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * A {@link CommandLine} instance that represents a merged command line combining CLI arguments with those from the
+ * {@code .mvn/maven.config} while reflecting the handling of {@link CLIManager#SET_SYSTEM_PROPERTY} versus all the
+ * other command line options (last wins vs first wins respectively).
+ */
+class MergedCommandLine
+    extends CommandLine
+{
+    MergedCommandLine( CommandLine commandLine, CommandLine configFile )
+    {
+        // such a pity that Commons CLI does not offer either a builder or a formatter and we need to extend
+        // to perform the merge. A formatter would mean we could unparse and reparse (not ideal but would work).
+        // A builder would be ideal for this kind of merge like processing.
+        super();
+        // the args are easy, cli first then config file
+        for ( String arg : commandLine.getArgs() )
+        {
+            addArg( arg );
+        }
+        for ( String arg : configFile.getArgs() )
+        {
+            addArg( arg );
+        }
+        // now add all options, except for -D with cli first then config file
+        List<Option> setPropertyOptions = new ArrayList<>();
+        for ( Option opt : commandLine.getOptions() )
+        {
+            if ( String.valueOf( CLIManager.SET_SYSTEM_PROPERTY ).equals( opt.getOpt() ) )
+            {
+                setPropertyOptions.add( opt );
+            }
+            else
+            {
+                addOption( opt );
+            }
+        }
+        for ( Option opt : configFile.getOptions() )
+        {
+            addOption( opt );
+        }
+        // finally add the CLI system properties
+        for ( Option opt : setPropertyOptions )
+        {
+            addOption( opt );
+        }
+    }
+
+}

http://git-wip-us.apache.org/repos/asf/maven/blob/dc9c4db4/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 66f247c..9b480ea 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
@@ -19,11 +19,10 @@ package org.apache.maven.cli;
  * under the License.
  */
 
-import java.io.File;
-
+import junit.framework.TestCase;
 import org.apache.commons.cli.ParseException;
 
-import junit.framework.TestCase;
+import java.io.File;
 
 public class MavenCliTest
     extends TestCase
@@ -75,7 +74,8 @@ public class MavenCliTest
     public void testMavenConfig()
         throws Exception
     {
-        System.setProperty( MavenCli.MULTIMODULE_PROJECT_DIRECTORY, new File( "src/test/projects/config" ).getCanonicalPath() );
+        System.setProperty( MavenCli.MULTIMODULE_PROJECT_DIRECTORY,
+                            new File( "src/test/projects/config" ).getCanonicalPath() );
         CliRequest request = new CliRequest( new String[0], null );
 
         // read .mvn/maven.config
@@ -85,7 +85,7 @@ public class MavenCliTest
         assertEquals( "8", request.commandLine.getOptionValue( CLIManager.THREADS ) );
 
         // override from command line
-        request = new CliRequest( new String[] { "--builder", "foobar" }, null );
+        request = new CliRequest( new String[]{ "--builder", "foobar" }, null );
         cli.cli( request );
         assertEquals( "foobar", request.commandLine.getOptionValue( "builder" ) );
     }
@@ -93,7 +93,8 @@ public class MavenCliTest
     public void testMavenConfigInvalid()
         throws Exception
     {
-        System.setProperty( MavenCli.MULTIMODULE_PROJECT_DIRECTORY, new File( "src/test/projects/config-illegal" ).getCanonicalPath() );
+        System.setProperty( MavenCli.MULTIMODULE_PROJECT_DIRECTORY,
+                            new File( "src/test/projects/config-illegal" ).getCanonicalPath() );
         CliRequest request = new CliRequest( new String[0], null );
 
         cli.initialize( request );
@@ -107,7 +108,7 @@ public class MavenCliTest
 
         }
     }
-    
+
     /**
      * Read .mvn/maven.config with the following definitions:
      * <pre>
@@ -116,19 +117,23 @@ public class MavenCliTest
      * </pre>
      * and check if the {@code -T 3} option can be overwritten via command line
      * argument.
+     *
      * @throws Exception in case of failure.
      */
-    public void testMVNConfigurationThreadCanBeOverwrittenViaCommandLine() throws Exception {
-        System.setProperty( MavenCli.MULTIMODULE_PROJECT_DIRECTORY, new File( "src/test/projects/mavenConfigProperties" ).getCanonicalPath() );
+    public void testMVNConfigurationThreadCanBeOverwrittenViaCommandLine()
+        throws Exception
+    {
+        System.setProperty( MavenCli.MULTIMODULE_PROJECT_DIRECTORY,
+                            new File( "src/test/projects/mavenConfigProperties" ).getCanonicalPath() );
         CliRequest request = new CliRequest( new String[]{ "-T", "5" }, null );
 
         cli.initialize( request );
         // read .mvn/maven.config
         cli.cli( request );
-        
+
         assertEquals( "5", request.commandLine.getOptionValue( CLIManager.THREADS ) );
     }
-    
+
     /**
      * Read .mvn/maven.config with the following definitions:
      * <pre>
@@ -137,17 +142,21 @@ public class MavenCliTest
      * </pre>
      * and check if the {@code -Drevision-1.3.0} option can be overwritten via command line
      * argument.
+     *
      * @throws Exception
      */
-    public void testMVNConfigurationDefinedPropertiesCanBeOverwrittenViaCommandLine() 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"}, null );
+    public void testMVNConfigurationDefinedPropertiesCanBeOverwrittenViaCommandLine()
+        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" }, null );
 
         cli.initialize( request );
         // read .mvn/maven.config
         cli.cli( request );
         cli.properties( request );
-        
+
         String revision = System.getProperty( "revision" );
         assertEquals( "8.1.0", revision );
     }
@@ -160,11 +169,15 @@ public class MavenCliTest
      * </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 );
+    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
@@ -174,4 +187,38 @@ public class MavenCliTest
         String revision = System.getProperty( "revision" );
         assertEquals( "8.2.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 when there are
+     * funky arguments present.
+     *
+     * @throws Exception
+     */
+    public void testMVNConfigurationFunkyArguments()
+        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", "--file=-Dpom.xml", "\"-Dfoo=bar ", "\"-Dfoo2=bar two\"",
+                "-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 );
+
+        assertEquals( "bar ", request.getSystemProperties().getProperty( "foo" ) );
+        assertEquals( "bar two", request.getSystemProperties().getProperty( "foo2" ) );
+
+        assertEquals( "-Dpom.xml", request.getCommandLine().getOptionValue( CLIManager.ALTERNATE_POM_FILE ) );
+    }
 }