[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-6164 Commit: dc9c4db4494b62e2231bb67b39678decf6329852 Parents: 5885e70 Author: Stephen Connolly <stephen.alan.conno...@gmail.com> Authored: Tue Feb 21 10:10:21 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 | 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 ) ); + } }
