[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 Commit: 5885e70e24a69914da892eb343906658d5463bfa Parents: 5cce371 Author: Stephen Connolly <stephen.alan.conno...@gmail.com> Authored: Tue Feb 21 00:11:27 2017 +0000 Committer: Stephen Connolly <stephen.alan.conno...@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 ); } }
