Skip to content

Commit

Permalink
[MNG-8244] Fix before:all and after:all phases
Browse files Browse the repository at this point in the history
  • Loading branch information
@gnodet
gnodet committed Dec 13, 2024
1 parent 8f6458b commit 63ca953
Show file tree
Hide file tree
Showing 11 changed files with 323 additions and 170 deletions.
21 changes: 11 additions & 10 deletions api/maven-api-core/src/main/java/org/apache/maven/api/Lifecycle.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@

import java.util.Collection;
import java.util.List;
import java.util.Optional;
import java.util.stream.Stream;

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

/**
* Collection of phases for this lifecycle
* Collection of main phases for this lifecycle
*/
Collection<Phase> phases();

/**
* Collection of main phases for this lifecycle used with the Maven 3 builders.
* Those builders does not operate on a graph, but on the list and expect a slightly
* different ordering (mainly unit test being executed before packaging).
*/
default Collection<Phase> v3phases() {
return phases();
}

/**
* Stream of phases containing all child phases recursively.
*/
Expand All @@ -82,14 +90,6 @@ default Stream<Phase> allPhases() {
*/
Collection<Alias> aliases();

/**
* Pre-ordered list of phases.
* If not provided, a default order will be computed.
*/
default Optional<List<String>> orderedPhases() {
return Optional.empty();
}

/**
* A phase in the lifecycle.
*
Expand All @@ -101,6 +101,7 @@ interface Phase {
// ======================
// Maven defined phases
// ======================
String ALL = "all";
String BUILD = "build";
String INITIALIZE = "initialize";
String VALIDATE = "validate";
Expand Down
107 changes: 49 additions & 58 deletions impl/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultLifecycleRegistry.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import javax.inject.Singleton;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
Expand All @@ -50,6 +49,9 @@
import org.codehaus.plexus.PlexusContainer;
import org.codehaus.plexus.component.repository.exception.ComponentLookupException;

import static org.apache.maven.api.Lifecycle.AFTER;
import static org.apache.maven.api.Lifecycle.BEFORE;
import static org.apache.maven.api.Lifecycle.Phase.ALL;
import static org.apache.maven.api.Lifecycle.Phase.BUILD;
import static org.apache.maven.api.Lifecycle.Phase.COMPILE;
import static org.apache.maven.api.Lifecycle.Phase.DEPLOY;
Expand Down Expand Up @@ -129,34 +131,20 @@ public Optional<Lifecycle> lookup(String id) {

public List<String> computePhases(Lifecycle lifecycle) {
Graph graph = new Graph();
lifecycle.phases().forEach(phase -> addPhase(graph, null, null, phase));
addPhases(graph, null, null, lifecycle.v3phases());
List<String> allPhases = graph.visitAll();
Collections.reverse(allPhases);
List<String> computed =
allPhases.stream().filter(s -> !s.startsWith("$")).collect(Collectors.toList());
List<String> given = lifecycle.orderedPhases().orElse(null);
if (given != null) {
if (given.size() != computed.size()) {
Set<String> s1 =
given.stream().filter(s -> !computed.contains(s)).collect(Collectors.toSet());
Set<String> s2 =
computed.stream().filter(s -> !given.contains(s)).collect(Collectors.toSet());
throw new IllegalStateException(
"List of phases differ in size: expected " + computed.size() + ", but received " + given.size()
+ (s1.isEmpty() ? "" : ", missing " + s1)
+ (s2.isEmpty() ? "" : ", unexpected " + s2));
}
return given;
}
return computed;
}

private static void addPhase(
Graph graph, Graph.Vertex before, Graph.Vertex after, org.apache.maven.api.Lifecycle.Phase phase) {
Graph.Vertex ep0 = graph.addVertex("$" + phase.name());
Graph.Vertex ep0 = graph.addVertex(BEFORE + phase.name());
Graph.Vertex ep1 = graph.addVertex("$$" + phase.name());
Graph.Vertex ep2 = graph.addVertex(phase.name());
Graph.Vertex ep3 = graph.addVertex("$$$" + phase.name());
Graph.Vertex ep3 = graph.addVertex(AFTER + phase.name());
graph.addEdge(ep0, ep1);
graph.addEdge(ep1, ep2);
graph.addEdge(ep2, ep3);
Expand All @@ -171,11 +159,28 @@ private static void addPhase(
if (link.kind() == Lifecycle.Link.Kind.AFTER) {
graph.addEdge(graph.addVertex(link.pointer().phase()), ep0);
} else {
graph.addEdge(ep3, graph.addVertex("$" + link.pointer().phase()));
graph.addEdge(ep3, graph.addVertex(BEFORE + link.pointer().phase()));
}
}
});
phase.phases().forEach(child -> addPhase(graph, ep1, ep2, child));
addPhases(graph, ep1, ep2, phase.phases());
}

private static void addPhases(
Graph graph, Graph.Vertex before, Graph.Vertex after, Collection<Lifecycle.Phase> phases) {
// We add ordering between internal phases.
// This would be wrong at execution time, but we are here computing a list and not a graph,
// so in order to obtain the expected order, we add these links between phases.
Lifecycle.Phase prev = null;
for (Lifecycle.Phase child : phases) {
// add phase
addPhase(graph, before, after, child);
if (prev != null) {
// add link between end of previous phase and beginning of this one
graph.addEdge(graph.addVertex(AFTER + prev.name()), graph.addVertex(BEFORE + child.name()));
}
prev = child;
}
}

@Named
Expand Down Expand Up @@ -375,8 +380,8 @@ public String id() {
@Override
public Collection<Phase> phases() {
return List.of(phase(
"all",
phase(INITIALIZE, phase(VALIDATE)),
ALL,
phase(VALIDATE, phase(INITIALIZE)),
phase(
BUILD,
after(VALIDATE),
Expand Down Expand Up @@ -407,6 +412,28 @@ public Collection<Phase> phases() {
phase(DEPLOY, after(PACKAGE))));
}

@Override
public Collection<Phase> v3phases() {
return List.of(phase(
ALL,
phase(INITIALIZE, phase(VALIDATE)),
phase(
BUILD,
phase(SOURCES),
phase(RESOURCES),
phase(COMPILE),
phase(READY),
phase(TEST_SOURCES),
phase(TEST_RESOURCES),
phase(TEST_COMPILE),
phase(TEST),
phase(UNIT_TEST),
phase(PACKAGE)),
phase(VERIFY, phase(INTEGRATION_TEST)),
phase(INSTALL),
phase(DEPLOY)));
}

@Override
public Collection<Alias> aliases() {
return List.of(
Expand All @@ -424,42 +451,6 @@ public Collection<Alias> aliases() {
alias("pre-integration-test", BEFORE + INTEGRATION_TEST),
alias("post-integration-test", AFTER + INTEGRATION_TEST));
}

@Override
public Optional<List<String>> orderedPhases() {
return Optional.of(Arrays.asList(
VALIDATE,
INITIALIZE,
// "generate-sources",
SOURCES,
// "process-sources",
// "generate-resources",
RESOURCES,
// "process-resources",
COMPILE,
// "process-classes",
READY,
// "generate-test-sources",
TEST_SOURCES,
// "process-test-sources",
// "generate-test-resources",
TEST_RESOURCES,
// "process-test-resources",
TEST_COMPILE,
// "process-test-classes",
TEST,
UNIT_TEST,
// "prepare-package",
PACKAGE,
BUILD,
// "pre-integration-test",
INTEGRATION_TEST,
// "post-integration-test",
VERIFY,
INSTALL,
DEPLOY,
"all"));
}
}

static class SiteLifecycle implements Lifecycle {
Expand Down
8 changes: 1 addition & 7 deletions impl/maven-core/src/main/java/org/apache/maven/lifecycle/Lifecycle.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,9 @@
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
import java.util.stream.Stream;

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

import static org.apache.maven.api.Lifecycle.AFTER;
import static org.apache.maven.api.Lifecycle.BEFORE;

/**
* Lifecycle definition, with eventual plugin bindings (when they are not packaging-specific).
*/
Expand All @@ -46,9 +42,7 @@ public Lifecycle(
org.apache.maven.api.services.LifecycleRegistry registry, org.apache.maven.api.Lifecycle lifecycle) {
this.lifecycle = lifecycle;
this.id = lifecycle.id();
this.phases = registry.computePhases(lifecycle).stream()
.flatMap(p -> Stream.of(BEFORE + p, p, AFTER + p))
.toList();
this.phases = registry.computePhases(lifecycle);
this.defaultPhases = getDefaultPhases(lifecycle);
}

Expand Down
8 changes: 2 additions & 6 deletions ...ain/java/org/apache/maven/lifecycle/internal/DefaultLifecycleExecutionPlanCalculator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@

import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
Expand Down Expand Up @@ -264,13 +263,10 @@ private Map<String, List<MojoExecution>> calculateLifecycleMappings(
}

LifecycleMappingDelegate delegate;
if (Arrays.binarySearch(DefaultLifecycles.STANDARD_LIFECYCLES, lifecycle.getId()) >= 0) {
if (List.of(DefaultLifecycles.STANDARD_LIFECYCLES).contains(lifecycle.getId())) {
delegate = standardDelegate;
} else {
delegate = delegates.get(lifecycle.getId());
if (delegate == null) {
delegate = standardDelegate;
}
delegate = delegates.getOrDefault(lifecycle.getId(), standardDelegate);
}

return delegate.calculateLifecycleMappings(session, project, lifecycle, lifecyclePhase);
Expand Down
23 changes: 16 additions & 7 deletions ...re/src/main/java/org/apache/maven/lifecycle/internal/DefaultLifecycleMappingDelegate.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,14 +82,23 @@ public Map<String, List<MojoExecution>> calculateLifecycleMappings(
lifecyclePhase = PhaseId.of(aliases.get(lifecyclePhase)).phase();
}

boolean passed = false;
for (String phase : lifecycle.getPhases()) {
Map<PhaseId, List<MojoExecution>> phaseBindings =
new TreeMap<>(Comparator.comparing(PhaseId::toString, new PhaseComparator(lifecycle.getPhases())));

mappings.put(phase, phaseBindings);

boolean include = true;
if (phase.equals(lifecyclePhase)) {
break;
passed = true;
} else if (passed) {
if (phase.startsWith(org.apache.maven.api.Lifecycle.AFTER)) {
String realPhase = phase.substring(org.apache.maven.api.Lifecycle.AFTER.length());
include = mappings.containsKey(org.apache.maven.api.Lifecycle.BEFORE + realPhase);
} else {
include = false;
}
}
if (include) {
Map<PhaseId, List<MojoExecution>> phaseBindings = new TreeMap<>(
Comparator.comparing(PhaseId::toString, new PhaseComparator(lifecycle.getPhases())));
mappings.put(phase, phaseBindings);
}
}

Expand Down Expand Up @@ -163,7 +172,7 @@ private Map<PhaseId, List<MojoExecution>> getPhaseBindings(
Map<String, Map<PhaseId, List<MojoExecution>>> mappings, String phase) {
if (phase != null) {
PhaseId id = PhaseId.of(phase);
return mappings.get(id.phase());
return mappings.get(id.executionPoint().prefix() + id.phase());
}
return null;
}
Expand Down
11 changes: 2 additions & 9 deletions impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/PhaseComparator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ public PhaseComparator(List<String> lifecyclePhases) {
public int compare(String o1, String o2) {
PhaseId p1 = PhaseId.of(o1);
PhaseId p2 = PhaseId.of(o2);
int i1 = lifecyclePhases.indexOf(p1.phase());
int i2 = lifecyclePhases.indexOf(p2.phase());
int i1 = lifecyclePhases.indexOf(p1.executionPoint().prefix() + p1.phase());
int i2 = lifecyclePhases.indexOf(p2.executionPoint().prefix() + p2.phase());
if (i1 == -1 && i2 == -1) {
// unknown phases, leave in existing order
return 0;
Expand All @@ -61,13 +61,6 @@ public int compare(String o1, String o2) {
if (rv != 0) {
return rv;
}
// same phase, now compare execution points
i1 = p1.executionPoint().ordinal();
i2 = p2.executionPoint().ordinal();
rv = Integer.compare(i1, i2);
if (rv != 0) {
return rv;
}
// same execution point, now compare priorities
return Integer.compare(p1.priority(), p2.priority());
}
Expand Down
8 changes: 5 additions & 3 deletions ...-core/src/main/java/org/apache/maven/lifecycle/internal/concurrent/BuildPlanExecutor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -683,9 +683,7 @@ public BuildPlan calculateLifecycleMappings(
+ " or a goal in the format <plugin-prefix>:<goal> or"
+ " <plugin-group-id>:<plugin-artifact-id>[:<plugin-version>]:<goal>. Available lifecycle phases are: "
+ lifecycles.stream()
.flatMap(l -> l.orderedPhases()
.map(List::stream)
.orElseGet(() -> l.allPhases().map(Lifecycle.Phase::name)))
.flatMap(l -> l.allPhases().map(Lifecycle.Phase::name))
.collect(Collectors.joining(", "))
+ ".",
lifecyclePhase));
Expand Down Expand Up @@ -738,6 +736,10 @@ public BuildPlan calculateLifecycleMappings(
? lifecyclePhase.substring(AT.length())
: AFTER + lifecyclePhase;
Set<BuildStep> toKeep = steps.get(endPhase).allPredecessors().collect(Collectors.toSet());
toKeep.addAll(toKeep.stream()
.filter(s -> s.name.startsWith(BEFORE))
.map(s -> steps.get(AFTER + s.name.substring(BEFORE.length())))
.toList());
steps.values().stream().filter(n -> !toKeep.contains(n)).forEach(BuildStep::skip);

plan.addProject(project, steps);
Expand Down
Loading

0 comments on commit 63ca953

Please sign in to comment.