From e6a7cadc14c10a6bd23d8b258c66f20c07882d32 Mon Sep 17 00:00:00 2001 From: Slawomir Jaranowski Date: Thu, 5 May 2022 23:23:58 +0200 Subject: [PATCH] [MRELEASE-1088] Remove parsing of CLI arguments --- maven-release-manager/pom.xml | 11 - .../release/exec/InvokerMavenExecutor.java | 346 +----------------- .../exec/InvokerMavenExecutorTest.java | 79 +--- .../it/projects/prepare/MRELEASE-667/pom.xml | 4 +- 4 files changed, 23 insertions(+), 417 deletions(-) diff --git a/maven-release-manager/pom.xml b/maven-release-manager/pom.xml index 81501fb17..591dac371 100644 --- a/maven-release-manager/pom.xml +++ b/maven-release-manager/pom.xml @@ -95,17 +95,6 @@ commons-lang3 3.12.0 - - commons-cli - commons-cli - 1.2 - - - commons-lang - commons-lang - - - org.apache.maven.scm diff --git a/maven-release-manager/src/main/java/org/apache/maven/shared/release/exec/InvokerMavenExecutor.java b/maven-release-manager/src/main/java/org/apache/maven/shared/release/exec/InvokerMavenExecutor.java index 2a6e061bc..b52e4c865 100644 --- a/maven-release-manager/src/main/java/org/apache/maven/shared/release/exec/InvokerMavenExecutor.java +++ b/maven-release-manager/src/main/java/org/apache/maven/shared/release/exec/InvokerMavenExecutor.java @@ -22,18 +22,12 @@ import java.io.File; import java.io.FileWriter; import java.io.IOException; -import java.util.Arrays; +import java.util.ArrayList; import java.util.List; -import java.util.Properties; -import org.apache.commons.cli.CommandLine; -import org.apache.commons.cli.OptionBuilder; -import org.apache.commons.cli.Options; -import org.apache.commons.cli.PosixParser; import org.apache.maven.settings.io.xpp3.SettingsXpp3Writer; import org.apache.maven.shared.invoker.DefaultInvocationRequest; import org.apache.maven.shared.invoker.DefaultInvoker; -import org.apache.maven.shared.invoker.InvocationOutputHandler; import org.apache.maven.shared.invoker.InvocationRequest; import org.apache.maven.shared.invoker.InvocationResult; import org.apache.maven.shared.invoker.Invoker; @@ -43,316 +37,40 @@ import org.apache.maven.shared.release.env.ReleaseEnvironment; import org.codehaus.plexus.component.annotations.Component; import org.codehaus.plexus.logging.Logger; -import org.codehaus.plexus.util.cli.CommandLineUtils; /** * Fork Maven using the maven-invoker shared library. */ @Component( role = MavenExecutor.class, hint = "invoker" ) -@SuppressWarnings( "static-access" ) public class InvokerMavenExecutor extends AbstractMavenExecutor { - private static final Options OPTIONS = new Options(); - - private static final char SET_SYSTEM_PROPERTY = 'D'; - - private static final char OFFLINE = 'o'; - - private static final char REACTOR = 'r'; - - private static final char QUIET = 'q'; - - private static final char DEBUG = 'X'; - - private static final char ERRORS = 'e'; - - private static final char NON_RECURSIVE = 'N'; - - private static final char UPDATE_SNAPSHOTS = 'U'; - - private static final char ACTIVATE_PROFILES = 'P'; - - private static final char CHECKSUM_FAILURE_POLICY = 'C'; - - private static final char CHECKSUM_WARNING_POLICY = 'c'; - - private static final char ALTERNATE_USER_SETTINGS = 's'; - - private static final String ALTERNATE_GLOBAL_SETTINGS = "gs"; - - private static final String FAIL_FAST = "ff"; - - private static final String FAIL_AT_END = "fae"; - - private static final String FAIL_NEVER = "fn"; - - private static final String ALTERNATE_POM_FILE = "f"; - - private static final String THREADS = "T"; - - private static final String BATCH_MODE = "B"; - - /** Constant ALTERNATE_USER_TOOLCHAINS='t' */ - public static final char ALTERNATE_USER_TOOLCHAINS = 't'; - - static - { - OPTIONS.addOption( - OptionBuilder.withLongOpt( "define" ).hasArg().withDescription( "Define a system property" ).create( - SET_SYSTEM_PROPERTY ) ); - - OPTIONS.addOption( OptionBuilder.withLongOpt( "offline" ).withDescription( "Work offline" ).create( OFFLINE ) ); - - OPTIONS.addOption( - OptionBuilder.withLongOpt( "quiet" ).withDescription( "Quiet output - only show errors" ).create( QUIET ) ); - - OPTIONS.addOption( - OptionBuilder.withLongOpt( "debug" ).withDescription( "Produce execution debug output" ).create( DEBUG ) ); - - OPTIONS.addOption( - OptionBuilder.withLongOpt( "errors" ).withDescription( "Produce execution error messages" ).create( - ERRORS ) ); - - OPTIONS.addOption( OptionBuilder.withLongOpt( "reactor" ).withDescription( - "Execute goals for project found in the reactor" ).create( REACTOR ) ); - - OPTIONS.addOption( - OptionBuilder.withLongOpt( "non-recursive" ).withDescription( "Do not recurse into sub-projects" ).create( - NON_RECURSIVE ) ); - - OPTIONS.addOption( OptionBuilder.withLongOpt( "update-snapshots" ).withDescription( - "Forces a check for updated releases and snapshots on remote repositories" ).create( UPDATE_SNAPSHOTS ) ); - - OPTIONS.addOption( OptionBuilder.withLongOpt( "activate-profiles" ).withDescription( - "Comma-delimited list of profiles to activate" ).hasArg().create( ACTIVATE_PROFILES ) ); - - OPTIONS.addOption( OptionBuilder.withLongOpt( "strict-checksums" ).withDescription( - "Fail the build if checksums don't match" ).create( CHECKSUM_FAILURE_POLICY ) ); - - OPTIONS.addOption( - OptionBuilder.withLongOpt( "lax-checksums" ).withDescription( "Warn if checksums don't match" ).create( - CHECKSUM_WARNING_POLICY ) ); - - OPTIONS.addOption( OptionBuilder.withLongOpt( "settings" ).withDescription( - "Alternate path for the user settings file" ).hasArg().create( ALTERNATE_USER_SETTINGS ) ); - - OPTIONS.addOption( OptionBuilder.withLongOpt( "global-settings" ).withDescription( - " Alternate path for the global settings file" ).hasArg().create( ALTERNATE_GLOBAL_SETTINGS ) ); - - OPTIONS.addOption( OptionBuilder.withLongOpt( "fail-fast" ).withDescription( - "Stop at first failure in reactorized builds" ).create( FAIL_FAST ) ); - - OPTIONS.addOption( OptionBuilder.withLongOpt( "fail-at-end" ).withDescription( - "Only fail the build afterwards; allow all non-impacted builds to continue" ).create( FAIL_AT_END ) ); - - OPTIONS.addOption( OptionBuilder.withLongOpt( "fail-never" ).withDescription( - "NEVER fail the build, regardless of project result" ).create( FAIL_NEVER ) ); - - OPTIONS.addOption( OptionBuilder.withLongOpt( "file" ).withDescription( - "Force the use of an alternate POM file." ).hasArg().create( ALTERNATE_POM_FILE ) ); - - OPTIONS.addOption( OptionBuilder.withLongOpt( "threads" ).withDescription( - "Thread count, for instance 2.0C where C is core multiplied" ).hasArg().create( THREADS ) ); - - OPTIONS.addOption( OptionBuilder.withLongOpt( "batch-mode" ).withDescription( - "Run in non-interactive (batch) mode" ).create( BATCH_MODE ) ); - - OPTIONS.addOption( OptionBuilder.withLongOpt( "toolchains" ).withDescription( - "Alternate path for the user toolchains file" ).hasArg().create( ALTERNATE_USER_TOOLCHAINS ) ); - } - - // TODO: Configuring an invocation request from a command line could as well be part of the Invoker API - /** - *

setupRequest.

- * - * @param req a {@link org.apache.maven.shared.invoker.InvocationRequest} object - * @param bridge a {@link org.apache.maven.shared.invoker.InvokerLogger} object - * @param additionalArguments a {@link java.lang.String} object - * @throws org.apache.maven.shared.release.exec.MavenExecutorException if any. - */ - protected void setupRequest( InvocationRequest req, - InvokerLogger bridge, - String additionalArguments ) - throws MavenExecutorException - { - try - { - String[] args = CommandLineUtils.translateCommandline( additionalArguments ); - CommandLine cli = new PosixParser().parse( OPTIONS, args ); - - if ( cli.hasOption( SET_SYSTEM_PROPERTY ) ) - { - String[] properties = cli.getOptionValues( SET_SYSTEM_PROPERTY ); - Properties props = new Properties(); - for ( int i = 0; i < properties.length; i++ ) - { - String property = properties[i]; - String name, value; - int sep = property.indexOf( "=" ); - if ( sep <= 0 ) - { - name = property.trim(); - value = "true"; - } - else - { - name = property.substring( 0, sep ).trim(); - value = property.substring( sep + 1 ).trim(); - } - props.setProperty( name, value ); - } - - req.setProperties( props ); - } - - if ( cli.hasOption( OFFLINE ) ) - { - req.setOffline( true ); - } - - if ( cli.hasOption( QUIET ) ) - { - req.setQuiet( true ); - } - else if ( cli.hasOption( DEBUG ) ) - { - req.setDebug( true ); - } - else if ( cli.hasOption( ERRORS ) ) - { - req.setShowErrors( true ); - } - - if ( cli.hasOption( REACTOR ) ) - { - req.setRecursive( true ); - } - else if ( cli.hasOption( NON_RECURSIVE ) ) - { - req.setRecursive( false ); - } - - if ( cli.hasOption( UPDATE_SNAPSHOTS ) ) - { - req.setUpdateSnapshots( true ); - } - - if ( cli.hasOption( ACTIVATE_PROFILES ) ) - { - String[] profiles = cli.getOptionValues( ACTIVATE_PROFILES ); - - if ( profiles != null ) - { - req.setProfiles( Arrays.asList( profiles ) ); - } - } - - if ( cli.hasOption( CHECKSUM_FAILURE_POLICY ) ) - { - req.setGlobalChecksumPolicy( InvocationRequest.CheckSumPolicy.Fail ); - } - else if ( cli.hasOption( CHECKSUM_WARNING_POLICY ) ) - { - req.setGlobalChecksumPolicy( InvocationRequest.CheckSumPolicy.Warn ); - } - - if ( cli.hasOption( ALTERNATE_USER_SETTINGS ) ) - { - req.setUserSettingsFile( new File( cli.getOptionValue( ALTERNATE_USER_SETTINGS ) ) ); - } - - if ( cli.hasOption( ALTERNATE_GLOBAL_SETTINGS ) ) - { - req.setGlobalSettingsFile( new File( cli.getOptionValue( ALTERNATE_GLOBAL_SETTINGS ) ) ); - } - - if ( cli.hasOption( FAIL_AT_END ) ) - { - req.setReactorFailureBehavior( InvocationRequest.ReactorFailureBehavior.FailAtEnd ); - } - else if ( cli.hasOption( FAIL_FAST ) ) - { - req.setReactorFailureBehavior( InvocationRequest.ReactorFailureBehavior.FailFast ); - } - if ( cli.hasOption( FAIL_NEVER ) ) - { - req.setReactorFailureBehavior( InvocationRequest.ReactorFailureBehavior.FailNever ); - } - if ( cli.hasOption( ALTERNATE_POM_FILE ) ) - { - if ( req.getPomFileName() != null ) - { - getLogger().info( "pomFileName is already set, ignoring the -f argument" ); - } - else - { - req.setPomFileName( cli.getOptionValue( ALTERNATE_POM_FILE ) ); - } - } - - if ( cli.hasOption( THREADS ) ) - { - req.setThreads( cli.getOptionValue( THREADS ) ); - } - - if ( cli.hasOption( BATCH_MODE ) ) - { - req.setBatchMode( true ); - } - - if ( cli.hasOption( ALTERNATE_USER_TOOLCHAINS ) ) - { - req.setToolchainsFile( new File( cli.getOptionValue( ALTERNATE_USER_TOOLCHAINS ) ) ); - } - - } - catch ( Exception e ) - { - throw new MavenExecutorException( "Failed to re-parse additional arguments for Maven invocation.", e ); - } - } - @Override public void executeGoals( File workingDirectory, List goals, ReleaseEnvironment releaseEnvironment, boolean interactive, String additionalArguments, String pomFileName, ReleaseResult result ) throws MavenExecutorException { - InvocationOutputHandler handler = getOutputHandler(); InvokerLogger bridge = getInvokerLogger(); - File mavenPath = null; - // if null we use the current one - if ( releaseEnvironment.getMavenHome() != null ) - { - mavenPath = releaseEnvironment.getMavenHome(); - } - else - { - String mavenHome = System.getProperty( "maven.home" ); - if ( mavenHome == null ) - { - mavenHome = System.getenv( "MAVEN_HOME" ); - } - if ( mavenHome == null ) - { - mavenHome = System.getenv( "M2_HOME" ); - } - mavenPath = mavenHome == null ? null : new File( mavenHome ); - } - Invoker invoker = new DefaultInvoker() - .setMavenHome( mavenPath ) + .setMavenHome( releaseEnvironment.getMavenHome() ) + .setLocalRepositoryDirectory( releaseEnvironment.getLocalRepositoryDirectory() ) .setLogger( bridge ); InvocationRequest req = new DefaultInvocationRequest() .setDebug( getLogger().isDebugEnabled() ) .setBaseDirectory( workingDirectory ) .setBatchMode( !interactive ) - .setOutputHandler( handler ) - .setErrorHandler( handler ); + .setOutputHandler( getLogger()::info ) + .setErrorHandler( getLogger()::error ); + + // for interactive mode we need some inputs stream + if ( interactive ) + { + req.setInputStream( System.in ); + } if ( pomFileName != null ) { @@ -379,17 +97,18 @@ public void executeGoals( File workingDirectory, List goals, ReleaseEnvi throw new MavenExecutorException( "Could not create temporary file for release settings.xml", e ); } } + try { - File localRepoDir = releaseEnvironment.getLocalRepositoryDirectory(); - if ( localRepoDir != null ) + List targetGoals = new ArrayList<>( goals ); + + if ( additionalArguments != null && !additionalArguments.isEmpty() ) { - req.setLocalRepositoryDirectory( localRepoDir ); + // additionalArguments will be parsed be MavenInvoker + targetGoals.add( additionalArguments ); } - setupRequest( req, bridge, additionalArguments ); - - req.setGoals( goals ); + req.setGoals( targetGoals ); try { @@ -400,10 +119,11 @@ public void executeGoals( File workingDirectory, List goals, ReleaseEnvi throw new MavenExecutorException( "Error executing Maven.", invocationResult.getExecutionException() ); } + if ( invocationResult.getExitCode() != 0 ) { throw new MavenExecutorException( - "Maven execution failed, exit code: \'" + invocationResult.getExitCode() + "\'", + "Maven execution failed, exit code: '" + invocationResult.getExitCode() + "'", invocationResult.getExitCode() ); } } @@ -431,32 +151,6 @@ protected InvokerLogger getInvokerLogger() return new LoggerBridge( getLogger() ); } - /** - *

getOutputHandler.

- * - * @return a {@link org.apache.maven.shared.invoker.InvocationOutputHandler} object - */ - protected InvocationOutputHandler getOutputHandler() - { - return new Handler( getLogger() ); - } - - private static final class Handler - implements InvocationOutputHandler - { - private Logger logger; - - Handler( Logger logger ) - { - this.logger = logger; - } - - public void consumeLine( String line ) - { - logger.info( line ); - } - } - private static final class LoggerBridge implements InvokerLogger { diff --git a/maven-release-manager/src/test/java/org/apache/maven/shared/release/exec/InvokerMavenExecutorTest.java b/maven-release-manager/src/test/java/org/apache/maven/shared/release/exec/InvokerMavenExecutorTest.java index cdb2cedae..928556960 100644 --- a/maven-release-manager/src/test/java/org/apache/maven/shared/release/exec/InvokerMavenExecutorTest.java +++ b/maven-release-manager/src/test/java/org/apache/maven/shared/release/exec/InvokerMavenExecutorTest.java @@ -58,82 +58,7 @@ protected void setUp() executor = (InvokerMavenExecutor) lookup( MavenExecutor.class, "invoker" ); - secDispatcher = (SecDispatcher) lookup( SecDispatcher.class, "mng-4384" ); - } - - @Test - public void testThreads() - throws Exception - { - Logger logger = mock( Logger.class ); - executor.enableLogging( logger ); - - InvocationRequest req = new DefaultInvocationRequest(); - executor.setupRequest( req, null, "-T 3" ); - assertEquals( "3", req.getThreads() ); - - req = new DefaultInvocationRequest(); - executor.setupRequest( req, null, "-T4" ); - assertEquals( "4", req.getThreads() ); - - req = new DefaultInvocationRequest(); - executor.setupRequest( req, null, "\"-T5\"" ); - assertEquals( "5", req.getThreads() ); - } - - @Test - public void testBatch() - throws Exception - { - Logger logger = mock( Logger.class ); - executor.enableLogging( logger ); - - InvocationRequest req = new DefaultInvocationRequest(); - - req.setBatchMode( false ); - executor.setupRequest( req, null, "-B" ); - assertTrue( req.isBatchMode() ); - - req = new DefaultInvocationRequest(); - req.setBatchMode( false ); - executor.setupRequest( req, null, "\"-B\"" ); - assertTrue( req.isBatchMode() ); - } - - @Test - public void testUserToolchains() - throws Exception - { - Logger logger = mock( Logger.class ); - executor.enableLogging( logger ); - - InvocationRequest req = new DefaultInvocationRequest(); - executor.setupRequest( req, null, "-t mytoolchains.xml" ); - assertEquals( new File( "mytoolchains.xml" ), req.getToolchainsFile() ); - - req = new DefaultInvocationRequest(); - executor.setupRequest( req, null, "-tmytoolchains.xml" ); - assertEquals( new File( "mytoolchains.xml" ), req.getToolchainsFile() ); - - req = new DefaultInvocationRequest(); - executor.setupRequest( req, null, "\"-tmytoolchains.xml\"" ); - assertEquals( new File( "mytoolchains.xml" ), req.getToolchainsFile() ); - } - - @Test - public void testGlobalSettings() - throws Exception - { - Logger logger = mock( Logger.class ); - executor.enableLogging( logger ); - - InvocationRequest req = new DefaultInvocationRequest(); - executor.setupRequest( req, null, "-gs custom-settings.xml" ); - assertEquals( "custom-settings.xml", req.getGlobalSettingsFile().getPath() ); - - req = new DefaultInvocationRequest(); - executor.setupRequest( req, null, "--global-settings other-settings.xml" ); - assertEquals( "other-settings.xml", req.getGlobalSettingsFile().getPath() ); + secDispatcher = lookup( SecDispatcher.class, "mng-4384" ); } public void testEncryptSettings() @@ -163,8 +88,6 @@ public void testEncryptSettings() ArgumentCaptor encryptedSettings = ArgumentCaptor.forClass( Settings.class ); when( executorSpy.getSettingsWriter() ).thenReturn( settingsWriter ); - when( executorSpy.getOutputHandler() ).thenReturn( null ); - when( executorSpy.getInvokerLogger() ).thenReturn( null ); try { diff --git a/maven-release-plugin/src/it/projects/prepare/MRELEASE-667/pom.xml b/maven-release-plugin/src/it/projects/prepare/MRELEASE-667/pom.xml index 216c5555c..8ad50f771 100644 --- a/maven-release-plugin/src/it/projects/prepare/MRELEASE-667/pom.xml +++ b/maven-release-plugin/src/it/projects/prepare/MRELEASE-667/pom.xml @@ -37,7 +37,7 @@ - -Prelease,!mrelease-677 ${arguments} + -Prelease,!mrelease-677 help:all-profiles @@ -49,4 +49,4 @@ - \ No newline at end of file +