mirror of https://github.com/apache/maven.git
[MNG-6078] Undo the order reversal hack
- ca43030313
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
This commit is contained in:
parent
5cce371c8a
commit
5885e70e24
|
@ -19,15 +19,12 @@ package org.apache.maven.cli;
|
||||||
* under the License.
|
* under the License.
|
||||||
*/
|
*/
|
||||||
|
|
||||||
import static org.apache.maven.shared.utils.logging.MessageUtils.buffer;
|
|
||||||
|
|
||||||
import com.google.common.base.Charsets;
|
import com.google.common.base.Charsets;
|
||||||
import com.google.common.io.Files;
|
import com.google.common.io.Files;
|
||||||
import com.google.inject.AbstractModule;
|
import com.google.inject.AbstractModule;
|
||||||
import org.apache.commons.cli.CommandLine;
|
import org.apache.commons.cli.CommandLine;
|
||||||
import org.apache.commons.cli.ParseException;
|
import org.apache.commons.cli.ParseException;
|
||||||
import org.apache.commons.cli.UnrecognizedOptionException;
|
import org.apache.commons.cli.UnrecognizedOptionException;
|
||||||
import org.apache.commons.lang3.ArrayUtils;
|
|
||||||
import org.apache.maven.BuildAbort;
|
import org.apache.maven.BuildAbort;
|
||||||
import org.apache.maven.InternalErrorException;
|
import org.apache.maven.InternalErrorException;
|
||||||
import org.apache.maven.Maven;
|
import org.apache.maven.Maven;
|
||||||
|
@ -102,7 +99,6 @@ import java.io.IOException;
|
||||||
import java.io.InputStream;
|
import java.io.InputStream;
|
||||||
import java.io.PrintStream;
|
import java.io.PrintStream;
|
||||||
import java.util.ArrayList;
|
import java.util.ArrayList;
|
||||||
import java.util.Arrays;
|
|
||||||
import java.util.Collections;
|
import java.util.Collections;
|
||||||
import java.util.HashSet;
|
import java.util.HashSet;
|
||||||
import java.util.LinkedHashMap;
|
import java.util.LinkedHashMap;
|
||||||
|
@ -115,6 +111,8 @@ import java.util.StringTokenizer;
|
||||||
import java.util.regex.Matcher;
|
import java.util.regex.Matcher;
|
||||||
import java.util.regex.Pattern;
|
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
|
// 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
|
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()] ) );
|
cliRequest.commandLine = cliManager.parse( args.toArray( new String[args.size()] ) );
|
||||||
}
|
}
|
||||||
catch ( ParseException e )
|
catch ( ParseException e )
|
||||||
|
@ -1587,11 +1598,6 @@ public class MavenCli
|
||||||
|
|
||||||
if ( defStrs != null )
|
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 )
|
for ( String defStr : defStrs )
|
||||||
{
|
{
|
||||||
setCliProperty( defStr, userProperties );
|
setCliProperty( defStr, userProperties );
|
||||||
|
|
|
@ -150,6 +150,28 @@ public class MavenCliTest
|
||||||
|
|
||||||
String revision = System.getProperty( "revision" );
|
String revision = System.getProperty( "revision" );
|
||||||
assertEquals( "8.1.0", 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 );
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue