MartinKanters commented on a change in pull request #659:
URL: https://github.com/apache/maven/pull/659#discussion_r805338630
##########
File path:
maven-core/src/main/java/org/apache/maven/graph/DefaultGraphBuilder.java
##########
@@ -242,106 +260,86 @@ public DefaultGraphBuilder(
BuildResumptionDataRepository buildResumptionDataRep
return selectedProjects;
}
- private List<MavenProject> trimResumedProjects( List<MavenProject>
projects, ProjectDependencyGraph graph,
- MavenExecutionRequest
request )
+ private Set<MavenProject> getResumedProjects( Set<MavenProject>
projectsToActivate,
+ List<MavenProject>
allSortedProjects, MavenExecutionRequest request )
throws MavenExecutionException
{
- List<MavenProject> result = projects;
-
- if ( StringUtils.isNotEmpty( request.getResumeFrom() ) )
+ if ( StringUtils.isEmpty( request.getResumeFrom() ) )
{
- File reactorDirectory = getReactorDirectory( request );
-
- String selector = request.getResumeFrom();
+ return projectsToActivate;
+ }
- MavenProject resumingFromProject = projects.stream()
- .filter( project -> isMatchingProject( project, selector,
reactorDirectory ) )
- .findFirst()
- .orElseThrow( () -> new MavenExecutionException(
- "Could not find project to resume reactor build
from: " + selector + " vs "
- + formatProjects( projects ), request.getPom() ) );
- int resumeFromProjectIndex = projects.indexOf( resumingFromProject
);
- List<MavenProject> retainingProjects = result.subList(
resumeFromProjectIndex, projects.size() );
+ File reactorDirectory = getReactorDirectory( request );
+ List<MavenProject> projectSelection = new ArrayList<>(
projectsToActivate );
+ projectSelection.sort( comparing( allSortedProjects::indexOf ) );
- result = includeAlsoMakeTransitively( retainingProjects, request,
graph );
- }
+ String selector = request.getResumeFrom();
+ MavenProject resumingFromProject = projectsToActivate.stream()
+ .filter( project -> isMatchingProject( project, selector,
reactorDirectory ) )
+ .findFirst()
+ .orElseThrow( () -> new MavenExecutionException(
+ "Could not find project to resume reactor build from:
" + selector + " vs "
+ + formatProjects( projectSelection ), request.getPom()
) );
- return result;
+ int resumeFromProjectIndex = projectSelection.indexOf(
resumingFromProject );
+ return new HashSet<>( projectSelection.subList(
resumeFromProjectIndex, projectSelection.size() ) );
}
- private List<MavenProject> trimExcludedProjects( List<MavenProject>
projects, ProjectDependencyGraph graph,
- MavenExecutionRequest
request )
+ private Set<MavenProject> getExcludedProjects( Set<MavenProject>
projectsToActivate, ProjectDependencyGraph graph,
+ MavenExecutionRequest
request )
throws MavenExecutionException
{
- List<MavenProject> result = projects;
-
ProjectActivation projectActivation = request.getProjectActivation();
Set<String> requiredSelectors =
projectActivation.getRequiredInactiveProjectSelectors();
Set<String> optionalSelectors =
projectActivation.getOptionalInactiveProjectSelectors();
- if ( !requiredSelectors.isEmpty() || !optionalSelectors.isEmpty() )
+ if ( requiredSelectors.isEmpty() && optionalSelectors.isEmpty() )
{
- Set<MavenProject> excludedProjects = new HashSet<>(
requiredSelectors.size() + optionalSelectors.size() );
- List<MavenProject> allProjects = graph.getAllProjects();
- excludedProjects.addAll( getProjectsBySelectors( request,
allProjects, requiredSelectors, true ) );
- excludedProjects.addAll( getProjectsBySelectors( request,
allProjects, optionalSelectors, false ) );
+ return Collections.emptySet();
+ }
- result = new ArrayList<>( projects );
- result.removeAll( excludedProjects );
+ Set<MavenProject> excludedProjects = new HashSet<>(
requiredSelectors.size() + optionalSelectors.size() );
+ List<MavenProject> allProjects = graph.getAllProjects();
+ excludedProjects.addAll( getProjectsBySelectors( request, allProjects,
requiredSelectors, true ) );
+ excludedProjects.addAll( getProjectsBySelectors( request, allProjects,
optionalSelectors, false ) );
- if ( result.isEmpty() )
- {
- boolean isPlural = excludedProjects.size() > 1;
- String message = String.format( "The project exclusion%s in
--projects/-pl resulted in an "
- + "empty reactor, please correct %s.", isPlural ? "s"
: "", isPlural ? "them" : "it" );
- throw new MavenExecutionException( message, request.getPom() );
- }
+ if ( excludedProjects.size() >= projectsToActivate.size() )
+ {
+ boolean isPlural = excludedProjects.size() > 1;
+ String message = String.format( "The project exclusion%s in
--projects/-pl resulted in an "
+ + "empty reactor, please correct %s.", isPlural ? "s" :
"", isPlural ? "them" : "it" );
+ throw new MavenExecutionException( message, request.getPom() );
}
- return result;
+ return excludedProjects;
}
- private List<MavenProject> includeAlsoMakeTransitively( List<MavenProject>
projects, MavenExecutionRequest request,
-
ProjectDependencyGraph graph )
+ private Set<MavenProject> getAlsoMakeDependencies( Set<MavenProject>
projectsToActivate,
+ MavenExecutionRequest
request, ProjectDependencyGraph graph )
throws MavenExecutionException
{
- List<MavenProject> result = projects;
-
String makeBehavior = request.getMakeBehavior();
boolean makeBoth = MavenExecutionRequest.REACTOR_MAKE_BOTH.equals(
makeBehavior );
boolean makeUpstream = makeBoth ||
MavenExecutionRequest.REACTOR_MAKE_UPSTREAM.equals( makeBehavior );
boolean makeDownstream = makeBoth ||
MavenExecutionRequest.REACTOR_MAKE_DOWNSTREAM.equals( makeBehavior );
- if ( StringUtils.isNotEmpty( makeBehavior ) && !makeUpstream &&
!makeDownstream )
+ if ( !makeUpstream && !makeDownstream )
{
- throw new MavenExecutionException( "Invalid reactor make behavior:
" + makeBehavior,
- request.getPom() );
- }
-
- if ( makeUpstream || makeDownstream )
- {
- Set<MavenProject> projectsSet = new HashSet<>( projects );
-
- for ( MavenProject project : projects )
+ if ( StringUtils.isNotEmpty( makeBehavior ) )
{
- if ( makeUpstream )
- {
- projectsSet.addAll( graph.getUpstreamProjects( project,
true ) );
- }
- if ( makeDownstream )
- {
- projectsSet.addAll( graph.getDownstreamProjects( project,
true ) );
- }
+ throw new MavenExecutionException( "Invalid reactor make
behavior: " + makeBehavior, request.getPom() );
}
- result = new ArrayList<>( projectsSet );
-
- // Order the new list in the original order
- List<MavenProject> sortedProjects = graph.getSortedProjects();
- result.sort( comparing( sortedProjects::indexOf ) );
+ return Collections.emptySet();
}
- return result;
+ List<MavenProject> emptyList = Collections.emptyList();
+ return projectsToActivate.stream()
Review comment:
Mostly readability, but I'm okay to revert that if you disagree that it
reads better.
##########
File path:
maven-core/src/main/java/org/apache/maven/graph/DefaultGraphBuilder.java
##########
@@ -137,72 +139,88 @@ public DefaultGraphBuilder( BuildResumptionDataRepository
buildResumptionDataRep
throws CycleDetectedException, DuplicateProjectException,
MavenExecutionException
{
ProjectDependencyGraph projectDependencyGraph = new
DefaultProjectDependencyGraph( projects );
- List<MavenProject> activeProjects =
projectDependencyGraph.getSortedProjects();
- activeProjects = trimProjectsToRequest( activeProjects,
projectDependencyGraph, session.getRequest() );
- activeProjects = trimSelectedProjects( activeProjects,
projectDependencyGraph, session.getRequest() );
- activeProjects = trimResumedProjects( activeProjects,
projectDependencyGraph, session.getRequest() );
- activeProjects = trimExcludedProjects( activeProjects,
projectDependencyGraph, session.getRequest() );
+ List<MavenProject> allSortedProjects =
projectDependencyGraph.getSortedProjects();
+ MavenExecutionRequest request = session.getRequest();
+
+ // Select projects into the reactor
+ Set<MavenProject> projectsToActivate = new HashSet<>(
allSortedProjects.size() );
+ Set<MavenProject> projectsBySelection = getProjectsBySelection(
allSortedProjects, request );
+ projectsToActivate.addAll( projectsBySelection );
- if ( activeProjects.size() !=
projectDependencyGraph.getSortedProjects().size() )
+ // If no projects were selected deliberately by the user, fall back to
all projects in this dir and below.
+ if ( projectsToActivate.isEmpty() )
{
- projectDependencyGraph = new FilteredProjectDependencyGraph(
projectDependencyGraph, activeProjects );
+ Set<MavenProject> projectsByDirectory = getProjectsByDirectory(
allSortedProjects, request );
+ projectsToActivate.addAll( projectsByDirectory );
+ }
+
+ // Add all transitive dependencies
+ Set<MavenProject> alsoMakeDependencies = getAlsoMakeDependencies(
projectsToActivate, request,
+ projectDependencyGraph );
+ projectsToActivate.addAll( alsoMakeDependencies );
+
+ // Trim until resumed project
+ Set<MavenProject> resumedProjects = getResumedProjects(
projectsToActivate, allSortedProjects, request );
+ projectsToActivate.retainAll( resumedProjects );
+
+ // Add back all transitive dependencies after projects were trimmed
+ projectsToActivate.addAll( alsoMakeDependencies );
+
+ // Remove unwanted projects from the reactor
+ getExcludedProjects( projectsToActivate, projectDependencyGraph,
request )
+ .forEach( projectsToActivate::remove );
+
+ if ( projectsToActivate.size() !=
projectDependencyGraph.getSortedProjects().size() )
+ {
+ // Sort all projects in build order
+ List<MavenProject> activeProjects = new ArrayList<>(
projectsToActivate );
+ activeProjects.sort( comparing( allSortedProjects::indexOf ) );
+
+ return Result.success( new FilteredProjectDependencyGraph(
projectDependencyGraph, activeProjects ) );
}
return Result.success( projectDependencyGraph );
}
- private List<MavenProject> trimProjectsToRequest( List<MavenProject>
activeProjects,
- ProjectDependencyGraph
graph,
+ private Set<MavenProject> getProjectsByDirectory( List<MavenProject>
allSortedProjects,
MavenExecutionRequest
request )
throws MavenExecutionException
{
- List<MavenProject> result = activeProjects;
-
- if ( request.getPom() != null )
+ if ( request.getPom() == null )
{
- result = getProjectsInRequestScope( request, activeProjects );
-
- List<MavenProject> sortedProjects = graph.getSortedProjects();
- result.sort( comparing( sortedProjects::indexOf ) );
-
- result = includeAlsoMakeTransitively( result, request, graph );
+ return new HashSet<>( allSortedProjects );
}
- return result;
+ return getProjectsInRequestScope( request, allSortedProjects );
}
- private List<MavenProject> trimSelectedProjects( List<MavenProject>
projects, ProjectDependencyGraph graph,
- MavenExecutionRequest
request )
+ private Set<MavenProject> getProjectsBySelection( List<MavenProject>
allSortedProjects,
+ MavenExecutionRequest
request )
throws MavenExecutionException
{
- List<MavenProject> result = projects;
-
ProjectActivation projectActivation = request.getProjectActivation();
Set<String> requiredSelectors =
projectActivation.getRequiredActiveProjectSelectors();
Set<String> optionalSelectors =
projectActivation.getOptionalActiveProjectSelectors();
- if ( !requiredSelectors.isEmpty() || !optionalSelectors.isEmpty() )
+ if ( requiredSelectors.isEmpty() && optionalSelectors.isEmpty() )
{
- Set<MavenProject> selectedProjects = new HashSet<>(
requiredSelectors.size() + optionalSelectors.size() );
- selectedProjects.addAll( getProjectsBySelectors( request,
projects, requiredSelectors, true ) );
- selectedProjects.addAll( getProjectsBySelectors( request,
projects, optionalSelectors, false ) );
-
- // it can be empty when an optional project is missing from the
reactor, fallback to returning all projects
- if ( !selectedProjects.isEmpty() )
- {
- result = new ArrayList<>( selectedProjects );
+ return Collections.emptySet();
+ }
- result = includeAlsoMakeTransitively( result, request, graph );
+ Set<MavenProject> selectedProjects = new HashSet<>(
requiredSelectors.size() + optionalSelectors.size() );
+ selectedProjects.addAll( getProjectsBySelectors( request,
allSortedProjects, requiredSelectors, true ) );
+ selectedProjects.addAll( getProjectsBySelectors( request,
allSortedProjects, optionalSelectors, false ) );
- // Order the new list in the original order
- List<MavenProject> sortedProjects = graph.getSortedProjects();
- result.sort( comparing( sortedProjects::indexOf ) );
- }
+ // it can be empty when an optional project is missing from the reactor
Review comment:
Actually this if-check is redundant. Basically it does: if (set ==
empty) return emptySet(); else return set.
I'll remove it :)
##########
File path:
maven-core/src/main/java/org/apache/maven/graph/DefaultGraphBuilder.java
##########
@@ -137,72 +139,88 @@ public DefaultGraphBuilder( BuildResumptionDataRepository
buildResumptionDataRep
throws CycleDetectedException, DuplicateProjectException,
MavenExecutionException
{
ProjectDependencyGraph projectDependencyGraph = new
DefaultProjectDependencyGraph( projects );
- List<MavenProject> activeProjects =
projectDependencyGraph.getSortedProjects();
- activeProjects = trimProjectsToRequest( activeProjects,
projectDependencyGraph, session.getRequest() );
- activeProjects = trimSelectedProjects( activeProjects,
projectDependencyGraph, session.getRequest() );
- activeProjects = trimResumedProjects( activeProjects,
projectDependencyGraph, session.getRequest() );
- activeProjects = trimExcludedProjects( activeProjects,
projectDependencyGraph, session.getRequest() );
+ List<MavenProject> allSortedProjects =
projectDependencyGraph.getSortedProjects();
+ MavenExecutionRequest request = session.getRequest();
+
+ // Select projects into the reactor
+ Set<MavenProject> projectsToActivate = new HashSet<>(
allSortedProjects.size() );
+ Set<MavenProject> projectsBySelection = getProjectsBySelection(
allSortedProjects, request );
+ projectsToActivate.addAll( projectsBySelection );
- if ( activeProjects.size() !=
projectDependencyGraph.getSortedProjects().size() )
+ // If no projects were selected deliberately by the user, fall back to
all projects in this dir and below.
+ if ( projectsToActivate.isEmpty() )
{
- projectDependencyGraph = new FilteredProjectDependencyGraph(
projectDependencyGraph, activeProjects );
+ Set<MavenProject> projectsByDirectory = getProjectsByDirectory(
allSortedProjects, request );
+ projectsToActivate.addAll( projectsByDirectory );
+ }
+
+ // Add all transitive dependencies
+ Set<MavenProject> alsoMakeDependencies = getAlsoMakeDependencies(
projectsToActivate, request,
+ projectDependencyGraph );
+ projectsToActivate.addAll( alsoMakeDependencies );
+
+ // Trim until resumed project
+ Set<MavenProject> resumedProjects = getResumedProjects(
projectsToActivate, allSortedProjects, request );
+ projectsToActivate.retainAll( resumedProjects );
+
+ // Add back all transitive dependencies after projects were trimmed
+ projectsToActivate.addAll( alsoMakeDependencies );
+
+ // Remove unwanted projects from the reactor
+ getExcludedProjects( projectsToActivate, projectDependencyGraph,
request )
+ .forEach( projectsToActivate::remove );
+
+ if ( projectsToActivate.size() !=
projectDependencyGraph.getSortedProjects().size() )
+ {
+ // Sort all projects in build order
+ List<MavenProject> activeProjects = new ArrayList<>(
projectsToActivate );
+ activeProjects.sort( comparing( allSortedProjects::indexOf ) );
+
+ return Result.success( new FilteredProjectDependencyGraph(
projectDependencyGraph, activeProjects ) );
}
return Result.success( projectDependencyGraph );
}
- private List<MavenProject> trimProjectsToRequest( List<MavenProject>
activeProjects,
- ProjectDependencyGraph
graph,
+ private Set<MavenProject> getProjectsByDirectory( List<MavenProject>
allSortedProjects,
MavenExecutionRequest
request )
throws MavenExecutionException
{
- List<MavenProject> result = activeProjects;
-
- if ( request.getPom() != null )
+ if ( request.getPom() == null )
{
- result = getProjectsInRequestScope( request, activeProjects );
-
- List<MavenProject> sortedProjects = graph.getSortedProjects();
- result.sort( comparing( sortedProjects::indexOf ) );
-
- result = includeAlsoMakeTransitively( result, request, graph );
+ return new HashSet<>( allSortedProjects );
}
- return result;
+ return getProjectsInRequestScope( request, allSortedProjects );
}
- private List<MavenProject> trimSelectedProjects( List<MavenProject>
projects, ProjectDependencyGraph graph,
- MavenExecutionRequest
request )
+ private Set<MavenProject> getProjectsBySelection( List<MavenProject>
allSortedProjects,
+ MavenExecutionRequest
request )
throws MavenExecutionException
{
- List<MavenProject> result = projects;
-
ProjectActivation projectActivation = request.getProjectActivation();
Set<String> requiredSelectors =
projectActivation.getRequiredActiveProjectSelectors();
Set<String> optionalSelectors =
projectActivation.getOptionalActiveProjectSelectors();
- if ( !requiredSelectors.isEmpty() || !optionalSelectors.isEmpty() )
+ if ( requiredSelectors.isEmpty() && optionalSelectors.isEmpty() )
{
- Set<MavenProject> selectedProjects = new HashSet<>(
requiredSelectors.size() + optionalSelectors.size() );
- selectedProjects.addAll( getProjectsBySelectors( request,
projects, requiredSelectors, true ) );
- selectedProjects.addAll( getProjectsBySelectors( request,
projects, optionalSelectors, false ) );
-
- // it can be empty when an optional project is missing from the
reactor, fallback to returning all projects
- if ( !selectedProjects.isEmpty() )
- {
- result = new ArrayList<>( selectedProjects );
+ return Collections.emptySet();
+ }
- result = includeAlsoMakeTransitively( result, request, graph );
+ Set<MavenProject> selectedProjects = new HashSet<>(
requiredSelectors.size() + optionalSelectors.size() );
+ selectedProjects.addAll( getProjectsBySelectors( request,
allSortedProjects, requiredSelectors, true ) );
+ selectedProjects.addAll( getProjectsBySelectors( request,
allSortedProjects, optionalSelectors, false ) );
- // Order the new list in the original order
- List<MavenProject> sortedProjects = graph.getSortedProjects();
- result.sort( comparing( sortedProjects::indexOf ) );
- }
+ // it can be empty when an optional project is missing from the reactor
Review comment:
Actually just realised that this if-check is redundant. Basically it
does: if (set == empty) return emptySet(); else return set.
I'll remove it :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
