gnodet-bot commented on code in PR #13233:
URL: https://github.com/apache/maven/pull/13233#discussion_r4071663521
##########
impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/concurrent/BuildPlanExecutor.java:
##########
@@ -681,6 +707,32 @@ private void plan() {
}
}
+ /**
+ * Expands the set of explicitly skipped phase names to include all
their descendant
+ * (child/sub) phases in the lifecycle DAG.
+ *
+ * <p>When a user specifies {@code --skip-phases=verify}, all mojos
bound to sub-phases
+ * of {@code verify} (e.g. {@code test}, {@code unit-test}, {@code
integration-test}) must
+ * also be skipped, since those sub-phases are part of the skipped
phase.</p>
+ *
+ * @param explicit the set of phase names explicitly listed in {@code
--skip-phases}
+ * @return a new set containing the original phase names plus all
their descendants
+ */
+ private Set<String> expandSkippedPhases(Set<String> explicit) {
Review Comment:
**Behavioral asymmetry: `expandSkippedPhases` only exists in the concurrent
path.**
`expandSkippedPhases` ensures that `--skip-phases=verify` also suppresses
mojos bound to sub-phases of `verify` (e.g. `test`, `integration-test`). This
is the correct semantic — skipping a parent phase should skip everything inside
it.
But this expansion only happens here, in `BuildPlanExecutor` (the concurrent
builder, used with `-T`). The sequential builder —
`DefaultLifecycleExecutionPlanCalculator.calculateMojoExecutions()` — is the
default for Maven 4 single-threaded builds (`builderId = "singlethreaded"` in
`DefaultMavenExecutionRequest`). Its filter is exact-match:
```java
if (skippedPhases.contains(entry.getKey())) {
continue;
}
```
With that code, `--skip-phases=verify` skips only mojos bound to the literal
phase `"verify"`. Surefire, bound to `"test"`, still runs — even though `test`
is a child of `verify` in the lifecycle DAG.
The result: `--skip-phases=verify` behaves differently depending on whether
the user passes `-T` or not. That is a silent correctness bug for the default
(most common) build mode.
**Fix:** extract `expandSkippedPhases` to a shared utility (e.g. a static
method in a helper class or on `LifecycleRegistry`) and call it from both
`BuildPlanExecutor.BuildContext` and
`DefaultLifecycleExecutionPlanCalculator.calculateMojoExecutions()`. Then add a
test to `DefaultLifecycleExecutionPlanCalculatorTest` mirroring
`skippingParentPhaseAlsoSkipsSubPhases`.
--
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]
