From 46ec597ac1a448a4bfc796ade3b2542da23a9a3b Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Thu, 27 Mar 2025 15:10:06 +0100 Subject: [PATCH] [MNG-5668] Execute after:* phases when build fails JIRA issue: [MNG-5668](https://issues.apache.org/jira/browse/MNG-5668) When a build step fails, Maven should still execute its corresponding after:* phases to ensure proper cleanup. This fix modifies the BuildPlanExecutor to: - Execute after:* phases when their corresponding before:* phase has been executed - Maintain proper phase ordering during failure handling - Handle cleanup phase failures gracefully without affecting the original error - Preserve concurrent build capabilities This ensures cleanup tasks (like resource cleanup or test environment teardown) are properly executed even when the build fails. --- .../concurrent/BuildPlanExecutor.java | 41 +++++++++++ ...MavenITmng5668AfterPhaseExecutionTest.java | 68 +++++++++++++++++++ .../apache/maven/it/TestSuiteOrdering.java | 1 + .../mng-5668-after-phase-execution/pom.xml | 60 ++++++++++++++++ 4 files changed, 170 insertions(+) create mode 100644 its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng5668AfterPhaseExecutionTest.java create mode 100644 its/core-it-suite/src/test/resources/mng-5668-after-phase-execution/pom.xml 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 664509d2a1c4..f8924eb13e12 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 @@ -343,6 +343,10 @@ private void 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); } }); @@ -352,6 +356,43 @@ private void executePlan() { } } + 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); + }) + .filter(step -> step.status.compareAndSet(CREATED, SCHEDULED)) + .forEach(afterStep -> { + try { + executeStep(afterStep); + afterStep.status.compareAndSet(SCHEDULED, EXECUTED); + } 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); + } + }); + } finally { + lock.readLock().unlock(); + } + } + private void executeStep(BuildStep step) throws IOException, LifecycleExecutionException { Clock clock = getClock(step.project); switch (step.name) { diff --git a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng5668AfterPhaseExecutionTest.java b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng5668AfterPhaseExecutionTest.java new file mode 100644 index 000000000000..96369af09ad8 --- /dev/null +++ b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng5668AfterPhaseExecutionTest.java @@ -0,0 +1,68 @@ +/* + * 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.it; + +import java.io.File; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * This is a test for MNG-5668: + * Verifies that after:xxx phases are executed even when the build fails + */ +class MavenITmng5668AfterPhaseExecutionTest extends AbstractMavenIntegrationTestCase { + + MavenITmng5668AfterPhaseExecutionTest() { + super("[4.0.0-rc-4,)"); // test is only relevant for Maven 4.0+ + } + + @Test + void testAfterPhaseExecutionOnFailure() throws Exception { + File testDir = extractResources("/mng-5668-after-phase-execution"); + + Verifier verifier = newVerifier(testDir.getAbsolutePath()); + verifier.setAutoclean(false); + verifier.deleteDirectory("target"); + + try { + verifier.addCliArgument("-b"); + verifier.addCliArgument("concurrent"); + verifier.addCliArgument("verify"); + verifier.execute(); + fail("Build should have failed"); + } catch (VerificationException e) { + // expected + } + + // Verify that marker files were created in the expected order + verifier.verifyFilePresent("target/before-verify.txt"); + verifier.verifyFilePresent("target/verify-failed.txt"); + verifier.verifyFilePresent("target/after-verify.txt"); + + // Verify the execution order through timestamps + long beforeTime = new File(testDir, "target/before-verify.txt").lastModified(); + long failTime = new File(testDir, "target/verify-failed.txt").lastModified(); + long afterTime = new File(testDir, "target/after-verify.txt").lastModified(); + + assertTrue(beforeTime <= failTime); + assertTrue(failTime <= afterTime); + } +} 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 dcbfb085c678..f0e31755dde3 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 @@ -101,6 +101,7 @@ public TestSuiteOrdering() { * the tests are to finishing. Newer tests are also more likely to fail, so this is * a fail fast technique as well. */ + suite.addTestSuite(MavenITmng5668AfterPhaseExecutionTest.class); suite.addTestSuite(MavenITmng8594AtFileTest.class); suite.addTestSuite(MavenITmng8561SourceRootTest.class); suite.addTestSuite(MavenITmng8523ModelPropertiesTest.class); diff --git a/its/core-it-suite/src/test/resources/mng-5668-after-phase-execution/pom.xml b/its/core-it-suite/src/test/resources/mng-5668-after-phase-execution/pom.xml new file mode 100644 index 000000000000..46bcda5f36b3 --- /dev/null +++ b/its/core-it-suite/src/test/resources/mng-5668-after-phase-execution/pom.xml @@ -0,0 +1,60 @@ + + + 4.0.0 + + org.apache.maven.its.mng5668 + test + 1.0-SNAPSHOT + + Test + Test that verifies after:xxx phase execution when build fails + + + + + org.apache.maven.plugins + maven-antrun-plugin + 3.1.0 + + + before-verify + + run + + before:verify + + + + + + + + verify + + run + + verify + + + + + + + + + after-verify + + run + + after:verify + + + + + + + + + + +