You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/02/12 21:20:40 UTC
[GitHub] [maven] mthmulders commented on a change in pull request #659: [MNG-7390] Allow selecting modules outside the cwd into the reactor using --projects.
mthmulders commented on a change in pull request #659:
URL: https://github.com/apache/maven/pull/659#discussion_r805204885
##########
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:
I _think_ this does the same as the previous code, but to be honest, I'm having a hard time to make sure it does. Why did you introduce a bunch of Streams here? Does it give any benefit over the previous, Collection-based approach?
##########
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:
If I understand the situation correctly:
```suggestion
// The set can be empty when an optional project is selected that doesn't exist in the reactor
```
(If that doesn't make sense, feel free to reject)
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org