[MNG-5760] Several fixes for the --resume feature.

BuildResumptionDataRepository is not used in MavenCli

Make setResume() on MavenExecutionRequest a traditional setter

Fix resolution of resume.properties file

Add unit test for DefaultBuildResumptionDataRepository#applyResumptionData

Avoid storing and using an empty excludedProjects field in the resume.properties file.

Avoid star imports

Don't create a unneeded Path when resolving resume.properties

Support the scenario where the first project was failed, but subsequent projects succeeded. (e.g. by fail-at-end or parallel builds)

Maven invocations without project shouldn't fail
This commit is contained in:
Maarten Mulders 2020-06-20 22:09:04 +02:00 committed by Martin Kanters
parent 3b442badfc
commit 117cfde44e
9 changed files with 98 additions and 32 deletions

View File

@ -20,6 +20,9 @@ package org.apache.maven.execution;
*/
import java.util.List;
import java.util.Optional;
import static java.util.Collections.emptyList;
/**
* This class holds the information required to enable resuming a Maven build with {@code --resume}.
@ -32,7 +35,7 @@ public class BuildResumptionData
private final String resumeFrom;
/**
* List of projects to skip if the build would be resumed from {@link #resumeFrom}.
* List of projects to skip.
*/
private final List<String> projectsToSkip;
@ -42,13 +45,23 @@ public class BuildResumptionData
this.projectsToSkip = projectsToSkip;
}
public String getResumeFrom()
/**
* Returns the project where the next build can resume from.
* This is usually the first failed project in the order of the reactor.
* @return An optional containing the group and artifact id of the project. It does not make sense to resume
* the build when the first project of the reactor has failed, so then it will return an empty optional.
*/
public Optional<String> getResumeFrom()
{
return this.resumeFrom;
return Optional.ofNullable( this.resumeFrom );
}
/**
* A list of projects which can be skipped in the next build.
* @return A list of group and artifact ids. Can be empty when no projects can be skipped.
*/
public List<String> getProjectsToSkip()
{
return this.projectsToSkip;
return ( projectsToSkip != null ) ? projectsToSkip : emptyList();
}
}

View File

@ -22,6 +22,7 @@ package org.apache.maven.execution;
import org.apache.maven.lifecycle.LifecycleExecutionException;
import org.apache.maven.model.Dependency;
import org.apache.maven.project.MavenProject;
import org.codehaus.plexus.util.StringUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@ -56,16 +57,31 @@ public class DefaultBuildResumptionAnalyzer implements BuildResumptionAnalyzer
final MavenProject resumeFromProject = failedProjects.get( 0 );
final String resumeFromSelector;
final List<String> projectsToSkip;
if ( isFailedProjectFirstInBuild( result, resumeFromProject ) )
{
LOGGER.info( "The first module in the build failed, resuming the build would not make sense." );
return Optional.empty();
// As the first module in the build failed, there is no need to specify this as the resumeFrom project.
resumeFromSelector = null;
projectsToSkip = determineProjectsToSkip( result, failedProjects, 0 );
}
else
{
resumeFromSelector = resumeFromProject.getGroupId() + ":" + resumeFromProject.getArtifactId();
List<MavenProject> allProjects = result.getTopologicallySortedProjects();
int resumeFromProjectIndex = allProjects.indexOf( resumeFromProject );
projectsToSkip = determineProjectsToSkip( result, failedProjects, resumeFromProjectIndex + 1 );
}
final String resumeFromSelector = resumeFromProject.getGroupId() + ":" + resumeFromProject.getArtifactId();
final List<String> projectsToSkip = determineProjectsToSkip( result, failedProjects, resumeFromProject );
return Optional.of( new BuildResumptionData( resumeFromSelector, projectsToSkip ) );
boolean canBuildBeResumed = StringUtils.isNotEmpty( resumeFromSelector ) || !projectsToSkip.isEmpty();
if ( canBuildBeResumed )
{
return Optional.of( new BuildResumptionData( resumeFromSelector, projectsToSkip ) );
}
else
{
return Optional.empty();
}
}
private boolean isFailedProjectFirstInBuild( final MavenExecutionResult result, final MavenProject failedProject )
@ -82,6 +98,7 @@ public class DefaultBuildResumptionAnalyzer implements BuildResumptionAnalyzer
.filter( LifecycleExecutionException.class::isInstance )
.map( LifecycleExecutionException.class::cast )
.map( LifecycleExecutionException::getProject )
.filter( Objects::nonNull )
.sorted( comparing( sortedProjects::indexOf ) )
.collect( Collectors.toList() );
}
@ -92,16 +109,15 @@ public class DefaultBuildResumptionAnalyzer implements BuildResumptionAnalyzer
* This is not the case these projects are dependent on one of the failed projects.
* @param result The result of the Maven build.
* @param failedProjects The list of failed projects in the build.
* @param resumeFromProject The project where the build will be resumed with in the next run.
* @param startFromProjectIndex Start looking for projects which can be skipped from a certain index.
* @return A list of projects which can be skipped in a later build.
*/
private List<String> determineProjectsToSkip( MavenExecutionResult result,
List<MavenProject> failedProjects,
MavenProject resumeFromProject )
int startFromProjectIndex )
{
List<MavenProject> allProjects = result.getTopologicallySortedProjects();
int resumeFromProjectIndex = allProjects.indexOf( resumeFromProject );
List<MavenProject> remainingProjects = allProjects.subList( resumeFromProjectIndex + 1, allProjects.size() );
List<MavenProject> remainingProjects = allProjects.subList( startFromProjectIndex, allProjects.size() );
List<GroupArtifactPair> failedProjectsGAList = failedProjects.stream()
.map( GroupArtifactPair::new )

View File

@ -74,9 +74,15 @@ public class DefaultBuildResumptionDataRepository implements BuildResumptionData
private Properties convertToProperties( final BuildResumptionData buildResumptionData )
{
Properties properties = new Properties();
properties.setProperty( RESUME_FROM_PROPERTY, buildResumptionData.getResumeFrom() );
String excludedProjects = String.join( PROPERTY_DELIMITER, buildResumptionData.getProjectsToSkip() );
properties.setProperty( EXCLUDED_PROJECTS_PROPERTY, excludedProjects );
buildResumptionData.getResumeFrom()
.ifPresent( resumeFrom -> properties.setProperty( RESUME_FROM_PROPERTY, resumeFrom ) );
if ( !buildResumptionData.getProjectsToSkip().isEmpty() )
{
String excludedProjects = String.join( PROPERTY_DELIMITER, buildResumptionData.getProjectsToSkip() );
properties.setProperty( EXCLUDED_PROJECTS_PROPERTY, excludedProjects );
}
return properties;
}
@ -105,7 +111,7 @@ public class DefaultBuildResumptionDataRepository implements BuildResumptionData
private Properties loadResumptionFile( Path rootBuildDirectory )
{
Properties properties = new Properties();
Path path = Paths.get( RESUME_PROPERTIES_FILENAME ).resolve( rootBuildDirectory );
Path path = rootBuildDirectory.resolve( RESUME_PROPERTIES_FILENAME );
if ( !Files.exists( path ) )
{
LOGGER.warn( "The {} file does not exist. The --resume / -r feature will not work.", path );
@ -137,9 +143,12 @@ public class DefaultBuildResumptionDataRepository implements BuildResumptionData
if ( properties.containsKey( EXCLUDED_PROJECTS_PROPERTY ) )
{
String propertyValue = properties.getProperty( EXCLUDED_PROJECTS_PROPERTY );
String[] excludedProjects = propertyValue.split( PROPERTY_DELIMITER );
request.getExcludedProjects().addAll( Arrays.asList( excludedProjects ) );
LOGGER.info( "Additionally excluding projects '{}' due to the --resume / -r feature.", propertyValue );
if ( !StringUtils.isEmpty( propertyValue ) )
{
String[] excludedProjects = propertyValue.split( PROPERTY_DELIMITER );
request.getExcludedProjects().addAll( Arrays.asList( excludedProjects ) );
LOGGER.info( "Additionally excluding projects '{}' due to the --resume / -r feature.", propertyValue );
}
}
}
}

View File

@ -607,9 +607,9 @@ public class DefaultMavenExecutionRequest
}
@Override
public MavenExecutionRequest setResume()
public MavenExecutionRequest setResume( boolean resume )
{
resume = true;
this.resume = resume;
return this;
}

View File

@ -173,9 +173,10 @@ public interface MavenExecutionRequest
/**
* Sets whether the build should be resumed from the data in the resume.properties file.
* @param resume Whether or not to resume a previous build.
* @return This request, never {@code null}.
*/
MavenExecutionRequest setResume();
MavenExecutionRequest setResume( boolean resume );
/**
* @return Whether the build should be resumed from the data in the resume.properties file.

View File

@ -55,7 +55,7 @@ public class DefaultBuildResumptionAnalyzerTest
Optional<BuildResumptionData> result = analyzer.determineBuildResumptionData( executionResult );
assertThat( result.isPresent(), is( true ) );
assertThat( result.get().getResumeFrom(), is( "test:B" ) );
assertThat( result.get().getResumeFrom(), is( Optional.of ( "test:B" ) ) );
}
@Test
@ -111,7 +111,7 @@ public class DefaultBuildResumptionAnalyzerTest
Optional<BuildResumptionData> result = analyzer.determineBuildResumptionData( executionResult );
assertThat( result.isPresent(), is( true ) );
assertThat( result.get().getResumeFrom(), is( "test:B" ) );
assertThat( result.get().getResumeFrom(), is( Optional.of ( "test:B" ) ) );
assertThat( result.get().getProjectsToSkip(), contains( "test:C" ) );
assertThat( result.get().getProjectsToSkip(), not( contains( "test:D" ) ) );
}

View File

@ -19,6 +19,8 @@ package org.apache.maven.execution;
* under the License.
*/
import org.apache.maven.model.Build;
import org.apache.maven.project.MavenProject;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.junit.MockitoJUnitRunner;
@ -29,6 +31,7 @@ import java.util.Properties;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.is;
@RunWith( MockitoJUnitRunner.class )
@ -75,4 +78,31 @@ public class DefaultBuildResumptionDataRepositoryTest
assertThat( request.getExcludedProjects(), contains( ":module-a", ":module-b", ":module-c" ) );
}
@Test
public void excludedProjectsAreNotAddedWhenPropertyValueIsEmpty()
{
MavenExecutionRequest request = new DefaultMavenExecutionRequest();
Properties properties = new Properties();
properties.setProperty( "excludedProjects", "" );
repository.applyResumptionProperties( request, properties );
assertThat( request.getExcludedProjects(), is( empty() ) );
}
@Test
public void applyResumptionData_shouldLoadData()
{
MavenExecutionRequest request = new DefaultMavenExecutionRequest();
Build build = new Build();
build.setDirectory( "src/test/resources/org/apache/maven/execution/" );
MavenProject rootProject = new MavenProject();
rootProject.setBuild( build );
repository.applyResumptionData( request, rootProject );
assertThat( request.getResumeFrom(), is( "example:module-c" ) );
assertThat( request.getExcludedProjects(), contains( "example:module-a", "example:module-b" ) );
}
}

View File

@ -0,0 +1,2 @@
resumeFrom=example:module-c
excludedProjects=example:module-a, example:module-b

View File

@ -48,7 +48,6 @@ import org.apache.maven.eventspy.internal.EventSpyDispatcher;
import org.apache.maven.exception.DefaultExceptionHandler;
import org.apache.maven.exception.ExceptionHandler;
import org.apache.maven.exception.ExceptionSummary;
import org.apache.maven.execution.BuildResumptionDataRepository;
import org.apache.maven.execution.DefaultMavenExecutionRequest;
import org.apache.maven.execution.ExecutionListener;
import org.apache.maven.execution.MavenExecutionRequest;
@ -169,8 +168,6 @@ public class MavenCli
private Map<String, ConfigurationProcessor> configurationProcessors;
private BuildResumptionDataRepository buildResumptionDataRepository;
public MavenCli()
{
this( null );
@ -708,8 +705,6 @@ public class MavenCli
dispatcher = (DefaultSecDispatcher) container.lookup( SecDispatcher.class, "maven" );
buildResumptionDataRepository = container.lookup( BuildResumptionDataRepository.class );
return container;
}
@ -1536,7 +1531,7 @@ public class MavenCli
if ( commandLine.hasOption( CLIManager.RESUME ) )
{
request.setResume();
request.setResume( true );
}
if ( commandLine.hasOption( CLIManager.RESUME_FROM ) )