Skip to content

Commit 35e10b8

Browse files
gnodetgnodet
authored
[MNG-8653] Fix 'all' phase and add 'each' phase (#2191)
Fixes the existing 'all' phase and introduces a new 'each' phase to better support hierarchical builds and concurrent execution: - 'all': Properly coordinates execution across parent-child project hierarchies - 'each': A new phase that executes for each individual project in isolation Key changes: - Enhanced BuildPlanExecutor to properly handle parent-child relationships in phases - Added new children() pointer type for explicit phase dependencies - Improved phase dependency management to ensure: - Parent's before:all executes before children's phases - Children's after:all executes before parent's after:all - Each project's phases execute in isolation within the 'each' phase - Added comprehensive documentation to BuildPlanExecutor - Added integration test to verify hierarchical phase execution This commit also fixes thread-safety issues in concurrent builds: - Added synchronization to ReactorReader for project operations - Improved thread-safety for project-level clean and install operations - Enhanced build step state management in concurrent scenarios Fixes: MNG-8653
1 parent 7cfbdc5 commit 35e10b8

File tree

12 files changed

+383
-61
lines changed

12 files changed

+383
-61
lines changed

‎api/maven-api-core/src/main/java/org/apache/maven/api/Lifecycle.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ interface Phase {
102102
// Maven defined phases
103103
// ======================
104104
String ALL = "all";
105+
String EACH = "each";
105106
String BUILD = "build";
106107
String INITIALIZE = "initialize";
107108
String VALIDATE = "validate";

‎impl/maven-core/src/main/java/org/apache/maven/ReactorReader.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -338,14 +338,18 @@ private void processEvent(ExecutionEvent event) {
338338
if (!Objects.equals(phase, phases.peekLast())) {
339339
phases.addLast(phase);
340340
if ("clean".equals(phase)) {
341-
cleanProjectLocalRepository(project);
341+
synchronized (project) {
342+
cleanProjectLocalRepository(project);
343+
}
342344
}
343345
}
344346
}
345347
break;
346348
case ProjectSucceeded:
347349
case ForkedProjectSucceeded:
348-
installIntoProjectLocalRepository(project);
350+
synchronized (project) {
351+
installIntoProjectLocalRepository(project);
352+
}
349353
break;
350354
default:
351355
break;

‎impl/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultLifecycleRegistry.java‎

Lines changed: 37 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import java.util.stream.Collectors;
3838
import java.util.stream.Stream;
3939

40+
import org.apache.maven.api.DependencyScope;
4041
import org.apache.maven.api.Lifecycle;
4142
import org.apache.maven.api.model.InputLocation;
4243
import org.apache.maven.api.model.InputSource;
@@ -55,6 +56,7 @@
5556
import static org.apache.maven.api.Lifecycle.Phase.BUILD;
5657
import static org.apache.maven.api.Lifecycle.Phase.COMPILE;
5758
import static org.apache.maven.api.Lifecycle.Phase.DEPLOY;
59+
import static org.apache.maven.api.Lifecycle.Phase.EACH;
5860
import static org.apache.maven.api.Lifecycle.Phase.INITIALIZE;
5961
import static org.apache.maven.api.Lifecycle.Phase.INSTALL;
6062
import static org.apache.maven.api.Lifecycle.Phase.INTEGRATION_TEST;
@@ -71,6 +73,7 @@
7173
import static org.apache.maven.api.Lifecycle.Phase.VERIFY;
7274
import static org.apache.maven.internal.impl.Lifecycles.after;
7375
import static org.apache.maven.internal.impl.Lifecycles.alias;
76+
import static org.apache.maven.internal.impl.Lifecycles.children;
7477
import static org.apache.maven.internal.impl.Lifecycles.dependencies;
7578
import static org.apache.maven.internal.impl.Lifecycles.phase;
7679
import static org.apache.maven.internal.impl.Lifecycles.plugin;
@@ -91,6 +94,11 @@ public class DefaultLifecycleRegistry implements LifecycleRegistry {
9194
public static final InputLocation DEFAULT_LIFECYCLE_INPUT_LOCATION =
9295
new InputLocation(new InputSource(DEFAULT_LIFECYCLE_MODELID, null));
9396

97+
public static final String SCOPE_COMPILE = DependencyScope.COMPILE.id();
98+
public static final String SCOPE_RUNTIME = DependencyScope.RUNTIME.id();
99+
public static final String SCOPE_TEST_ONLY = DependencyScope.TEST_ONLY.id();
100+
public static final String SCOPE_TEST = DependencyScope.TEST.id();
101+
94102
private final List<LifecycleProvider> providers;
95103

96104
public DefaultLifecycleRegistry() {
@@ -384,35 +392,38 @@ public Collection<Phase> phases() {
384392
// START SNIPPET: default
385393
return List.of(phase(
386394
ALL,
387-
phase(VALIDATE, phase(INITIALIZE)),
395+
children(ALL),
388396
phase(
389-
BUILD,
390-
after(VALIDATE),
391-
phase(SOURCES),
392-
phase(RESOURCES),
393-
phase(COMPILE, after(SOURCES), dependencies(COMPILE, READY)),
394-
phase(READY, after(COMPILE), after(RESOURCES)),
395-
phase(PACKAGE, after(READY), dependencies("runtime", PACKAGE))),
396-
phase(
397-
VERIFY,
398-
after(VALIDATE),
397+
EACH,
398+
phase(VALIDATE, phase(INITIALIZE)),
399399
phase(
400-
UNIT_TEST,
401-
phase(TEST_SOURCES),
402-
phase(TEST_RESOURCES),
403-
phase(
404-
TEST_COMPILE,
405-
after(TEST_SOURCES),
406-
after(READY),
407-
dependencies("test-only", READY)),
400+
BUILD,
401+
after(VALIDATE),
402+
phase(SOURCES),
403+
phase(RESOURCES),
404+
phase(COMPILE, after(SOURCES), dependencies(SCOPE_COMPILE, READY)),
405+
phase(READY, after(COMPILE), after(RESOURCES)),
406+
phase(PACKAGE, after(READY), dependencies(SCOPE_RUNTIME, PACKAGE))),
407+
phase(
408+
VERIFY,
409+
after(VALIDATE),
408410
phase(
409-
TEST,
410-
after(TEST_COMPILE),
411-
after(TEST_RESOURCES),
412-
dependencies("test", READY))),
413-
phase(INTEGRATION_TEST)),
414-
phase(INSTALL, after(PACKAGE)),
415-
phase(DEPLOY, after(PACKAGE))));
411+
UNIT_TEST,
412+
phase(TEST_SOURCES),
413+
phase(TEST_RESOURCES),
414+
phase(
415+
TEST_COMPILE,
416+
after(TEST_SOURCES),
417+
after(READY),
418+
dependencies(SCOPE_TEST_ONLY, READY)),
419+
phase(
420+
TEST,
421+
after(TEST_COMPILE),
422+
after(TEST_RESOURCES),
423+
dependencies(SCOPE_TEST, READY))),
424+
phase(INTEGRATION_TEST)),
425+
phase(INSTALL, after(PACKAGE)),
426+
phase(DEPLOY, after(PACKAGE)))));
416427
// END SNIPPET: default
417428
}
418429

‎impl/maven-core/src/main/java/org/apache/maven/internal/impl/Lifecycles.java‎

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ static Plugin plugin(String coords, String phase) {
8989
}
9090

9191
/** Indicates the phase is after the phases given in arguments */
92-
static Lifecycle.Link after(String b) {
92+
static Lifecycle.Link after(String phase) {
9393
return new Lifecycle.Link() {
9494
@Override
9595
public Kind kind() {
@@ -101,10 +101,20 @@ public Lifecycle.Pointer pointer() {
101101
return new Lifecycle.PhasePointer() {
102102
@Override
103103
public String phase() {
104-
return b;
104+
return phase;
105+
}
106+
107+
@Override
108+
public String toString() {
109+
return "phase(" + phase + ")";
105110
}
106111
};
107112
}
113+
114+
@Override
115+
public String toString() {
116+
return "after(" + pointer() + ")";
117+
}
108118
};
109119
}
110120

@@ -128,8 +138,47 @@ public String phase() {
128138
public String scope() {
129139
return scope;
130140
}
141+
142+
@Override
143+
public String toString() {
144+
return "dependencies(" + scope + ", " + phase + ")";
145+
}
146+
};
147+
}
148+
149+
@Override
150+
public String toString() {
151+
return "after(" + pointer() + ")";
152+
}
153+
};
154+
}
155+
156+
static Lifecycle.Link children(String phase) {
157+
return new Lifecycle.Link() {
158+
@Override
159+
public Kind kind() {
160+
return Kind.AFTER;
161+
}
162+
163+
@Override
164+
public Lifecycle.Pointer pointer() {
165+
return new Lifecycle.ChildrenPointer() {
166+
@Override
167+
public String phase() {
168+
return phase;
169+
}
170+
171+
@Override
172+
public String toString() {
173+
return "children(" + phase + ")";
174+
}
131175
};
132176
}
177+
178+
@Override
179+
public String toString() {
180+
return "after(" + pointer() + ")";
181+
}
133182
};
134183
}
135184

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

Lines changed: 85 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -103,17 +103,46 @@
103103
import static org.apache.maven.lifecycle.internal.concurrent.BuildStep.TEARDOWN;
104104

105105
/**
106-
* Builds the full lifecycle in weave-mode (phase by phase as opposed to project-by-project).
107-
* <p>
108-
* This builder uses a number of threads equal to the minimum of the degree of concurrency (which is the thread count
109-
* set with <code>-T</code> on the command-line) and the number of projects to build. As such, building a single project
110-
* will always result in a sequential build, regardless of the thread count.
111-
* </p>
112-
* <strong>NOTE:</strong> This class is not part of any public api and can be changed or deleted without prior notice.
106+
* Executes the Maven build plan in a concurrent manner, handling the lifecycle phases and plugin executions.
107+
* This executor implements a weave-mode build strategy, where builds are executed phase-by-phase rather than
108+
* project-by-project.
109+
*
110+
* <h2>Key Features:</h2>
111+
* <ul>
112+
* <li>Concurrent execution of compatible build steps across projects</li>
113+
* <li>Thread-safety validation for plugins</li>
114+
* <li>Support for forked executions and lifecycle phases</li>
115+
* <li>Dynamic build plan adjustment during execution</li>
116+
* </ul>
117+
*
118+
* <h2>Execution Strategy:</h2>
119+
* <p>The executor follows these main steps:</p>
120+
* <ol>
121+
* <li>Initial plan creation based on project dependencies and task segments</li>
122+
* <li>Concurrent execution of build steps while maintaining dependency order</li>
123+
* <li>Dynamic replanning when necessary (e.g., for forked executions)</li>
124+
* <li>Project setup, execution, and teardown phases management</li>
125+
* </ol>
126+
*
127+
* <h2>Thread Management:</h2>
128+
* <p>The number of threads used is determined by:</p>
129+
* <pre>
130+
* min(degreeOfConcurrency, numberOfProjects)
131+
* </pre>
132+
* where degreeOfConcurrency is set via the -T command-line option.
133+
*
134+
* <h2>Build Step States:</h2>
135+
* <ul>
136+
* <li>CREATED: Initial state of a build step</li>
137+
* <li>PLANNING: Step is being planned</li>
138+
* <li>SCHEDULED: Step is queued for execution</li>
139+
* <li>EXECUTED: Step has completed successfully</li>
140+
* <li>FAILED: Step execution failed</li>
141+
* </ul>
142+
*
143+
* <p><strong>NOTE:</strong> This class is not part of any public API and can be changed or deleted without prior notice.</p>
113144
*
114145
* @since 3.0
115-
* Builds one or more lifecycles for a full module
116-
* NOTE: This class is not part of any public api and can be changed or deleted without prior notice.
117146
*/
118147
@Named
119148
public class BuildPlanExecutor {
@@ -225,6 +254,7 @@ public BuildPlan buildInitialPlan(List<TaskSegment> taskSegments) {
225254
pplan.status.set(PLANNING); // the plan step always need planning
226255
BuildStep setup = new BuildStep(SETUP, project, null);
227256
BuildStep teardown = new BuildStep(TEARDOWN, project, null);
257+
teardown.executeAfter(setup);
228258
setup.executeAfter(pplan);
229259
plan.steps(project).forEach(step -> {
230260
if (step.predecessors.isEmpty()) {
@@ -322,6 +352,10 @@ private void executePlan() {
322352
global.start();
323353
lock.readLock().lock();
324354
try {
355+
// Get all build steps that are:
356+
// 1. Not yet started (CREATED status)
357+
// 2. Have all their prerequisites completed (predecessors EXECUTED)
358+
// 3. Successfully transition from CREATED to SCHEDULED state
325359
plan.sortedNodes().stream()
326360
.filter(step -> step.status.get() == CREATED)
327361
.filter(step -> step.predecessors.stream().allMatch(s -> s.status.get() == EXECUTED))
@@ -356,6 +390,17 @@ private void executePlan() {
356390
}
357391
}
358392

393+
/**
394+
* Executes all pending after:* phases for a failed project.
395+
* This ensures proper cleanup is performed even when a build fails.
396+
* Only executes after:xxx phases if their corresponding before:xxx phase
397+
* has been either executed or failed.
398+
*
399+
* For example, if a project fails during 'compile', this will execute
400+
* any configured 'after:compile' phases to ensure proper cleanup.
401+
*
402+
* @param failedStep The build step that failed, containing the project that needs cleanup
403+
*/
359404
private void executeAfterPhases(BuildStep failedStep) {
360405
if (failedStep == null || failedStep.project == null) {
361406
return;
@@ -393,6 +438,17 @@ private void executeAfterPhases(BuildStep failedStep) {
393438
}
394439
}
395440

441+
/**
442+
* Executes a single build step, which can be one of:
443+
* - PLAN: Project build planning
444+
* - SETUP: Project initialization
445+
* - TEARDOWN: Project cleanup
446+
* - Default: Actual mojo/plugin executions
447+
*
448+
* @param step The build step to execute
449+
* @throws IOException If there's an IO error during execution
450+
* @throws LifecycleExecutionException If there's a lifecycle execution error
451+
*/
396452
private void executeStep(BuildStep step) throws IOException, LifecycleExecutionException {
397453
Clock clock = getClock(step.project);
398454
switch (step.name) {
@@ -796,16 +852,27 @@ public BuildPlan calculateLifecycleMappings(
796852
plan.allSteps().filter(step -> step.phase != null).forEach(step -> {
797853
Lifecycle.Phase phase = step.phase;
798854
MavenProject project = step.project;
799-
phase.links().stream()
800-
.filter(l -> l.pointer().type() != Lifecycle.Pointer.Type.PROJECT)
801-
.forEach(link -> {
802-
String n1 = phase.name();
803-
String n2 = link.pointer().phase();
804-
// for each project, if the phase in the build, link after it
805-
getLinkedProjects(projects, project, link).forEach(p -> plan.step(p, AFTER + n2)
806-
.ifPresent(a -> plan.requiredStep(project, BEFORE + n1)
807-
.executeAfter(a)));
855+
phase.links().stream().forEach(link -> {
856+
BuildStep before = plan.requiredStep(project, BEFORE + phase.name());
857+
BuildStep after = plan.requiredStep(project, AFTER + phase.name());
858+
Lifecycle.Pointer pointer = link.pointer();
859+
String n2 = pointer.phase();
860+
if (pointer instanceof Lifecycle.DependenciesPointer) {
861+
// For dependencies: ensure current project's phase starts after dependency's phase completes
862+
// Example: project's compile starts after dependency's package completes
863+
// TODO: String scope = ((Lifecycle.DependenciesPointer) pointer).scope();
864+
projects.get(project)
865+
.forEach(p -> plan.step(p, AFTER + n2).ifPresent(before::executeAfter));
866+
} else if (pointer instanceof Lifecycle.ChildrenPointer) {
867+
// For children: ensure bidirectional phase coordination
868+
project.getCollectedProjects().forEach(p -> {
869+
// 1. Child's phase start waits for parent's phase start
870+
plan.step(p, BEFORE + n2).ifPresent(before::executeBefore);
871+
// 2. Parent's phase completion waits for child's phase completion
872+
plan.step(p, AFTER + n2).ifPresent(after::executeAfter);
808873
});
874+
}
875+
});
809876
});
810877

811878
// Keep projects in reactors by GAV
@@ -832,19 +899,6 @@ public BuildPlan calculateLifecycleMappings(
832899

833900
return plan;
834901
}
835-
836-
private List<MavenProject> getLinkedProjects(
837-
Map<MavenProject, List<MavenProject>> projects, MavenProject project, Lifecycle.Link link) {
838-
if (link.pointer().type() == Lifecycle.Pointer.Type.DEPENDENCIES) {
839-
// TODO: String scope = ((Lifecycle.DependenciesPointer) link.pointer()).scope();
840-
return projects.get(project);
841-
} else if (link.pointer().type() == Lifecycle.Pointer.Type.CHILDREN) {
842-
return project.getCollectedProjects();
843-
} else {
844-
throw new IllegalArgumentException(
845-
"Unsupported pointer type: " + link.pointer().type());
846-
}
847-
}
848902
}
849903

850904
private void resolvePlugin(MavenSession session, List<RemoteRepository> repositories, Plugin plugin) {

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,10 @@ public void executeAfter(BuildStep stepToExecuteBefore) {
103103
}
104104
}
105105

106+
public void executeBefore(BuildStep stepToExecuteAfter) {
107+
stepToExecuteAfter.executeAfter(this);
108+
}
109+
106110
public Stream<MojoExecution> executions() {
107111
return mojos.values().stream().flatMap(m -> m.values().stream());
108112
}

0 commit comments

Comments
 (0)