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/13 13:54:38 UTC

[GitHub] [maven] MartinKanters commented on a change in pull request #659: [MNG-7390] Allow selecting modules outside the cwd into the reactor using --projects.

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.




-- 
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