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 55c10f8fa4e5..6866649fad03 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 @@ -386,9 +386,9 @@ private void processStep(BuildStep step) { try { executeStep(step); executePlan(); - } catch (Exception e) { + } catch (Throwable e) { step.status.compareAndSet(SKIPPED, FAILED); - // Store the exception in the step for handling in the TEARDOWN phase + // Store the failure 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 @@ -423,10 +423,10 @@ private void processStep(BuildStep step) { } } executePlan(); - } catch (Exception e) { + } catch (Throwable e) { step.status.compareAndSet(SCHEDULED, FAILED); - // Store the exception in the step for handling in the TEARDOWN phase + // Store the failure 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); @@ -510,7 +510,7 @@ private void executeStep(BuildStep step) throws IOException, LifecycleExecutionE List failures = null; boolean allWorkExecuted = true; for (BuildStep projectStep : plan.steps(step.project).toList()) { - Exception exception = projectStep.exception; + Throwable exception = projectStep.exception; if (exception != null) { if (failures == null) { failures = new ArrayList<>(); @@ -545,7 +545,7 @@ private void executeStep(BuildStep step) throws IOException, LifecycleExecutionE failure = new LifecycleExecutionException("Error building project"); failures.forEach(failure::addSuppressed); } - handleBuildError(reactorContext, session, step.project, failure); + handleBuildError(reactorContext, session, step.project, failure, isFatal(failures)); } else if (projectStarted && allWorkExecuted) { // If there were no failures, report success projectExecutionListener.afterProjectExecutionSuccess( @@ -578,6 +578,20 @@ private void executeStep(BuildStep step) throws IOException, LifecycleExecutionE step.status.compareAndSet(SCHEDULED, EXECUTED); } + /** + * Tells whether any of the failures collected for a project must halt the build. Several failures are + * reported through a wrapper, and a wrapper is always a checked exception, so an {@link Error} among + * them can only be seen by looking at the failures themselves. + * + * @param failures The failures collected for a single project + * @return {@code true} if the build must be halted; checked exceptions (ordinary plugin failures) are + * soft and allow the reactor to continue with other projects, while {@link RuntimeException}s + * and {@link Error}s indicate an unexpected JVM or framework state and halt the build + */ + private static boolean isFatal(List failures) { + return failures.stream().anyMatch(t -> t instanceof RuntimeException || !(t instanceof Exception)); + } + private void attachToThread(BuildStep step) { BuildPlanExecutor.attachToThread(step.project); session.setCurrentProject(step.project); @@ -848,13 +862,16 @@ private String getResolvedPhase(String phase) { * @param buildContext The reactor context * @param session The Maven session * @param mavenProject The project that failed - * @param t The exception that caused the failure + * @param t The exception that caused the failure, possibly a wrapper around several failures + * @param fatal Whether the failure must halt the build. This cannot be read off {@code t}, because a + * wrapper around several failures hides what is inside it. See {@link #isFatal(List)}. */ protected void handleBuildError( final ReactorContext buildContext, final MavenSession session, final MavenProject mavenProject, - Throwable t) { + Throwable t, + boolean fatal) { // record the error and mark the project as failed Clock clock = getClock(mavenProject); buildContext.getResult().addException(t); @@ -863,12 +880,12 @@ protected void handleBuildError( .addBuildSummary(new BuildFailure(mavenProject, clock.execTime(), clock.wallTime(), t)); // notify listeners about "soft" project build failures only - if (t instanceof Exception exception && !(t instanceof RuntimeException)) { + if (!fatal && t instanceof Exception exception) { eventCatapult.fire(ExecutionEvent.Type.ProjectFailed, session, null, exception); } // reactor failure modes - if (t instanceof RuntimeException || !(t instanceof Exception)) { + if (fatal) { // fail fast on RuntimeExceptions, Errors and "other" Throwables // assume these are system errors and further build is meaningless buildContext.getReactorBuildStatus().halt(); 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 1eef183059f6..8a5f645c1db9 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 @@ -61,7 +61,7 @@ public class BuildStep { final Collection successors = new HashSet<>(); final AtomicInteger status = new AtomicInteger(); final AtomicBoolean skip = new AtomicBoolean(); - volatile Exception exception; + volatile Throwable exception; public BuildStep(String name, MavenProject project, Lifecycle.Phase phase) { this.name = Objects.requireNonNull(name, "name cannot be null"); diff --git a/impl/maven-core/src/test/java/org/apache/maven/lifecycle/internal/concurrent/BuildPlanExecutorTest.java b/impl/maven-core/src/test/java/org/apache/maven/lifecycle/internal/concurrent/BuildPlanExecutorTest.java new file mode 100644 index 000000000000..9ef77e95bf04 --- /dev/null +++ b/impl/maven-core/src/test/java/org/apache/maven/lifecycle/internal/concurrent/BuildPlanExecutorTest.java @@ -0,0 +1,289 @@ +/* + * 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 java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.function.Consumer; + +import org.apache.maven.api.Lifecycle; +import org.apache.maven.execution.DefaultMavenExecutionRequest; +import org.apache.maven.execution.DefaultMavenExecutionResult; +import org.apache.maven.execution.MavenExecutionRequest; +import org.apache.maven.execution.MavenSession; +import org.apache.maven.execution.ProjectDependencyGraph; +import org.apache.maven.execution.ProjectExecutionEvent; +import org.apache.maven.execution.ProjectExecutionListener; +import org.apache.maven.internal.impl.DefaultLifecycleRegistry; +import org.apache.maven.internal.transformation.TransformerManager; +import org.apache.maven.lifecycle.LifecycleExecutionException; +import org.apache.maven.lifecycle.internal.LifecycleTask; +import org.apache.maven.lifecycle.internal.ReactorBuildStatus; +import org.apache.maven.lifecycle.internal.ReactorContext; +import org.apache.maven.lifecycle.internal.TaskSegment; +import org.apache.maven.lifecycle.internal.stub.ExecutionEventCatapultStub; +import org.apache.maven.project.MavenProject; +import org.eclipse.aether.DefaultRepositorySystemSession; +import org.eclipse.aether.RepositorySystemSession; +import org.eclipse.aether.deployment.DeployRequest; +import org.eclipse.aether.installation.InstallRequest; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class BuildPlanExecutorTest { + + /** + * A build step that throws an {@link Error} must be reported as a build failure, the same way the + * single threaded builder reports it. Otherwise the build ends with no exception at all and Maven + * prints BUILD SUCCESS while nothing was built. + */ + @Test + void errorThrownByBuildStepIsRecordedAsBuildFailure() throws Exception { + Error thrown = new NoClassDefFoundError("some/Class"); + MavenProject project = newProject(); + MavenSession session = newSession(project); + + execute(session, project, event -> { + throw thrown; + }); + + List exceptions = session.getResult().getExceptions(); + assertEquals(1, exceptions.size(), "expected the error to be recorded, but got: " + exceptions); + assertSame(thrown, exceptions.get(0)); + assertTrue(session.getResult().getBuildSummary(project) instanceof org.apache.maven.execution.BuildFailure); + } + + /** + * The same for an exception, which already worked. This pins the existing behaviour so the widened + * catch does not change it. + */ + @Test + void exceptionThrownByBuildStepIsRecordedAsBuildFailure() throws Exception { + RuntimeException thrown = new IllegalStateException("nope"); + MavenProject project = newProject(); + MavenSession session = newSession(project); + + execute(session, project, event -> { + throw thrown; + }); + + List exceptions = session.getResult().getExceptions(); + assertEquals(1, exceptions.size(), "expected the exception to be recorded, but got: " + exceptions); + assertSame(thrown, exceptions.get(0)); + assertTrue(session.getResult().getBuildSummary(project) instanceof org.apache.maven.execution.BuildFailure); + } + + /** + * A checked exception thrown by a build step is a soft failure: the reactor must not be halted, and + * other projects must still be built when {@code REACTOR_FAIL_AT_END} is in effect. This pins the + * soft-failure path of {@code isFatal} so a future change does not silently break {@code --fail-at-end} + * for ordinary plugin failures. + */ + @Test + void checkedExceptionThrownByBuildStepDoesNotHaltReactor() throws Exception { + MavenProject project = newProject(); + MavenSession session = newSession(project); + session.getRequest().setReactorFailureBehavior(MavenExecutionRequest.REACTOR_FAIL_AT_END); + + ReactorContext reactorContext = execute(session, project, event -> {}, plan -> { + BuildStep step = plan.step(project, "validate") + .orElseThrow(() -> new IllegalStateException("no validate step in the plan")); + // LifecycleExecutionException is a checked exception — the soft-failure case for isFatal. + step.exception = new LifecycleExecutionException("plugin failure"); + step.status.set(BuildStep.FAILED); + }); + + assertTrue( + session.getResult().getBuildSummary(project) instanceof org.apache.maven.execution.BuildFailure, + "the project must be recorded as a failure"); + assertTrue( + !reactorContext.getReactorBuildStatus().isHalted(), + "a checked exception must not halt the reactor when REACTOR_FAIL_AT_END is set"); + } + + /** + * A project can end up with more than one failure: when a build step fails, the matching after:* step is + * still run for cleanup and may fail on its own. Those failures are reported through a wrapper exception, + * and the wrapper is a checked exception, so reading the severity off the wrapper hides the {@link Error} + * that is inside it. A cleanup step failing after an Error must not downgrade the build from "halt" to a + * plain per-project failure. + */ + @Test + void errorIsStillFatalWhenASecondFailureJoinsIt() throws Exception { + Error thrown = new NoClassDefFoundError("some/Class"); + MavenProject project = newProject(); + MavenSession session = newSession(project); + // with fail-fast the build halts whatever happens, so the downgrade is only observable with fail-at-end + session.getRequest().setReactorFailureBehavior(MavenExecutionRequest.REACTOR_FAIL_AT_END); + + ReactorContext reactorContext = execute( + session, + project, + event -> { + throw thrown; + }, + plan -> { + BuildStep cleanup = plan.step(project, Lifecycle.AFTER + "validate") + .orElseThrow(() -> new IllegalStateException("no after:validate step in the plan")); + // stands in for the cleanup step failing after the Error, the way processStep records it + cleanup.exception = new LifecycleExecutionException("cleanup failed"); + }); + + assertTrue( + reactorContext.getReactorBuildStatus().isHalted(), + "an Error must halt the reactor even when a second failure is recorded for the same project, but" + + " the build was not halted; recorded exceptions: " + + session.getResult().getExceptions()); + } + + private ReactorContext execute(MavenSession session, MavenProject project, BeforeProjectExecution listener) + throws Exception { + ReactorContext reactorContext = newReactorContext(session); + newExecutor(listener).execute(session, reactorContext, List.of(newTaskSegment())); + return reactorContext; + } + + /** + * Same, but the plan is handed to {@code planCustomizer} after it is created and before it is executed. + * A step that runs no mojo cannot be made to fail from the outside, so this is the only way to put a + * second failure on the project. + */ + private ReactorContext execute( + MavenSession session, + MavenProject project, + BeforeProjectExecution listener, + Consumer planCustomizer) + throws Exception { + ReactorContext reactorContext = newReactorContext(session); + try (BuildPlanExecutor.BuildContext context = + newExecutor(listener).new BuildContext(session, reactorContext, List.of(newTaskSegment()))) { + planCustomizer.accept(context.plan); + context.execute(); + } + return reactorContext; + } + + private ReactorContext newReactorContext(MavenSession session) { + return new ReactorContext( + session.getResult(), + Thread.currentThread().getContextClassLoader(), + new ReactorBuildStatus(session.getProjectDependencyGraph())); + } + + private TaskSegment newTaskSegment() { + TaskSegment taskSegment = new TaskSegment(false); + taskSegment.getTasks().add(new LifecycleTask("validate")); + return taskSegment; + } + + private BuildPlanExecutor newExecutor(ProjectExecutionListener listener) { + return new BuildPlanExecutor( + null, + new ExecutionEventCatapultStub(), + List.of(listener), + new NoopTransformerManager(), + new BuildPlanLogger(), + Map.of(), + null, + null, + new DefaultLifecycleRegistry(Collections.emptyList())); + } + + private MavenProject newProject() { + MavenProject result = new MavenProject(); + result.setArtifactId("a"); + result.setCollectedProjects(List.of()); + return result; + } + + private MavenSession newSession(MavenProject project) { + MavenExecutionRequest request = new DefaultMavenExecutionRequest(); + request.setGoals(List.of("validate")); + MavenSession result = new MavenSession( + null, new DefaultRepositorySystemSession(h -> false), request, new DefaultMavenExecutionResult()); + result.setProjectDependencyGraph(new SingleProjectDependencyGraph(project)); + result.setProjects(List.of(project)); + return result; + } + + private static final class SingleProjectDependencyGraph implements ProjectDependencyGraph { + + private final List projects; + + private SingleProjectDependencyGraph(MavenProject project) { + this.projects = List.of(project); + } + + @Override + public List getAllProjects() { + return projects; + } + + @Override + public List getSortedProjects() { + return projects; + } + + @Override + public List getDownstreamProjects(MavenProject project, boolean transitive) { + return List.of(); + } + + @Override + public List getUpstreamProjects(MavenProject project, boolean transitive) { + return List.of(); + } + } + + private static final class NoopTransformerManager implements TransformerManager { + + @Override + public InstallRequest remapInstallArtifacts(RepositorySystemSession session, InstallRequest request) { + return request; + } + + @Override + public DeployRequest remapDeployArtifacts(RepositorySystemSession session, DeployRequest request) { + return request; + } + + @Override + public void injectTransformedArtifacts(RepositorySystemSession repositorySession, MavenProject project) {} + } + + @FunctionalInterface + private interface BeforeProjectExecution extends ProjectExecutionListener { + + @Override + void beforeProjectExecution(ProjectExecutionEvent event); + + @Override + default void beforeProjectLifecycleExecution(ProjectExecutionEvent event) {} + + @Override + default void afterProjectExecutionSuccess(ProjectExecutionEvent event) {} + + @Override + default void afterProjectExecutionFailure(ProjectExecutionEvent event) {} + } +}