Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MNG-8244] Using before:all / all / after:all is not triggered #1973

Merged
merged 3 commits into from
Dec 13, 2024
Merged

[MNG-8244] Using before:all / all / after:all is not triggered #1973

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
15531fb
[MNG-5729] Fix possible NPE with introduction of mononic clock
gnodet Dec 12, 2024
8f6458b
Prevent NPE when a mojo execution is not bound at all
gnodet Dec 12, 2024
a8ed59d
[MNG-8244] Fix before:all and after:all phases
gnodet Dec 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions api/maven-api-core/src/main/java/org/apache/maven/api/Lifecycle.java
View file
Open in desktop
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
5 changes: 4 additions & 1 deletion impl/maven-cli/src/main/java/org/apache/maven/cling/event/ExecutionEventLogger.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import java.time.Duration;
import java.time.Instant;
import java.time.ZoneId;
import java.time.ZonedDateTime;
import java.time.temporal.ChronoUnit;
import java.util.List;
import java.util.Objects;

Expand Down Expand Up @@ -278,7 +280,8 @@ private void logStats(MavenSession session) {

logger.info("Total time: {}{}", formatDuration(time), wallClock);

logger.info("Finished at: {}", formatTimestamp(finish.atZone(ZoneId.systemDefault())));
ZonedDateTime rounded = finish.truncatedTo(ChronoUnit.SECONDS).atZone(ZoneId.systemDefault());
logger.info("Finished at: {}", formatTimestamp(rounded));
}

@Override
Expand Down
107 changes: 49 additions & 58 deletions impl/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultLifecycleRegistry.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
13 changes: 9 additions & 4 deletions ...-core/src/main/java/org/apache/maven/lifecycle/internal/concurrent/BuildPlanExecutor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,9 @@ private void plan() {
MojoDescriptor mojoDescriptor = getMojoDescriptor(project, plugin, goal);
String phase =
execution.getPhase() != null ? execution.getPhase() : mojoDescriptor.getPhase();
if (phase == null) {
continue;
}
String tmpResolvedPhase = plan.aliases().getOrDefault(phase, phase);
String resolvedPhase = tmpResolvedPhase.startsWith(AT)
? tmpResolvedPhase.substring(AT.length())
Expand Down Expand Up @@ -680,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 @@ -735,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 Expand Up @@ -927,7 +932,7 @@ protected void stop() {
}

protected Duration wallTime() {
return Duration.between(start, end);
return start != null && end != null ? Duration.between(start, end) : Duration.ZERO;
}

protected Duration execTime() {
Expand Down
Loading
Loading