From 44dba0ddd2d02ef7bc27abb267a07ddde6bf7d3f Mon Sep 17 00:00:00 2001 From: Stefan Oehme Date: Tue, 8 Apr 2025 13:22:13 +0200 Subject: [PATCH 1/2] [MNG-8670] Test case for missing/wrong project events Rather than adding another test, I've expanded on the one I previously wrote for ProjectStarted events. --- ...a => MavenITmng8648ProjectEventsTest.java} | 30 ++++++++++++------- .../apache/maven/it/TestSuiteOrdering.java | 2 +- .../maven/its/mng8648/ProjectEventSpy.java | 11 +++++-- .../test/resources/mng-8648/project/pom.xml | 2 ++ .../mng-8648/project/subproject-c/pom.xml | 12 ++++++++ .../subproject-c/src/main/java/Foo.java | 19 ++++++++++++ .../mng-8648/project/subproject-d/pom.xml | 19 ++++++++++++ 7 files changed, 81 insertions(+), 14 deletions(-) rename its/core-it-suite/src/test/java/org/apache/maven/it/{MavenITmng8648ProjectStartedEventsTest.java => MavenITmng8648ProjectEventsTest.java} (61%) create mode 100644 its/core-it-suite/src/test/resources/mng-8648/project/subproject-c/pom.xml create mode 100644 its/core-it-suite/src/test/resources/mng-8648/project/subproject-c/src/main/java/Foo.java create mode 100644 its/core-it-suite/src/test/resources/mng-8648/project/subproject-d/pom.xml diff --git a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng8648ProjectStartedEventsTest.java b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng8648ProjectEventsTest.java similarity index 61% rename from its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng8648ProjectStartedEventsTest.java rename to its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng8648ProjectEventsTest.java index c117cbb4a8b5..39cc6ea76a50 100644 --- a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng8648ProjectStartedEventsTest.java +++ b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng8648ProjectEventsTest.java @@ -22,9 +22,9 @@ import org.junit.jupiter.api.Test; -public class MavenITmng8648ProjectStartedEventsTest extends AbstractMavenIntegrationTestCase { +public class MavenITmng8648ProjectEventsTest extends AbstractMavenIntegrationTestCase { - public MavenITmng8648ProjectStartedEventsTest() { + public MavenITmng8648ProjectEventsTest() { super("[4.0.0-rc-4,)"); } @@ -41,15 +41,25 @@ public void test() throws Exception { verifier = newVerifier(projectDir.getAbsolutePath()); verifier.addCliArguments("compile", "-b", "concurrent"); - verifier.execute(); - verifier.verifyErrorFreeLog(); + try { + verifier.execute(); + } catch (VerificationException expected) { + } - verifier.verifyTextInLog("org.apache.maven.its.mng8648:root:pom:1-SNAPSHOT started"); - verifier.verifyTextInLog("org.apache.maven.its.mng8648:root:pom:1-SNAPSHOT finished"); - verifier.verifyTextInLog("org.apache.maven.its.mng8648:subproject-a:jar:1-SNAPSHOT started"); - verifier.verifyTextInLog("org.apache.maven.its.mng8648:subproject-a:jar:1-SNAPSHOT finished"); - verifier.verifyTextInLog("org.apache.maven.its.mng8648:subproject-b:jar:1-SNAPSHOT started"); - verifier.verifyTextInLog("org.apache.maven.its.mng8648:subproject-b:jar:1-SNAPSHOT finished"); + // The root project is marked as successful with the traditional builder + // With the concurrent builder, it gets no finish event and in the reactor summary it is listed as "skipped" + verifier.verifyTextInLog("org.apache.maven.its.mng8648:root:pom:1-SNAPSHOT ProjectStarted"); + verifier.verifyTextInLog("org.apache.maven.its.mng8648:root:pom:1-SNAPSHOT ProjectSucceeded"); + verifier.verifyTextInLog("org.apache.maven.its.mng8648:subproject-a:jar:1-SNAPSHOT ProjectStarted"); + verifier.verifyTextInLog("org.apache.maven.its.mng8648:subproject-a:jar:1-SNAPSHOT ProjectSucceeded"); + verifier.verifyTextInLog("org.apache.maven.its.mng8648:subproject-b:jar:1-SNAPSHOT ProjectStarted"); + verifier.verifyTextInLog("org.apache.maven.its.mng8648:subproject-b:jar:1-SNAPSHOT ProjectSucceeded"); + verifier.verifyTextInLog("org.apache.maven.its.mng8648:subproject-c:jar:1-SNAPSHOT ProjectStarted"); + verifier.verifyTextInLog("org.apache.maven.its.mng8648:subproject-c:jar:1-SNAPSHOT ProjectFailed"); + // With the traditional builder, project D is not reported at all (it is never even started) and in the reactor + // summary it is listed as "skipped" + // With the concurrent builder, it is currently reported as successful, which seems wrong, since it depends on + // a failed project verifier.verifyTextNotInLog("Failed to notify spy org.apache.maven.its.mng8648.ProjectEventSpy"); } } diff --git a/its/core-it-suite/src/test/java/org/apache/maven/it/TestSuiteOrdering.java b/its/core-it-suite/src/test/java/org/apache/maven/it/TestSuiteOrdering.java index 470a7a9f56f7..425a4db23a20 100644 --- a/its/core-it-suite/src/test/java/org/apache/maven/it/TestSuiteOrdering.java +++ b/its/core-it-suite/src/test/java/org/apache/maven/it/TestSuiteOrdering.java @@ -107,7 +107,7 @@ public TestSuiteOrdering() { suite.addTestSuite(MavenITmng8598JvmConfigSubstitutionTest.class); suite.addTestSuite(MavenITmng8653AfterAndEachPhasesWithConcurrentBuilderTest.class); suite.addTestSuite(MavenITmng5668AfterPhaseExecutionTest.class); - suite.addTestSuite(MavenITmng8648ProjectStartedEventsTest.class); + suite.addTestSuite(MavenITmng8648ProjectEventsTest.class); suite.addTestSuite(MavenITmng8645ConsumerPomDependencyManagementTest.class); suite.addTestSuite(MavenITmng8594AtFileTest.class); suite.addTestSuite(MavenITmng8561SourceRootTest.class); diff --git a/its/core-it-suite/src/test/resources/mng-8648/extension/src/main/java/org/apache/maven/its/mng8648/ProjectEventSpy.java b/its/core-it-suite/src/test/resources/mng-8648/extension/src/main/java/org/apache/maven/its/mng8648/ProjectEventSpy.java index 6678757acae4..017a2769e706 100644 --- a/its/core-it-suite/src/test/resources/mng-8648/extension/src/main/java/org/apache/maven/its/mng8648/ProjectEventSpy.java +++ b/its/core-it-suite/src/test/resources/mng-8648/extension/src/main/java/org/apache/maven/its/mng8648/ProjectEventSpy.java @@ -38,12 +38,17 @@ public void onEvent(Object event) { MavenProject project = executionEvent.getProject(); switch (executionEvent.getType()) { case ProjectStarted: - System.out.println(project.getId() + " started"); - projects.put(project.getId(), project); + System.out.println(project.getId() + " " + executionEvent.getType()); + MavenProject existing = projects.put(project.getId(), project); + if (existing != null) { + throw new IllegalStateException("Project " + project.getId() + " was already started"); + } break; case ProjectSucceeded: + case ProjectFailed: + case ProjectSkipped: + System.out.println(project.getId() + " " + executionEvent.getType()); MavenProject mavenProject = projects.get(project.getId()); - System.out.println(project.getId() + " finished"); if (mavenProject == null) { throw new IllegalStateException("Project " + project.getId() + " was never started"); } diff --git a/its/core-it-suite/src/test/resources/mng-8648/project/pom.xml b/its/core-it-suite/src/test/resources/mng-8648/project/pom.xml index acf5b1832cc4..8512077a1075 100644 --- a/its/core-it-suite/src/test/resources/mng-8648/project/pom.xml +++ b/its/core-it-suite/src/test/resources/mng-8648/project/pom.xml @@ -11,5 +11,7 @@ subproject-a subproject-b + subproject-c + subproject-d diff --git a/its/core-it-suite/src/test/resources/mng-8648/project/subproject-c/pom.xml b/its/core-it-suite/src/test/resources/mng-8648/project/subproject-c/pom.xml new file mode 100644 index 000000000000..46b8aee507ca --- /dev/null +++ b/its/core-it-suite/src/test/resources/mng-8648/project/subproject-c/pom.xml @@ -0,0 +1,12 @@ + + + + 4.0.0 + + + org.apache.maven.its.mng8648 + root + 1-SNAPSHOT + + subproject-c + diff --git a/its/core-it-suite/src/test/resources/mng-8648/project/subproject-c/src/main/java/Foo.java b/its/core-it-suite/src/test/resources/mng-8648/project/subproject-c/src/main/java/Foo.java new file mode 100644 index 000000000000..2196c1bf1da2 --- /dev/null +++ b/its/core-it-suite/src/test/resources/mng-8648/project/subproject-c/src/main/java/Foo.java @@ -0,0 +1,19 @@ +/* + * 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. + */ +public class Foo implements Object {} diff --git a/its/core-it-suite/src/test/resources/mng-8648/project/subproject-d/pom.xml b/its/core-it-suite/src/test/resources/mng-8648/project/subproject-d/pom.xml new file mode 100644 index 000000000000..cd9af4b59551 --- /dev/null +++ b/its/core-it-suite/src/test/resources/mng-8648/project/subproject-d/pom.xml @@ -0,0 +1,19 @@ + + + + 4.0.0 + + + org.apache.maven.its.mng8648 + root + 1-SNAPSHOT + + subproject-d + + + ${project.groupId} + subproject-c + ${project.version} + + + From 6be4d44e95e41c01208444dbf4f5560bdf85bbd0 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Thu, 24 Apr 2025 08:59:22 +0200 Subject: [PATCH 2/2] [MNG-8670] Rework BuildPlanExecutor for better events --- .../concurrent/BuildPlanExecutor.java | 293 ++++++++++++------ .../internal/concurrent/BuildStep.java | 23 +- .../it/MavenITmng8648ProjectEventsTest.java | 12 +- 3 files changed, 221 insertions(+), 107 deletions(-) diff --git a/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/concurrent/BuildPlanExecutor.java b/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/concurrent/BuildPlanExecutor.java index 8b330cc048da..3bca5cf6b3f3 100644 --- a/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/concurrent/BuildPlanExecutor.java +++ b/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/concurrent/BuildPlanExecutor.java @@ -70,6 +70,7 @@ import org.apache.maven.lifecycle.internal.LifecycleTask; import org.apache.maven.lifecycle.internal.MojoDescriptorCreator; import org.apache.maven.lifecycle.internal.MojoExecutor; +import org.apache.maven.lifecycle.internal.ReactorBuildStatus; import org.apache.maven.lifecycle.internal.ReactorContext; import org.apache.maven.lifecycle.internal.Task; import org.apache.maven.lifecycle.internal.TaskSegment; @@ -100,6 +101,7 @@ import static org.apache.maven.lifecycle.internal.concurrent.BuildStep.PLANNING; import static org.apache.maven.lifecycle.internal.concurrent.BuildStep.SCHEDULED; import static org.apache.maven.lifecycle.internal.concurrent.BuildStep.SETUP; +import static org.apache.maven.lifecycle.internal.concurrent.BuildStep.SKIPPED; import static org.apache.maven.lifecycle.internal.concurrent.BuildStep.TEARDOWN; /** @@ -257,9 +259,9 @@ public BuildPlan buildInitialPlan(List taskSegments) { teardown.executeAfter(setup); setup.executeAfter(pplan); plan.steps(project).forEach(step -> { - if (step.predecessors.isEmpty()) { + if (step.predecessors.stream().noneMatch(s -> s.project == project)) { step.executeAfter(setup); - } else if (step.successors.isEmpty()) { + } else if (step.successors.stream().noneMatch(s -> s.project == project)) { teardown.executeAfter(step); } }); @@ -344,95 +346,135 @@ public void close() { this.executor.close(); } - private void executePlan() { - if (reactorContext.getReactorBuildStatus().isHalted()) { - return; - } - Clock global = clocks.computeIfAbsent(GLOBAL, p -> new Clock()); - global.start(); - lock.readLock().lock(); - try { - // Get all build steps that are: - // 1. Not yet started (CREATED status) - // 2. Have all their prerequisites completed (predecessors EXECUTED) - // 3. Successfully transition from CREATED to SCHEDULED state - plan.sortedNodes().stream() - .filter(step -> step.status.get() == CREATED) - .filter(step -> step.predecessors.stream().allMatch(s -> s.status.get() == EXECUTED)) - .filter(step -> step.status.compareAndSet(CREATED, SCHEDULED)) - .forEach(step -> { - boolean nextIsPlanning = step.successors.stream().anyMatch(st -> PLAN.equals(st.name)); - executor.execute(() -> { - try { - executeStep(step); - if (nextIsPlanning) { - lock.writeLock().lock(); - try { - plan(); - } finally { - lock.writeLock().unlock(); - } - } - executePlan(); - } catch (Exception e) { - step.status.compareAndSet(SCHEDULED, FAILED); - global.stop(); - - // Find and execute all pending after:* phases for this project - executeAfterPhases(step); - - handleBuildError(reactorContext, session, step.project, e, global); - } - }); - }); - } finally { - lock.readLock().unlock(); - } - } - /** - * Executes all pending after:* phases for a failed project. - * This ensures proper cleanup is performed even when a build fails. - * Only executes after:xxx phases if their corresponding before:xxx phase - * has been either executed or failed. + * Processes a single build step, deciding whether to schedule it for execution or skip it. * - * For example, if a project fails during 'compile', this will execute - * any configured 'after:compile' phases to ensure proper cleanup. - * - * @param failedStep The build step that failed, containing the project that needs cleanup + * @param step The build step to process */ - private void executeAfterPhases(BuildStep failedStep) { - if (failedStep == null || failedStep.project == null) { - return; - } - - lock.readLock().lock(); - try { - // Find all after:* phases that should be executed - plan.steps(failedStep.project) - .filter(step -> step.name != null && step.name.startsWith(AFTER)) - .filter(step -> step.status.get() == CREATED) - .filter(step -> { - // Only execute after:xxx if before:xxx has been executed or failed - String phaseName = step.name.substring(AFTER.length()); - return plan.step(failedStep.project, BEFORE + phaseName) - .map(s -> { - int status = s.status.get(); - return status == EXECUTED || status == FAILED; - }) - .orElse(false); + private void processStep(BuildStep step) { + // 1. Apply reactor failure behavior to decide whether to schedule or skip + ReactorBuildStatus status = reactorContext.getReactorBuildStatus(); + boolean isAfterStep = step.name.startsWith(AFTER); + boolean shouldExecute; + + // Check if all predecessors are executed successfully + boolean allPredecessorsExecuted = step.predecessors.stream().allMatch(s -> s.status.get() == EXECUTED); + + // Special case for after:* steps - they should run if their corresponding before:* step ran + if (isAfterStep) { + String phaseName = step.name.substring(AFTER.length()); + // Always process after:* steps for cleanup if their before:* step ran + shouldExecute = plan.step(step.project, BEFORE + phaseName) + .map(s -> { + int stepStatus = s.status.get(); + return stepStatus == EXECUTED; }) - .filter(step -> step.status.compareAndSet(CREATED, SCHEDULED)) - .forEach(afterStep -> { + .orElse(false); + + // Check if any predecessor failed - if so, we'll run the step but mark it as SKIPPED + boolean anyPredecessorFailed = step.predecessors.stream().anyMatch(s -> s.status.get() == FAILED); + + // If any predecessor failed, we'll use a special status transition: CREATED -> SKIPPED + // This ensures the step runs for cleanup but is marked as skipped in the end + if (shouldExecute && anyPredecessorFailed) { + // We'll run the step but mark it as SKIPPED instead of SCHEDULED + if (step.status.compareAndSet(CREATED, SKIPPED)) { + logger.debug( + "Running after:* step {} for cleanup but marking it as SKIPPED because a predecessor failed", + step); + executor.execute(() -> { try { - executeStep(afterStep); - afterStep.status.compareAndSet(SCHEDULED, EXECUTED); + executeStep(step); + executePlan(); } catch (Exception e) { - // Log but don't fail - we're already in error handling - logger.error("Error executing cleanup phase " + afterStep.name, e); - afterStep.status.compareAndSet(SCHEDULED, FAILED); + step.status.compareAndSet(SKIPPED, FAILED); + // Store the exception in the step for handling in the TEARDOWN phase + step.exception = e; + logger.debug("Stored exception for step {} to be handled in TEARDOWN phase", step, e); + // Let the scheduler handle after:* phases and TEARDOWN in the next cycle + executePlan(); } }); + return; // Skip the rest of the method since we've handled this step + } + } + } else if (TEARDOWN.equals(step.name)) { + // TEARDOWN should always run to ensure proper cleanup and error handling + // We'll handle success/failure reporting inside the TEARDOWN phase + shouldExecute = true; + } else { + // For regular steps: + // Don't run for halted builds, blacklisted projects, or if predecessors failed + shouldExecute = !status.isHalted() && !status.isBlackListed(step.project) && allPredecessorsExecuted; + } + + // 2. Either schedule the step or mark it as skipped based on the decision + if (shouldExecute && step.status.compareAndSet(CREATED, SCHEDULED)) { + boolean nextIsPlanning = step.successors.stream().anyMatch(st -> PLAN.equals(st.name)); + executor.execute(() -> { + try { + executeStep(step); + if (nextIsPlanning) { + lock.writeLock().lock(); + try { + plan(); + } finally { + lock.writeLock().unlock(); + } + } + executePlan(); + } catch (Exception e) { + step.status.compareAndSet(SCHEDULED, FAILED); + + // Store the exception in the step for handling in the TEARDOWN phase + step.exception = e; + logger.debug("Stored exception for step {} to be handled in TEARDOWN phase", step, e); + + // Let the scheduler handle after:* phases and TEARDOWN in the next cycle + executePlan(); + } + }); + } else if (step.status.compareAndSet(CREATED, SKIPPED)) { + // Skip the step and provide a specific reason + if (!shouldExecute) { + if (status.isHalted()) { + logger.debug("Skipping step {} because the build is halted", step); + } else if (status.isBlackListed(step.project)) { + logger.debug("Skipping step {} because the project is blacklisted", step); + } else if (TEARDOWN.equals(step.name)) { + // This should never happen given we always process TEARDOWN steps + logger.warn("Unexpected skipping of TEARDOWN step {}", step); + } else { + logger.debug("Skipping step {} because a dependency has failed", step); + } + } else { + // Skip because predecessors failed or were skipped + logger.debug( + "Skipping step {} because one or more predecessors did not execute successfully", step); + } + // Recursively call executePlan to process steps that depend on this one + executePlan(); + } + } + + private void executePlan() { + // Even if the build is halted, we still want to execute TEARDOWN and after:* steps + // for proper cleanup, so we don't return early here + Clock global = getClock(GLOBAL); + global.start(); + lock.readLock().lock(); + try { + // Process build steps in a logical order: + // 1. Find steps that are not yet started (CREATED status) + // 2. Check if all their predecessors have completed (in a terminal state) + // 3. Process each step (schedule or skip based on reactor failure behavior) + plan.sortedNodes().stream() + // 1. Filter steps that are in CREATED state + .filter(BuildStep::isCreated) + // 2. Check if all predecessors are in a terminal state + .filter(step -> step.predecessors.stream().allMatch(BuildStep::isDone)) + // 3. Process each step + .forEach(this::processStep); } finally { lock.readLock().unlock(); } @@ -464,24 +506,58 @@ private void executeStep(BuildStep step) throws IOException, LifecycleExecutionE break; case TEARDOWN: attachToThread(step); - projectExecutionListener.afterProjectExecutionSuccess( - new ProjectExecutionEvent(session, step.project, Collections.emptyList())); - reactorContext - .getResult() - .addBuildSummary(new BuildSuccess(step.project, clock.wallTime(), clock.execTime())); - eventCatapult.fire(ExecutionEvent.Type.ProjectSucceeded, session, null); + + // Check if there are any stored exceptions for this project + List failures = null; + boolean allStepsExecuted = true; + for (BuildStep projectStep : plan.steps(step.project).toList()) { + Exception exception = projectStep.exception; + if (exception != null) { + if (failures == null) { + failures = new ArrayList<>(); + } + failures.add(exception); + } + allStepsExecuted &= step == projectStep || projectStep.status.get() == EXECUTED; + } + + if (failures != null) { + // Handle the stored exception + Throwable failure; + if (failures.size() == 1) { + failure = failures.get( + 0); // Single exception, no need to wrap it in a LifecycleExecutionException + } else { + failure = new LifecycleExecutionException("Error building project"); + failures.forEach(failure::addSuppressed); + } + handleBuildError(reactorContext, session, step.project, failure); + } else if (allStepsExecuted) { + // If there were no failures, report success + projectExecutionListener.afterProjectExecutionSuccess( + new ProjectExecutionEvent(session, step.project, Collections.emptyList())); + reactorContext + .getResult() + .addBuildSummary(new BuildSuccess(step.project, clock.wallTime(), clock.execTime())); + eventCatapult.fire(ExecutionEvent.Type.ProjectSucceeded, session, null); + } else { + eventCatapult.fire(ExecutionEvent.Type.ProjectSkipped, session, null); + } break; default: - List executions = step.executions().collect(Collectors.toList()); + List executions = step.executions().toList(); if (!executions.isEmpty()) { attachToThread(step); clock.start(); - executions.forEach(mojoExecution -> { - mojoExecutionConfigurator(mojoExecution).configure(step.project, mojoExecution, true); - finalizeMojoConfiguration(mojoExecution); - }); - mojoExecutor.execute(session, executions); - clock.stop(); + try { + executions.forEach(mojoExecution -> { + mojoExecutionConfigurator(mojoExecution).configure(step.project, mojoExecution, true); + finalizeMojoConfiguration(mojoExecution); + }); + mojoExecutor.execute(session, executions); + } finally { + clock.stop(); + } } break; } @@ -501,7 +577,7 @@ private void plan() { lock.writeLock().lock(); try { Set planSteps = plan.allSteps() - .filter(st -> PLAN.equals(st.name)) + .filter(step -> PLAN.equals(step.name)) .filter(step -> step.predecessors.stream().allMatch(s -> s.status.get() == EXECUTED)) .filter(step -> step.status.compareAndSet(PLANNING, SCHEDULED)) .collect(Collectors.toSet()); @@ -664,13 +740,30 @@ private String getResolvedPhase(String phase) { .orElse(phase); } + /** + * Handles build errors by recording the error, notifying listeners, and updating the ReactorBuildStatus + * based on the reactor failure behavior. + *

+ * This method works in conjunction with the filtering in executePlan(): + * - For FAIL_FAST: Sets ReactorBuildStatus to halted, which causes executePlan to only process after:* steps + * - For FAIL_AT_END: Blacklists the project and its dependents, which causes executePlan to skip them + * - For FAIL_NEVER: Does nothing special, allowing all projects to continue building + *

+ * Note: TEARDOWN steps are not executed for failed or blacklisted projects, as they're designed for + * successful project completions. + * + * @param buildContext The reactor context + * @param session The Maven session + * @param mavenProject The project that failed + * @param t The exception that caused the failure + */ protected void handleBuildError( final ReactorContext buildContext, final MavenSession session, final MavenProject mavenProject, - Throwable t, - final Clock clock) { + Throwable t) { // record the error and mark the project as failed + Clock clock = getClock(mavenProject); buildContext.getResult().addException(t); buildContext .getResult() @@ -812,7 +905,7 @@ public BuildPlan calculateLifecycleMappings( return Stream.of(a, b, c); }) .collect(Collectors.toMap(n -> n.name, n -> n)); - // for each phase, make sure children phases are execute between pre and post steps + // for each phase, make sure children phases are executed between before and after steps lifecycle.allPhases().forEach(phase -> phase.phases().forEach(child -> { steps.get(BEFORE + child.name()).executeAfter(steps.get(BEFORE + phase.name())); steps.get(AFTER + phase.name()).executeAfter(steps.get(AFTER + child.name())); diff --git a/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/concurrent/BuildStep.java b/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/concurrent/BuildStep.java index a97b641ca797..baf77e26893f 100644 --- a/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/concurrent/BuildStep.java +++ b/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/concurrent/BuildStep.java @@ -30,6 +30,8 @@ import java.util.stream.Stream; import org.apache.maven.api.Lifecycle; +import org.apache.maven.api.annotations.Nonnull; +import org.apache.maven.api.annotations.Nullable; import org.apache.maven.plugin.MojoExecution; import org.apache.maven.project.MavenProject; @@ -39,26 +41,43 @@ public class BuildStep { public static final int SCHEDULED = 2; public static final int EXECUTED = 3; public static final int FAILED = 4; + public static final int SKIPPED = 5; public static final String PLAN = "$plan$"; public static final String SETUP = "$setup$"; public static final String TEARDOWN = "$teardown$"; + @Nonnull final MavenProject project; + + @Nonnull final String name; + + @Nullable final Lifecycle.Phase phase; + final Map> mojos = new TreeMap<>(); final Collection predecessors = new HashSet<>(); final Collection successors = new HashSet<>(); final AtomicInteger status = new AtomicInteger(); final AtomicBoolean skip = new AtomicBoolean(); + volatile Exception exception; public BuildStep(String name, MavenProject project, Lifecycle.Phase phase) { - this.name = name; - this.project = project; + this.name = Objects.requireNonNull(name, "name cannot be null"); + this.project = Objects.requireNonNull(project, "project cannot be null"); this.phase = phase; } + public boolean isCreated() { + return status.get() == CREATED; + } + + public boolean isDone() { + int state = status.get(); + return state == EXECUTED || state == FAILED || state == SKIPPED; + } + public Stream allPredecessors() { return preds(new HashSet<>()).stream(); } diff --git a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng8648ProjectEventsTest.java b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng8648ProjectEventsTest.java index 39cc6ea76a50..12b75ec60b42 100644 --- a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng8648ProjectEventsTest.java +++ b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng8648ProjectEventsTest.java @@ -40,7 +40,7 @@ public void test() throws Exception { File projectDir = extractResources("/mng-8648/project"); verifier = newVerifier(projectDir.getAbsolutePath()); - verifier.addCliArguments("compile", "-b", "concurrent"); + verifier.addCliArguments("compile", "-b", "concurrent", "-T5"); try { verifier.execute(); } catch (VerificationException expected) { @@ -56,10 +56,12 @@ public void test() throws Exception { verifier.verifyTextInLog("org.apache.maven.its.mng8648:subproject-b:jar:1-SNAPSHOT ProjectSucceeded"); verifier.verifyTextInLog("org.apache.maven.its.mng8648:subproject-c:jar:1-SNAPSHOT ProjectStarted"); verifier.verifyTextInLog("org.apache.maven.its.mng8648:subproject-c:jar:1-SNAPSHOT ProjectFailed"); - // With the traditional builder, project D is not reported at all (it is never even started) and in the reactor - // summary it is listed as "skipped" - // With the concurrent builder, it is currently reported as successful, which seems wrong, since it depends on - // a failed project + // With the traditional builder, project D is not reported at all (it is never even started), + // and in the reactor summary it is listed as "skipped". + // With the concurrent builder, it should be started and later reported as "skipped" + verifier.verifyTextInLog("org.apache.maven.its.mng8648:subproject-d:jar:1-SNAPSHOT ProjectStarted"); + verifier.verifyTextInLog("org.apache.maven.its.mng8648:subproject-d:jar:1-SNAPSHOT ProjectSkipped"); + // Make sure there's no problem with the event spy verifier.verifyTextNotInLog("Failed to notify spy org.apache.maven.its.mng8648.ProjectEventSpy"); } }