[maven-4.0.x] Fix #12704: Do not report an independent, built module as skipped

impl/maven-cli/src/main/java/org/apache/maven/cling/event/ExecutionEventLogger.java

@@ -316,12 +316,34 @@ public void projectSkipped(ExecutionEvent event) {
             infoLine('-');
             String name = event.getProject().getName();
             infoMain("Skipping " + name);
-            logger.info("{} was not built because a module it depends on failed to build.", name);
+            if (dependsOnFailedProject(event)) {
+                logger.info("{} was not built because a module it depends on failed to build.", name);
+            } else {
+                logger.info("{} was not built because the build was stopped after an earlier failure.", name);
+            }
 
             infoLine('-');
         }
     }
 
+    /**
+     * A project can be skipped for two different reasons: one of the modules it depends on failed,
+     * or the reactor was stopped after an unrelated module failed. Only the first one lets us blame
+     * a dependency, so tell them apart instead of always reporting the same cause.
+     * When the answer cannot be established, the dependency wording is kept.
+     */
+    private boolean dependsOnFailedProject(ExecutionEvent event) {
+        MavenSession session = event.getSession();
+        MavenProject project = event.getProject();
+        if (session == null || project == null || session.getProjectDependencyGraph() == null) {
+            return true;
+        }
+        MavenExecutionResult result = session.getResult();
+        return result == null
+                || session.getProjectDependencyGraph().getUpstreamProjects(project, true).stream()
+                        .anyMatch(upstream -> result.getBuildSummary(upstream) instanceof BuildFailure);
+    }
+
     @Override
     public void projectStarted(ExecutionEvent event) {
         if (logger.isInfoEnabled()) {

impl/maven-cli/src/test/java/org/apache/maven/cling/event/ExecutionEventLoggerTest.java

@@ -20,6 +20,7 @@
 
 import java.io.File;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 
 import org.apache.maven.execution.BuildFailure;
@@ -446,6 +447,66 @@ void testSessionEndedFailureMultimoduleWithSeparatedFailures() {
         inOrder.verify(logger).info("------------------------------------------------------------------------");
     }
 
+    @Test
+    void testProjectSkippedBecauseADependencyFailed() {
+        // prepare
+        MavenProject failed = generateMavenProject("Maven Project artifact1");
+        MavenProject skipped = generateMavenProject("Maven Project artifact2");
+
+        DefaultMavenExecutionResult executionResult = new DefaultMavenExecutionResult();
+        executionResult.addBuildSummary(new BuildFailure(failed, 1000, new Exception("Failure")));
+
+        ExecutionEvent event = skipEvent(skipped, executionResult, Arrays.asList(failed));
+
+        // execute
+        executionEventLogger.projectSkipped(event);
+
+        // verify
+        InOrder inOrder = inOrder(logger);
+        inOrder.verify(logger).info("Skipping Maven Project artifact2");
+        inOrder.verify(logger)
+                .info("{} was not built because a module it depends on failed to build.", "Maven Project artifact2");
+    }
+
+    @Test
+    void testProjectSkippedBecauseTheBuildWasStopped() {
+        // prepare
+        MavenProject unrelated = generateMavenProject("Maven Project artifact1");
+        MavenProject skipped = generateMavenProject("Maven Project artifact2");
+
+        DefaultMavenExecutionResult executionResult = new DefaultMavenExecutionResult();
+        executionResult.addBuildSummary(new BuildFailure(unrelated, 1000, new Exception("Failure")));
+
+        // the skipped project has no upstream projects at all, so the failure cannot be blamed on one
+        ExecutionEvent event = skipEvent(skipped, executionResult, Collections.emptyList());
+
+        // execute
+        executionEventLogger.projectSkipped(event);
+
+        // verify
+        InOrder inOrder = inOrder(logger);
+        inOrder.verify(logger).info("Skipping Maven Project artifact2");
+        inOrder.verify(logger)
+                .info(
+                        "{} was not built because the build was stopped after an earlier failure.",
+                        "Maven Project artifact2");
+    }
+
+    private ExecutionEvent skipEvent(
+            MavenProject skipped, MavenExecutionResult executionResult, List<MavenProject> upstream) {
+        ProjectDependencyGraph projectDependencyGraph = mock(ProjectDependencyGraph.class);
+        when(projectDependencyGraph.getUpstreamProjects(skipped, true)).thenReturn(upstream);
+
+        MavenSession mavenSession = mock(MavenSession.class);
+        when(mavenSession.getResult()).thenReturn(executionResult);
+        when(mavenSession.getProjectDependencyGraph()).thenReturn(projectDependencyGraph);
+
+        ExecutionEvent event = mock(ExecutionEvent.class);
+        when(event.getProject()).thenReturn(skipped);
+        when(event.getSession()).thenReturn(mavenSession);
+        return event;
+    }
+
     private static MavenProject generateMavenProject(String projectName) {
         MavenProject project = mock(MavenProject.class);
         lenient().when(project.getPackaging()).thenReturn("jar");

impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/concurrent/BuildPlanExecutor.java

@@ -508,7 +508,7 @@ private void executeStep(BuildStep step) throws IOException, LifecycleExecutionE
 
                     // Check if there are any stored exceptions for this project
                     List<Throwable> failures = null;
-                    boolean allStepsExecuted = true;
+                    boolean allWorkExecuted = true;
                     for (BuildStep projectStep : plan.steps(step.project).toList()) {
                         Exception exception = projectStep.exception;
                         if (exception != null) {
@@ -517,9 +517,24 @@ private void executeStep(BuildStep step) throws IOException, LifecycleExecutionE
                             }
                             failures.add(exception);
                         }
-                        allStepsExecuted &= step == projectStep || projectStep.status.get() == EXECUTED;
+                        // Only steps that carry mojo executions represent work that was asked for.
+                        // The plan holds a step for every phase of the lifecycle, so a project that
+                        // has run everything requested of it still has empty steps left over for the
+                        // phases beyond the requested tasks. Those get skipped as soon as the reactor
+                        // is halted, and must not turn a completed project into a skipped one.
+                        if (projectStep != step
+                                && projectStep.status.get() != EXECUTED
+                                && projectStep.hasExecutions()) {
+                            allWorkExecuted = false;
+                        }
                     }
 
+                    // A project whose setup never ran was never started at all: it is genuinely skipped,
+                    // even when it has no work of its own (an aggregator, for instance).
+                    boolean projectStarted = plan.step(step.project, SETUP)
+                            .map(setup -> setup.status.get() == EXECUTED)
+                            .orElse(false);
+
                     if (failures != null) {
                         // Handle the stored exception
                         Throwable failure;
@@ -531,7 +546,7 @@ private void executeStep(BuildStep step) throws IOException, LifecycleExecutionE
                             failures.forEach(failure::addSuppressed);
                         }
                         handleBuildError(reactorContext, session, step.project, failure);
-                    } else if (allStepsExecuted) {
+                    } else if (projectStarted && allWorkExecuted) {
                         // If there were no failures, report success
                         projectExecutionListener.afterProjectExecutionSuccess(
                                 new ProjectExecutionEvent(session, step.project, Collections.emptyList()));

impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/concurrent/BuildStep.java

@@ -130,6 +130,16 @@ public Stream<MojoExecution> executions() {
         return mojos.values().stream().flatMap(m -> m.values().stream());
     }
 
+    /**
+     * Indicates whether executing this step performs any actual work.
+     * Steps without mojo executions are pure ordering nodes: this is the case for the
+     * {@code before:} and {@code after:} steps of a phase, and for every phase that lies
+     * outside the scope of the tasks the user requested.
+     */
+    public boolean hasExecutions() {
+        return !mojos.isEmpty();
+    }
+
     @Override
     public boolean equals(Object o) {
         if (this == o) {

impl/maven-core/src/test/java/org/apache/maven/lifecycle/internal/concurrent/BuildStepTest.java

@@ -0,0 +1,60 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.lifecycle.internal.concurrent;
+
+import org.apache.maven.plugin.MojoExecution;
+import org.apache.maven.plugin.descriptor.MojoDescriptor;
+import org.apache.maven.project.MavenProject;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+class BuildStepTest {
+
+    @Test
+    void stepWithoutMojosDoesNoWork() {
+        BuildStep step = new BuildStep("compile", new MavenProject(), null);
+
+        assertFalse(step.hasExecutions());
+    }
+
+    @Test
+    void stepCarryingAMojoDoesWork() {
+        BuildStep step = new BuildStep("compile", new MavenProject(), null);
+        step.addMojo(mojoExecution("compile"), 0);
+
+        assertTrue(step.hasExecutions());
+    }
+
+    @Test
+    void skippedStepDoesNoWork() {
+        BuildStep step = new BuildStep("compile", new MavenProject(), null);
+        step.addMojo(mojoExecution("compile"), 0);
+        step.skip();
+
+        assertFalse(step.hasExecutions());
+    }
+
+    private static MojoExecution mojoExecution(String goal) {
+        MojoDescriptor descriptor = new MojoDescriptor();
+        descriptor.setGoal(goal);
+        return new MojoExecution(descriptor, "default-" + goal);
+    }
+}