Skip to content

Commit 0b34ce5

Browse files
gnodetgnodet
committed
[MNG-8244] Fix before:all and after:all phases
1 parent 8f6458b commit 0b34ce5

File tree

11 files changed

+359
-170
lines changed

11 files changed

+359
-170
lines changed

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

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020

2121
import java.util.Collection;
2222
import java.util.List;
23-
import java.util.Optional;
2423
import java.util.stream.Stream;
2524

2625
import org.apache.maven.api.annotations.Experimental;
@@ -66,10 +65,19 @@ public interface Lifecycle extends ExtensibleEnum {
6665
String id();
6766

6867
/**
69-
* Collection of phases for this lifecycle
68+
* Collection of main phases for this lifecycle
7069
*/
7170
Collection<Phase> phases();
7271

72+
/**
73+
* Collection of main phases for this lifecycle used with the Maven 3 builders.
74+
* Those builders does not operate on a graph, but on the list and expect a slightly
75+
* different ordering (mainly unit test being executed before packaging).
76+
*/
77+
default Collection<Phase> v3phases() {
78+
return phases();
79+
}
80+
7381
/**
7482
* Stream of phases containing all child phases recursively.
7583
*/
@@ -82,14 +90,6 @@ default Stream<Phase> allPhases() {
8290
*/
8391
Collection<Alias> aliases();
8492

85-
/**
86-
* Pre-ordered list of phases.
87-
* If not provided, a default order will be computed.
88-
*/
89-
default Optional<List<String>> orderedPhases() {
90-
return Optional.empty();
91-
}
92-
9393
/**
9494
* A phase in the lifecycle.
9595
*
@@ -101,6 +101,7 @@ interface Phase {
101101
// ======================
102102
// Maven defined phases
103103
// ======================
104+
String ALL = "all";
104105
String BUILD = "build";
105106
String INITIALIZE = "initialize";
106107
String VALIDATE = "validate";

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

Lines changed: 49 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import javax.inject.Singleton;
2525

2626
import java.util.ArrayList;
27-
import java.util.Arrays;
2827
import java.util.Collection;
2928
import java.util.Collections;
3029
import java.util.HashSet;
@@ -50,6 +49,9 @@
5049
import org.codehaus.plexus.PlexusContainer;
5150
import org.codehaus.plexus.component.repository.exception.ComponentLookupException;
5251

52+
import static org.apache.maven.api.Lifecycle.AFTER;
53+
import static org.apache.maven.api.Lifecycle.BEFORE;
54+
import static org.apache.maven.api.Lifecycle.Phase.ALL;
5355
import static org.apache.maven.api.Lifecycle.Phase.BUILD;
5456
import static org.apache.maven.api.Lifecycle.Phase.COMPILE;
5557
import static org.apache.maven.api.Lifecycle.Phase.DEPLOY;
@@ -129,34 +131,20 @@ public Optional<Lifecycle> lookup(String id) {
129131

130132
public List<String> computePhases(Lifecycle lifecycle) {
131133
Graph graph = new Graph();
132-
lifecycle.phases().forEach(phase -> addPhase(graph, null, null, phase));
134+
addPhases(graph, null, null, lifecycle.v3phases());
133135
List<String> allPhases = graph.visitAll();
134136
Collections.reverse(allPhases);
135137
List<String> computed =
136138
allPhases.stream().filter(s -> !s.startsWith("$")).collect(Collectors.toList());
137-
List<String> given = lifecycle.orderedPhases().orElse(null);
138-
if (given != null) {
139-
if (given.size() != computed.size()) {
140-
Set<String> s1 =
141-
given.stream().filter(s -> !computed.contains(s)).collect(Collectors.toSet());
142-
Set<String> s2 =
143-
computed.stream().filter(s -> !given.contains(s)).collect(Collectors.toSet());
144-
throw new IllegalStateException(
145-
"List of phases differ in size: expected " + computed.size() + ", but received " + given.size()
146-
+ (s1.isEmpty() ? "" : ", missing " + s1)
147-
+ (s2.isEmpty() ? "" : ", unexpected " + s2));
148-
}
149-
return given;
150-
}
151139
return computed;
152140
}
153141

154142
private static void addPhase(
155143
Graph graph, Graph.Vertex before, Graph.Vertex after, org.apache.maven.api.Lifecycle.Phase phase) {
156-
Graph.Vertex ep0 = graph.addVertex("$" + phase.name());
144+
Graph.Vertex ep0 = graph.addVertex(BEFORE + phase.name());
157145
Graph.Vertex ep1 = graph.addVertex("$$" + phase.name());
158146
Graph.Vertex ep2 = graph.addVertex(phase.name());
159-
Graph.Vertex ep3 = graph.addVertex("$$$" + phase.name());
147+
Graph.Vertex ep3 = graph.addVertex(AFTER + phase.name());
160148
graph.addEdge(ep0, ep1);
161149
graph.addEdge(ep1, ep2);
162150
graph.addEdge(ep2, ep3);
@@ -171,11 +159,28 @@ private static void addPhase(
171159
if (link.kind() == Lifecycle.Link.Kind.AFTER) {
172160
graph.addEdge(graph.addVertex(link.pointer().phase()), ep0);
173161
} else {
174-
graph.addEdge(ep3, graph.addVertex("$" + link.pointer().phase()));
162+
graph.addEdge(ep3, graph.addVertex(BEFORE + link.pointer().phase()));
175163
}
176164
}
177165
});
178-
phase.phases().forEach(child -> addPhase(graph, ep1, ep2, child));
166+
addPhases(graph, ep1, ep2, phase.phases());
167+
}
168+
169+
private static void addPhases(
170+
Graph graph, Graph.Vertex before, Graph.Vertex after, Collection<Lifecycle.Phase> phases) {
171+
// We add ordering between internal phases.
172+
// This would be wrong at execution time, but we are here computing a list and not a graph,
173+
// so in order to obtain the expected order, we add these links between phases.
174+
Lifecycle.Phase prev = null;
175+
for (Lifecycle.Phase child : phases) {
176+
// add phase
177+
addPhase(graph, before, after, child);
178+
if (prev != null) {
179+
// add link between end of previous phase and beginning of this one
180+
graph.addEdge(graph.addVertex(AFTER + prev.name()), graph.addVertex(BEFORE + child.name()));
181+
}
182+
prev = child;
183+
}
179184
}
180185

181186
@Named
@@ -375,8 +380,8 @@ public String id() {
375380
@Override
376381
public Collection<Phase> phases() {
377382
return List.of(phase(
378-
"all",
379-
phase(INITIALIZE, phase(VALIDATE)),
383+
ALL,
384+
phase(VALIDATE, phase(INITIALIZE)),
380385
phase(
381386
BUILD,
382387
after(VALIDATE),
@@ -407,6 +412,28 @@ public Collection<Phase> phases() {
407412
phase(DEPLOY, after(PACKAGE))));
408413
}
409414

415+
@Override
416+
public Collection<Phase> v3phases() {
417+
return List.of(phase(
418+
ALL,
419+
phase(INITIALIZE, phase(VALIDATE)),
420+
phase(
421+
BUILD,
422+
phase(SOURCES),
423+
phase(RESOURCES),
424+
phase(COMPILE),
425+
phase(READY),
426+
phase(TEST_SOURCES),
427+
phase(TEST_RESOURCES),
428+
phase(TEST_COMPILE),
429+
phase(TEST),
430+
phase(UNIT_TEST),
431+
phase(PACKAGE)),
432+
phase(VERIFY, phase(INTEGRATION_TEST)),
433+
phase(INSTALL),
434+
phase(DEPLOY)));
435+
}
436+
410437
@Override
411438
public Collection<Alias> aliases() {
412439
return List.of(
@@ -424,42 +451,6 @@ public Collection<Alias> aliases() {
424451
alias("pre-integration-test", BEFORE + INTEGRATION_TEST),
425452
alias("post-integration-test", AFTER + INTEGRATION_TEST));
426453
}
427-
428-
@Override
429-
public Optional<List<String>> orderedPhases() {
430-
return Optional.of(Arrays.asList(
431-
VALIDATE,
432-
INITIALIZE,
433-
// "generate-sources",
434-
SOURCES,
435-
// "process-sources",
436-
// "generate-resources",
437-
RESOURCES,
438-
// "process-resources",
439-
COMPILE,
440-
// "process-classes",
441-
READY,
442-
// "generate-test-sources",
443-
TEST_SOURCES,
444-
// "process-test-sources",
445-
// "generate-test-resources",
446-
TEST_RESOURCES,
447-
// "process-test-resources",
448-
TEST_COMPILE,
449-
// "process-test-classes",
450-
TEST,
451-
UNIT_TEST,
452-
// "prepare-package",
453-
PACKAGE,
454-
BUILD,
455-
// "pre-integration-test",
456-
INTEGRATION_TEST,
457-
// "post-integration-test",
458-
VERIFY,
459-
INSTALL,
460-
DEPLOY,
461-
"all"));
462-
}
463454
}
464455

465456
static class SiteLifecycle implements Lifecycle {

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

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,9 @@
2323
import java.util.List;
2424
import java.util.Map;
2525
import java.util.stream.Collectors;
26-
import java.util.stream.Stream;
2726

2827
import org.apache.maven.lifecycle.mapping.LifecyclePhase;
2928

30-
import static org.apache.maven.api.Lifecycle.AFTER;
31-
import static org.apache.maven.api.Lifecycle.BEFORE;
32-
3329
/**
3430
* Lifecycle definition, with eventual plugin bindings (when they are not packaging-specific).
3531
*/
@@ -46,9 +42,7 @@ public Lifecycle(
4642
org.apache.maven.api.services.LifecycleRegistry registry, org.apache.maven.api.Lifecycle lifecycle) {
4743
this.lifecycle = lifecycle;
4844
this.id = lifecycle.id();
49-
this.phases = registry.computePhases(lifecycle).stream()
50-
.flatMap(p -> Stream.of(BEFORE + p, p, AFTER + p))
51-
.toList();
45+
this.phases = registry.computePhases(lifecycle);
5246
this.defaultPhases = getDefaultPhases(lifecycle);
5347
}
5448

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

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525

2626
import java.io.IOException;
2727
import java.util.ArrayList;
28-
import java.util.Arrays;
2928
import java.util.Collection;
3029
import java.util.Collections;
3130
import java.util.HashMap;
@@ -264,13 +263,10 @@ private Map<String, List<MojoExecution>> calculateLifecycleMappings(
264263
}
265264

266265
LifecycleMappingDelegate delegate;
267-
if (Arrays.binarySearch(DefaultLifecycles.STANDARD_LIFECYCLES, lifecycle.getId()) >= 0) {
266+
if (List.of(DefaultLifecycles.STANDARD_LIFECYCLES).contains(lifecycle.getId())) {
268267
delegate = standardDelegate;
269268
} else {
270-
delegate = delegates.get(lifecycle.getId());
271-
if (delegate == null) {
272-
delegate = standardDelegate;
273-
}
269+
delegate = delegates.getOrDefault(lifecycle.getId(), standardDelegate);
274270
}
275271

276272
return delegate.calculateLifecycleMappings(session, project, lifecycle, lifecyclePhase);

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

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -82,14 +82,23 @@ public Map<String, List<MojoExecution>> calculateLifecycleMappings(
8282
lifecyclePhase = PhaseId.of(aliases.get(lifecyclePhase)).phase();
8383
}
8484

85+
boolean passed = false;
8586
for (String phase : lifecycle.getPhases()) {
86-
Map<PhaseId, List<MojoExecution>> phaseBindings =
87-
new TreeMap<>(Comparator.comparing(PhaseId::toString, new PhaseComparator(lifecycle.getPhases())));
88-
89-
mappings.put(phase, phaseBindings);
90-
87+
boolean include = true;
9188
if (phase.equals(lifecyclePhase)) {
92-
break;
89+
passed = true;
90+
} else if (passed) {
91+
if (phase.startsWith(org.apache.maven.api.Lifecycle.AFTER)) {
92+
String realPhase = phase.substring(org.apache.maven.api.Lifecycle.AFTER.length());
93+
include = mappings.containsKey(org.apache.maven.api.Lifecycle.BEFORE + realPhase);
94+
} else {
95+
include = false;
96+
}
97+
}
98+
if (include) {
99+
Map<PhaseId, List<MojoExecution>> phaseBindings = new TreeMap<>(
100+
Comparator.comparing(PhaseId::toString, new PhaseComparator(lifecycle.getPhases())));
101+
mappings.put(phase, phaseBindings);
93102
}
94103
}
95104

@@ -163,7 +172,7 @@ private Map<PhaseId, List<MojoExecution>> getPhaseBindings(
163172
Map<String, Map<PhaseId, List<MojoExecution>>> mappings, String phase) {
164173
if (phase != null) {
165174
PhaseId id = PhaseId.of(phase);
166-
return mappings.get(id.phase());
175+
return mappings.get(id.executionPoint().prefix() + id.phase());
167176
}
168177
return null;
169178
}

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

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,8 @@ public PhaseComparator(List<String> lifecyclePhases) {
4343
public int compare(String o1, String o2) {
4444
PhaseId p1 = PhaseId.of(o1);
4545
PhaseId p2 = PhaseId.of(o2);
46-
int i1 = lifecyclePhases.indexOf(p1.phase());
47-
int i2 = lifecyclePhases.indexOf(p2.phase());
46+
int i1 = lifecyclePhases.indexOf(p1.executionPoint().prefix() + p1.phase());
47+
int i2 = lifecyclePhases.indexOf(p2.executionPoint().prefix() + p2.phase());
4848
if (i1 == -1 && i2 == -1) {
4949
// unknown phases, leave in existing order
5050
return 0;
@@ -61,13 +61,6 @@ public int compare(String o1, String o2) {
6161
if (rv != 0) {
6262
return rv;
6363
}
64-
// same phase, now compare execution points
65-
i1 = p1.executionPoint().ordinal();
66-
i2 = p2.executionPoint().ordinal();
67-
rv = Integer.compare(i1, i2);
68-
if (rv != 0) {
69-
return rv;
70-
}
7164
// same execution point, now compare priorities
7265
return Integer.compare(p1.priority(), p2.priority());
7366
}

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -683,9 +683,7 @@ public BuildPlan calculateLifecycleMappings(
683683
+ " or a goal in the format <plugin-prefix>:<goal> or"
684684
+ " <plugin-group-id>:<plugin-artifact-id>[:<plugin-version>]:<goal>. Available lifecycle phases are: "
685685
+ lifecycles.stream()
686-
.flatMap(l -> l.orderedPhases()
687-
.map(List::stream)
688-
.orElseGet(() -> l.allPhases().map(Lifecycle.Phase::name)))
686+
.flatMap(l -> l.allPhases().map(Lifecycle.Phase::name))
689687
.collect(Collectors.joining(", "))
690688
+ ".",
691689
lifecyclePhase));
@@ -738,6 +736,10 @@ public BuildPlan calculateLifecycleMappings(
738736
? lifecyclePhase.substring(AT.length())
739737
: AFTER + lifecyclePhase;
740738
Set<BuildStep> toKeep = steps.get(endPhase).allPredecessors().collect(Collectors.toSet());
739+
toKeep.addAll(toKeep.stream()
740+
.filter(s -> s.name.startsWith(BEFORE))
741+
.map(s -> steps.get(AFTER + s.name.substring(BEFORE.length())))
742+
.toList());
741743
steps.values().stream().filter(n -> !toKeep.contains(n)).forEach(BuildStep::skip);
742744

743745
plan.addProject(project, steps);

0 commit comments

Comments
 (0)