gnodet-bot commented on code in PR #13647:
URL: https://github.com/apache/maven/pull/13647#discussion_r4323721526
##########
impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/concurrent/BuildPlanExecutor.java:
##########
@@ -560,11 +565,23 @@ private void executeStep(BuildStep step) throws
IOException, LifecycleExecutionE
failure = new LifecycleExecutionException("Error
building project");
failures.forEach(failure::addSuppressed);
}
+ List<MojoExecution> failedPlan =
executionPlan(step.project);
+ projectExecutionListener.afterProjectExecutionFailure(
+ new ProjectExecutionEvent(session,
step.project, failedPlan, failure));
+ try {
+
LifecycleParticipantDispatcher.afterProjectExecution(
+ session, step.project, failedPlan,
failure);
+ } catch (LifecycleExecutionException e) {
+ failure.addSuppressed(e);
+ }
handleBuildError(reactorContext, session,
step.project, failure, isFatal(failures));
} else if (projectStarted && allWorkExecuted) {
// If there were no failures, report success
+ List<MojoExecution> successfulPlan =
executionPlan(step.project);
projectExecutionListener.afterProjectExecutionSuccess(
- new ProjectExecutionEvent(session,
step.project, Collections.emptyList()));
+ new ProjectExecutionEvent(session,
step.project, successfulPlan));
+ LifecycleParticipantDispatcher.afterProjectExecution(
+ session, step.project, successfulPlan, null);
Review Comment:
⚠️ **Unguarded `afterProjectExecution` on success path.** The failure path
at line ~572 wraps this call in `try/catch (LifecycleExecutionException e) {
failure.addSuppressed(e); }`. This success path has no such guard. A
`LifecycleParticipantException` from `afterProjectExecution` escapes into the
outer `catch (Throwable e)` Runnable, gets stored as `step.exception`, and at
TEARDOWN causes the project to be reported as FAILED even though all mojos
succeeded.
```suggestion
try {
LifecycleParticipantDispatcher.afterProjectExecution(
session, step.project, successfulPlan,
null);
} catch (LifecycleExecutionException e) {
// Participant failure on success path: report
the project as failed.
// The project's mojos succeeded, but a
participant aborted the session.
handleBuildError(reactorContext, session,
step.project, e, true);
return;
}
```
##########
api/maven-api-core/src/main/java/org/apache/maven/api/services/ProjectManager.java:
##########
@@ -59,6 +60,15 @@
*/
@Experimental
public interface ProjectManager extends Service {
+ /**
+ * Replaces the direct dependencies of the given project for the current
build session.
+ * This method does not modify the project's POM on disk.
+ *
+ * @param project the project to update
+ * @param dependencies the new direct dependencies
+ */
+ void setDependencies(@Nonnull Project project, @Nonnull
List<DependencyCoordinates> dependencies);
Review Comment:
💡 **Missing `@since` on new method.** The class is `@since 4.0.0` but this
method is new in 4.1.0. Needs `@since 4.1.0` to match `LifecycleParticipant`
and `ProjectExecutionEvent`.
```suggestion
/**
* Replaces the direct dependencies of the given project for the current
build session.
* This method does not modify the project's POM on disk.
*
* @param project the project to update
* @param dependencies the new direct dependencies
* @since 4.1.0
*/
void setDependencies(@Nonnull Project project, @Nonnull
List<DependencyCoordinates> dependencies);
```
##########
impl/maven-core/src/main/java/org/apache/maven/internal/impl/LifecycleParticipantDispatcher.java:
##########
@@ -0,0 +1,151 @@
+/*
+ * 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.internal.impl;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.Optional;
+
+import org.apache.maven.MavenExecutionException;
+import org.apache.maven.api.MojoExecution;
+import org.apache.maven.api.Project;
+import org.apache.maven.api.SessionData;
+import org.apache.maven.api.spi.LifecycleParticipant;
+import org.apache.maven.api.spi.LifecycleParticipantException;
+import org.apache.maven.api.spi.ProjectExecutionEvent;
+import org.apache.maven.execution.MavenSession;
+import org.apache.maven.lifecycle.LifecycleExecutionException;
+import org.apache.maven.project.MavenProject;
+
+/**
+ * Bridges lifecycle callbacks from the legacy execution engine to the Maven 4
SPI.
+ */
+public final class LifecycleParticipantDispatcher {
+
+ @SuppressWarnings({"rawtypes", "unchecked"})
+ private static final SessionData.Key<List<LifecycleParticipant>>
PARTICIPANTS =
+ (SessionData.Key) SessionData.key(List.class,
LifecycleParticipant.class);
+
+ private LifecycleParticipantDispatcher() {}
+
+ public static void setParticipants(MavenSession session,
Collection<LifecycleParticipant> participants) {
+ session.getSession().getData().set(PARTICIPANTS,
List.copyOf(participants));
+ }
+
+ public static void afterProjectsRead(MavenSession session) throws
MavenExecutionException {
+ try {
+ call(session, participant ->
participant.afterProjectsRead(session.getSession()));
+ } catch (LifecycleParticipantException e) {
+ throw new MavenExecutionException("Lifecycle participant failed
after projects were read", e);
+ }
+ }
+
+ public static void beforeProjectExecution(
+ MavenSession session, MavenProject project,
List<org.apache.maven.plugin.MojoExecution> executionPlan)
+ throws LifecycleExecutionException {
+ if (!hasParticipants(session)) {
+ return;
+ }
+ ProjectExecutionEvent event = event(session, project, executionPlan,
null);
+ try {
+ call(session, participant ->
participant.beforeProjectExecution(event));
+ } catch (LifecycleParticipantException e) {
+ throw new LifecycleExecutionException("Lifecycle participant
failed before project execution", e);
+ }
+ }
+
+ public static void afterProjectExecution(
+ MavenSession session,
+ MavenProject project,
+ List<org.apache.maven.plugin.MojoExecution> executionPlan,
+ Throwable failure)
+ throws LifecycleExecutionException {
+ if (!hasParticipants(session)) {
+ return;
+ }
+ ProjectExecutionEvent event = event(session, project, executionPlan,
failure);
+ try {
+ call(session, participant ->
participant.afterProjectExecution(event));
+ } catch (LifecycleParticipantException e) {
+ throw new LifecycleExecutionException("Lifecycle participant
failed after project execution", e);
+ }
+ }
+
+ public static void afterSessionEnd(MavenSession session) throws
MavenExecutionException {
+ try {
+ call(session, participant ->
participant.afterSessionEnd(session.getSession()));
+ } catch (LifecycleParticipantException e) {
+ throw new MavenExecutionException("Lifecycle participant failed at
session end", e);
+ }
+ }
+
+ private static ProjectExecutionEvent event(
+ MavenSession session,
+ MavenProject project,
+ List<org.apache.maven.plugin.MojoExecution> executionPlan,
+ Throwable failure) {
+ InternalMavenSession apiSession =
InternalMavenSession.from(session.getSession());
+ Project apiProject = apiSession.getProject(project);
+ List<MojoExecution> apiPlan = executionPlan.stream()
+ .map(execution -> (MojoExecution) new
DefaultMojoExecution(apiSession, execution))
+ .toList();
+ return new DefaultProjectExecutionEvent(apiSession, apiProject,
apiPlan, failure);
+ }
+
+ private static void call(MavenSession session, ParticipantCall call)
throws LifecycleParticipantException {
+ List<LifecycleParticipant> participants = participants(session);
+ if (participants == null) {
+ return;
+ }
+ Thread thread = Thread.currentThread();
+ ClassLoader originalClassLoader = thread.getContextClassLoader();
+ try {
+ for (LifecycleParticipant participant : participants) {
+
thread.setContextClassLoader(participant.getClass().getClassLoader());
+ call.run(participant);
+ }
+ } finally {
+ thread.setContextClassLoader(originalClassLoader);
+ }
+ }
+
Review Comment:
⚠️ **No per-participant exception isolation.** If any participant throws
here, the loop exits and all subsequent participants are silently skipped —
losing their `afterProjectExecution` or `afterSessionEnd` callbacks. One bad
extension prevents all others from cleaning up.
Fix: catch each participant's exception, continue to the next participant,
rethrow the first (with subsequent ones as suppressed) after all have run:
```suggestion
LifecycleParticipantException first = null;
for (LifecycleParticipant participant : participants) {
thread.setContextClassLoader(participant.getClass().getClassLoader());
try {
call.run(participant);
} catch (LifecycleParticipantException e) {
if (first == null) {
first = e;
} else {
first.addSuppressed(e);
}
}
}
if (first != null) {
throw first;
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
