Merge pull request #287 from famod/issue-269-pl-am-changed

Build upstream of -pl also if not changed + some fixes if nothing changed
gitflow-incremental-builder · Dec 13, 2020 · 4350eaa · 4350eaa
2 parents 47dcb5b + 8bbeb15
commit 4350eaa
Show file tree

Hide file tree

Showing 4 changed files with 94 additions and 80 deletions.
diff --git a/README.md b/README.md
@@ -628,7 +628,7 @@ Since 3.10.0, special rules appy when `mvn -pl ...` (or `--projects ...`) is use
 - Upstream projects of these selected projects are built if:
   - `-am` (`--also-make`) is used
   - _and_:
-    - they are **changed** (or depend on other changed upstream modules) _and_ [`buildUpstream`](#gibbuildUpstream) is _not_ `never` or `false`
+    - [`buildUpstream`](#gibbuildUpstream) is _not_ `never` or `false`
     - _or_ [`buildAll`](#gibbuildall) is enabled
 
 Other properties/features are applied as usual to the resulting subset of modules/projects.

diff --git a/pom.xml b/pom.xml
@@ -639,7 +639,7 @@
                                         <limit>
                                             <counter>BRANCH</counter>
                                             <value>COVEREDRATIO</value>
-                                            <minimum>0.83</minimum>
+                                            <minimum>0.82</minimum>
                                         </limit>
                                         <limit>
                                             <counter>CLASS</counter>

diff --git a/src/main/java/com/vackosar/gitflowincrementalbuild/boundary/UnchangedProjectsRemover.java b/src/main/java/com/vackosar/gitflowincrementalbuild/boundary/UnchangedProjectsRemover.java
@@ -118,21 +118,6 @@ private void doAct(Configuration config) throws GitAPIException, IOException {
         }
     }
 
-    private void writeImpactedLogFile(Set<MavenProject> impacted, Path logFilePath, MavenSession mavenSession) {
-        List<String> projectsToLog = impacted.isEmpty()
-                ? Collections.emptyList()
-                : mavenSession.getProjects().stream()   // write in proper order
-                        .filter(impacted::contains)
-                        .map(proj -> proj.getBasedir().getPath())
-                        .collect(Collectors.toList());
-        logger.debug("Writing impacted projects to {}: {}", logFilePath, projectsToLog);
-        try {
-            Files.write(logFilePath, projectsToLog, StandardCharsets.UTF_8, StandardOpenOption.CREATE);
-        } catch (IOException e) {
-            throw new IllegalStateException("Failed to write impacted projects to " + logFilePath + ": " + impacted, e);
-        }
-    }
-
     private boolean onlySelectedModulesPresent(Set<MavenProject> selected, MavenSession mavenSession) {
         return !selected.isEmpty() && mavenSession.getProjects().equals(new ArrayList<>(selected));
     }
@@ -144,16 +129,31 @@ private boolean onlySingleLeafModulePresent(Configuration config) {
 
     private void handleNoChangesDetected(Set<MavenProject> selected, Configuration config) {
         if (!selected.isEmpty()) {
-            // note: need to check make behaviour additionally because downstream projects will only be present for -pl when also -amd is set
-            if (config.buildDownstream && Configuration.isMakeBehaviourActive(MavenExecutionRequest.REACTOR_MAKE_DOWNSTREAM, config.mavenSession)) {
-                logger.info("No changed artifacts detected: Building explicitly selected projects and their dependents.");
-                config.mavenSession.setProjects(selected.stream()
-                        .flatMap(proj -> streamProjectWithDownstreamProjects(proj, config))
-                        .distinct()
+            logger.info("No changed artifacts detected: Building just explicitly selected projects (and their upstream and/or downstream, if requested).");
+            // note: "only selected" case was handled before, so we have up- and/or downstream projects in the session as well
+            Set<MavenProject> selectedAndDownstream = selected.stream()
+                    .flatMap(proj -> streamProjectWithDownstreamProjects(proj, config))
+                    .collect(Collectors.toSet());
+            // handle upstream
+            if (Configuration.isMakeBehaviourActive(MavenExecutionRequest.REACTOR_MAKE_UPSTREAM, config.mavenSession)) {
+                if (config.buildUpstreamMode == BuildUpstreamMode.NONE) {
+                    // only retain selected and downstream
+                    config.mavenSession.setProjects(config.mavenSession.getProjects().stream()  // retain order
+                            .filter(selectedAndDownstream::contains)
+                            .collect(Collectors.toList()));
+                } else {
+                    // applyUpstreamModuleArgs
+                    config.mavenSession.getProjects().stream()
+                            .filter(proj -> !selectedAndDownstream.contains(proj))
+                            .forEach(proj -> applyUpstreamModuleArgs(proj, config));
+                }
+            }
+            // handle downstream (remove downstream projects if configured)
+            if (Configuration.isMakeBehaviourActive(MavenExecutionRequest.REACTOR_MAKE_DOWNSTREAM, config.mavenSession)
+                    && !config.buildDownstream) {
+                config.mavenSession.setProjects(config.mavenSession.getProjects().stream()  // retain order
+                        .filter(proj -> selected.contains(proj) || !selectedAndDownstream.contains(proj))
                         .collect(Collectors.toList()));
-            } else {
-                logger.info("No changed artifacts detected: Building just explicitly selected projects.");
-                config.mavenSession.setProjects(new ArrayList<>(selected));
             }
         } else if (config.buildAllIfNoChanges) {
             logger.info("No changed artifacts detected: Building all modules in buildAll mode.");
@@ -169,7 +169,7 @@ private void handleNoChangesDetected(Set<MavenProject> selected, Configuration c
     }
 
     private Set<MavenProject> calculateImpactedProjects(Set<MavenProject> selected, Set<MavenProject> changed, Configuration config) {
-        Stream<MavenProject> impacted = selected.isEmpty() ? changed.stream() : Stream.concat(selected.stream(), changed.stream()).distinct();
+        Stream<MavenProject> impacted = selected.isEmpty() ? changed.stream() : selected.stream();
         // note: buildAll *always* needs impacted incl. downstream, otherwise applyNotImpactedModuleArgs() might disable tests etc. for downstream modules!
         if (config.buildAll || config.buildDownstream) {
             impacted = impacted.flatMap(proj -> streamProjectWithDownstreamProjects(proj, config));
@@ -179,6 +179,21 @@ private Set<MavenProject> calculateImpactedProjects(Set<MavenProject> selected,
                 .collect(Collectors.toCollection(LinkedHashSet::new));
     }
 
+    private void writeImpactedLogFile(Set<MavenProject> impacted, Path logFilePath, MavenSession mavenSession) {
+        List<String> projectsToLog = impacted.isEmpty()
+                ? Collections.emptyList()
+                : mavenSession.getProjects().stream()   // write in proper order
+                        .filter(impacted::contains)
+                        .map(proj -> proj.getBasedir().getPath())
+                        .collect(Collectors.toList());
+        logger.debug("Writing impacted projects to {}: {}", logFilePath, projectsToLog);
+        try {
+            Files.write(logFilePath, projectsToLog, StandardCharsets.UTF_8, StandardOpenOption.CREATE);
+        } catch (IOException e) {
+            throw new IllegalStateException("Failed to write impacted projects to " + logFilePath + ": " + impacted, e);
+        }
+    }
+
     private void modifyProjectList(Set<MavenProject> selected, Set<MavenProject> changed, Set<MavenProject> impacted, Configuration config) {
         Set<MavenProject> rebuild = calculateRebuildProjects(selected, changed, impacted, config);
         if (rebuild.isEmpty()) {
@@ -200,54 +215,26 @@ private void modifyProjectList(Set<MavenProject> selected, Set<MavenProject> cha
     private Set<MavenProject> calculateRebuildProjects(Set<MavenProject> selected, Set<MavenProject> changed, Set<MavenProject> impacted,
             Configuration config) {
         BuildUpstreamMode buildUpstreamMode = config.buildUpstreamMode;
-        Set<MavenProject> upstreamProjects;
 
-        if (!selected.isEmpty()) {
-            // note: buildDownstream=false is not relevant here since -amd might have been specified
-            //       and we need all downstreams to subtract them from the project list to find the upstreams
-            Set<MavenProject> selectedWithDownstream = selected.stream()
-                    .flatMap(proj -> streamProjectWithDownstreamProjects(proj, config))
-                    .collect(Collectors.toCollection(LinkedHashSet::new));
-            switch (buildUpstreamMode) {
-                case NONE:
-                    // just use impacted that are selected and the downstreams of the selected
-                    return impacted.stream()
-                            .filter(selectedWithDownstream::contains)
-                            .collect(Collectors.toCollection(LinkedHashSet::new));
-                case CHANGED:
-                    // fall-through
-                case IMPACTED:
-                    upstreamProjects = changed.stream()
-                            .filter(proj -> !selectedWithDownstream.contains(proj))
-                            .flatMap(proj -> streamProjectWithDownstreamProjects(proj, config))
-                            .filter(proj -> !selectedWithDownstream.contains(proj))
-                            .collect(Collectors.toCollection(LinkedHashSet::new));
-                    break;
-                default:
-                    throw new IllegalStateException("Unsupported BuildUpstreamMode: " + buildUpstreamMode);
-            }
-        } else {
-            Set<MavenProject> upstreamRequiringProjects;
-            switch (buildUpstreamMode) {
-                case NONE:
-                    // just use impacted
-                    return impacted;
-                case CHANGED:
-                    upstreamRequiringProjects = changed;
-                    break;
-                case IMPACTED:
-                    upstreamRequiringProjects = impacted;
-                    break;
-                default:
-                    throw new IllegalStateException("Unsupported BuildUpstreamMode: " + buildUpstreamMode);
-            }
-            upstreamProjects = upstreamRequiringProjects.stream()
-                    .flatMap(proj -> streamUpstreamProjects(proj, config.mavenSession))
-                    .filter(proj -> !impacted.contains(proj))
-                    .collect(Collectors.toCollection(LinkedHashSet::new));
+        Set<MavenProject> upstreamRequiringProjects;
+        switch (buildUpstreamMode) {
+            case NONE:
+                // just use impacted
+                return impacted;
+            case CHANGED:
+                upstreamRequiringProjects = selected.isEmpty() ? changed : selected;
+                break;
+            case IMPACTED:
+                upstreamRequiringProjects = impacted;
+                break;
+            default:
+                throw new IllegalStateException("Unsupported BuildUpstreamMode: " + buildUpstreamMode);
         }
-
-        upstreamProjects.forEach(proj -> applyUpstreamModuleArgs(proj, config));
+        Set<MavenProject> upstreamProjects = upstreamRequiringProjects.stream()
+                .flatMap(proj -> streamUpstreamProjects(proj, config.mavenSession))
+                .filter(proj -> !impacted.contains(proj))
+                .peek(proj -> applyUpstreamModuleArgs(proj, config))
+                .collect(Collectors.toCollection(LinkedHashSet::new));
 
         return config.mavenSession.getProjects().stream()
                 .filter(proj -> impacted.contains(proj) || upstreamProjects.contains(proj))

diff --git a/...ckosar/gitflowincrementalbuild/boundary/UnchangedProjectsRemoverSelectedProjectsTest.java b/...ckosar/gitflowincrementalbuild/boundary/UnchangedProjectsRemoverSelectedProjectsTest.java
@@ -116,6 +116,26 @@ public void nothingChanged_makeUpstream() throws GitAPIException, IOException {
 
         assertThat(mavenSessionMock.getGoals()).as("Unexpected goals").isEqualTo(Collections.emptyList());
 
+        verify(mavenSessionMock, never()).setProjects(anyList());
+
+        assertProjectPropertiesEqual(moduleA, "maven.test.skip", "true");
+        assertProjectPropertiesEqual(moduleB);
+    }
+
+    // mvn -pl :module-B -am
+    @Test
+    public void nothingChanged_makeUpstream_buildUpstreamDisabled() throws GitAPIException, IOException {
+        MavenProject moduleB = addModuleMock(AID_MODULE_B, false);
+        setProjectSelections(moduleB);
+
+        when(mavenExecutionRequestMock.getMakeBehavior()).thenReturn(MavenExecutionRequest.REACTOR_MAKE_UPSTREAM);
+
+        addGibProperty(Property.buildUpstream, "false");
+
+        underTest.act(config());
+
+        assertThat(mavenSessionMock.getGoals()).as("Unexpected goals").isEqualTo(Collections.emptyList());
+
         verify(mavenSessionMock).setProjects(Collections.singletonList(moduleB));
 
         assertProjectPropertiesEqual(moduleB);
@@ -139,7 +159,8 @@ public void nothingChanged_makeDownstream() throws GitAPIException, IOException
 
         assertThat(mavenSessionMock.getGoals()).as("Unexpected goals").isEqualTo(Collections.emptyList());
 
-        verify(mavenSessionMock).setProjects(Arrays.asList(moduleB, moduleC));
+        // verify(mavenSessionMock).setProjects(Arrays.asList(moduleB, moduleC));
+        verify(mavenSessionMock, never()).setProjects(anyList());
 
         assertProjectPropertiesEqual(moduleB);
         assertProjectPropertiesEqual(moduleC);
@@ -196,7 +217,7 @@ public void nothingChanged_makeDownstream_twoSelected() throws GitAPIException,
 
         assertThat(mavenSessionMock.getGoals()).as("Unexpected goals").isEqualTo(Collections.emptyList());
 
-        verify(mavenSessionMock).setProjects(Arrays.asList(moduleB, moduleC, moduleD, moduleE));
+        verify(mavenSessionMock, never()).setProjects(anyList());
 
         assertProjectPropertiesEqual(moduleB);
         assertProjectPropertiesEqual(moduleC);
@@ -221,8 +242,9 @@ public void nothingChanged_makeBoth() throws GitAPIException, IOException {
 
         assertThat(mavenSessionMock.getGoals()).as("Unexpected goals").isEqualTo(Collections.emptyList());
 
-        verify(mavenSessionMock).setProjects(Arrays.asList(moduleB, moduleC));
+        verify(mavenSessionMock, never()).setProjects(anyList());
 
+        assertProjectPropertiesEqual(moduleA, "maven.test.skip", "true");
         assertProjectPropertiesEqual(moduleB);
         assertProjectPropertiesEqual(moduleC);
     }
@@ -536,7 +558,7 @@ public void moduleBChanged_makeUpstream() throws GitAPIException, IOException {
 
         underTest.act(config());
 
-        verify(mavenSessionMock).setProjects(Collections.singletonList(moduleB));
+        verify(mavenSessionMock).setProjects(Arrays.asList(moduleA, moduleB));
 
         assertProjectPropertiesEqual(moduleB);
     }
@@ -590,7 +612,7 @@ public void moduleBChanged_makeBoth() throws GitAPIException, IOException {
 
         underTest.act(config());
 
-        verify(mavenSessionMock).setProjects(Arrays.asList(moduleB, moduleC));
+        verify(mavenSessionMock).setProjects(Arrays.asList(moduleA, moduleB, moduleC));
 
         assertProjectPropertiesEqual(moduleB);
         assertProjectPropertiesEqual(moduleC);
@@ -724,7 +746,9 @@ public void twoSelected_differentUpstreams_bothChanged() throws GitAPIException,
 
         underTest.act(config());
 
-        verify(mavenSessionMock).setProjects(Arrays.asList(moduleB, moduleC, moduleD, moduleE));
+        verify(mavenSessionMock).setProjects(Arrays.asList(moduleA, moduleB, moduleC, moduleD, moduleE));
+
+        assertProjectPropertiesEqual(moduleA, "maven.test.skip", "true");
 
         assertProjectPropertiesEqual(moduleB, "maven.test.skip", "true");
         assertProjectPropertiesEqual(moduleC);
@@ -756,8 +780,11 @@ public void twoSelected_differentUpstreams_oneChanged() throws GitAPIException,
 
         underTest.act(config());
 
-        verify(mavenSessionMock).setProjects(Arrays.asList(moduleC, moduleD, moduleE));
+        verify(mavenSessionMock).setProjects(Arrays.asList(moduleA, moduleB, moduleC, moduleD, moduleE));
+
+        assertProjectPropertiesEqual(moduleA, "maven.test.skip", "true");
 
+        assertProjectPropertiesEqual(moduleB, "maven.test.skip", "true");
         assertProjectPropertiesEqual(moduleC);
 
         assertProjectPropertiesEqual(moduleD, "maven.test.skip", "true");