You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "gnodet (via GitHub)" <gi...@apache.org> on 2026/09/23 10:24:43 UTC
[PR] [MNG-12537] reactor.xml: named build aliases, lifecycle filter, structured build report, BuildEnvironment API [maven]
gnodet opened a new pull request, #13249:
URL: https://github.com/apache/maven/pull/13249
## Overview
This PR introduces four inter-related features for Maven 4.1.0, developed together because they share the same extension points and data model:
1. **`reactor.xml`** — a single `.mvn/reactor.xml` file that consolidates CLI defaults (replaces `.mvn/maven.config`), core extensions (replaces `.mvn/extensions.xml`), named build aliases, and lifecycle phase injections.
2. **Named build aliases** — a short name on the command line that expands to a full argument list, defined in `reactor.xml`.
3. **Lifecycle execution filter** — a `maven.lifecycle.filter` user property that skips mojo executions matching a coordinate or phase predicate.
4. **Structured build report** — a JSON file written to `target/build-reports/` at the end of each build, capturing timing, status, per-mojo log output, and environment info; accessible via `Session.buildEnvironment()` and a new `BuildReport` API.
---
## Feature 1: `reactor.xml`
**Location:** `.mvn/reactor.xml` (project root directory)
**Schema:** `api/maven-api-core/src/main/mdo/reactor.mdo` — generates StAX reader/writer via Modello.
**Format:**
```xml
<reactor xmlns="http://maven.apache.org/REACTOR/1.0.0">
<!-- Replaces .mvn/maven.config -->
<options>-T4 --no-transfer-progress</options>
<!-- Replaces .mvn/extensions.xml -->
<extensions>
<extension groupId="io.takari.maven" artifactId="takari-smart-builder" version="0.6.1"/>
</extensions>
<!-- Named build aliases (see Feature 2) -->
<aliases>
<alias name="ci">
<content>verify -Dmaven.test.skip=true</content>
</alias>
<alias name="release">
<args><arg>-Prelease</arg><arg>deploy</arg></args>
</alias>
</aliases>
<!-- Custom lifecycle phase injections -->
<phases>
<phase name="pre-integration" after="test" before="integration-test"/>
</phases>
</reactor>
```
**Key design decisions:**
- `reactor.xml` replaces both `maven.config` and `extensions.xml` when present. Legacy files are still read when `reactor.xml` is absent, preserving full backwards compatibility.
- `<alias name="...">` uses an XML attribute (not a child element) because aliases are short and the attribute form is more readable for what is expected to be a common file.
- `<options>` is a whitespace/quote-aware inline string (same tokenization as shell); `<optionArgs>` provides a structured alternative when values contain spaces.
- Parsing happens in `BaseParser` before any DI container is started, so reactor.xml config (including extensions) is available at the earliest possible point.
---
## Feature 2: Named Build Aliases
**Parsing:** `MavenParser.expandAlias()` — pure string manipulation before any Maven DI or session setup.
**Semantics:**
- Every argument in the args list is checked against the alias map in order; any match is replaced by the alias expansion tokens.
- Expansion is **not recursive** — tokens produced by an alias are passed through as-is.
- Aliases can be flag-style (e.g. `--skip-tests`) to provide shorthand for long `-D` expressions.
- An empty alias expansion (no `<content>` and no `<args>`) is valid — the alias token is simply removed.
**Validation:**
- Blank alias names are rejected at expansion time with `IllegalArgumentException`.
- Unclosed quotes in `<content>` strings throw `IllegalArgumentException` (via `ArgumentTokenizer`).
**Example:**
```
# reactor.xml defines alias "ci" → "verify -Dmaven.test.skip=true"
mvn ci -B
# is equivalent to:
mvn verify -Dmaven.test.skip=true -B
```
**Tests:** `MavenParserAliasTest`, `ArgumentTokenizerTest`, IT `mng-12537-reactor-xml-alias`.
---
## Feature 3: Lifecycle Execution Filter
**Activation:** `-Dmaven.lifecycle.filter=<expression>` (property name: `Constants.MAVEN_LIFECYCLE_FILTER`)
**Expression syntax** (comma-separated, OR-ed predicates):
| Token | Meaning |
|---|---|
| `*` | Skip every mojo execution |
| `:A` | Skip by artifactId (e.g. `:maven-enforcer-plugin`) |
| `G:A` | Skip by groupId:artifactId |
| `P` | Skip by goal prefix (e.g. `enforcer`) |
| `P:v:g` | Skip by prefix + version + goal |
| `P:v:g@e` | Skip by prefix + version + goal + executionId |
| `phase(name)` | Skip all mojos bound to the named phase |
**Implementation:** `MojoExecutionFilter` (parser) + `CoordinatePredicate` / `PhasePredicate` (matchers). Applied in `MojoExecutor` before each mojo execution; skipped mojos fire a `MojoSkipped` execution event.
A `DefaultMojoExecution` with a `null` session is created for predicate evaluation — only string fields (goal, groupId, artifactId, prefix, executionId, phase) are accessed; no session-bound fields are touched.
**Example:**
```
# Skip all test-phase mojos and the enforcer plugin
mvn verify -Dmaven.lifecycle.filter=phase(test),:maven-enforcer-plugin
```
**Tests:** `CoordinatePredicateTest`, `MojoExecutionFilterTest`, `PhasePredicateTest`, IT `mng-12538-lifecycle-filter`.
---
## Feature 4: Structured Build Report
**Output location:** `<topDirectory>/target/build-reports/`
- `build-report-<timestamp>Z.json` — timestamped per-build file
- `build-report-latest.json` — symlink (or copy on Windows) always pointing to the most recent report
**Content:**
```jsonc
{
"formatVersion": 1,
"status": "SUCCESS", // SUCCESS | FAILURE | ERROR | SKIPPED
"startTime": "2025-07-29T14:30:00Z",
"duration": "PT1M23.456S",
"mavenVersion": "4.1.0-SNAPSHOT",
"javaVersion": "25.0.1",
"goals": ["verify"],
"multiModule": true,
"threads": 4,
"environment": {
"os": "Linux 6.1.0", "arch": "amd64",
"userProperties": { "-Dmaven.test.skip": "true" },
"systemProperties": { "java.version": "25.0.1", ... }
},
"modules": [
{
"groupId": "org.apache.maven",
"artifactId": "maven-core",
"status": "SUCCESS",
"duration": "PT12.3S",
"mojos": [
{
"groupId": "org.apache.maven.plugins",
"artifactId": "maven-compiler-plugin",
"goal": "compile",
"executionId": "default-compile",
"phase": "compile",
"status": "SUCCESS",
"duration": "PT2.1S",
"output": [
{ "level": "INFO", "timestamp": "...", "message": "Compiling 42 source files" }
]
}
]
}
],
"failures": [...],
"output": [...] // build-level log lines not attributed to a module
}
```
**Implementation:**
- `BuildReportCollector` — `@Singleton` `ExecutionListener` that captures timings and routes log events via `ProjectBuildLogAppender`. Thread-safe: uses `ConcurrentHashMap` + per-list `synchronized` blocks (properly guarded — no synchronization on the shared `Collections.emptyList()` singleton).
- `BuildReportJsonWriter` — hand-written JSON serializer (no external JSON dependency; produces UTF-8 output compatible with `jq`).
- `DefaultBuildEnvironment` — captures OS, JVM, Maven version, and user/system properties (sensitive keys redacted to `***`).
**API access:** `Session.buildEnvironment()` returns a `BuildEnvironment` (lazy, volatile, thread-safe).
**New API types** (`api/maven-api-core`, package `org.apache.maven.api.build.report`):
- `BuildReport` — top-level report
- `ModuleReport` — per-module timing and mojo list
- `MojoReport` — per-mojo timing and captured log output
- `LogEvent` — a single captured log line (level + timestamp + message + optional throwable)
- `FailureReport` — structured failure info (module, mojo, message, stack trace excerpt)
- `BuildStatus` — enum: `SUCCESS`, `FAILURE`, `ERROR`, `SKIPPED`
- `BuildEnvironment` — OS/JVM/Maven/property snapshot
All API types are immutable records with noun-style accessors (`groupId()`, `status()`, `duration()`) and full Javadoc. Lists are defensively copied in compact constructors.
**Tests:** `BuildReportCollectorTest`, `BuildReportIntegrationTest`.
---
## What Was Not Changed
- `.mvn/maven.config` and `.mvn/extensions.xml` continue to be read when `reactor.xml` is absent — no migration required.
- No existing behaviour changes when these features are not activated.
- The `maven.lifecycle.filter` property is not set by default — zero overhead when unused.
- Build reports are written on a best-effort basis (failure to write logs a `WARN`, build is not aborted).
---
## Checklist
- [x] Unit tests for all new classes (`ArgumentTokenizerTest`, `MavenParserAliasTest`, `CoordinatePredicateTest`, `MojoExecutionFilterTest`, `PhasePredicateTest`, `BuildReportCollectorTest`, `BuildReportIntegrationTest`)
- [x] Integration tests (`mng-12537-reactor-xml-alias`, `mng-12538-lifecycle-filter`)
- [x] `spotless:apply` applied
- [x] No new compiler warnings
- [x] API types follow Maven 4 immutable-type conventions (noun accessors, compact constructors, `List.copyOf()` at construction time)
- [x] Concurrency: `BuildReportCollector` reviewed — `ConcurrentHashMap` + per-list `synchronized`, no spurious synchronization on shared singletons
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] reactor.xml: named build aliases and lifecycle phase injections [maven]
Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
gnodet-bot commented on code in PR #13249:
URL: https://github.com/apache/maven/pull/13249#discussion_r4324836148
##########
impl/maven-core/src/test/java/org/apache/maven/internal/impl/PhaseEnrichedLifecycleTest.java:
##########
@@ -0,0 +1,332 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.internal.impl;
+
+import java.util.Collection;
+import java.util.List;
+
+import org.apache.maven.api.Lifecycle;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Unit tests for {@link PhaseEnrichedLifecycle}.
+ *
+ * <p>In Maven 4 the "default" lifecycle is a <em>tree</em> of nested phases. Phases like
+ * {@code unit-test} and {@code integration-test} are children of {@code verify}, which is itself
+ * a child of {@code each}. Custom phases must therefore declare their {@code parent} explicitly.
+ *
+ * <p>Ordering is verified via {@link DefaultLifecycleRegistry#computePhases(Lifecycle)}, which
+ * performs the full topological sort on the lifecycle DAG.
+ */
+class PhaseEnrichedLifecycleTest {
+
+ /** InjectedPhase(name, parent, after, before) */
+ private static PhaseEnrichedLifecycle.InjectedPhase ip(String name, String parent, String after, String before) {
+ return new PhaseEnrichedLifecycle.InjectedPhase(name, parent, after, before);
+ }
+
+ private static Lifecycle defaultLifecycle() {
+ DefaultLifecycleRegistry registry = new DefaultLifecycleRegistry(List.of());
+ return registry.stream()
+ .filter(lc -> Lifecycle.DEFAULT.equals(lc.id()))
+ .findFirst()
+ .orElseThrow(() -> new AssertionError("Default lifecycle not found"));
+ }
+
+ private static DefaultLifecycleRegistry registry() {
+ return new DefaultLifecycleRegistry(List.of());
+ }
+
+ // -------------------------------------------------------------------------
+ // Phase presence
+ // -------------------------------------------------------------------------
+
+ @Test
+ void injectedPhaseAppearsInAllPhases() {
+ Lifecycle base = defaultLifecycle();
+ PhaseEnrichedLifecycle enriched =
+ new PhaseEnrichedLifecycle(base, List.of(ip("pre-integration", "verify", "unit-test", null)));
+
+ List<String> names = enriched.allPhases().map(Lifecycle.Phase::name).toList();
+ assertTrue(names.contains("pre-integration"), "'pre-integration' must appear in allPhases()");
+ assertTrue(names.contains("unit-test"), "'unit-test' must still be present");
+ assertTrue(names.contains("integration-test"), "'integration-test' must still be present");
+ }
+
+ @Test
+ void injectedPhaseAppearsInV3Phases() {
+ Lifecycle base = defaultLifecycle();
+ PhaseEnrichedLifecycle enriched =
+ new PhaseEnrichedLifecycle(base, List.of(ip("pre-integration", "verify", "unit-test", null)));
+
+ List<String> names = enriched.v3phases().stream()
+ .flatMap(Lifecycle.Phase::allPhases)
+ .map(Lifecycle.Phase::name)
+ .toList();
+ assertTrue(names.contains("pre-integration"), "'pre-integration' must appear in v3phases() tree");
+ }
+
+ // -------------------------------------------------------------------------
+ // Topological ordering via computePhases()
+ // -------------------------------------------------------------------------
+
+ @Test
+ void computedPhasesRespectAfterConstraint() {
+ Lifecycle base = defaultLifecycle();
+ PhaseEnrichedLifecycle enriched =
+ new PhaseEnrichedLifecycle(base, List.of(ip("pre-integration", "verify", "unit-test", null)));
+
+ List<String> computed = registry().computePhases(enriched);
+
+ int unitTestIdx = computed.indexOf("unit-test");
+ int newIdx = computed.indexOf("pre-integration");
+ int itIdx = computed.indexOf("integration-test");
+ assertTrue(unitTestIdx >= 0, "'unit-test' must appear in computePhases()");
+ assertTrue(newIdx >= 0, "'pre-integration' must appear in computePhases()");
+ assertTrue(itIdx >= 0, "'integration-test' must appear in computePhases()");
+ assertTrue(unitTestIdx < newIdx, "'pre-integration' must come after 'unit-test'");
+ assertTrue(newIdx < itIdx, "'pre-integration' must come before 'integration-test'");
+ }
+
+ @Test
+ void computedPhasesRespectBeforeConstraint() {
+ Lifecycle base = defaultLifecycle();
+ PhaseEnrichedLifecycle enriched =
+ new PhaseEnrichedLifecycle(base, List.of(ip("pre-integration", "verify", null, "integration-test")));
+
+ List<String> computed = registry().computePhases(enriched);
+
+ int itIdx = computed.indexOf("integration-test");
+ int newIdx = computed.indexOf("pre-integration");
+ assertTrue(itIdx >= 0, "'integration-test' must appear in computePhases()");
+ assertTrue(newIdx >= 0, "'pre-integration' must appear in computePhases()");
+ assertTrue(newIdx < itIdx, "'pre-integration' must precede 'integration-test' in topological order");
+ }
+
+ @Test
+ void computedPhasesWithAfterAndBefore() {
+ Lifecycle base = defaultLifecycle();
+ PhaseEnrichedLifecycle enriched = new PhaseEnrichedLifecycle(
+ base, List.of(ip("pre-integration", "verify", "unit-test", "integration-test")));
+
+ List<String> computed = registry().computePhases(enriched);
+
+ int unitTestIdx = computed.indexOf("unit-test");
+ int itIdx = computed.indexOf("integration-test");
+ int newIdx = computed.indexOf("pre-integration");
+ assertTrue(unitTestIdx >= 0, "'unit-test' must appear");
+ assertTrue(itIdx >= 0, "'integration-test' must appear");
+ assertTrue(newIdx >= 0, "'pre-integration' must appear");
+ assertTrue(unitTestIdx < newIdx, "'pre-integration' must come after 'unit-test'");
+ assertTrue(newIdx < itIdx, "'pre-integration' must come before 'integration-test'");
+ }
+
+ // -------------------------------------------------------------------------
+ // DAG links on the injected phase
+ // -------------------------------------------------------------------------
+
+ @Test
+ void injectedPhaseHasAfterLink() {
+ Lifecycle base = defaultLifecycle();
+ PhaseEnrichedLifecycle enriched =
+ new PhaseEnrichedLifecycle(base, List.of(ip("pre-integration", "verify", "unit-test", null)));
+
+ Lifecycle.Phase phase = enriched.allPhases()
+ .filter(p -> "pre-integration".equals(p.name()))
+ .findFirst()
+ .orElseThrow(() -> new AssertionError("'pre-integration' not found in allPhases()"));
+
+ Collection<Lifecycle.Link> links = phase.links();
+ assertEquals(1, links.size(), "injected phase must have exactly one AFTER link");
+ Lifecycle.Link link = links.iterator().next();
+ assertEquals(Lifecycle.Link.Kind.AFTER, link.kind(), "link must be AFTER");
+ assertEquals("unit-test", link.pointer().phase(), "AFTER link must point to 'unit-test'");
+ }
+
+ @Test
+ void injectedPhaseBeforeHasAfterLinkToLeftSibling() {
+ // When only 'before' is specified, the AFTER link points to the left sibling (not the before anchor)
+ Lifecycle base = defaultLifecycle();
+ PhaseEnrichedLifecycle enriched =
+ new PhaseEnrichedLifecycle(base, List.of(ip("pre-integration", "verify", null, "integration-test")));
+
+ Lifecycle.Phase phase = enriched.allPhases()
+ .filter(p -> "pre-integration".equals(p.name()))
+ .findFirst()
+ .orElseThrow(() -> new AssertionError("'pre-integration' not found"));
+
+ Collection<Lifecycle.Link> links = phase.links();
+ assertEquals(1, links.size(), "injected phase must have exactly one AFTER link");
+ assertEquals(Lifecycle.Link.Kind.AFTER, links.iterator().next().kind());
+ // Left sibling when inserting before 'integration-test' in [unit-test, integration-test] is 'unit-test'
+ assertEquals(
+ "unit-test",
+ links.iterator().next().pointer().phase(),
+ "AFTER link must point to 'unit-test' (left sibling)");
+ }
+
+ // -------------------------------------------------------------------------
+ // Identity / delegation
+ // -------------------------------------------------------------------------
+
+ @Test
+ void idIsPreserved() {
+ Lifecycle base = defaultLifecycle();
+ PhaseEnrichedLifecycle enriched =
+ new PhaseEnrichedLifecycle(base, List.of(ip("x", "verify", "unit-test", null)));
+ assertEquals(Lifecycle.DEFAULT, enriched.id());
+ }
+
+ @Test
+ void aliasesAreDelegated() {
+ Lifecycle base = defaultLifecycle();
+ PhaseEnrichedLifecycle enriched =
+ new PhaseEnrichedLifecycle(base, List.of(ip("x", "verify", "unit-test", null)));
+ List<String> baseV3 =
+ base.aliases().stream().map(Lifecycle.Alias::v3Phase).toList();
+ List<String> enrichedV3 =
+ enriched.aliases().stream().map(Lifecycle.Alias::v3Phase).toList();
+ assertEquals(baseV3, enrichedV3, "aliases must be fully delegated to the base lifecycle");
+ }
+
+ @Test
+ void originalPhasesAreRetained() {
+ Lifecycle base = defaultLifecycle();
+ long originalCount = base.allPhases().count();
+
+ PhaseEnrichedLifecycle enriched =
+ new PhaseEnrichedLifecycle(base, List.of(ip("x", "verify", "unit-test", null)));
+ long enrichedCount = enriched.allPhases().count();
+
+ assertEquals(originalCount + 1, enrichedCount, "exactly one phase should be added");
+ }
+
+ // -------------------------------------------------------------------------
+ // Comma-separated after/before anchors
+ // -------------------------------------------------------------------------
+
+ @Test
+ void commaSeparatedAfterAnchorsUsesLatest() {
+ Lifecycle base = defaultLifecycle();
+ // after="unit-test,integration-test" → insert after integration-test (the latest)
+ PhaseEnrichedLifecycle enriched =
+ new PhaseEnrichedLifecycle(base, List.of(ip("post-all", "verify", "unit-test,integration-test", null)));
+
+ List<String> computed = registry().computePhases(enriched);
+
+ int unitTestIdx = computed.indexOf("unit-test");
+ int itIdx = computed.indexOf("integration-test");
+ int newIdx = computed.indexOf("post-all");
+ assertTrue(unitTestIdx >= 0, "'unit-test' must appear");
+ assertTrue(itIdx >= 0, "'integration-test' must appear");
+ assertTrue(newIdx >= 0, "'post-all' must appear");
+ assertTrue(itIdx < newIdx, "'post-all' must come after 'integration-test'");
+ }
+
+ @Test
+ void commaSeparatedBeforeAnchorsUsesEarliest() {
+ Lifecycle base = defaultLifecycle();
+ // before="unit-test,integration-test" → insert before unit-test (the earliest)
+ PhaseEnrichedLifecycle enriched =
+ new PhaseEnrichedLifecycle(base, List.of(ip("pre-all", "verify", null, "unit-test,integration-test")));
+
+ List<String> computed = registry().computePhases(enriched);
+
+ int unitTestIdx = computed.indexOf("unit-test");
+ int newIdx = computed.indexOf("pre-all");
+ assertTrue(unitTestIdx >= 0, "'unit-test' must appear");
+ assertTrue(newIdx >= 0, "'pre-all' must appear");
+ assertTrue(newIdx < unitTestIdx, "'pre-all' must come before 'unit-test'");
Review Comment:
⚠️ **Test gap: reversed `before=` anchor order not covered**
`before="unit-test,integration-test"` happens to be in lifecycle order so `get(0)` = `"unit-test"` (correct). Add a test with reversed order to pin the behavior:
```java
@Test
void commaSeparatedBeforeAnchorsUsesEarliestRegardlessOfStringOrder() {
Lifecycle base = defaultLifecycle();
// before="integration-test,unit-test" — reversed string order, unit-test is still the earliest
PhaseEnrichedLifecycle enriched =
new PhaseEnrichedLifecycle(base, List.of(ip("pre-all", "verify", null, "integration-test,unit-test")));
List<String> computed = registry().computePhases(enriched);
int unitTestIdx = computed.indexOf("unit-test");
int newIdx = computed.indexOf("pre-all");
assertTrue(unitTestIdx >= 0, "'unit-test' must appear");
assertTrue(newIdx >= 0, "'pre-all' must appear");
assertTrue(newIdx < unitTestIdx, "'pre-all' must come before 'unit-test' regardless of string order");
}
```
##########
impl/maven-core/src/main/java/org/apache/maven/internal/impl/PhaseEnrichedLifecycle.java:
##########
@@ -0,0 +1,481 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.internal.impl;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Optional;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import org.apache.maven.api.Lifecycle;
+import org.apache.maven.api.model.Plugin;
+
+/**
+ * A {@link Lifecycle} decorator that injects custom phases into the lifecycle phase tree.
+ *
+ * <p>Maven 4 lifecycles carry two parallel phase trees:
+ * <ul>
+ * <li>{@link Lifecycle#phases()} — the native Maven 4 hierarchical tree.</li>
+ * <li>{@link Lifecycle#v3phases()} — a flatter, Maven-3-compatible tree used exclusively by
+ * {@link DefaultLifecycleRegistry#computePhases} to produce the ordered phase list.</li>
+ * </ul>
+ *
+ * <p>Injection strategy:
+ * <ul>
+ * <li>In {@code phases()}: the new phase is inserted as a child of the named {@code parent}
+ * container, positioned by {@code after}/{@code before} among that container's children.
+ * The injected phase's {@code AFTER} link points to its immediate left sibling so that
+ * {@code computePhases()} can build a correct DAG edge.</li>
+ * <li>In {@code v3phases()}: the anchor ({@code after} or {@code before}) is searched
+ * recursively; the new phase is inserted as a sibling with an explicit {@code AFTER} link
+ * to that anchor. This is sufficient for {@code computePhases()} which resolves order
+ * from DAG edges. Falls back to top-level append if the anchor is not found.</li>
+ * </ul>
+ *
+ * <p>Only {@code AFTER} links are emitted; {@code BEFORE} links are omitted to avoid
+ * introducing cycles when {@code addPhases()} also adds implicit sibling-ordering edges.
+ *
+ * <p>{@code after} and {@code before} accept comma-separated lists when multiple anchors are
+ * needed, and both can be specified together to constrain the insertion from both sides.
+ *
+ * <p>Example {@code reactor.xml} declarations:
+ * <pre>{@code
+ * <phase name="pre-integration" parent="verify" after="unit-test" before="integration-test"/>
+ * <phase name="post-all" parent="verify" after="unit-test,integration-test"/>
+ * }</pre>
+ *
+ * @since 4.1.0
+ */
+public class PhaseEnrichedLifecycle implements Lifecycle {
+
+ /**
+ * Descriptor of a single phase to inject.
+ *
+ * @param name name of the new phase
+ * @param parent name of the parent container phase (in the Maven 4 tree) whose children
+ * list receives the injection
+ * @param after comma-separated sibling anchor(s): insert after the latest (rightmost)
+ * match; also used as DAG anchor(s) in v3phases (nullable)
+ * @param before comma-separated sibling anchor(s): insert before the earliest (leftmost)
+ * match (nullable). Can be combined with {@code after}.
+ */
+ public record InjectedPhase(String name, String parent, String after, String before) {}
+
+ private final Lifecycle delegate;
+ private final Collection<Phase> enrichedPhases;
+ private final Collection<Phase> enrichedV3Phases;
+
+ public PhaseEnrichedLifecycle(Lifecycle delegate, List<InjectedPhase> injections) {
+ this.delegate = delegate;
+ this.enrichedPhases = injectAll(new ArrayList<>(delegate.phases()), injections, false);
+ this.enrichedV3Phases = injectAll(new ArrayList<>(delegate.v3phases()), injections, true);
+ }
+
+ // -------------------------------------------------------------------------
+ // Lifecycle delegation
+ // -------------------------------------------------------------------------
+
+ @Override
+ public String id() {
+ return delegate.id();
+ }
+
+ @Override
+ public Collection<Phase> phases() {
+ return enrichedPhases;
+ }
+
+ @Override
+ public Collection<Phase> v3phases() {
+ return enrichedV3Phases;
+ }
+
+ @Override
+ public Stream<Phase> allPhases() {
+ return enrichedPhases.stream().flatMap(Phase::allPhases);
+ }
+
+ @Override
+ public Collection<Alias> aliases() {
+ return delegate.aliases();
+ }
+
+ // -------------------------------------------------------------------------
+ // Top-level dispatcher
+ // -------------------------------------------------------------------------
+
+ /**
+ * Applies all injections to the given phase tree.
+ *
+ * @param phases mutable top-level phase list to enrich
+ * @param injections phases to inject
+ * @param v3Mode when {@code true} uses v3-style injection (sibling of anchor with
+ * explicit AFTER link); when {@code false} uses the parent-based injection
+ * @return unmodifiable enriched list
+ */
+ private static Collection<Phase> injectAll(List<Phase> phases, List<InjectedPhase> injections, boolean v3Mode) {
+ for (InjectedPhase inj : injections) {
+ if (v3Mode) {
+ phases = injectV3(phases, inj);
+ } else {
+ phases = injectMaven4(phases, inj);
+ }
+ }
+ return Collections.unmodifiableList(phases);
+ }
+
+ // -------------------------------------------------------------------------
+ // Maven 4 tree injection (phases())
+ // -------------------------------------------------------------------------
+
+ /**
+ * Injects into the Maven 4 tree by finding the named {@code parent} node and inserting
+ * the new phase among its direct children.
+ *
+ * <p>Falls back to appending at the top level (no link) if the parent is not found.
+ */
+ private static List<Phase> injectMaven4(List<Phase> phases, InjectedPhase inj) {
+ if (inj.parent() == null) {
+ return insertAmongSiblings(phases, inj);
+ }
+ Optional<List<Phase>> result = tryInjectIntoParent(phases, inj);
+ if (result.isPresent()) {
+ return result.get();
+ }
+ // Parent not found — append at top level without a link
+ List<Phase> fallback = new ArrayList<>(phases);
+ fallback.add(buildPhase(inj.name(), null));
+ return fallback;
+ }
+
+ /**
+ * Recursively searches for {@code inj.parent()} in the tree rooted at {@code phases} and,
+ * when found, inserts the new phase among that node's children.
+ *
+ * @return the updated top-level list wrapped in an {@code Optional}, or empty if the parent
+ * was not found at this level or any descendant level
+ */
+ private static Optional<List<Phase>> tryInjectIntoParent(List<Phase> phases, InjectedPhase inj) {
+ for (int i = 0; i < phases.size(); i++) {
+ Phase phase = phases.get(i);
+ if (inj.parent().equals(phase.name())) {
+ // Found the parent: insert among its children
+ List<Phase> newChildren = insertAmongSiblings(new ArrayList<>(phase.phases()), inj);
+ List<Phase> result = new ArrayList<>(phases);
+ result.set(i, withChildren(phase, newChildren));
+ return Optional.of(result);
+ }
+ // Recurse into descendants
+ if (!phase.phases().isEmpty()) {
+ Optional<List<Phase>> sub = tryInjectIntoParent(new ArrayList<>(phase.phases()), inj);
+ if (sub.isPresent()) {
+ List<Phase> result = new ArrayList<>(phases);
+ result.set(i, withChildren(phase, sub.get()));
+ return Optional.of(result);
+ }
+ }
+ }
+ return Optional.empty();
+ }
+
+ // -------------------------------------------------------------------------
+ // v3 tree injection (v3phases() / computePhases)
+ // -------------------------------------------------------------------------
+
+ /**
+ * Injects into the v3 tree by searching for the {@code after}/{@code before} anchors anywhere
+ * in the tree and inserting the new phase as a sibling with explicit {@code AFTER} links to
+ * all {@code after} anchors.
+ *
+ * <p>The explicit links are what {@code computePhases()} uses for DAG ordering — the exact
+ * position in the v3 tree is secondary. Falls back to top-level append when no anchor is
+ * found in the v3 tree.
+ */
+ private static List<Phase> injectV3(List<Phase> phases, InjectedPhase inj) {
+ List<String> afterAnchors = inj.after() != null ? splitAnchors(inj.after()) : List.of();
+ List<String> beforeAnchors = inj.before() != null ? splitAnchors(inj.before()) : List.of();
+
+ // Primary anchor for tree positioning: prefer the last after anchor,
+ // fall back to the first before anchor
+ String primaryAnchor = !afterAnchors.isEmpty()
+ ? afterAnchors.get(afterAnchors.size() - 1)
+ : (!beforeAnchors.isEmpty() ? beforeAnchors.get(0) : null);
Review Comment:
⚠️ **`before`-only comma list: v3 tree positioning uses string order, not lifecycle order**
`primaryAnchor = beforeAnchors.get(0)` picks the first element in the comma-separated string. But the intended semantic is the *earliest in lifecycle order* — which is what `insertAmongSiblings` correctly implements (`i < upperBound`). When string order and lifecycle order differ, the two trees disagree.
For `before="integration-test,unit-test"` (reversed order):
- `beforeAnchors.get(0)` = `"integration-test"` (later in lifecycle, index ~8)
- `tryInjectV3Sibling` finds `integration-test` and inserts BEFORE it in the v3 list
- `addPhases` adds sibling-ordering edges: `AFTER(unit-test) → BEFORE(new-phase)`, placing `new-phase` AFTER `unit-test`
- Result: phase ends up between `unit-test` and `integration-test`, violating "before unit-test"
`insertAmongSiblings` (Maven 4 tree) correctly resolves this by scanning for the minimum-index anchor (`i < upperBound`). `injectV3` should do the same — find the earliest lifecycle-positioned anchor, not the first string token.
Fix: make `injectV3` mirror `insertAmongSiblings`'s logic for the before case — find the anchor that appears earliest in the flattened v3 phase list. The simplest fix is to pass all `before` anchors to `tryInjectV3Sibling` and scan rather than assuming string position order, or (simpler) pick the minimum by scanning `phases` once before calling into the tree search.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] reactor.xml: named build aliases and lifecycle execution filter [maven]
Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
gnodet-bot commented on code in PR #13249:
URL: https://github.com/apache/maven/pull/13249#discussion_r4094015243
##########
impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/filter/CoordinatePredicate.java:
##########
@@ -0,0 +1,248 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.lifecycle.internal.filter;
+
+import org.apache.maven.api.MojoExecution;
+import org.apache.maven.api.plugin.descriptor.PluginDescriptor;
+
+/**
+ * A {@link FilterPredicate} that matches {@link MojoExecution}s by plugin coordinate or prefix.
+ *
+ * <h2>Syntax: {@code ([G[:A]]|P)[:v][:g[@e]]}</h2>
+ *
+ * <p>The {@code @} separator for execution ID is compatible with Maven's existing
+ * {@code plugin:version:goal@executionId} notation used in
+ * {@code DefaultLifecycleExecutionPlanCalculator} for goal tasks.
+ *
+ * <p>Matching uses {@link MojoExecution#getDescriptor()} for goal-level fields and
+ * {@link MojoExecution#getPlugin()} for plugin-level coordinates (groupId, artifactId, version,
+ * goal prefix).
+ *
+ * <p>Forms:
+ * <ul>
+ * <li>{@code *} — matches every mojo execution</li>
+ * <li>{@code :A} — any groupId, specific artifactId (e.g. {@code :maven-enforcer-plugin})</li>
+ * <li>{@code G:A} — exact groupId:artifactId (e.g. {@code org.apache.maven.plugins:maven-enforcer-plugin}).
+ * The groupId <strong>must contain at least one {@code '.'}</strong> for this form to be recognised;
+ * groupIds without a dot (e.g. {@code commons-io}) are routed to prefix-based matching instead.
+ * Use the {@code :A} form to match by artifactId only when the groupId has no dot.</li>
+ * <li>{@code P} — plugin prefix (e.g. {@code enforcer}), resolved against
+ * {@link MojoExecution#getMojoDescriptor()} goal prefix</li>
+ * <li>{@code P:v:g} — prefix + version + goal</li>
+ * <li>{@code P:v:g@e} — prefix + version + goal + executionId</li>
Review Comment:
⚠️ **Undocumented symmetric failure case: dotted prefix misrouted to G:A**
The previous review raised two misclassification cases:
1. Dotless groupId → misrouted to prefix mode (now documented ✅)
2. **Dotted prefix → misrouted to G:A mode** (e.g. `io.smallrye:enforce` where the user intended prefix-based matching) — still not documented.
This second case is just as silent as the first: a user writing `io.smallrye:enforce` in a lifecycle filter expecting to skip the SmallRye plugin by goal prefix will get no error, but the filter won't match anything (the G:A form will try to match groupId=`io.smallrye`, artifactId=`enforce`, which doesn't correspond to any real groupId/artifactId pair). The Javadoc update documents the dotless-groupId limitation but leaves the dotted-prefix limitation undocumented.
Add a note to the `P` bullet in the class-level Javadoc:
```suggestion
* <li>{@code P} — plugin prefix (e.g. {@code enforcer}), resolved against
* {@link MojoExecution#getMojoDescriptor()} goal prefix.
* The prefix <strong>must not contain a {@code '.'}</strong>; prefixes with a dot
* (e.g. {@code io.smallrye}) are routed to groupId:artifactId matching instead.</li>
```
##########
impl/maven-core/src/test/java/org/apache/maven/lifecycle/internal/filter/CoordinatePredicateTest.java:
##########
@@ -0,0 +1,180 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.lifecycle.internal.filter;
+
+import org.apache.maven.api.MojoExecution;
+import org.apache.maven.api.Plugin;
+import org.apache.maven.api.plugin.descriptor.PluginDescriptor;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+/**
+ * Unit tests for {@link CoordinatePredicate}.
+ */
+class CoordinatePredicateTest {
+
+ private static MojoExecution execution(
+ String groupId, String artifactId, String version, String goalPrefix, String goal, String executionId) {
+ PluginDescriptor descriptor = mock(PluginDescriptor.class);
+ when(descriptor.getGroupId()).thenReturn(groupId);
+ when(descriptor.getArtifactId()).thenReturn(artifactId);
+ when(descriptor.getVersion()).thenReturn(version);
+ when(descriptor.getGoalPrefix()).thenReturn(goalPrefix);
+
+ Plugin plugin = mock(Plugin.class);
+ when(plugin.getDescriptor()).thenReturn(descriptor);
+
+ MojoExecution exec = mock(MojoExecution.class);
+ when(exec.getPlugin()).thenReturn(plugin);
+ when(exec.getGoal()).thenReturn(goal);
+ when(exec.getExecutionId()).thenReturn(executionId);
+ return exec;
+ }
+
+ private static MojoExecution surefire() {
+ return execution(
+ "org.apache.maven.plugins", "maven-surefire-plugin", "3.2.5", "surefire", "test", "default-test");
+ }
+
+ private static MojoExecution enforcer() {
+ return execution(
+ "org.apache.maven.plugins", "maven-enforcer-plugin", "3.4.1", "enforcer", "enforce", "enforce-rules");
+ }
+
+ @Test
+ void wildcardMatchesAll() {
+ assertTrue(CoordinatePredicate.parse("*").matches(surefire()));
+ assertTrue(CoordinatePredicate.parse("*").matches(enforcer()));
+ }
+
+ @Test
+ void artifactIdOnlyMatchesByArtifactId() {
+ CoordinatePredicate p = CoordinatePredicate.parse(":maven-surefire-plugin");
+ assertTrue(p.matches(surefire()));
+ assertFalse(p.matches(enforcer()));
+ }
+
+ @Test
+ void colonAloneMatchesAll() {
+ CoordinatePredicate p = CoordinatePredicate.parse(":");
+ assertTrue(p.matches(surefire()));
+ assertTrue(p.matches(enforcer()));
+ }
+
+ @Test
+ void groupAndArtifactMatchesExact() {
+ CoordinatePredicate p = CoordinatePredicate.parse("org.apache.maven.plugins:maven-surefire-plugin");
+ assertTrue(p.matches(surefire()));
+ assertFalse(p.matches(enforcer()));
+ }
+
+ @Test
+ void groupWithWildcardArtifact() {
+ CoordinatePredicate p = CoordinatePredicate.parse("org.apache.maven.plugins:");
+ assertTrue(p.matches(surefire()));
+ assertTrue(p.matches(enforcer()));
+ }
+
+ @Test
+ void groupMismatchDoesNotMatch() {
+ CoordinatePredicate p = CoordinatePredicate.parse("com.example:maven-surefire-plugin");
+ assertFalse(p.matches(surefire()));
+ }
+
+ @Test
+ void prefixMatchesByGoalPrefix() {
+ CoordinatePredicate p = CoordinatePredicate.parse("surefire");
+ assertTrue(p.matches(surefire()));
+ assertFalse(p.matches(enforcer()));
+ }
+
+ @Test
+ void prefixWithGoalMatchesGoal() {
+ CoordinatePredicate p = CoordinatePredicate.parse("surefire::test");
+ assertTrue(p.matches(surefire()));
+ assertFalse(p.matches(enforcer()));
+ }
+
+ @Test
+ void prefixWithVersionGoalAndExecution() {
+ CoordinatePredicate p = CoordinatePredicate.parse("surefire:3.2.5:test@default-test");
+ assertTrue(p.matches(surefire()));
+ }
+
+ @Test
+ void prefixVersionMismatch() {
+ CoordinatePredicate p = CoordinatePredicate.parse("surefire:99.0:test@default-test");
+ assertFalse(p.matches(surefire()));
+ }
+
+ @Test
+ void prefixExecutionIdMismatch() {
+ CoordinatePredicate p = CoordinatePredicate.parse("surefire::test@other-id");
+ assertFalse(p.matches(surefire()));
+ }
+
+ @Test
+ void fullCoordinateWithGoalAndExecution() {
+ CoordinatePredicate p =
+ CoordinatePredicate.parse("org.apache.maven.plugins:maven-surefire-plugin:3.2.5:test@default-test");
+ assertTrue(p.matches(surefire()));
+ }
+
+ @Test
+ void coordinateGoalMismatch() {
+ CoordinatePredicate p =
+ CoordinatePredicate.parse("org.apache.maven.plugins:maven-surefire-plugin:3.2.5:other@default-test");
+ assertFalse(p.matches(surefire()));
+ }
+
+ @Test
+ void noDescriptorDoesNotMatchSpecificPlugin() {
+ MojoExecution exec = mock(MojoExecution.class);
+ when(exec.getPlugin()).thenReturn(null);
+ when(exec.getGoal()).thenReturn("test");
+ when(exec.getExecutionId()).thenReturn("default-test");
+ assertFalse(CoordinatePredicate.parse(":maven-surefire-plugin").matches(exec));
+ }
+
+ @Test
+ void wildcardMatchesEvenWithNullPlugin() {
+ MojoExecution exec = mock(MojoExecution.class);
+ when(exec.getPlugin()).thenReturn(null);
+ assertTrue(CoordinatePredicate.MATCH_ALL.matches(exec));
+ }
+
+ @Test
+ void dotlessGroupIdIsMisroutedToPrefixMode() {
+ // GroupIds without a '.' (e.g. "commons-io") cannot be used in G:A form — they are routed
+ // to prefix-based matching. Document this: the token "commons-io:commons-io" is treated as
+ // prefix="commons-io", version="commons-io" — it will NOT match an execution with groupId
+ // "commons-io". Users should use ":commons-io" (artifactId-only form) instead.
+ MojoExecution exec = execution("commons-io", "commons-io", "2.15.1", "commons-io", "copy", "default");
+ // Prefix "commons-io" won't match goal prefix "commons-io" if the descriptor prefix happens
+ // to be the same, but the version segment "commons-io" will never match "2.15.1" → no match.
+ CoordinatePredicate p = CoordinatePredicate.parse("commons-io:commons-io");
Review Comment:
💡 **Test comment is inaccurate — the prefix DOES match, the version does not**
The comment says _"Prefix 'commons-io' won't match goal prefix 'commons-io' if the descriptor prefix happens to be the same"_ — but the test sets `goalPrefix = "commons-io"`, so the prefix check succeeds (`"commons-io".equals("commons-io") == true`). The test returns `false` because the version segment mismatches (`"commons-io" != "2.15.1"`), not because the prefix fails.
The comment as written implies the test would behave differently if the prefix happened to match, which is wrong — it always matches on prefix here. Fix the comment to accurately describe what the test asserts:
```suggestion
// "commons-io:commons-io" is parsed as P:v form (prefix="commons-io", version="commons-io").
// The prefix check succeeds (goalPrefix="commons-io" matches), but the version check fails
// ("commons-io" != "2.15.1"), so the predicate returns false. This demonstrates that the
// misrouting produces a non-matching predicate, not a silent match.
```
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] reactor.xml: named build aliases and lifecycle phase injections [maven]
Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
gnodet-bot commented on code in PR #13249:
URL: https://github.com/apache/maven/pull/13249#discussion_r4291479979
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/BaseParser.java:
##########
@@ -499,11 +538,31 @@ protected List<CoreExtensions> readCoreExtensionsDescriptor(LocalContext context
Map<String, String> eff = new HashMap<>(context.systemProperties);
eff.putAll(context.userProperties);
- // project
- file = context.cwd.resolve(eff.get(Constants.MAVEN_PROJECT_EXTENSIONS));
- loaded = readCoreExtensionsDescriptorFromFile(file, false);
- if (!loaded.isEmpty()) {
- result.add(new CoreExtensions(file, loaded));
+ // project extensions: use reactor.xml <extensions> when present, otherwise extensions.xml
+ if (context.reactorConfig != null
+ && context.reactorConfig.getExtensions() != null
+ && !context.reactorConfig.getExtensions().isEmpty()) {
+ // reactor.xml is authoritative for project extensions
+ Path dir = context.rootDirectory != null ? context.rootDirectory : context.topDirectory;
+ Path reactorXml = dir != null ? dir.resolve(".mvn/reactor.xml") : null;
+ loaded = context.reactorConfig.getExtensions().stream()
+ .map(re -> org.apache.maven.api.cli.extensions.CoreExtension.newBuilder()
+ .groupId(re.getGroupId())
+ .artifactId(re.getArtifactId())
+ .version(re.getVersion())
+ .classLoadingStrategy(re.getClassLoadingStrategy())
+ .build())
Review Comment:
⚠️ **`ReactorExtension.configuration` is silently dropped**
The `ReactorConfig` model defines a `<configuration>` field on `ReactorExtension` (type `DOM`, for extension-specific initialization configuration). The conversion to `CoreExtension` here omits it — extensions declared in `reactor.xml` with a `<configuration>` block will have their configuration silently discarded. The legacy `extensions.xml` path correctly maps this via `CoreExtensionsStaxReader`.
```suggestion
.map(re -> org.apache.maven.api.cli.extensions.CoreExtension.newBuilder()
.groupId(re.getGroupId())
.artifactId(re.getArtifactId())
.version(re.getVersion())
.classLoadingStrategy(re.getClassLoadingStrategy())
.configuration(re.getConfiguration())
.build())
```
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/MavenParser.java:
##########
@@ -47,14 +56,103 @@ protected Options parseCliOptions(LocalContext context) {
throw new IllegalArgumentException("Specified file does not exists (" + file + ")");
}
}
- // maven.config; if exists
- Path mavenConfig = context.rootDirectory != null ? context.rootDirectory.resolve(".mvn/maven.config") : null;
- if (mavenConfig != null && Files.isRegularFile(mavenConfig)) {
- result.add(parseMavenConfigOptions(mavenConfig));
+
+ // reactor.xml <options> — used instead of maven.config when reactor.xml is present
+ if (context.reactorConfig != null) {
+ List<String> optionArgs = resolveReactorOptions(context.reactorConfig);
+ if (!optionArgs.isEmpty()) {
+ result.add(parseMavenReactorOptions(optionArgs));
+ }
+ } else {
+ // legacy maven.config; if exists
+ Path mavenConfig =
+ context.rootDirectory != null ? context.rootDirectory.resolve(".mvn/maven.config") : null;
+ if (mavenConfig != null && Files.isRegularFile(mavenConfig)) {
+ result.add(parseMavenConfigOptions(mavenConfig));
+ }
}
+
return LayeredMavenOptions.layerMavenOptions(result);
}
+ /**
+ * Resolves the {@code <options>} or {@code <optionArgs>} block of a {@link ReactorConfig}
+ * to a flat list of argument strings.
+ *
+ * <p>When the config has structured {@code <arg>} children ({@code optionArgs}), those are used as-is.
+ * When it has inline text content ({@code options}), it is tokenized with {@link ArgumentTokenizer}.
+ */
+ private static List<String> resolveReactorOptions(ReactorConfig rc) {
+ List<String> optionArgs = rc.getOptionArgs();
+ if (optionArgs != null && !optionArgs.isEmpty()) {
+ return optionArgs;
+ }
+ String options = rc.getOptions();
+ if (options != null && !options.isBlank()) {
+ return ArgumentTokenizer.tokenize(options);
+ }
+ return List.of();
+ }
+
+ /**
+ * Expands alias arguments in {@code args} using aliases defined in {@code reactorConfig}.
+ *
+ * <p>Alias expansion is a pure string manipulation — it happens before any Maven DI or session setup.
+ * The argument list is rebuilt on the fly: each argument is checked against the alias map in order;
+ * if it matches an alias name, it is replaced by the alias expansion tokens; otherwise it is kept as-is.
+ * Expansion is not recursive — tokens produced by an alias are passed through as-is.
+ *
+ * @param args original argument list
+ * @param reactorConfig parsed reactor config, or {@code null} if reactor.xml is absent
+ * @return the (possibly modified) argument list
+ */
+ static List<String> expandAlias(List<String> args, ReactorConfig reactorConfig) {
+ if (reactorConfig == null
+ || reactorConfig.getAliases() == null
+ || reactorConfig.getAliases().isEmpty()) {
+ return args;
+ }
+
+ // Build a name → expansion map for O(1) lookup per arg
+ Map<String, List<String>> aliasMap = new LinkedHashMap<>();
+ for (Alias alias : reactorConfig.getAliases()) {
+ String name = alias.getName();
+ if (name == null || name.isBlank()) {
+ throw new IllegalArgumentException("Alias name must not be blank in reactor.xml");
+ }
+ aliasMap.put(name, resolveAlias(alias));
Review Comment:
💡 **Duplicate alias names are silently overwritten**
`aliasMap.put(name, resolveAlias(alias))` overwrites any existing entry without warning. If a user accidentally declares two `<alias name="ci">` blocks in `reactor.xml`, the second wins silently — no log, no error. Given that blank names already throw (`line 121`), duplicate names should at minimum warn. A throw is more consistent:
```suggestion
if (aliasMap.containsKey(name)) {
throw new IllegalArgumentException(
"Duplicate alias name '" + name + "' in reactor.xml");
}
aliasMap.put(name, resolveAlias(alias));
```
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] reactor.xml: named build aliases and lifecycle execution filter [maven]
Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
gnodet-bot commented on code in PR #13249:
URL: https://github.com/apache/maven/pull/13249#discussion_r4084627803
##########
api/maven-api-spi/src/main/java/org/apache/maven/api/spi/LifecycleProcessor.java:
##########
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.api.spi;
+
+import org.apache.maven.api.Lifecycle;
+import org.apache.maven.api.annotations.Consumer;
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Nonnull;
+import org.apache.maven.api.di.Named;
+
+/**
+ * SPI for transforming Maven lifecycles at session initialisation time.
+ *
+ * <p>A {@code LifecycleProcessor} receives each built-in (or extension-contributed) lifecycle
+ * and may return a modified version of it — for example to inject additional phases into the
+ * lifecycle DAG. Multiple processors are applied in order; each one receives the output of the
+ * previous one.
+ *
+ * <p>Processors are called each time the lifecycle phase map is rebuilt (potentially multiple
+ * times per build). Implementations must be stateless and idempotent — the same lifecycle
+ * input must always produce the same output. They receive the output of the previous processor
+ * in the chain.
+ *
+ * <p>Example — injecting a {@code docker-push} phase after {@code deploy}:
+ * <pre>{@code
+ * @Named
+ * public class DockerLifecycleProcessor implements LifecycleProcessor {
+ * public Lifecycle process(Lifecycle lifecycle) {
+ * if (!Lifecycle.DEFAULT.equals(lifecycle.id())) return lifecycle;
+ * // Use PhaseEnrichedLifecycle (from impl/maven-core) to inject custom phases
+ * return new PhaseEnrichedLifecycle(lifecycle,
+ * List.of(new PhaseEnrichedLifecycle.InjectedPhase("docker-push", null, "deploy", null)));
Review Comment:
⚠️ **SPI Javadoc example leaks an internal implementation class**
`PhaseEnrichedLifecycle` lives in `org.apache.maven.internal.impl` (module `impl/maven-core`). The SPI interface (`api/maven-api-spi`) does not depend on `impl/maven-core` — the comment itself acknowledges this with `"// Use PhaseEnrichedLifecycle (from impl/maven-core)"`. External SPI implementors would need to add a compile dependency on `maven-core` to write the example shown here, which violates the `internal.*` contract.
Options:
1. **Remove the code example** and document the concept in prose, linking to `ReactorXmlLifecycleProcessor` as a reference implementation: `"See {@code ReactorXmlLifecycleProcessor} in impl/maven-core for a concrete example."`
2. **Promote `PhaseEnrichedLifecycle` to the SPI layer** — move it to `api/maven-api-spi` or `api/maven-api-core` as an `@Experimental` type if you want it to be part of the public surface.
3. **Add a factory method to the interface** or a companion utility class in `api/maven-api-spi` that wraps `PhaseEnrichedLifecycle` internally (keeping the impl detail hidden).
```suggestion
* <p>Example — injecting a {@code docker-push} phase after {@code deploy}:
* <pre>
* {@literal @}Named
* public class DockerLifecycleProcessor implements LifecycleProcessor {
* public Lifecycle process(Lifecycle lifecycle) {
* if (!Lifecycle.DEFAULT.equals(lifecycle.id())) return lifecycle;
* // Wrap the lifecycle using a concrete implementation from impl/maven-core,
* // e.g. PhaseEnrichedLifecycle — see ReactorXmlLifecycleProcessor for a
* // full working example.
* return lifecycle; // replace with your enriched lifecycle
* }
* }
* </pre>
```
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] reactor.xml: named build aliases and lifecycle phase injections [maven]
Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
gnodet-bot commented on code in PR #13249:
URL: https://github.com/apache/maven/pull/13249#discussion_r4291927817
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/MavenParser.java:
##########
@@ -47,14 +56,106 @@ protected Options parseCliOptions(LocalContext context) {
throw new IllegalArgumentException("Specified file does not exists (" + file + ")");
}
}
- // maven.config; if exists
- Path mavenConfig = context.rootDirectory != null ? context.rootDirectory.resolve(".mvn/maven.config") : null;
- if (mavenConfig != null && Files.isRegularFile(mavenConfig)) {
- result.add(parseMavenConfigOptions(mavenConfig));
+
+ // reactor.xml <options> — used instead of maven.config when reactor.xml is present
+ if (context.reactorConfig != null) {
+ List<String> optionArgs = resolveReactorOptions(context.reactorConfig);
+ if (!optionArgs.isEmpty()) {
+ result.add(parseMavenReactorOptions(optionArgs));
+ }
+ } else {
+ // legacy maven.config; if exists
+ Path mavenConfig =
+ context.rootDirectory != null ? context.rootDirectory.resolve(".mvn/maven.config") : null;
+ if (mavenConfig != null && Files.isRegularFile(mavenConfig)) {
+ result.add(parseMavenConfigOptions(mavenConfig));
+ }
}
+
return LayeredMavenOptions.layerMavenOptions(result);
}
+ /**
+ * Resolves the {@code <options>} or {@code <optionArgs>} block of a {@link ReactorConfig}
+ * to a flat list of argument strings.
+ *
+ * <p>When the config has structured {@code <arg>} children ({@code optionArgs}), those are used as-is.
+ * When it has inline text content ({@code options}), it is tokenized with {@link ArgumentTokenizer}.
+ */
+ private static List<String> resolveReactorOptions(ReactorConfig rc) {
+ List<String> optionArgs = rc.getOptionArgs();
+ if (optionArgs != null && !optionArgs.isEmpty()) {
+ return optionArgs;
+ }
+ String options = rc.getOptions();
+ if (options != null && !options.isBlank()) {
+ return ArgumentTokenizer.tokenize(options);
+ }
+ return List.of();
Review Comment:
⚠️ **`options`/`optionArgs` mutual exclusivity not enforced** — the MDO Javadoc on `optionArgs` says *"Mutually exclusive with options"*, but the code silently prefers `optionArgs` when both are set, with no warning or error. A user who accidentally sets both gets confusing behavior (their `<options>` is silently ignored). Consider logging a warning or throwing if both are present:
```java
private static List<String> resolveReactorOptions(ReactorConfig rc) {
List<String> optionArgs = rc.getOptionArgs();
String options = rc.getOptions();
if (optionArgs != null && !optionArgs.isEmpty() && options != null && !options.isBlank()) {
throw new IllegalStateException(
"reactor.xml: <options> and <optionArgs> are mutually exclusive; set only one");
}
if (optionArgs != null && !optionArgs.isEmpty()) {
return optionArgs;
}
if (options != null && !options.isBlank()) {
return ArgumentTokenizer.tokenize(options);
}
return List.of();
}
```
##########
impl/maven-core/src/main/java/org/apache/maven/internal/impl/PhaseEnrichedLifecycle.java:
##########
@@ -0,0 +1,430 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.internal.impl;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Optional;
+import java.util.stream.Stream;
+
+import org.apache.maven.api.Lifecycle;
+import org.apache.maven.api.model.Plugin;
+
+/**
+ * A {@link Lifecycle} decorator that injects custom phases into the lifecycle phase tree.
+ *
+ * <p>Maven 4 lifecycles carry two parallel phase trees:
+ * <ul>
+ * <li>{@link Lifecycle#phases()} — the native Maven 4 hierarchical tree.</li>
+ * <li>{@link Lifecycle#v3phases()} — a flatter, Maven-3-compatible tree used exclusively by
+ * {@link DefaultLifecycleRegistry#computePhases} to produce the ordered phase list.</li>
+ * </ul>
+ *
+ * <p>Injection strategy:
+ * <ul>
+ * <li>In {@code phases()}: the new phase is inserted as a child of the named {@code parent}
+ * container, positioned by {@code after}/{@code before} among that container's children.
+ * The injected phase's {@code AFTER} link points to its immediate left sibling so that
+ * {@code computePhases()} can build a correct DAG edge.</li>
+ * <li>In {@code v3phases()}: the anchor ({@code after} or {@code before}) is searched
+ * recursively; the new phase is inserted as a sibling with an explicit {@code AFTER} link
+ * to that anchor. This is sufficient for {@code computePhases()} which resolves order
+ * from DAG edges. Falls back to top-level append if the anchor is not found.</li>
+ * </ul>
+ *
+ * <p>Only {@code AFTER} links are emitted; {@code BEFORE} links are omitted to avoid
+ * introducing cycles when {@code addPhases()} also adds implicit sibling-ordering edges.
+ *
+ * <p>Example {@code reactor.xml} declaration:
+ * <pre>{@code
+ * <phase name="pre-integration" parent="verify" after="unit-test" before="integration-test"/>
+ * }</pre>
+ *
+ * @since 4.1.0
+ */
+public class PhaseEnrichedLifecycle implements Lifecycle {
+
+ /**
+ * Descriptor of a single phase to inject.
+ *
+ * @param name name of the new phase
+ * @param parent name of the parent container phase (in the Maven 4 tree) whose children
+ * list receives the injection
+ * @param after sibling anchor: insert after this phase (takes precedence over
+ * {@code before}); also used as the DAG anchor in v3phases (nullable)
+ * @param before sibling anchor: insert before this phase (nullable; used when
+ * {@code after} is absent)
+ */
+ public record InjectedPhase(String name, String parent, String after, String before) {}
+
+ private final Lifecycle delegate;
+ private final Collection<Phase> enrichedPhases;
+ private final Collection<Phase> enrichedV3Phases;
+
+ public PhaseEnrichedLifecycle(Lifecycle delegate, List<InjectedPhase> injections) {
+ this.delegate = delegate;
+ this.enrichedPhases = injectAll(new ArrayList<>(delegate.phases()), injections, false);
+ this.enrichedV3Phases = injectAll(new ArrayList<>(delegate.v3phases()), injections, true);
+ }
+
+ // -------------------------------------------------------------------------
+ // Lifecycle delegation
+ // -------------------------------------------------------------------------
+
+ @Override
+ public String id() {
+ return delegate.id();
+ }
+
+ @Override
+ public Collection<Phase> phases() {
+ return enrichedPhases;
+ }
+
+ @Override
+ public Collection<Phase> v3phases() {
+ return enrichedV3Phases;
+ }
+
+ @Override
+ public Stream<Phase> allPhases() {
+ return enrichedPhases.stream().flatMap(Phase::allPhases);
+ }
+
+ @Override
+ public Collection<Alias> aliases() {
+ return delegate.aliases();
+ }
+
+ // -------------------------------------------------------------------------
+ // Top-level dispatcher
+ // -------------------------------------------------------------------------
+
+ /**
+ * Applies all injections to the given phase tree.
+ *
+ * @param phases mutable top-level phase list to enrich
+ * @param injections phases to inject
+ * @param v3Mode when {@code true} uses v3-style injection (sibling of anchor with
+ * explicit AFTER link); when {@code false} uses the parent-based injection
+ * @return unmodifiable enriched list
+ */
+ private static Collection<Phase> injectAll(List<Phase> phases, List<InjectedPhase> injections, boolean v3Mode) {
+ for (InjectedPhase inj : injections) {
+ if (v3Mode) {
+ phases = injectV3(phases, inj);
+ } else {
+ phases = injectMaven4(phases, inj);
+ }
+ }
+ return Collections.unmodifiableList(phases);
+ }
+
+ // -------------------------------------------------------------------------
+ // Maven 4 tree injection (phases())
+ // -------------------------------------------------------------------------
+
+ /**
+ * Injects into the Maven 4 tree by finding the named {@code parent} node and inserting
+ * the new phase among its direct children.
+ *
+ * <p>Falls back to appending at the top level (no link) if the parent is not found.
+ */
+ private static List<Phase> injectMaven4(List<Phase> phases, InjectedPhase inj) {
+ if (inj.parent() == null) {
+ return insertAmongSiblings(phases, inj, null);
+ }
+ Optional<List<Phase>> result = tryInjectIntoParent(phases, inj);
+ if (result.isPresent()) {
+ return result.get();
+ }
+ // Parent not found — append at top level without a link
+ List<Phase> fallback = new ArrayList<>(phases);
+ fallback.add(buildPhase(inj.name(), null));
+ return fallback;
+ }
+
+ /**
+ * Recursively searches for {@code inj.parent()} in the tree rooted at {@code phases} and,
+ * when found, inserts the new phase among that node's children.
+ *
+ * @return the updated top-level list wrapped in an {@code Optional}, or empty if the parent
+ * was not found at this level or any descendant level
+ */
+ private static Optional<List<Phase>> tryInjectIntoParent(List<Phase> phases, InjectedPhase inj) {
+ for (int i = 0; i < phases.size(); i++) {
+ Phase phase = phases.get(i);
+ if (inj.parent().equals(phase.name())) {
+ // Found the parent: insert among its children
+ String leftSiblingLink = null; // computed by insertAmongSiblings
+ List<Phase> newChildren = insertAmongSiblings(new ArrayList<>(phase.phases()), inj, leftSiblingLink);
+ List<Phase> result = new ArrayList<>(phases);
+ result.set(i, withChildren(phase, newChildren));
+ return Optional.of(result);
+ }
+ // Recurse into descendants
+ if (!phase.phases().isEmpty()) {
+ Optional<List<Phase>> sub = tryInjectIntoParent(new ArrayList<>(phase.phases()), inj);
+ if (sub.isPresent()) {
+ List<Phase> result = new ArrayList<>(phases);
+ result.set(i, withChildren(phase, sub.get()));
+ return Optional.of(result);
+ }
+ }
+ }
+ return Optional.empty();
+ }
+
+ // -------------------------------------------------------------------------
+ // v3 tree injection (v3phases() / computePhases)
+ // -------------------------------------------------------------------------
+
+ /**
+ * Injects into the v3 tree by searching for the {@code after}/{@code before} anchor anywhere
+ * in the tree and inserting the new phase as its sibling with an explicit {@code AFTER} link.
+ *
+ * <p>The explicit link is what {@code computePhases()} uses for DAG ordering — the exact
+ * position in the v3 tree is secondary. Falls back to top-level append when the anchor is
+ * not found in the v3 tree.
+ */
+ private static List<Phase> injectV3(List<Phase> phases, InjectedPhase inj) {
+ String anchor = inj.after() != null ? inj.after() : inj.before();
+ boolean insertAfter = inj.after() != null;
+
+ Optional<List<Phase>> result = tryInjectV3Sibling(phases, anchor, inj.name(), insertAfter);
+ if (result.isPresent()) {
+ return result.get();
+ }
+ // Anchor not found — append at top level.
+ // Use an AFTER link only for 'after' anchors; 'before' anchors can't safely be used
+ // as AFTER links since we'd be after integration-test but claiming to be before it.
+ String fallbackLink = insertAfter ? anchor : null;
+ List<Phase> fallback = new ArrayList<>(phases);
+ fallback.add(buildPhase(inj.name(), fallbackLink));
+ return fallback;
+ }
+
+ /**
+ * Recursively searches for {@code anchor} and inserts the new phase as its sibling.
+ *
+ * @param phases current sibling list
+ * @param anchor name of the anchor phase ({@code after} or {@code before} target)
+ * @param newName name of the phase to inject
+ * @param insertAfter {@code true} to insert after the anchor, {@code false} to insert before
+ * @return updated list wrapped in {@code Optional}, or empty if not found
+ */
+ private static Optional<List<Phase>> tryInjectV3Sibling(
+ List<Phase> phases, String anchor, String newName, boolean insertAfter) {
+ if (anchor == null) {
+ return Optional.empty();
+ }
+ // Search in direct siblings
+ for (int i = 0; i < phases.size(); i++) {
+ if (anchor.equals(phases.get(i).name())) {
+ int insertAt = insertAfter ? i + 1 : i;
+ // For AFTER anchor: link points to the anchor (= left sibling).
+ // For BEFORE anchor: link points to the left sibling of insertAt (may be null
+ // when inserting at position 0), NOT the anchor — pointing to the anchor would
+ // create a cycle since addPhases() already adds an edge from the anchor to us.
+ String afterLink = insertAt > 0 ? phases.get(insertAt - 1).name() : null;
+ Phase newPhase = buildPhase(newName, afterLink);
+ List<Phase> result = new ArrayList<>(phases);
+ result.add(insertAt, newPhase);
+ return Optional.of(result);
+ }
+ }
+ // Recurse into sub-phases
+ for (int i = 0; i < phases.size(); i++) {
+ Phase phase = phases.get(i);
+ if (!phase.phases().isEmpty()) {
+ Optional<List<Phase>> sub =
+ tryInjectV3Sibling(new ArrayList<>(phase.phases()), anchor, newName, insertAfter);
+ if (sub.isPresent()) {
+ List<Phase> result = new ArrayList<>(phases);
+ result.set(i, withChildren(phase, sub.get()));
+ return Optional.of(result);
+ }
+ }
+ }
+ return Optional.empty();
+ }
+
+ // -------------------------------------------------------------------------
+ // Shared helper: sibling insertion for Maven 4 tree
+ // -------------------------------------------------------------------------
+
+ /**
+ * Inserts the new phase into {@code siblings} at the position given by
+ * {@code inj.after()}/{@code inj.before()}, defaulting to append.
+ *
+ * <p>The injected phase's {@code AFTER} link points to its immediate left sibling so that,
+ * together with the implicit sibling-ordering edges added by {@code addPhases()}, the DAG
+ * correctly orders the new phase relative to its neighbours.
+ *
+ * @param siblings the mutable sibling list to insert into
+ * @param inj the injection descriptor
+ * @param ignoredLinkHint reserved for future use; pass {@code null}
+ */
+ private static List<Phase> insertAmongSiblings(List<Phase> siblings, InjectedPhase inj, String ignoredLinkHint) {
Review Comment:
💡 **YAGNI: dead parameter** — `ignoredLinkHint` is never read inside the method body; the AFTER anchor is always derived from position in the list instead. The Javadoc says *"reserved for future use; pass null"*, but if that future use never materializes this parameter stays dead forever. Consider removing it now and adding it back when actually needed, keeping the API surface minimal.
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/MavenInvoker.java:
##########
@@ -314,6 +315,12 @@ protected void populateRequest(MavenContext context, Lookup lookup, MavenExecuti
if (context.options().builder().isPresent()) {
request.setBuilderId(context.options().builder().get());
}
+
+ // Store ReactorConfig on the request data map so it is accessible to other components
+ // (e.g. future ReactorXmlPhaseInjector) without adding a dependency on maven-cli.
Review Comment:
💡 **Stale class name in comment** — `ReactorXmlPhaseInjector` doesn't exist; the class introduced by this same PR is `ReactorXmlLifecycleProcessor`. Either drop the forward-reference entirely or update to the correct name:
```suggestion
// (e.g. future ReactorXmlLifecycleProcessor) without adding a dependency on maven-cli.
```
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-12537] reactor.xml: named build aliases and lifecycle execution filter [maven]
Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
gnodet-bot commented on code in PR #13249:
URL: https://github.com/apache/maven/pull/13249#discussion_r4081516927
##########
api/maven-api-core/src/main/mdo/reactor.mdo:
##########
@@ -0,0 +1,237 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+
+-->
+
+<model xmlns="http://codehaus-plexus.github.io/MODELLO/2.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://codehaus-plexus.github.io/MODELLO/2.0.0 https://codehaus-plexus.github.io/modello/xsd/modello-2.0.0.xsd"
+ xml.namespace="http://maven.apache.org/REACTOR/${version}"
+ xml.schemaLocation="https://maven.apache.org/xsd/reactor-${version}.xsd">
+
+ <id>reactor</id>
+ <name>ReactorConfig</name>
+ <description><![CDATA[
+ <p>This is a reference for the Reactor Configuration descriptor.</p>
+ <p>The default location for the Reactor Configuration descriptor file is <code>${maven.projectBasedir}/.mvn/reactor.xml</code></p>
+ <p>It consolidates reactor-scoped build configuration previously split across <code>.mvn/maven.config</code>
+ (CLI flag defaults) and <code>.mvn/extensions.xml</code> (core extensions), and adds support for
+ named build aliases and reactor-level lifecycle phase injection.</p>
+ ]]></description>
+
+ <defaults>
+ <default>
+ <key>package</key>
+ <value>org.apache.maven.api.reactor</value>
+ </default>
+ </defaults>
+
+ <classes>
+ <class rootElement="true" xml.tagName="reactor" xsd.compositor="sequence">
+ <name>ReactorConfig</name>
+ <description>Reactor-scoped build configuration for a Maven project.</description>
+ <version>1.0.0+</version>
+ <fields>
+ <field>
+ <name>options</name>
+ <description>CLI options to prepend to every invocation. Replaces .mvn/maven.config when present.
+ Expressed as whitespace-separated, quote-aware inline text (e.g. {@code -T4 --no-transfer-progress}).
+ For options with spaces in their values, use the structured form via {@link #getOptionArgs()}.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <type>String</type>
+ </field>
+ <field>
+ <name>optionArgs</name>
+ <description>Structured CLI options — each <arg> element is one option token.
+ Use instead of {@link #getOptions()} when option values contain spaces.
+ Mutually exclusive with options.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <association xml.itemsStyle="flat" xml.tagName="arg">
+ <type>String</type>
+ <multiplicity>*</multiplicity>
+ </association>
+ </field>
+ <field>
+ <name>extensions</name>
+ <description>Core extensions to load. Replaces .mvn/extensions.xml when present.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <association xml.itemsStyle="flat" xml.tagName="extension">
+ <type>ReactorExtension</type>
+ <multiplicity>*</multiplicity>
+ </association>
+ </field>
+ <field>
+ <name>aliases</name>
+ <description>Named build aliases. The first non-flag argument on the command line is matched against
+ alias names; if found it is replaced by the alias expansion.</description>
Review Comment:
⚠️ **Documentation/implementation mismatch: alias expansion semantics**
The description says *"The first non-flag argument on the command line is matched against alias names"*, but the actual implementation in `MavenParser.expandAlias()` expands **every** argument that matches an alias name — not just the first non-flag one. The PR description and the code's own Javadoc both correctly describe the all-args behavior.
This description will end up in the generated Javadoc and XSD documentation, misleading users who define multiple aliases and expect all of them to be expanded in a single invocation (e.g. `mvn ci rel` where both `ci` and `rel` are aliases).
##########
api/maven-api-core/src/main/mdo/reactor.mdo:
##########
@@ -0,0 +1,237 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+
+-->
+
+<model xmlns="http://codehaus-plexus.github.io/MODELLO/2.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://codehaus-plexus.github.io/MODELLO/2.0.0 https://codehaus-plexus.github.io/modello/xsd/modello-2.0.0.xsd"
+ xml.namespace="http://maven.apache.org/REACTOR/${version}"
+ xml.schemaLocation="https://maven.apache.org/xsd/reactor-${version}.xsd">
+
+ <id>reactor</id>
+ <name>ReactorConfig</name>
+ <description><![CDATA[
+ <p>This is a reference for the Reactor Configuration descriptor.</p>
+ <p>The default location for the Reactor Configuration descriptor file is <code>${maven.projectBasedir}/.mvn/reactor.xml</code></p>
+ <p>It consolidates reactor-scoped build configuration previously split across <code>.mvn/maven.config</code>
+ (CLI flag defaults) and <code>.mvn/extensions.xml</code> (core extensions), and adds support for
+ named build aliases and reactor-level lifecycle phase injection.</p>
+ ]]></description>
+
+ <defaults>
+ <default>
+ <key>package</key>
+ <value>org.apache.maven.api.reactor</value>
+ </default>
+ </defaults>
+
+ <classes>
+ <class rootElement="true" xml.tagName="reactor" xsd.compositor="sequence">
+ <name>ReactorConfig</name>
+ <description>Reactor-scoped build configuration for a Maven project.</description>
+ <version>1.0.0+</version>
+ <fields>
+ <field>
+ <name>options</name>
+ <description>CLI options to prepend to every invocation. Replaces .mvn/maven.config when present.
+ Expressed as whitespace-separated, quote-aware inline text (e.g. {@code -T4 --no-transfer-progress}).
+ For options with spaces in their values, use the structured form via {@link #getOptionArgs()}.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <type>String</type>
+ </field>
+ <field>
+ <name>optionArgs</name>
+ <description>Structured CLI options — each <arg> element is one option token.
+ Use instead of {@link #getOptions()} when option values contain spaces.
+ Mutually exclusive with options.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <association xml.itemsStyle="flat" xml.tagName="arg">
+ <type>String</type>
+ <multiplicity>*</multiplicity>
+ </association>
+ </field>
+ <field>
+ <name>extensions</name>
+ <description>Core extensions to load. Replaces .mvn/extensions.xml when present.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <association xml.itemsStyle="flat" xml.tagName="extension">
+ <type>ReactorExtension</type>
+ <multiplicity>*</multiplicity>
+ </association>
+ </field>
+ <field>
+ <name>aliases</name>
+ <description>Named build aliases. The first non-flag argument on the command line is matched against
+ alias names; if found it is replaced by the alias expansion.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <association xml.itemsStyle="flat" xml.tagName="alias">
+ <type>Alias</type>
+ <multiplicity>*</multiplicity>
+ </association>
+ </field>
+ <field>
+ <name>phases</name>
+ <description>Custom lifecycle phase injections. Phases are registered in the lifecycle DAG
+ with the declared after/before edges.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <association xml.itemsStyle="flat" xml.tagName="phase">
+ <type>PhaseInjection</type>
+ <multiplicity>*</multiplicity>
+ </association>
+ </field>
+ </fields>
+ </class>
+
+ <class xml.tagName="extension">
+ <name>ReactorExtension</name>
+ <description>Describes a core extension to load.</description>
+ <version>1.0.0+</version>
+ <fields>
+ <field>
+ <name>groupId</name>
+ <description>The group ID of the extension's artifact.</description>
+ <version>1.0.0+</version>
+ <required>true</required>
+ <type>String</type>
+ </field>
+ <field>
+ <name>artifactId</name>
+ <description>The artifact ID of the extension.</description>
+ <version>1.0.0+</version>
+ <required>true</required>
+ <type>String</type>
+ </field>
+ <field>
+ <name>version</name>
+ <description>The version of the extension.</description>
+ <version>1.0.0+</version>
+ <required>true</required>
+ <type>String</type>
+ </field>
+ <field>
+ <name>classLoadingStrategy</name>
+ <description>The class loading strategy: 'self-first' (the default), 'parent-first' (loads classes from the
+ parent, then from the extension) or 'plugin' (follows the rules from extensions defined as plugins).</description>
+ <version>1.0.0+</version>
+ <defaultValue>self-first</defaultValue>
+ <required>false</required>
+ <type>String</type>
+ </field>
+ <field>
+ <name>configuration</name>
+ <description>Extension-specific configuration passed to the extension during initialization.
+ The structure is extension-defined.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <type>DOM</type>
+ </field>
+ </fields>
+ <codeSegments>
+ <codeSegment>
+ <version>1.0.0+</version>
+ <code>
+ <![CDATA[
+ /**
+ * Gets the identifier of the extension.
+ *
+ * @return The extension id in the form {@code <groupId>:<artifactId>:<version>}, never {@code null}.
+ */
+ public String getId() {
+ return (getGroupId() == null ? "[unknown-group-id]" : getGroupId())
+ + ":" + (getArtifactId() == null ? "[unknown-artifact-id]" : getArtifactId())
+ + ":" + (getVersion() == null ? "[unknown-version]" : getVersion());
+ }
+ ]]>
+ </code>
+ </codeSegment>
+ </codeSegments>
+ </class>
+
+ <class xml.tagName="alias">
+ <name>Alias</name>
+ <description>A named build alias. When the first non-flag argument matches the alias name,
+ it is replaced by the alias expansion. Supports both inline text (shorthand, whitespace-separated)
Review Comment:
⚠️ **Same doc mismatch on the `Alias` class itself**
Same issue as the `aliases` field description: says "first non-flag argument" but the implementation expands all matching arguments.
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/MavenParser.java:
##########
@@ -76,6 +174,21 @@ protected MavenOptions parseMavenAtFileOptions(Path atFile) {
}
}
+ protected MavenOptions parseMavenReactorOptions(List<String> args) {
+ try {
+ MavenOptions options = parseArgs("reactor.xml", args);
+ if (options.goals().isPresent()) {
+ // <options> can only contain options, not goals/phases
+ throw new IllegalArgumentException("Unrecognized entries in reactor.xml <options>: "
+ + options.goals().get());
+ }
Review Comment:
💡 ** drops the itself as the cause**
`org.apache.commons.cli.ParseException` has no cause (it is the root exception); `e.getCause()` returns `null`, so the `IllegalArgumentException` is constructed with a `null` cause. The original exception is swallowed — only its message survives. The same pattern appears in `parseMavenCliOptions` and `parseMavenAtFileOptions`, so this is pre-existing and presumably intentional, but it means stack traces are lost when reactor option parsing fails.
If intentional (Commons CLI parse errors carry enough info in the message alone), the three catch blocks should have a matching comment explaining the choice. If not, the fix is:
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] reactor.xml: named build aliases and lifecycle phase injections [maven]
Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
gnodet-bot commented on code in PR #13249:
URL: https://github.com/apache/maven/pull/13249#discussion_r4296193174
##########
its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng13249ReactorXmlPhaseInjectionTest.java:
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.it;
+
+import java.nio.file.Path;
+import java.util.List;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Verifies that custom lifecycle phases defined in {@code .mvn/reactor.xml} are injected
+ * into the lifecycle DAG and that plugin goals bound to those phases execute correctly.
+ *
+ * @see <a href="https://github.com/apache/maven/pull/13249">#13249</a>
+ */
+class MavenITmng13249ReactorXmlPhaseInjectionTest extends AbstractMavenIntegrationTestCase {
+
+ /**
+ * A plugin goal bound to a reactor.xml-injected phase must execute when running
+ * through that phase (via {@code verify}).
+ */
+ @Test
+ void pluginBoundToInjectedPhaseExecutesDuringVerify() throws Exception {
+ Path testDir = extractResources("/mng-13249-reactor-xml-phase-injection");
+
+ Verifier verifier = newVerifier(testDir);
+ verifier.setAutoclean(false);
+ verifier.deleteDirectory("target");
+ verifier.addCliArgument("verify");
+ verifier.execute();
+ verifier.verifyErrorFreeLog();
+
+ verifier.verifyFilePresent("target/pre-integration-marker.txt");
+ }
+
+ /**
+ * The injected phase can be used as a direct CLI target.
+ */
+ @Test
+ void injectedPhaseCanBeUsedAsCliTarget() throws Exception {
+ Path testDir = extractResources("/mng-13249-reactor-xml-phase-injection");
+
+ Verifier verifier = newVerifier(testDir);
+ verifier.setAutoclean(false);
+ verifier.deleteDirectory("target");
+ verifier.addCliArgument("pre-integration");
+ verifier.execute();
+ verifier.verifyErrorFreeLog();
+
+ verifier.verifyFilePresent("target/pre-integration-marker.txt");
+ }
+
+ /**
+ * The injected phase respects ordering: running only up to {@code compile}
+ * (which comes before {@code pre-integration}) must NOT trigger the plugin.
+ */
+ @Test
+ void pluginDoesNotRunBeforeInjectedPhase() throws Exception {
+ Path testDir = extractResources("/mng-13249-reactor-xml-phase-injection");
+
+ Verifier verifier = newVerifier(testDir);
+ verifier.setAutoclean(false);
+ verifier.deleteDirectory("target");
+ verifier.addCliArgument("compile");
+ verifier.execute();
+ verifier.verifyErrorFreeLog();
+
+ verifier.verifyFileNotPresent("target/pre-integration-marker.txt");
+ }
+
+ /**
+ * Verify the injected phase runs in the correct position: after unit-test
+ * related phases but before integration-test phases. We check log ordering
+ * by looking for the execution banner lines ({@code [INFO] ---}) which only
+ * appear at actual phase-execution time, not at plan-calculation time.
+ */
+ @Test
+ void injectedPhaseRunsInCorrectOrder() throws Exception {
+ Path testDir = extractResources("/mng-13249-reactor-xml-phase-injection");
+
+ Verifier verifier = newVerifier(testDir);
+ verifier.setAutoclean(false);
+ verifier.deleteDirectory("target");
+ verifier.addCliArgument("verify");
+ verifier.execute();
+ verifier.verifyErrorFreeLog();
+
+ List<String> logLines = verifier.loadLogLines();
+
+ int touchIdx = -1;
+ int compileIdx = -1;
+ for (int i = 0; i < logLines.size(); i++) {
+ String line = logLines.get(i);
+ if (line.contains("compiler") && line.contains(":compile")) {
Review Comment:
**Javadoc/code mismatch — and the pattern is still overly broad**
The method Javadoc (line 92) says _"looking for the execution banner lines `[INFO] ---` which only appear at actual phase-execution time, not at plan-calculation time"_ — but the implementation dropped `[INFO] ---` from the check. The Javadoc describes the original design intent (anchored to the execution banner) while the code reverted to a broad substring scan.
The new pattern `line.contains("compiler") && line.contains(":compile")` will match the Maven 4 execution banner `[INFO] --- compiler:3.16.0:compile (default-compile) @ project ---`, which is correct. But it will also match any DEBUG or INFO planning line that includes the goal coordinates (e.g. `[DEBUG] Resolving plugin compiler:3.16.0:compile`), potentially capturing a line earlier in the log than the actual execution — which is the exact fragility the original review flagged.
Fix: add `[INFO] ---` back as an anchor while keeping the Maven 4 format:
```suggestion
if (line.contains("[INFO] ---") && line.contains("compiler") && line.contains(":compile")) {
```
Then update the Javadoc on line 92 to describe what the code actually does — it already describes this correctly, so no Javadoc change is needed once the code matches.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] reactor.xml: named build aliases and lifecycle execution filter [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #13249:
URL: https://github.com/apache/maven/pull/13249#discussion_r4084575692
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/MavenParser.java:
##########
@@ -76,6 +174,21 @@ protected MavenOptions parseMavenAtFileOptions(Path atFile) {
}
}
+ protected MavenOptions parseMavenReactorOptions(List<String> args) {
+ try {
+ MavenOptions options = parseArgs("reactor.xml", args);
+ if (options.goals().isPresent()) {
+ // <options> can only contain options, not goals/phases
+ throw new IllegalArgumentException("Unrecognized entries in reactor.xml <options>: "
+ + options.goals().get());
+ }
+ return options;
+ } catch (ParseException e) {
+ throw new IllegalArgumentException(
+ "Failed to parse arguments from reactor.xml <options>: " + e.getMessage(), e.getCause());
Review Comment:
Fixed in 1802942432: changed `e.getCause()` to `e` so the original `ParseException` is preserved as the cause of the `IllegalArgumentException`. The pre-existing occurrences at lines 160/171/206 use `e.getCause()` for historical reasons but this new call is now consistent with preserving the full stack trace.
##########
api/maven-api-spi/src/main/java/org/apache/maven/api/spi/LifecycleProcessor.java:
##########
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.api.spi;
+
+import org.apache.maven.api.Lifecycle;
+import org.apache.maven.api.annotations.Consumer;
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Nonnull;
+import org.apache.maven.api.di.Named;
+
+/**
+ * SPI for transforming Maven lifecycles at session initialisation time.
+ *
+ * <p>A {@code LifecycleProcessor} receives each built-in (or extension-contributed) lifecycle
+ * and may return a modified version of it — for example to inject additional phases into the
+ * lifecycle DAG. Multiple processors are applied in order; each one receives the output of the
+ * previous one.
+ *
+ * <p>Processors are called once per lifecycle per session, after the base lifecycles have been
+ * assembled by the {@link LifecycleProvider}s, so they have access to full session context.
+ *
Review Comment:
Fixed in 1802942432: corrected the Javadoc to state that processors are called each time the lifecycle phase map is rebuilt (not once per session), and that implementations must be stateless and idempotent.
##########
api/maven-api-spi/src/main/java/org/apache/maven/api/spi/LifecycleProcessor.java:
##########
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.api.spi;
+
+import org.apache.maven.api.Lifecycle;
+import org.apache.maven.api.annotations.Consumer;
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Nonnull;
+import org.apache.maven.api.di.Named;
+
+/**
+ * SPI for transforming Maven lifecycles at session initialisation time.
+ *
+ * <p>A {@code LifecycleProcessor} receives each built-in (or extension-contributed) lifecycle
+ * and may return a modified version of it — for example to inject additional phases into the
+ * lifecycle DAG. Multiple processors are applied in order; each one receives the output of the
+ * previous one.
+ *
+ * <p>Processors are called once per lifecycle per session, after the base lifecycles have been
+ * assembled by the {@link LifecycleProvider}s, so they have access to full session context.
+ *
+ * <p>Example — injecting a {@code docker-push} phase after {@code deploy}:
+ * <pre>{@code
+ * @Named
+ * public class DockerLifecycleProcessor implements LifecycleProcessor {
+ * public Lifecycle process(Lifecycle lifecycle) {
+ * if (!Lifecycle.DEFAULT.equals(lifecycle.id())) return lifecycle;
+ * return LifecycleProcessor.withInjectedPhase(lifecycle,
+ * "docker-push", "deploy", null);
Review Comment:
Fixed in 1802942432: replaced the non-existent `LifecycleProcessor.withInjectedPhase()` call with `new PhaseEnrichedLifecycle(...)` using the actual constructor. Added a comment noting that `PhaseEnrichedLifecycle` lives in `impl/maven-core` since it's an implementation class, not part of the public API.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] reactor.xml: named build aliases and lifecycle execution filter [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #13249:
URL: https://github.com/apache/maven/pull/13249#discussion_r4094052340
##########
impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/filter/CoordinatePredicate.java:
##########
@@ -0,0 +1,248 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.lifecycle.internal.filter;
+
+import org.apache.maven.api.MojoExecution;
+import org.apache.maven.api.plugin.descriptor.PluginDescriptor;
+
+/**
+ * A {@link FilterPredicate} that matches {@link MojoExecution}s by plugin coordinate or prefix.
+ *
+ * <h2>Syntax: {@code ([G[:A]]|P)[:v][:g[@e]]}</h2>
+ *
+ * <p>The {@code @} separator for execution ID is compatible with Maven's existing
+ * {@code plugin:version:goal@executionId} notation used in
+ * {@code DefaultLifecycleExecutionPlanCalculator} for goal tasks.
+ *
+ * <p>Matching uses {@link MojoExecution#getDescriptor()} for goal-level fields and
+ * {@link MojoExecution#getPlugin()} for plugin-level coordinates (groupId, artifactId, version,
+ * goal prefix).
+ *
+ * <p>Forms:
+ * <ul>
+ * <li>{@code *} — matches every mojo execution</li>
+ * <li>{@code :A} — any groupId, specific artifactId (e.g. {@code :maven-enforcer-plugin})</li>
+ * <li>{@code G:A} — exact groupId:artifactId (e.g. {@code org.apache.maven.plugins:maven-enforcer-plugin}).
+ * The groupId <strong>must contain at least one {@code '.'}</strong> for this form to be recognised;
+ * groupIds without a dot (e.g. {@code commons-io}) are routed to prefix-based matching instead.
+ * Use the {@code :A} form to match by artifactId only when the groupId has no dot.</li>
+ * <li>{@code P} — plugin prefix (e.g. {@code enforcer}), resolved against
+ * {@link MojoExecution#getMojoDescriptor()} goal prefix</li>
+ * <li>{@code P:v:g} — prefix + version + goal</li>
+ * <li>{@code P:v:g@e} — prefix + version + goal + executionId</li>
Review Comment:
Fixed in 54de779c5a: added note to the `P` bullet in class-level Javadoc that prefixes containing a `.` (e.g. `io.smallrye`) are routed to G:A matching instead of prefix-based matching.
##########
impl/maven-core/src/test/java/org/apache/maven/lifecycle/internal/filter/CoordinatePredicateTest.java:
##########
@@ -0,0 +1,180 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.lifecycle.internal.filter;
+
+import org.apache.maven.api.MojoExecution;
+import org.apache.maven.api.Plugin;
+import org.apache.maven.api.plugin.descriptor.PluginDescriptor;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+/**
+ * Unit tests for {@link CoordinatePredicate}.
+ */
+class CoordinatePredicateTest {
+
+ private static MojoExecution execution(
+ String groupId, String artifactId, String version, String goalPrefix, String goal, String executionId) {
+ PluginDescriptor descriptor = mock(PluginDescriptor.class);
+ when(descriptor.getGroupId()).thenReturn(groupId);
+ when(descriptor.getArtifactId()).thenReturn(artifactId);
+ when(descriptor.getVersion()).thenReturn(version);
+ when(descriptor.getGoalPrefix()).thenReturn(goalPrefix);
+
+ Plugin plugin = mock(Plugin.class);
+ when(plugin.getDescriptor()).thenReturn(descriptor);
+
+ MojoExecution exec = mock(MojoExecution.class);
+ when(exec.getPlugin()).thenReturn(plugin);
+ when(exec.getGoal()).thenReturn(goal);
+ when(exec.getExecutionId()).thenReturn(executionId);
+ return exec;
+ }
+
+ private static MojoExecution surefire() {
+ return execution(
+ "org.apache.maven.plugins", "maven-surefire-plugin", "3.2.5", "surefire", "test", "default-test");
+ }
+
+ private static MojoExecution enforcer() {
+ return execution(
+ "org.apache.maven.plugins", "maven-enforcer-plugin", "3.4.1", "enforcer", "enforce", "enforce-rules");
+ }
+
+ @Test
+ void wildcardMatchesAll() {
+ assertTrue(CoordinatePredicate.parse("*").matches(surefire()));
+ assertTrue(CoordinatePredicate.parse("*").matches(enforcer()));
+ }
+
+ @Test
+ void artifactIdOnlyMatchesByArtifactId() {
+ CoordinatePredicate p = CoordinatePredicate.parse(":maven-surefire-plugin");
+ assertTrue(p.matches(surefire()));
+ assertFalse(p.matches(enforcer()));
+ }
+
+ @Test
+ void colonAloneMatchesAll() {
+ CoordinatePredicate p = CoordinatePredicate.parse(":");
+ assertTrue(p.matches(surefire()));
+ assertTrue(p.matches(enforcer()));
+ }
+
+ @Test
+ void groupAndArtifactMatchesExact() {
+ CoordinatePredicate p = CoordinatePredicate.parse("org.apache.maven.plugins:maven-surefire-plugin");
+ assertTrue(p.matches(surefire()));
+ assertFalse(p.matches(enforcer()));
+ }
+
+ @Test
+ void groupWithWildcardArtifact() {
+ CoordinatePredicate p = CoordinatePredicate.parse("org.apache.maven.plugins:");
+ assertTrue(p.matches(surefire()));
+ assertTrue(p.matches(enforcer()));
+ }
+
+ @Test
+ void groupMismatchDoesNotMatch() {
+ CoordinatePredicate p = CoordinatePredicate.parse("com.example:maven-surefire-plugin");
+ assertFalse(p.matches(surefire()));
+ }
+
+ @Test
+ void prefixMatchesByGoalPrefix() {
+ CoordinatePredicate p = CoordinatePredicate.parse("surefire");
+ assertTrue(p.matches(surefire()));
+ assertFalse(p.matches(enforcer()));
+ }
+
+ @Test
+ void prefixWithGoalMatchesGoal() {
+ CoordinatePredicate p = CoordinatePredicate.parse("surefire::test");
+ assertTrue(p.matches(surefire()));
+ assertFalse(p.matches(enforcer()));
+ }
+
+ @Test
+ void prefixWithVersionGoalAndExecution() {
+ CoordinatePredicate p = CoordinatePredicate.parse("surefire:3.2.5:test@default-test");
+ assertTrue(p.matches(surefire()));
+ }
+
+ @Test
+ void prefixVersionMismatch() {
+ CoordinatePredicate p = CoordinatePredicate.parse("surefire:99.0:test@default-test");
+ assertFalse(p.matches(surefire()));
+ }
+
+ @Test
+ void prefixExecutionIdMismatch() {
+ CoordinatePredicate p = CoordinatePredicate.parse("surefire::test@other-id");
+ assertFalse(p.matches(surefire()));
+ }
+
+ @Test
+ void fullCoordinateWithGoalAndExecution() {
+ CoordinatePredicate p =
+ CoordinatePredicate.parse("org.apache.maven.plugins:maven-surefire-plugin:3.2.5:test@default-test");
+ assertTrue(p.matches(surefire()));
+ }
+
+ @Test
+ void coordinateGoalMismatch() {
+ CoordinatePredicate p =
+ CoordinatePredicate.parse("org.apache.maven.plugins:maven-surefire-plugin:3.2.5:other@default-test");
+ assertFalse(p.matches(surefire()));
+ }
+
+ @Test
+ void noDescriptorDoesNotMatchSpecificPlugin() {
+ MojoExecution exec = mock(MojoExecution.class);
+ when(exec.getPlugin()).thenReturn(null);
+ when(exec.getGoal()).thenReturn("test");
+ when(exec.getExecutionId()).thenReturn("default-test");
+ assertFalse(CoordinatePredicate.parse(":maven-surefire-plugin").matches(exec));
+ }
+
+ @Test
+ void wildcardMatchesEvenWithNullPlugin() {
+ MojoExecution exec = mock(MojoExecution.class);
+ when(exec.getPlugin()).thenReturn(null);
+ assertTrue(CoordinatePredicate.MATCH_ALL.matches(exec));
+ }
+
+ @Test
+ void dotlessGroupIdIsMisroutedToPrefixMode() {
+ // GroupIds without a '.' (e.g. "commons-io") cannot be used in G:A form — they are routed
+ // to prefix-based matching. Document this: the token "commons-io:commons-io" is treated as
+ // prefix="commons-io", version="commons-io" — it will NOT match an execution with groupId
+ // "commons-io". Users should use ":commons-io" (artifactId-only form) instead.
+ MojoExecution exec = execution("commons-io", "commons-io", "2.15.1", "commons-io", "copy", "default");
+ // Prefix "commons-io" won't match goal prefix "commons-io" if the descriptor prefix happens
+ // to be the same, but the version segment "commons-io" will never match "2.15.1" → no match.
+ CoordinatePredicate p = CoordinatePredicate.parse("commons-io:commons-io");
Review Comment:
Fixed in 54de779c5a: the comment now correctly states that the prefix check succeeds (goalPrefix="commons-io" matches) but the version check fails ("commons-io" != "2.15.1"), so the predicate returns false — demonstrating misrouting produces a non-matching predicate, not a silent match.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] reactor.xml: named build aliases and lifecycle phase injections [maven]
Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
gnodet-bot commented on code in PR #13249:
URL: https://github.com/apache/maven/pull/13249#discussion_r4295225850
##########
its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng13249ReactorXmlPhaseInjectionTest.java:
##########
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.it;
+
+import java.nio.file.Path;
+import java.util.List;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Verifies that custom lifecycle phases defined in {@code .mvn/reactor.xml} are injected
+ * into the lifecycle DAG and that plugin goals bound to those phases execute correctly.
+ *
+ * @see <a href="https://github.com/apache/maven/pull/13249">#13249</a>
+ */
+class MavenITmng13249ReactorXmlPhaseInjectionTest extends AbstractMavenIntegrationTestCase {
+
+ /**
+ * A plugin goal bound to a reactor.xml-injected phase must execute when running
+ * through that phase (via {@code verify}).
+ */
+ @Test
+ void pluginBoundToInjectedPhaseExecutesDuringVerify() throws Exception {
+ Path testDir = extractResources("/mng-13249-reactor-xml-phase-injection");
+
+ Verifier verifier = newVerifier(testDir);
+ verifier.setAutoclean(false);
+ verifier.deleteDirectory("target");
+ verifier.addCliArgument("verify");
+ verifier.execute();
+ verifier.verifyErrorFreeLog();
+
+ verifier.verifyFilePresent("target/pre-integration-marker.txt");
+ }
+
+ /**
+ * The injected phase can be used as a direct CLI target.
+ */
+ @Test
+ void injectedPhaseCanBeUsedAsCliTarget() throws Exception {
+ Path testDir = extractResources("/mng-13249-reactor-xml-phase-injection");
+
+ Verifier verifier = newVerifier(testDir);
+ verifier.setAutoclean(false);
+ verifier.deleteDirectory("target");
+ verifier.addCliArgument("pre-integration");
+ verifier.execute();
+ verifier.verifyErrorFreeLog();
+
+ verifier.verifyFilePresent("target/pre-integration-marker.txt");
+ }
+
+ /**
+ * The injected phase respects ordering: running only up to {@code compile}
+ * (which comes before {@code pre-integration}) must NOT trigger the plugin.
+ */
+ @Test
+ void pluginDoesNotRunBeforeInjectedPhase() throws Exception {
+ Path testDir = extractResources("/mng-13249-reactor-xml-phase-injection");
+
+ Verifier verifier = newVerifier(testDir);
+ verifier.setAutoclean(false);
+ verifier.deleteDirectory("target");
+ verifier.addCliArgument("compile");
+ verifier.execute();
+ verifier.verifyErrorFreeLog();
+
+ verifier.verifyFileNotPresent("target/pre-integration-marker.txt");
+ }
+
+ /**
+ * Verify the injected phase runs in the correct position: after unit-test
+ * related phases but before integration-test phases. We check log ordering
+ * by looking for the touch plugin log line relative to known phase markers.
+ */
+ @Test
+ void injectedPhaseRunsInCorrectOrder() throws Exception {
+ Path testDir = extractResources("/mng-13249-reactor-xml-phase-injection");
+
+ Verifier verifier = newVerifier(testDir);
+ verifier.setAutoclean(false);
+ verifier.deleteDirectory("target");
+ verifier.addCliArgument("verify");
+ verifier.execute();
+ verifier.verifyErrorFreeLog();
+
+ List<String> logLines = verifier.loadLogLines();
+
+ int touchIdx = -1;
+ int compileIdx = -1;
+ for (int i = 0; i < logLines.size(); i++) {
+ String line = logLines.get(i);
+ if (line.contains("maven-compiler-plugin") && line.contains("compile")) {
+ if (compileIdx == -1) {
+ compileIdx = i;
+ }
+ }
+ if (line.contains("pre-integration-marker.txt") || line.contains("touch-in-custom-phase")) {
Review Comment:
**Fragile log-line matching in ordering assertion**
`injectedPhaseRunsInCorrectOrder` detects the compiler plugin by scanning for lines containing both `maven-compiler-plugin` and `compile`. This matches any compile-related log line — including the goal resolution line that appears before execution, which fires at plan-calculation time, not at phase-execution time. The index you capture may be earlier than the actual `compile` phase execution, making the ordering assertion trivially pass even if the ordering were wrong.
A more reliable anchor: look for the `[INFO] --- maven-compiler-plugin` execution banner, or check for the `target/classes` directory creation timestamp (file-system ordering) rather than log-line position.
Alternatively, assert ordering via the filesystem: after `verify`, both `target/classes` (compile output) and `target/pre-integration-marker.txt` must exist, and since the touch plugin runs in `pre-integration` which comes after `compile`, any successful `verify` already proves the ordering — the third test (`pluginDoesNotRunBeforeInjectedPhase`) already validates the boundary. A separate log-order test that can produce false positives adds noise, not confidence.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] reactor.xml: named build aliases and lifecycle execution filter [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #13249:
URL: https://github.com/apache/maven/pull/13249#discussion_r4093973449
##########
impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/filter/CoordinatePredicate.java:
##########
@@ -0,0 +1,241 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.lifecycle.internal.filter;
+
+import org.apache.maven.api.MojoExecution;
+import org.apache.maven.api.plugin.descriptor.PluginDescriptor;
+
+/**
+ * A {@link FilterPredicate} that matches {@link MojoExecution}s by plugin coordinate or prefix.
+ *
+ * <h2>Syntax: {@code ([G[:A]]|P)[:v][:g[@e]]}</h2>
+ *
+ * <p>The {@code @} separator for execution ID is compatible with Maven's existing
+ * {@code plugin:version:goal@executionId} notation used in
+ * {@code DefaultLifecycleExecutionPlanCalculator} for goal tasks.
+ *
+ * <p>Matching uses {@link MojoExecution#getDescriptor()} for goal-level fields and
+ * {@link MojoExecution#getPlugin()} for plugin-level coordinates (groupId, artifactId, version,
+ * goal prefix).
+ *
+ * <p>Forms:
+ * <ul>
+ * <li>{@code *} — matches every mojo execution</li>
+ * <li>{@code :A} — any groupId, specific artifactId (e.g. {@code :maven-enforcer-plugin})</li>
+ * <li>{@code G:A} — exact groupId:artifactId (e.g. {@code org.apache.maven.plugins:maven-enforcer-plugin})</li>
+ * <li>{@code P} — plugin prefix (e.g. {@code enforcer}), resolved against
+ * {@link MojoExecution#getMojoDescriptor()} goal prefix</li>
+ * <li>{@code P:v:g} — prefix + version + goal</li>
+ * <li>{@code P:v:g@e} — prefix + version + goal + executionId</li>
+ * </ul>
+ *
+ * <p>When any field is {@code null} or not specified, it is treated as a wildcard (matches any value).
+ *
+ * @since 4.1.0
+ */
+public class CoordinatePredicate implements FilterPredicate {
+
+ /** Wildcard token — matches any value. */
+ private static final String ANY = null;
+
+ private final boolean matchAll;
+ private final String groupId; // null = any, non-null = exact match
+ private final String artifactId; // null = any, non-null = exact match
+ private final String prefix; // null = not used, non-null = match by goal prefix
+ private final String version; // null = any
+ private final String goal; // null = any
+ private final String executionId; // null = any
+
+ /** Wildcard predicate — matches everything. */
+ public static final CoordinatePredicate MATCH_ALL = new CoordinatePredicate();
+
+ private CoordinatePredicate() {
+ this.matchAll = true;
+ this.groupId = ANY;
+ this.artifactId = ANY;
+ this.prefix = ANY;
+ this.version = ANY;
+ this.goal = ANY;
+ this.executionId = ANY;
+ }
+
+ private CoordinatePredicate(
+ String groupId, String artifactId, String prefix, String version, String goal, String executionId) {
+ this.matchAll = false;
+ this.groupId = groupId;
+ this.artifactId = artifactId;
+ this.prefix = prefix;
+ this.version = version;
+ this.goal = goal;
+ this.executionId = executionId;
+ }
+
+ /**
+ * Parses a coordinate predicate from a string token.
+ *
+ * <p>Supported forms:
+ * <ul>
+ * <li>{@code *} — match all</li>
+ * <li>{@code :A} — by artifactId only</li>
+ * <li>{@code G:A} — by groupId:artifactId (token contains {@code :} after first char)</li>
+ * <li>{@code P} — by prefix</li>
+ * <li>{@code P:v:g} — by prefix + version + goal</li>
+ * <li>{@code P:v:g@e} — by prefix + version + goal + executionId</li>
+ * </ul>
+ *
+ * @param token the filter expression token (not {@code null}, not blank)
+ * @return the parsed predicate
+ */
+ public static CoordinatePredicate parse(String token) {
+ if ("*".equals(token)) {
+ return MATCH_ALL;
+ }
+
+ if (token.startsWith(":")) {
+ // :A form — any groupId, specific artifactId, optional :v:g[@e]
+ // e.g. ":maven-enforcer-plugin" or ":maven-enforcer-plugin:3.0.0:enforce@enforce-id"
+ String rest = token.substring(1); // remove leading ':'
+ String[] parts = rest.split(":", 3);
+ String artifactId = emptyToNull(parts[0]);
+ String version = parts.length > 1 ? emptyToNull(parts[1]) : null;
+ String goalAndExec = parts.length > 2 ? parts[2] : null;
+ String[] ge = splitGoalExecution(goalAndExec);
+ return new CoordinatePredicate(ANY, artifactId, ANY, version, ge[0], ge[1]);
+ }
+
+ // Try to detect G:A form: the token contains ':' AND the part before the first ':' looks like a
+ // groupId (contains a '.' suggesting it's a Java package name like org.apache.maven).
+ // This distinguishes "org.apache.maven.plugins:maven-enforcer-plugin" from "enforcer:3.1.0:enforce".
+ int firstColon = token.indexOf(':');
+ if (firstColon > 0 && token.substring(0, firstColon).contains(".")) {
Review Comment:
Fixed in 63ce016f02f6d96b684d9b778f2a0f1afa821db8: documented the dot-in-groupId constraint in the class Javadoc, parse() Javadoc, and inline comments. Added a test (dotlessGroupIdIsMisroutedToPrefixMode) that exposes the misrouting and demonstrates the :A workaround.
##########
impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/filter/CoordinatePredicate.java:
##########
@@ -0,0 +1,241 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.lifecycle.internal.filter;
+
+import org.apache.maven.api.MojoExecution;
+import org.apache.maven.api.plugin.descriptor.PluginDescriptor;
+
+/**
+ * A {@link FilterPredicate} that matches {@link MojoExecution}s by plugin coordinate or prefix.
+ *
+ * <h2>Syntax: {@code ([G[:A]]|P)[:v][:g[@e]]}</h2>
+ *
+ * <p>The {@code @} separator for execution ID is compatible with Maven's existing
+ * {@code plugin:version:goal@executionId} notation used in
+ * {@code DefaultLifecycleExecutionPlanCalculator} for goal tasks.
+ *
+ * <p>Matching uses {@link MojoExecution#getDescriptor()} for goal-level fields and
+ * {@link MojoExecution#getPlugin()} for plugin-level coordinates (groupId, artifactId, version,
+ * goal prefix).
+ *
+ * <p>Forms:
+ * <ul>
+ * <li>{@code *} — matches every mojo execution</li>
+ * <li>{@code :A} — any groupId, specific artifactId (e.g. {@code :maven-enforcer-plugin})</li>
+ * <li>{@code G:A} — exact groupId:artifactId (e.g. {@code org.apache.maven.plugins:maven-enforcer-plugin})</li>
+ * <li>{@code P} — plugin prefix (e.g. {@code enforcer}), resolved against
+ * {@link MojoExecution#getMojoDescriptor()} goal prefix</li>
+ * <li>{@code P:v:g} — prefix + version + goal</li>
+ * <li>{@code P:v:g@e} — prefix + version + goal + executionId</li>
+ * </ul>
+ *
+ * <p>When any field is {@code null} or not specified, it is treated as a wildcard (matches any value).
+ *
+ * @since 4.1.0
+ */
+public class CoordinatePredicate implements FilterPredicate {
+
+ /** Wildcard token — matches any value. */
+ private static final String ANY = null;
+
+ private final boolean matchAll;
+ private final String groupId; // null = any, non-null = exact match
+ private final String artifactId; // null = any, non-null = exact match
+ private final String prefix; // null = not used, non-null = match by goal prefix
+ private final String version; // null = any
+ private final String goal; // null = any
+ private final String executionId; // null = any
+
+ /** Wildcard predicate — matches everything. */
+ public static final CoordinatePredicate MATCH_ALL = new CoordinatePredicate();
+
+ private CoordinatePredicate() {
+ this.matchAll = true;
+ this.groupId = ANY;
+ this.artifactId = ANY;
+ this.prefix = ANY;
+ this.version = ANY;
+ this.goal = ANY;
+ this.executionId = ANY;
+ }
+
+ private CoordinatePredicate(
+ String groupId, String artifactId, String prefix, String version, String goal, String executionId) {
+ this.matchAll = false;
+ this.groupId = groupId;
+ this.artifactId = artifactId;
+ this.prefix = prefix;
+ this.version = version;
+ this.goal = goal;
+ this.executionId = executionId;
+ }
+
+ /**
+ * Parses a coordinate predicate from a string token.
+ *
+ * <p>Supported forms:
+ * <ul>
+ * <li>{@code *} — match all</li>
+ * <li>{@code :A} — by artifactId only</li>
+ * <li>{@code G:A} — by groupId:artifactId (token contains {@code :} after first char)</li>
+ * <li>{@code P} — by prefix</li>
+ * <li>{@code P:v:g} — by prefix + version + goal</li>
+ * <li>{@code P:v:g@e} — by prefix + version + goal + executionId</li>
+ * </ul>
+ *
+ * @param token the filter expression token (not {@code null}, not blank)
+ * @return the parsed predicate
+ */
+ public static CoordinatePredicate parse(String token) {
+ if ("*".equals(token)) {
+ return MATCH_ALL;
+ }
+
+ if (token.startsWith(":")) {
+ // :A form — any groupId, specific artifactId, optional :v:g[@e]
+ // e.g. ":maven-enforcer-plugin" or ":maven-enforcer-plugin:3.0.0:enforce@enforce-id"
+ String rest = token.substring(1); // remove leading ':'
+ String[] parts = rest.split(":", 3);
+ String artifactId = emptyToNull(parts[0]);
+ String version = parts.length > 1 ? emptyToNull(parts[1]) : null;
+ String goalAndExec = parts.length > 2 ? parts[2] : null;
+ String[] ge = splitGoalExecution(goalAndExec);
+ return new CoordinatePredicate(ANY, artifactId, ANY, version, ge[0], ge[1]);
+ }
+
+ // Try to detect G:A form: the token contains ':' AND the part before the first ':' looks like a
+ // groupId (contains a '.' suggesting it's a Java package name like org.apache.maven).
+ // This distinguishes "org.apache.maven.plugins:maven-enforcer-plugin" from "enforcer:3.1.0:enforce".
+ int firstColon = token.indexOf(':');
+ if (firstColon > 0 && token.substring(0, firstColon).contains(".")) {
Review Comment:
Fixed in 63ce016f02f6d96b684d9b778f2a0f1afa821db8: documented the dot-in-groupId constraint in the class Javadoc, parse() Javadoc, and inline comments. Added a test (dotlessGroupIdIsMisroutedToPrefixMode) that exposes the misrouting and demonstrates the :A workaround.
##########
impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/filter/MojoExecutionFilter.java:
##########
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.lifecycle.internal.filter;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.apache.maven.api.Constants;
+import org.apache.maven.api.MojoExecution;
+import org.apache.maven.internal.impl.DefaultMojoExecution;
+
+/**
+ * Parses the {@code maven.lifecycle.filter} user property value into a list of {@link FilterPredicate}s,
+ * and applies them at mojo execution time in {@code MojoExecutor}.
+ *
+ * <p>The property value is a comma-separated list of predicates, OR-ed together:
+ * a mojo execution matching <em>any</em> predicate is skipped (a {@code MojoSkipped} event is fired).
+ *
+ * <p>Supported predicate forms:
+ * <ul>
+ * <li>{@code *} — skip all mojo executions</li>
+ * <li>{@code :A} — skip by artifactId (e.g. {@code :maven-enforcer-plugin})</li>
+ * <li>{@code G:A} — skip by groupId:artifactId</li>
+ * <li>{@code P} — skip by plugin prefix (e.g. {@code enforcer})</li>
+ * <li>{@code P:v:g} — skip by prefix:version:goal</li>
+ * <li>{@code P:v:g@e} — skip by prefix:version:goal@executionId</li>
+ * <li>{@code phase(name)} — skip all mojos bound to the named phase</li>
+ * </ul>
+ *
+ * @since 4.1.0
+ */
+public class MojoExecutionFilter {
+
+ /** The name of the user property that activates the filter. */
+ public static final String PROPERTY_NAME = Constants.MAVEN_LIFECYCLE_FILTER;
+
+ private MojoExecutionFilter() {
+ // utility class
+ }
+
+ /**
+ * Parses a filter expression string into a list of {@link FilterPredicate}s.
+ *
+ * @param expression the comma-separated filter expression (may be {@code null} or blank)
+ * @return list of parsed predicates; empty if the expression is absent or blank
+ */
+ public static List<FilterPredicate> parse(String expression) {
+ if (expression == null || expression.isBlank()) {
+ return List.of();
+ }
+ List<FilterPredicate> predicates = new ArrayList<>();
+ for (String token : expression.split(",")) {
+ token = token.strip();
+ if (token.isEmpty()) {
+ continue;
+ }
+ predicates.add(parseToken(token));
+ }
+ return List.copyOf(predicates);
+ }
+
+ private static FilterPredicate parseToken(String token) {
+ if (token.startsWith("phase(") && token.endsWith(")")) {
+ String phaseName = token.substring("phase(".length(), token.length() - 1);
Review Comment:
Fixed in 63ce016f02f6d96b684d9b778f2a0f1afa821db8: added a blank check before constructing PhasePredicate — throws IllegalArgumentException for phase() and phase( ). Added emptyPhaseParensThrows and blankPhaseParensThrows tests.
##########
impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/filter/MojoExecutionFilter.java:
##########
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.lifecycle.internal.filter;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.apache.maven.api.Constants;
+import org.apache.maven.api.MojoExecution;
+import org.apache.maven.internal.impl.DefaultMojoExecution;
+
+/**
+ * Parses the {@code maven.lifecycle.filter} user property value into a list of {@link FilterPredicate}s,
+ * and applies them at mojo execution time in {@code MojoExecutor}.
+ *
+ * <p>The property value is a comma-separated list of predicates, OR-ed together:
+ * a mojo execution matching <em>any</em> predicate is skipped (a {@code MojoSkipped} event is fired).
+ *
+ * <p>Supported predicate forms:
+ * <ul>
+ * <li>{@code *} — skip all mojo executions</li>
+ * <li>{@code :A} — skip by artifactId (e.g. {@code :maven-enforcer-plugin})</li>
+ * <li>{@code G:A} — skip by groupId:artifactId</li>
+ * <li>{@code P} — skip by plugin prefix (e.g. {@code enforcer})</li>
+ * <li>{@code P:v:g} — skip by prefix:version:goal</li>
+ * <li>{@code P:v:g@e} — skip by prefix:version:goal@executionId</li>
+ * <li>{@code phase(name)} — skip all mojos bound to the named phase</li>
+ * </ul>
+ *
+ * @since 4.1.0
+ */
+public class MojoExecutionFilter {
+
+ /** The name of the user property that activates the filter. */
+ public static final String PROPERTY_NAME = Constants.MAVEN_LIFECYCLE_FILTER;
+
+ private MojoExecutionFilter() {
+ // utility class
+ }
+
+ /**
+ * Parses a filter expression string into a list of {@link FilterPredicate}s.
+ *
+ * @param expression the comma-separated filter expression (may be {@code null} or blank)
+ * @return list of parsed predicates; empty if the expression is absent or blank
+ */
+ public static List<FilterPredicate> parse(String expression) {
+ if (expression == null || expression.isBlank()) {
+ return List.of();
+ }
+ List<FilterPredicate> predicates = new ArrayList<>();
+ for (String token : expression.split(",")) {
+ token = token.strip();
+ if (token.isEmpty()) {
+ continue;
+ }
+ predicates.add(parseToken(token));
+ }
+ return List.copyOf(predicates);
+ }
+
+ private static FilterPredicate parseToken(String token) {
+ if (token.startsWith("phase(") && token.endsWith(")")) {
+ String phaseName = token.substring("phase(".length(), token.length() - 1);
+ return new PhasePredicate(phaseName);
Review Comment:
Fixed in 63ce016f02f6d96b684d9b778f2a0f1afa821db8: added a blank check before constructing PhasePredicate — throws IllegalArgumentException for phase() and phase( ). Added emptyPhaseParensThrows and blankPhaseParensThrows tests.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] reactor.xml: named build aliases and lifecycle execution filter [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #13249:
URL: https://github.com/apache/maven/pull/13249#discussion_r4086269048
##########
api/maven-api-spi/src/main/java/org/apache/maven/api/spi/LifecycleProcessor.java:
##########
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.api.spi;
+
+import org.apache.maven.api.Lifecycle;
+import org.apache.maven.api.annotations.Consumer;
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Nonnull;
+import org.apache.maven.api.di.Named;
+
+/**
+ * SPI for transforming Maven lifecycles at session initialisation time.
+ *
+ * <p>A {@code LifecycleProcessor} receives each built-in (or extension-contributed) lifecycle
+ * and may return a modified version of it — for example to inject additional phases into the
+ * lifecycle DAG. Multiple processors are applied in order; each one receives the output of the
+ * previous one.
+ *
+ * <p>Processors are called each time the lifecycle phase map is rebuilt (potentially multiple
+ * times per build). Implementations must be stateless and idempotent — the same lifecycle
+ * input must always produce the same output. They receive the output of the previous processor
+ * in the chain.
+ *
+ * <p>Example — injecting a {@code docker-push} phase after {@code deploy}:
+ * <pre>{@code
+ * @Named
+ * public class DockerLifecycleProcessor implements LifecycleProcessor {
+ * public Lifecycle process(Lifecycle lifecycle) {
+ * if (!Lifecycle.DEFAULT.equals(lifecycle.id())) return lifecycle;
+ * // Use PhaseEnrichedLifecycle (from impl/maven-core) to inject custom phases
+ * return new PhaseEnrichedLifecycle(lifecycle,
+ * List.of(new PhaseEnrichedLifecycle.InjectedPhase("docker-push", null, "deploy", null)));
Review Comment:
Fixed in 1e9fc25951: removed the code example entirely to avoid leaking `PhaseEnrichedLifecycle` (an internal impl class) from the SPI Javadoc. Replaced with a prose reference to `ReactorXmlLifecycleProcessor` as the concrete reference implementation. Also fixed the javadoc `unknown tag: arg` CI failure caused by `<arg>` in the `.mdo` description being rendered as a raw Javadoc tag.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] reactor.xml: named build aliases and lifecycle execution filter [maven]
Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
gnodet-bot commented on code in PR #13249:
URL: https://github.com/apache/maven/pull/13249#discussion_r4086302873
##########
api/maven-api-spi/src/main/java/org/apache/maven/api/spi/LifecycleProcessor.java:
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.api.spi;
+
+import org.apache.maven.api.Lifecycle;
+import org.apache.maven.api.annotations.Consumer;
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Nonnull;
+import org.apache.maven.api.di.Named;
+
+/**
+ * SPI for transforming Maven lifecycles at session initialisation time.
+ *
+ * <p>A {@code LifecycleProcessor} receives each built-in (or extension-contributed) lifecycle
+ * and may return a modified version of it — for example to inject additional phases into the
+ * lifecycle DAG. Multiple processors are applied in order; each one receives the output of the
+ * previous one.
+ *
+ * <p>Processors are called each time the lifecycle phase map is rebuilt (potentially multiple
+ * times per build). Implementations must be stateless and idempotent — the same lifecycle
+ * input must always produce the same output. They receive the output of the previous processor
+ * in the chain.
+ *
+ * <p>See {@code ReactorXmlLifecycleProcessor} in {@code impl/maven-core} for a concrete
+ * reference implementation that injects phases from {@code .mvn/reactor.xml}.
Review Comment:
⚠️ **Wrong module name in Javadoc reference**
`ReactorXmlLifecycleProcessor` lives in `impl/maven-cli` (`org.apache.maven.cling.invoker.mvn`), not in `impl/maven-core`. Anyone searching for the reference implementation in `impl/maven-core` won't find it.
```suggestion
* <p>See {@code ReactorXmlLifecycleProcessor} in {@code impl/maven-cli} for a concrete
```
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] reactor.xml: named build aliases and lifecycle execution filter [maven]
Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
gnodet-bot commented on code in PR #13249:
URL: https://github.com/apache/maven/pull/13249#discussion_r4090863415
##########
impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/filter/MojoExecutionFilter.java:
##########
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.lifecycle.internal.filter;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.apache.maven.api.Constants;
+import org.apache.maven.api.MojoExecution;
+import org.apache.maven.internal.impl.DefaultMojoExecution;
+
+/**
+ * Parses the {@code maven.lifecycle.filter} user property value into a list of {@link FilterPredicate}s,
+ * and applies them at mojo execution time in {@code MojoExecutor}.
+ *
+ * <p>The property value is a comma-separated list of predicates, OR-ed together:
+ * a mojo execution matching <em>any</em> predicate is skipped (a {@code MojoSkipped} event is fired).
+ *
+ * <p>Supported predicate forms:
+ * <ul>
+ * <li>{@code *} — skip all mojo executions</li>
+ * <li>{@code :A} — skip by artifactId (e.g. {@code :maven-enforcer-plugin})</li>
+ * <li>{@code G:A} — skip by groupId:artifactId</li>
+ * <li>{@code P} — skip by plugin prefix (e.g. {@code enforcer})</li>
+ * <li>{@code P:v:g} — skip by prefix:version:goal</li>
+ * <li>{@code P:v:g@e} — skip by prefix:version:goal@executionId</li>
+ * <li>{@code phase(name)} — skip all mojos bound to the named phase</li>
+ * </ul>
+ *
+ * @since 4.1.0
+ */
+public class MojoExecutionFilter {
+
+ /** The name of the user property that activates the filter. */
+ public static final String PROPERTY_NAME = Constants.MAVEN_LIFECYCLE_FILTER;
+
+ private MojoExecutionFilter() {
+ // utility class
+ }
+
+ /**
+ * Parses a filter expression string into a list of {@link FilterPredicate}s.
+ *
+ * @param expression the comma-separated filter expression (may be {@code null} or blank)
+ * @return list of parsed predicates; empty if the expression is absent or blank
+ */
+ public static List<FilterPredicate> parse(String expression) {
+ if (expression == null || expression.isBlank()) {
+ return List.of();
+ }
+ List<FilterPredicate> predicates = new ArrayList<>();
+ for (String token : expression.split(",")) {
+ token = token.strip();
+ if (token.isEmpty()) {
+ continue;
+ }
+ predicates.add(parseToken(token));
+ }
+ return List.copyOf(predicates);
+ }
+
+ private static FilterPredicate parseToken(String token) {
+ if (token.startsWith("phase(") && token.endsWith(")")) {
+ String phaseName = token.substring("phase(".length(), token.length() - 1);
Review Comment:
⚠️ **`phase()` with empty parens silently creates a non-matching predicate**
When receives `phase()` (empty parens), `phaseName` is the empty string ``. This is passed unchecked to `new PhasePredicate()`, which stores it and evaluates `.equals(execution.getLifecyclePhase())` — always `false` since no mojo has an empty phase name. The user gets no error and the filter silently does nothing.
Add a blank check before constructing `PhasePredicate`:
##########
impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/filter/CoordinatePredicate.java:
##########
@@ -0,0 +1,241 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.lifecycle.internal.filter;
+
+import org.apache.maven.api.MojoExecution;
+import org.apache.maven.api.plugin.descriptor.PluginDescriptor;
+
+/**
+ * A {@link FilterPredicate} that matches {@link MojoExecution}s by plugin coordinate or prefix.
+ *
+ * <h2>Syntax: {@code ([G[:A]]|P)[:v][:g[@e]]}</h2>
+ *
+ * <p>The {@code @} separator for execution ID is compatible with Maven's existing
+ * {@code plugin:version:goal@executionId} notation used in
+ * {@code DefaultLifecycleExecutionPlanCalculator} for goal tasks.
+ *
+ * <p>Matching uses {@link MojoExecution#getDescriptor()} for goal-level fields and
+ * {@link MojoExecution#getPlugin()} for plugin-level coordinates (groupId, artifactId, version,
+ * goal prefix).
+ *
+ * <p>Forms:
+ * <ul>
+ * <li>{@code *} — matches every mojo execution</li>
+ * <li>{@code :A} — any groupId, specific artifactId (e.g. {@code :maven-enforcer-plugin})</li>
+ * <li>{@code G:A} — exact groupId:artifactId (e.g. {@code org.apache.maven.plugins:maven-enforcer-plugin})</li>
+ * <li>{@code P} — plugin prefix (e.g. {@code enforcer}), resolved against
+ * {@link MojoExecution#getMojoDescriptor()} goal prefix</li>
+ * <li>{@code P:v:g} — prefix + version + goal</li>
+ * <li>{@code P:v:g@e} — prefix + version + goal + executionId</li>
+ * </ul>
+ *
+ * <p>When any field is {@code null} or not specified, it is treated as a wildcard (matches any value).
+ *
+ * @since 4.1.0
+ */
+public class CoordinatePredicate implements FilterPredicate {
+
+ /** Wildcard token — matches any value. */
+ private static final String ANY = null;
+
+ private final boolean matchAll;
+ private final String groupId; // null = any, non-null = exact match
+ private final String artifactId; // null = any, non-null = exact match
+ private final String prefix; // null = not used, non-null = match by goal prefix
+ private final String version; // null = any
+ private final String goal; // null = any
+ private final String executionId; // null = any
+
+ /** Wildcard predicate — matches everything. */
+ public static final CoordinatePredicate MATCH_ALL = new CoordinatePredicate();
+
+ private CoordinatePredicate() {
+ this.matchAll = true;
+ this.groupId = ANY;
+ this.artifactId = ANY;
+ this.prefix = ANY;
+ this.version = ANY;
+ this.goal = ANY;
+ this.executionId = ANY;
+ }
+
+ private CoordinatePredicate(
+ String groupId, String artifactId, String prefix, String version, String goal, String executionId) {
+ this.matchAll = false;
+ this.groupId = groupId;
+ this.artifactId = artifactId;
+ this.prefix = prefix;
+ this.version = version;
+ this.goal = goal;
+ this.executionId = executionId;
+ }
+
+ /**
+ * Parses a coordinate predicate from a string token.
+ *
+ * <p>Supported forms:
+ * <ul>
+ * <li>{@code *} — match all</li>
+ * <li>{@code :A} — by artifactId only</li>
+ * <li>{@code G:A} — by groupId:artifactId (token contains {@code :} after first char)</li>
+ * <li>{@code P} — by prefix</li>
+ * <li>{@code P:v:g} — by prefix + version + goal</li>
+ * <li>{@code P:v:g@e} — by prefix + version + goal + executionId</li>
+ * </ul>
+ *
+ * @param token the filter expression token (not {@code null}, not blank)
+ * @return the parsed predicate
+ */
+ public static CoordinatePredicate parse(String token) {
+ if ("*".equals(token)) {
+ return MATCH_ALL;
+ }
+
+ if (token.startsWith(":")) {
+ // :A form — any groupId, specific artifactId, optional :v:g[@e]
+ // e.g. ":maven-enforcer-plugin" or ":maven-enforcer-plugin:3.0.0:enforce@enforce-id"
+ String rest = token.substring(1); // remove leading ':'
+ String[] parts = rest.split(":", 3);
+ String artifactId = emptyToNull(parts[0]);
+ String version = parts.length > 1 ? emptyToNull(parts[1]) : null;
+ String goalAndExec = parts.length > 2 ? parts[2] : null;
+ String[] ge = splitGoalExecution(goalAndExec);
+ return new CoordinatePredicate(ANY, artifactId, ANY, version, ge[0], ge[1]);
+ }
+
+ // Try to detect G:A form: the token contains ':' AND the part before the first ':' looks like a
+ // groupId (contains a '.' suggesting it's a Java package name like org.apache.maven).
+ // This distinguishes "org.apache.maven.plugins:maven-enforcer-plugin" from "enforcer:3.1.0:enforce".
+ int firstColon = token.indexOf(':');
+ if (firstColon > 0 && token.substring(0, firstColon).contains(".")) {
Review Comment:
⚠️ **G:A detection heuristic is fragile — silent misclassification for dotless groupIds and dotted prefixes**
The heuristic uses `token.substring(0, firstColon).contains(".")` to distinguish a `G:A` form from a `P:v:g` form. This fails silently in two common cases:
1. **GroupId without a dot** — e.g. `commons-io:commons-io` has no dot before the colon. It gets misrouted to prefix-based matching, which will never match (no plugin has that goal prefix), silently skipping the skip.
2. **Goal prefix that contains a dot** — e.g. `io.smallrye:enforce` would be misrouted to G:A mode when the user intended prefix-based matching.
The consequence is silent: the filter expression appears to be accepted but does nothing. This is a correctness bug, not a crash.
A more robust approach is to use the actual colon count as the primary discriminator: a leading is artifact-only, two or more colons with no leading is ambiguous between G:A and P:v:g, but that ambiguity can be resolved by requiring users to use the full form (with groupId always containing a dot — document this constraint) or by adding an explicit prefix sentinel. At minimum, document the limitation clearly in the Javadoc.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] reactor.xml: named build aliases and lifecycle execution filter [maven]
Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
gnodet-bot commented on code in PR #13249:
URL: https://github.com/apache/maven/pull/13249#discussion_r4083969012
##########
api/maven-api-core/src/main/mdo/reactor.mdo:
##########
@@ -0,0 +1,249 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+
+-->
+
+<model xmlns="http://codehaus-plexus.github.io/MODELLO/2.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://codehaus-plexus.github.io/MODELLO/2.0.0 https://codehaus-plexus.github.io/modello/xsd/modello-2.0.0.xsd"
+ xml.namespace="http://maven.apache.org/REACTOR/${version}"
+ xml.schemaLocation="https://maven.apache.org/xsd/reactor-${version}.xsd">
+
+ <id>reactor</id>
+ <name>ReactorConfig</name>
+ <description><![CDATA[
+ <p>This is a reference for the Reactor Configuration descriptor.</p>
+ <p>The default location for the Reactor Configuration descriptor file is <code>${maven.projectBasedir}/.mvn/reactor.xml</code></p>
+ <p>It consolidates reactor-scoped build configuration previously split across <code>.mvn/maven.config</code>
+ (CLI flag defaults) and <code>.mvn/extensions.xml</code> (core extensions), and adds support for
+ named build aliases and reactor-level lifecycle phase injection.</p>
+ ]]></description>
+
+ <defaults>
+ <default>
+ <key>package</key>
+ <value>org.apache.maven.api.reactor</value>
+ </default>
+ </defaults>
+
+ <classes>
+ <class rootElement="true" xml.tagName="reactor" xsd.compositor="sequence">
+ <name>ReactorConfig</name>
+ <description>Reactor-scoped build configuration for a Maven project.</description>
+ <version>1.0.0+</version>
+ <fields>
+ <field>
+ <name>options</name>
+ <description>CLI options to prepend to every invocation. Replaces .mvn/maven.config when present.
+ Expressed as whitespace-separated, quote-aware inline text (e.g. {@code -T4 --no-transfer-progress}).
+ For options with spaces in their values, use the structured form via {@link #getOptionArgs()}.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <type>String</type>
+ </field>
+ <field>
+ <name>optionArgs</name>
+ <description>Structured CLI options — each <arg> element is one option token.
+ Use instead of {@link #getOptions()} when option values contain spaces.
+ Mutually exclusive with options.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <association xml.itemsStyle="flat" xml.tagName="arg">
+ <type>String</type>
+ <multiplicity>*</multiplicity>
+ </association>
+ </field>
+ <field>
+ <name>extensions</name>
+ <description>Core extensions to load. Replaces .mvn/extensions.xml when present.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <association xml.itemsStyle="flat" xml.tagName="extension">
+ <type>ReactorExtension</type>
+ <multiplicity>*</multiplicity>
+ </association>
+ </field>
+ <field>
+ <name>aliases</name>
+ <description>Named build aliases. The first non-flag argument on the command line is matched against
+ alias names; if found it is replaced by the alias expansion.</description>
Review Comment:
⚠️ **[Previous finding — NOT addressed]** Still says "first non-flag argument" but `MavenParser.expandAlias()` expands **every** argument that matches, not just the first one. This description ends up in the generated XSD and Javadoc and will confuse users who define multiple aliases (e.g. `mvn ci rel` where both are aliases — both expand).
```suggestion
<description>Named build aliases. Every argument on the command line is checked against alias
names in order; each match is replaced in-place by the alias expansion tokens. Not recursive.</description>
```
##########
api/maven-api-core/src/main/mdo/reactor.mdo:
##########
@@ -0,0 +1,249 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+
+-->
+
+<model xmlns="http://codehaus-plexus.github.io/MODELLO/2.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://codehaus-plexus.github.io/MODELLO/2.0.0 https://codehaus-plexus.github.io/modello/xsd/modello-2.0.0.xsd"
+ xml.namespace="http://maven.apache.org/REACTOR/${version}"
+ xml.schemaLocation="https://maven.apache.org/xsd/reactor-${version}.xsd">
+
+ <id>reactor</id>
+ <name>ReactorConfig</name>
+ <description><![CDATA[
+ <p>This is a reference for the Reactor Configuration descriptor.</p>
+ <p>The default location for the Reactor Configuration descriptor file is <code>${maven.projectBasedir}/.mvn/reactor.xml</code></p>
+ <p>It consolidates reactor-scoped build configuration previously split across <code>.mvn/maven.config</code>
+ (CLI flag defaults) and <code>.mvn/extensions.xml</code> (core extensions), and adds support for
+ named build aliases and reactor-level lifecycle phase injection.</p>
+ ]]></description>
+
+ <defaults>
+ <default>
+ <key>package</key>
+ <value>org.apache.maven.api.reactor</value>
+ </default>
+ </defaults>
+
+ <classes>
+ <class rootElement="true" xml.tagName="reactor" xsd.compositor="sequence">
+ <name>ReactorConfig</name>
+ <description>Reactor-scoped build configuration for a Maven project.</description>
+ <version>1.0.0+</version>
+ <fields>
+ <field>
+ <name>options</name>
+ <description>CLI options to prepend to every invocation. Replaces .mvn/maven.config when present.
+ Expressed as whitespace-separated, quote-aware inline text (e.g. {@code -T4 --no-transfer-progress}).
+ For options with spaces in their values, use the structured form via {@link #getOptionArgs()}.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <type>String</type>
+ </field>
+ <field>
+ <name>optionArgs</name>
+ <description>Structured CLI options — each <arg> element is one option token.
+ Use instead of {@link #getOptions()} when option values contain spaces.
+ Mutually exclusive with options.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <association xml.itemsStyle="flat" xml.tagName="arg">
+ <type>String</type>
+ <multiplicity>*</multiplicity>
+ </association>
+ </field>
+ <field>
+ <name>extensions</name>
+ <description>Core extensions to load. Replaces .mvn/extensions.xml when present.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <association xml.itemsStyle="flat" xml.tagName="extension">
+ <type>ReactorExtension</type>
+ <multiplicity>*</multiplicity>
+ </association>
+ </field>
+ <field>
+ <name>aliases</name>
+ <description>Named build aliases. The first non-flag argument on the command line is matched against
+ alias names; if found it is replaced by the alias expansion.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <association xml.itemsStyle="flat" xml.tagName="alias">
+ <type>Alias</type>
+ <multiplicity>*</multiplicity>
+ </association>
+ </field>
+ <field>
+ <name>phases</name>
+ <description>Custom lifecycle phase injections. Phases are registered in the lifecycle DAG
+ with the declared after/before edges.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <association xml.itemsStyle="flat" xml.tagName="phase">
+ <type>PhaseInjection</type>
+ <multiplicity>*</multiplicity>
+ </association>
+ </field>
+ </fields>
+ </class>
+
+ <class xml.tagName="extension">
+ <name>ReactorExtension</name>
+ <description>Describes a core extension to load.</description>
+ <version>1.0.0+</version>
+ <fields>
+ <field>
+ <name>groupId</name>
+ <description>The group ID of the extension's artifact.</description>
+ <version>1.0.0+</version>
+ <required>true</required>
+ <type>String</type>
+ </field>
+ <field>
+ <name>artifactId</name>
+ <description>The artifact ID of the extension.</description>
+ <version>1.0.0+</version>
+ <required>true</required>
+ <type>String</type>
+ </field>
+ <field>
+ <name>version</name>
+ <description>The version of the extension.</description>
+ <version>1.0.0+</version>
+ <required>true</required>
+ <type>String</type>
+ </field>
+ <field>
+ <name>classLoadingStrategy</name>
+ <description>The class loading strategy: 'self-first' (the default), 'parent-first' (loads classes from the
+ parent, then from the extension) or 'plugin' (follows the rules from extensions defined as plugins).</description>
+ <version>1.0.0+</version>
+ <defaultValue>self-first</defaultValue>
+ <required>false</required>
+ <type>String</type>
+ </field>
+ <field>
+ <name>configuration</name>
+ <description>Extension-specific configuration passed to the extension during initialization.
+ The structure is extension-defined.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <type>DOM</type>
+ </field>
+ </fields>
+ <codeSegments>
+ <codeSegment>
+ <version>1.0.0+</version>
+ <code>
+ <![CDATA[
+ /**
+ * Gets the identifier of the extension.
+ *
+ * @return The extension id in the form {@code <groupId>:<artifactId>:<version>}, never {@code null}.
+ */
+ public String getId() {
+ return (getGroupId() == null ? "[unknown-group-id]" : getGroupId())
+ + ":" + (getArtifactId() == null ? "[unknown-artifact-id]" : getArtifactId())
+ + ":" + (getVersion() == null ? "[unknown-version]" : getVersion());
+ }
+ ]]>
+ </code>
+ </codeSegment>
+ </codeSegments>
+ </class>
+
+ <class xml.tagName="alias">
+ <name>Alias</name>
+ <description>A named build alias. When the first non-flag argument matches the alias name,
+ it is replaced by the alias expansion. Supports both inline text (shorthand, whitespace-separated)
Review Comment:
⚠️ **[Previous finding — NOT addressed]** Same "first non-flag argument" description on the `Alias` class itself. The `multipleAliasesAllExpanded` test demonstrates the real behaviour: every matching argument is expanded.
```suggestion
<description>A named build alias. Every argument on the command line is checked against alias names;
when a match is found it is replaced in-place by the alias expansion tokens. Supports both inline
text (shorthand, whitespace-separated) and structured <arg> elements for values that contain spaces.</description>
```
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/MavenParser.java:
##########
@@ -76,6 +174,21 @@ protected MavenOptions parseMavenAtFileOptions(Path atFile) {
}
}
+ protected MavenOptions parseMavenReactorOptions(List<String> args) {
+ try {
+ MavenOptions options = parseArgs("reactor.xml", args);
+ if (options.goals().isPresent()) {
+ // <options> can only contain options, not goals/phases
+ throw new IllegalArgumentException("Unrecognized entries in reactor.xml <options>: "
+ + options.goals().get());
+ }
+ return options;
+ } catch (ParseException e) {
+ throw new IllegalArgumentException(
+ "Failed to parse arguments from reactor.xml <options>: " + e.getMessage(), e.getCause());
Review Comment:
⚠️ **[Previous finding — NOT addressed]** `e.getCause()` returns `null` for `ParseException` (it has no cause), so the `IllegalArgumentException` is constructed with a `null` cause. The original exception is silently dropped — only its message survives in the stack trace.
If intentional (matching the pre-existing pattern in `parseMavenCliOptions`/`parseMavenAtFileOptions`), add a comment explaining why `e.getCause()` is preferred over `e`. If not intentional:
```suggestion
throw new IllegalArgumentException(
"Failed to parse arguments from reactor.xml <options>: " + e.getMessage(), e);
```
##########
api/maven-api-spi/src/main/java/org/apache/maven/api/spi/LifecycleProcessor.java:
##########
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.api.spi;
+
+import org.apache.maven.api.Lifecycle;
+import org.apache.maven.api.annotations.Consumer;
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Nonnull;
+import org.apache.maven.api.di.Named;
+
+/**
+ * SPI for transforming Maven lifecycles at session initialisation time.
+ *
+ * <p>A {@code LifecycleProcessor} receives each built-in (or extension-contributed) lifecycle
+ * and may return a modified version of it — for example to inject additional phases into the
+ * lifecycle DAG. Multiple processors are applied in order; each one receives the output of the
+ * previous one.
+ *
+ * <p>Processors are called once per lifecycle per session, after the base lifecycles have been
+ * assembled by the {@link LifecycleProvider}s, so they have access to full session context.
+ *
Review Comment:
⚠️ **Inaccurate Javadoc — processors are NOT called once per session.**
`DefaultLifecycles.getPhaseToLifecycleMap()` explicitly states: *"Lifecycles cannot be cached as extensions might add custom lifecycles later in the execution."* Every call to `defaultLifecycles.get(phase)` (which happens per mojo execution in `DefaultLifecycleExecutionPlanCalculator`) rebuilds the map and re-runs the full processor chain. Processors are called once **per `getPhaseToLifecycleMap()` invocation**, not once per session.
This is important because `ReactorXmlLifecycleProcessor.process()` calls `legacySupport.getSession()` on every invocation — understanding the actual call frequency matters for performance reasoning.
```suggestion
* <p>Processors are called each time the lifecycle phase map is rebuilt (potentially multiple
* times per build). Implementations must be stateless and idempotent — the same lifecycle
* input must always produce the same output. They receive the output of the previous processor
* in the chain.
```
##########
api/maven-api-spi/src/main/java/org/apache/maven/api/spi/LifecycleProcessor.java:
##########
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.api.spi;
+
+import org.apache.maven.api.Lifecycle;
+import org.apache.maven.api.annotations.Consumer;
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Nonnull;
+import org.apache.maven.api.di.Named;
+
+/**
+ * SPI for transforming Maven lifecycles at session initialisation time.
+ *
+ * <p>A {@code LifecycleProcessor} receives each built-in (or extension-contributed) lifecycle
+ * and may return a modified version of it — for example to inject additional phases into the
+ * lifecycle DAG. Multiple processors are applied in order; each one receives the output of the
+ * previous one.
+ *
+ * <p>Processors are called once per lifecycle per session, after the base lifecycles have been
+ * assembled by the {@link LifecycleProvider}s, so they have access to full session context.
+ *
+ * <p>Example — injecting a {@code docker-push} phase after {@code deploy}:
+ * <pre>{@code
+ * @Named
+ * public class DockerLifecycleProcessor implements LifecycleProcessor {
+ * public Lifecycle process(Lifecycle lifecycle) {
+ * if (!Lifecycle.DEFAULT.equals(lifecycle.id())) return lifecycle;
+ * return LifecycleProcessor.withInjectedPhase(lifecycle,
+ * "docker-push", "deploy", null);
+ * }
+ * }
+ * }</pre>
Review Comment:
⚠️ **Javadoc example references `LifecycleProcessor.withInjectedPhase()` which does not exist.**
This static helper method is not declared on the `LifecycleProcessor` interface (or anywhere in the API). Anyone copying this example will get a compile error. Replace with an accurate example using the actual `PhaseEnrichedLifecycle` constructor, or remove the helper call if it was planned but not yet added.
```suggestion
* <p>Example — injecting a {@code docker-push} phase after {@code deploy}:
* <pre>{@code
* @Named
* public class DockerLifecycleProcessor implements LifecycleProcessor {
* public Lifecycle process(Lifecycle lifecycle) {
* if (!Lifecycle.DEFAULT.equals(lifecycle.id())) return lifecycle;
* return new PhaseEnrichedLifecycle(lifecycle,
* List.of(new PhaseEnrichedLifecycle.InjectedPhase("docker-push", null, "deploy", null)));
* }
* }
* }</pre>
```
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] reactor.xml: named build aliases and lifecycle execution filter [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #13249:
URL: https://github.com/apache/maven/pull/13249#discussion_r4084573359
##########
api/maven-api-core/src/main/mdo/reactor.mdo:
##########
@@ -0,0 +1,249 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+
+-->
+
+<model xmlns="http://codehaus-plexus.github.io/MODELLO/2.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://codehaus-plexus.github.io/MODELLO/2.0.0 https://codehaus-plexus.github.io/modello/xsd/modello-2.0.0.xsd"
+ xml.namespace="http://maven.apache.org/REACTOR/${version}"
+ xml.schemaLocation="https://maven.apache.org/xsd/reactor-${version}.xsd">
+
+ <id>reactor</id>
+ <name>ReactorConfig</name>
+ <description><![CDATA[
+ <p>This is a reference for the Reactor Configuration descriptor.</p>
+ <p>The default location for the Reactor Configuration descriptor file is <code>${maven.projectBasedir}/.mvn/reactor.xml</code></p>
+ <p>It consolidates reactor-scoped build configuration previously split across <code>.mvn/maven.config</code>
+ (CLI flag defaults) and <code>.mvn/extensions.xml</code> (core extensions), and adds support for
+ named build aliases and reactor-level lifecycle phase injection.</p>
+ ]]></description>
+
+ <defaults>
+ <default>
+ <key>package</key>
+ <value>org.apache.maven.api.reactor</value>
+ </default>
+ </defaults>
+
+ <classes>
+ <class rootElement="true" xml.tagName="reactor" xsd.compositor="sequence">
+ <name>ReactorConfig</name>
+ <description>Reactor-scoped build configuration for a Maven project.</description>
+ <version>1.0.0+</version>
+ <fields>
+ <field>
+ <name>options</name>
+ <description>CLI options to prepend to every invocation. Replaces .mvn/maven.config when present.
+ Expressed as whitespace-separated, quote-aware inline text (e.g. {@code -T4 --no-transfer-progress}).
+ For options with spaces in their values, use the structured form via {@link #getOptionArgs()}.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <type>String</type>
+ </field>
+ <field>
+ <name>optionArgs</name>
+ <description>Structured CLI options — each <arg> element is one option token.
+ Use instead of {@link #getOptions()} when option values contain spaces.
+ Mutually exclusive with options.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <association xml.itemsStyle="flat" xml.tagName="arg">
+ <type>String</type>
+ <multiplicity>*</multiplicity>
+ </association>
+ </field>
+ <field>
+ <name>extensions</name>
+ <description>Core extensions to load. Replaces .mvn/extensions.xml when present.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <association xml.itemsStyle="flat" xml.tagName="extension">
+ <type>ReactorExtension</type>
+ <multiplicity>*</multiplicity>
+ </association>
+ </field>
+ <field>
+ <name>aliases</name>
+ <description>Named build aliases. The first non-flag argument on the command line is matched against
+ alias names; if found it is replaced by the alias expansion.</description>
Review Comment:
Fixed in 1802942432: corrected description to reflect that every matching argument is expanded, not just the first one.
##########
api/maven-api-core/src/main/mdo/reactor.mdo:
##########
@@ -0,0 +1,249 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+
+-->
+
+<model xmlns="http://codehaus-plexus.github.io/MODELLO/2.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://codehaus-plexus.github.io/MODELLO/2.0.0 https://codehaus-plexus.github.io/modello/xsd/modello-2.0.0.xsd"
+ xml.namespace="http://maven.apache.org/REACTOR/${version}"
+ xml.schemaLocation="https://maven.apache.org/xsd/reactor-${version}.xsd">
+
+ <id>reactor</id>
+ <name>ReactorConfig</name>
+ <description><![CDATA[
+ <p>This is a reference for the Reactor Configuration descriptor.</p>
+ <p>The default location for the Reactor Configuration descriptor file is <code>${maven.projectBasedir}/.mvn/reactor.xml</code></p>
+ <p>It consolidates reactor-scoped build configuration previously split across <code>.mvn/maven.config</code>
+ (CLI flag defaults) and <code>.mvn/extensions.xml</code> (core extensions), and adds support for
+ named build aliases and reactor-level lifecycle phase injection.</p>
+ ]]></description>
+
+ <defaults>
+ <default>
+ <key>package</key>
+ <value>org.apache.maven.api.reactor</value>
+ </default>
+ </defaults>
+
+ <classes>
+ <class rootElement="true" xml.tagName="reactor" xsd.compositor="sequence">
+ <name>ReactorConfig</name>
+ <description>Reactor-scoped build configuration for a Maven project.</description>
+ <version>1.0.0+</version>
+ <fields>
+ <field>
+ <name>options</name>
+ <description>CLI options to prepend to every invocation. Replaces .mvn/maven.config when present.
+ Expressed as whitespace-separated, quote-aware inline text (e.g. {@code -T4 --no-transfer-progress}).
+ For options with spaces in their values, use the structured form via {@link #getOptionArgs()}.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <type>String</type>
+ </field>
+ <field>
+ <name>optionArgs</name>
+ <description>Structured CLI options — each <arg> element is one option token.
+ Use instead of {@link #getOptions()} when option values contain spaces.
+ Mutually exclusive with options.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <association xml.itemsStyle="flat" xml.tagName="arg">
+ <type>String</type>
+ <multiplicity>*</multiplicity>
+ </association>
+ </field>
+ <field>
+ <name>extensions</name>
+ <description>Core extensions to load. Replaces .mvn/extensions.xml when present.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <association xml.itemsStyle="flat" xml.tagName="extension">
+ <type>ReactorExtension</type>
+ <multiplicity>*</multiplicity>
+ </association>
+ </field>
+ <field>
+ <name>aliases</name>
+ <description>Named build aliases. The first non-flag argument on the command line is matched against
+ alias names; if found it is replaced by the alias expansion.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <association xml.itemsStyle="flat" xml.tagName="alias">
+ <type>Alias</type>
+ <multiplicity>*</multiplicity>
+ </association>
+ </field>
+ <field>
+ <name>phases</name>
+ <description>Custom lifecycle phase injections. Phases are registered in the lifecycle DAG
+ with the declared after/before edges.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <association xml.itemsStyle="flat" xml.tagName="phase">
+ <type>PhaseInjection</type>
+ <multiplicity>*</multiplicity>
+ </association>
+ </field>
+ </fields>
+ </class>
+
+ <class xml.tagName="extension">
+ <name>ReactorExtension</name>
+ <description>Describes a core extension to load.</description>
+ <version>1.0.0+</version>
+ <fields>
+ <field>
+ <name>groupId</name>
+ <description>The group ID of the extension's artifact.</description>
+ <version>1.0.0+</version>
+ <required>true</required>
+ <type>String</type>
+ </field>
+ <field>
+ <name>artifactId</name>
+ <description>The artifact ID of the extension.</description>
+ <version>1.0.0+</version>
+ <required>true</required>
+ <type>String</type>
+ </field>
+ <field>
+ <name>version</name>
+ <description>The version of the extension.</description>
+ <version>1.0.0+</version>
+ <required>true</required>
+ <type>String</type>
+ </field>
+ <field>
+ <name>classLoadingStrategy</name>
+ <description>The class loading strategy: 'self-first' (the default), 'parent-first' (loads classes from the
+ parent, then from the extension) or 'plugin' (follows the rules from extensions defined as plugins).</description>
+ <version>1.0.0+</version>
+ <defaultValue>self-first</defaultValue>
+ <required>false</required>
+ <type>String</type>
+ </field>
+ <field>
+ <name>configuration</name>
+ <description>Extension-specific configuration passed to the extension during initialization.
+ The structure is extension-defined.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <type>DOM</type>
+ </field>
+ </fields>
+ <codeSegments>
+ <codeSegment>
+ <version>1.0.0+</version>
+ <code>
+ <![CDATA[
+ /**
+ * Gets the identifier of the extension.
+ *
+ * @return The extension id in the form {@code <groupId>:<artifactId>:<version>}, never {@code null}.
+ */
+ public String getId() {
+ return (getGroupId() == null ? "[unknown-group-id]" : getGroupId())
+ + ":" + (getArtifactId() == null ? "[unknown-artifact-id]" : getArtifactId())
+ + ":" + (getVersion() == null ? "[unknown-version]" : getVersion());
+ }
+ ]]>
+ </code>
+ </codeSegment>
+ </codeSegments>
+ </class>
+
+ <class xml.tagName="alias">
+ <name>Alias</name>
+ <description>A named build alias. When the first non-flag argument matches the alias name,
+ it is replaced by the alias expansion. Supports both inline text (shorthand, whitespace-separated)
Review Comment:
Fixed in 1802942432: corrected description to reflect that every matching argument is expanded, not just the first one.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-12537] reactor.xml: named build aliases and lifecycle execution filter [maven]
Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
gnodet-bot commented on code in PR #13249:
URL: https://github.com/apache/maven/pull/13249#discussion_r4081516934
##########
api/maven-api-core/src/main/mdo/reactor.mdo:
##########
@@ -0,0 +1,237 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+
+-->
+
+<model xmlns="http://codehaus-plexus.github.io/MODELLO/2.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://codehaus-plexus.github.io/MODELLO/2.0.0 https://codehaus-plexus.github.io/modello/xsd/modello-2.0.0.xsd"
+ xml.namespace="http://maven.apache.org/REACTOR/${version}"
+ xml.schemaLocation="https://maven.apache.org/xsd/reactor-${version}.xsd">
+
+ <id>reactor</id>
+ <name>ReactorConfig</name>
+ <description><![CDATA[
+ <p>This is a reference for the Reactor Configuration descriptor.</p>
+ <p>The default location for the Reactor Configuration descriptor file is <code>${maven.projectBasedir}/.mvn/reactor.xml</code></p>
+ <p>It consolidates reactor-scoped build configuration previously split across <code>.mvn/maven.config</code>
+ (CLI flag defaults) and <code>.mvn/extensions.xml</code> (core extensions), and adds support for
+ named build aliases and reactor-level lifecycle phase injection.</p>
+ ]]></description>
+
+ <defaults>
+ <default>
+ <key>package</key>
+ <value>org.apache.maven.api.reactor</value>
+ </default>
+ </defaults>
+
+ <classes>
+ <class rootElement="true" xml.tagName="reactor" xsd.compositor="sequence">
+ <name>ReactorConfig</name>
+ <description>Reactor-scoped build configuration for a Maven project.</description>
+ <version>1.0.0+</version>
+ <fields>
+ <field>
+ <name>options</name>
+ <description>CLI options to prepend to every invocation. Replaces .mvn/maven.config when present.
+ Expressed as whitespace-separated, quote-aware inline text (e.g. {@code -T4 --no-transfer-progress}).
+ For options with spaces in their values, use the structured form via {@link #getOptionArgs()}.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <type>String</type>
+ </field>
+ <field>
+ <name>optionArgs</name>
+ <description>Structured CLI options — each <arg> element is one option token.
+ Use instead of {@link #getOptions()} when option values contain spaces.
+ Mutually exclusive with options.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <association xml.itemsStyle="flat" xml.tagName="arg">
+ <type>String</type>
+ <multiplicity>*</multiplicity>
+ </association>
+ </field>
+ <field>
+ <name>extensions</name>
+ <description>Core extensions to load. Replaces .mvn/extensions.xml when present.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <association xml.itemsStyle="flat" xml.tagName="extension">
+ <type>ReactorExtension</type>
+ <multiplicity>*</multiplicity>
+ </association>
+ </field>
+ <field>
+ <name>aliases</name>
+ <description>Named build aliases. The first non-flag argument on the command line is matched against
+ alias names; if found it is replaced by the alias expansion.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <association xml.itemsStyle="flat" xml.tagName="alias">
+ <type>Alias</type>
+ <multiplicity>*</multiplicity>
+ </association>
+ </field>
+ <field>
+ <name>phases</name>
+ <description>Custom lifecycle phase injections. Phases are registered in the lifecycle DAG
+ with the declared after/before edges.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <association xml.itemsStyle="flat" xml.tagName="phase">
+ <type>PhaseInjection</type>
+ <multiplicity>*</multiplicity>
+ </association>
+ </field>
+ </fields>
+ </class>
+
+ <class xml.tagName="extension">
+ <name>ReactorExtension</name>
+ <description>Describes a core extension to load.</description>
+ <version>1.0.0+</version>
+ <fields>
+ <field>
+ <name>groupId</name>
+ <description>The group ID of the extension's artifact.</description>
+ <version>1.0.0+</version>
+ <required>true</required>
+ <type>String</type>
+ </field>
+ <field>
+ <name>artifactId</name>
+ <description>The artifact ID of the extension.</description>
+ <version>1.0.0+</version>
+ <required>true</required>
+ <type>String</type>
+ </field>
+ <field>
+ <name>version</name>
+ <description>The version of the extension.</description>
+ <version>1.0.0+</version>
+ <required>true</required>
+ <type>String</type>
+ </field>
+ <field>
+ <name>classLoadingStrategy</name>
+ <description>The class loading strategy: 'self-first' (the default), 'parent-first' (loads classes from the
+ parent, then from the extension) or 'plugin' (follows the rules from extensions defined as plugins).</description>
+ <version>1.0.0+</version>
+ <defaultValue>self-first</defaultValue>
+ <required>false</required>
+ <type>String</type>
+ </field>
+ <field>
+ <name>configuration</name>
+ <description>Extension-specific configuration passed to the extension during initialization.
+ The structure is extension-defined.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <type>DOM</type>
+ </field>
+ </fields>
+ <codeSegments>
+ <codeSegment>
+ <version>1.0.0+</version>
+ <code>
+ <![CDATA[
+ /**
+ * Gets the identifier of the extension.
+ *
+ * @return The extension id in the form {@code <groupId>:<artifactId>:<version>}, never {@code null}.
+ */
+ public String getId() {
+ return (getGroupId() == null ? "[unknown-group-id]" : getGroupId())
+ + ":" + (getArtifactId() == null ? "[unknown-artifact-id]" : getArtifactId())
+ + ":" + (getVersion() == null ? "[unknown-version]" : getVersion());
+ }
+ ]]>
+ </code>
+ </codeSegment>
+ </codeSegments>
+ </class>
+
+ <class xml.tagName="alias">
+ <name>Alias</name>
+ <description>A named build alias. When the first non-flag argument matches the alias name,
+ it is replaced by the alias expansion. Supports both inline text (shorthand, whitespace-separated)
Review Comment:
⚠️ **Same doc mismatch on the `Alias` class itself**
Same issue as the `aliases` field description: says "first non-flag argument" but the implementation expands all matching arguments. The test `multipleAliasesAllExpanded` in `MavenParserAliasTest` demonstrates the real behavior: `[ci, rel]` expands both aliases.
```suggestion
<description>A named build alias. Every argument on the command line is checked against alias names;
when a match is found it is replaced in-place by the alias expansion tokens. Supports both inline
text (shorthand, whitespace-separated) and structured <arg> elements for values that contain spaces.</description>
```
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/MavenParser.java:
##########
@@ -76,6 +174,21 @@ protected MavenOptions parseMavenAtFileOptions(Path atFile) {
}
}
+ protected MavenOptions parseMavenReactorOptions(List<String> args) {
+ try {
+ MavenOptions options = parseArgs("reactor.xml", args);
+ if (options.goals().isPresent()) {
+ // <options> can only contain options, not goals/phases
+ throw new IllegalArgumentException("Unrecognized entries in reactor.xml <options>: "
+ + options.goals().get());
+ }
Review Comment:
💡 **`e.getCause()` drops the `ParseException` itself as the cause**
```java
throw new IllegalArgumentException(
"Failed to parse arguments from reactor.xml <options>: " + e.getMessage(), e.getCause());
```
`org.apache.commons.cli.ParseException` has no cause (it is the root exception); `e.getCause()` returns `null`, so the `IllegalArgumentException` is constructed with a `null` cause. The original exception is swallowed — only its message survives. The same pattern appears in `parseMavenCliOptions` and `parseMavenAtFileOptions` (pre-existing), so this is presumably intentional, but it means stack traces are lost when option parsing fails.
If intentional, add a comment explaining why `e.getCause()` rather than `e` is used. If not:
```suggestion
} catch (ParseException e) {
throw new IllegalArgumentException(
"Failed to parse arguments from reactor.xml <options>: " + e.getMessage(), e);
}
```
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] reactor.xml: named build aliases and lifecycle execution filter [maven]
Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
gnodet-bot commented on code in PR #13249:
URL: https://github.com/apache/maven/pull/13249#discussion_r4084065532
##########
api/maven-api-core/src/main/mdo/reactor.mdo:
##########
@@ -0,0 +1,249 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+
+-->
+
+<model xmlns="http://codehaus-plexus.github.io/MODELLO/2.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://codehaus-plexus.github.io/MODELLO/2.0.0 https://codehaus-plexus.github.io/modello/xsd/modello-2.0.0.xsd"
+ xml.namespace="http://maven.apache.org/REACTOR/${version}"
+ xml.schemaLocation="https://maven.apache.org/xsd/reactor-${version}.xsd">
+
+ <id>reactor</id>
+ <name>ReactorConfig</name>
+ <description><![CDATA[
+ <p>This is a reference for the Reactor Configuration descriptor.</p>
+ <p>The default location for the Reactor Configuration descriptor file is <code>${maven.projectBasedir}/.mvn/reactor.xml</code></p>
+ <p>It consolidates reactor-scoped build configuration previously split across <code>.mvn/maven.config</code>
+ (CLI flag defaults) and <code>.mvn/extensions.xml</code> (core extensions), and adds support for
+ named build aliases and reactor-level lifecycle phase injection.</p>
+ ]]></description>
+
+ <defaults>
+ <default>
+ <key>package</key>
+ <value>org.apache.maven.api.reactor</value>
+ </default>
+ </defaults>
+
+ <classes>
+ <class rootElement="true" xml.tagName="reactor" xsd.compositor="sequence">
+ <name>ReactorConfig</name>
+ <description>Reactor-scoped build configuration for a Maven project.</description>
+ <version>1.0.0+</version>
+ <fields>
+ <field>
+ <name>options</name>
+ <description>CLI options to prepend to every invocation. Replaces .mvn/maven.config when present.
+ Expressed as whitespace-separated, quote-aware inline text (e.g. {@code -T4 --no-transfer-progress}).
+ For options with spaces in their values, use the structured form via {@link #getOptionArgs()}.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <type>String</type>
+ </field>
+ <field>
+ <name>optionArgs</name>
+ <description>Structured CLI options — each <arg> element is one option token.
+ Use instead of {@link #getOptions()} when option values contain spaces.
+ Mutually exclusive with options.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <association xml.itemsStyle="flat" xml.tagName="arg">
+ <type>String</type>
+ <multiplicity>*</multiplicity>
+ </association>
+ </field>
+ <field>
+ <name>extensions</name>
+ <description>Core extensions to load. Replaces .mvn/extensions.xml when present.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <association xml.itemsStyle="flat" xml.tagName="extension">
+ <type>ReactorExtension</type>
+ <multiplicity>*</multiplicity>
+ </association>
+ </field>
+ <field>
+ <name>aliases</name>
+ <description>Named build aliases. The first non-flag argument on the command line is matched against
+ alias names; if found it is replaced by the alias expansion.</description>
Review Comment:
⚠️ **[3rd occurrence — still unaddressed]** The description says *"The first non-flag argument on the command line is matched against alias names"* but `MavenParser.expandAlias()` expands **every** argument that matches — not just the first one. The `multipleAliasesAllExpanded` test confirms this: `[ci, rel]` expands both.
This text ends up in the generated XSD and Javadoc — it will mislead users who define multiple aliases and expect them all to expand in a single invocation.
```suggestion
<description>Named build aliases. Every argument on the command line is checked against
alias names in order; each match is replaced in-place by the alias expansion tokens.
Expansion is not recursive.</description>
```
##########
api/maven-api-core/src/main/mdo/reactor.mdo:
##########
@@ -0,0 +1,249 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+
+-->
+
+<model xmlns="http://codehaus-plexus.github.io/MODELLO/2.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://codehaus-plexus.github.io/MODELLO/2.0.0 https://codehaus-plexus.github.io/modello/xsd/modello-2.0.0.xsd"
+ xml.namespace="http://maven.apache.org/REACTOR/${version}"
+ xml.schemaLocation="https://maven.apache.org/xsd/reactor-${version}.xsd">
+
+ <id>reactor</id>
+ <name>ReactorConfig</name>
+ <description><![CDATA[
+ <p>This is a reference for the Reactor Configuration descriptor.</p>
+ <p>The default location for the Reactor Configuration descriptor file is <code>${maven.projectBasedir}/.mvn/reactor.xml</code></p>
+ <p>It consolidates reactor-scoped build configuration previously split across <code>.mvn/maven.config</code>
+ (CLI flag defaults) and <code>.mvn/extensions.xml</code> (core extensions), and adds support for
+ named build aliases and reactor-level lifecycle phase injection.</p>
+ ]]></description>
+
+ <defaults>
+ <default>
+ <key>package</key>
+ <value>org.apache.maven.api.reactor</value>
+ </default>
+ </defaults>
+
+ <classes>
+ <class rootElement="true" xml.tagName="reactor" xsd.compositor="sequence">
+ <name>ReactorConfig</name>
+ <description>Reactor-scoped build configuration for a Maven project.</description>
+ <version>1.0.0+</version>
+ <fields>
+ <field>
+ <name>options</name>
+ <description>CLI options to prepend to every invocation. Replaces .mvn/maven.config when present.
+ Expressed as whitespace-separated, quote-aware inline text (e.g. {@code -T4 --no-transfer-progress}).
+ For options with spaces in their values, use the structured form via {@link #getOptionArgs()}.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <type>String</type>
+ </field>
+ <field>
+ <name>optionArgs</name>
+ <description>Structured CLI options — each <arg> element is one option token.
+ Use instead of {@link #getOptions()} when option values contain spaces.
+ Mutually exclusive with options.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <association xml.itemsStyle="flat" xml.tagName="arg">
+ <type>String</type>
+ <multiplicity>*</multiplicity>
+ </association>
+ </field>
+ <field>
+ <name>extensions</name>
+ <description>Core extensions to load. Replaces .mvn/extensions.xml when present.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <association xml.itemsStyle="flat" xml.tagName="extension">
+ <type>ReactorExtension</type>
+ <multiplicity>*</multiplicity>
+ </association>
+ </field>
+ <field>
+ <name>aliases</name>
+ <description>Named build aliases. The first non-flag argument on the command line is matched against
+ alias names; if found it is replaced by the alias expansion.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <association xml.itemsStyle="flat" xml.tagName="alias">
+ <type>Alias</type>
+ <multiplicity>*</multiplicity>
+ </association>
+ </field>
+ <field>
+ <name>phases</name>
+ <description>Custom lifecycle phase injections. Phases are registered in the lifecycle DAG
+ with the declared after/before edges.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <association xml.itemsStyle="flat" xml.tagName="phase">
+ <type>PhaseInjection</type>
+ <multiplicity>*</multiplicity>
+ </association>
+ </field>
+ </fields>
+ </class>
+
+ <class xml.tagName="extension">
+ <name>ReactorExtension</name>
+ <description>Describes a core extension to load.</description>
+ <version>1.0.0+</version>
+ <fields>
+ <field>
+ <name>groupId</name>
+ <description>The group ID of the extension's artifact.</description>
+ <version>1.0.0+</version>
+ <required>true</required>
+ <type>String</type>
+ </field>
+ <field>
+ <name>artifactId</name>
+ <description>The artifact ID of the extension.</description>
+ <version>1.0.0+</version>
+ <required>true</required>
+ <type>String</type>
+ </field>
+ <field>
+ <name>version</name>
+ <description>The version of the extension.</description>
+ <version>1.0.0+</version>
+ <required>true</required>
+ <type>String</type>
+ </field>
+ <field>
+ <name>classLoadingStrategy</name>
+ <description>The class loading strategy: 'self-first' (the default), 'parent-first' (loads classes from the
+ parent, then from the extension) or 'plugin' (follows the rules from extensions defined as plugins).</description>
+ <version>1.0.0+</version>
+ <defaultValue>self-first</defaultValue>
+ <required>false</required>
+ <type>String</type>
+ </field>
+ <field>
+ <name>configuration</name>
+ <description>Extension-specific configuration passed to the extension during initialization.
+ The structure is extension-defined.</description>
+ <version>1.0.0+</version>
+ <required>false</required>
+ <type>DOM</type>
+ </field>
+ </fields>
+ <codeSegments>
+ <codeSegment>
+ <version>1.0.0+</version>
+ <code>
+ <![CDATA[
+ /**
+ * Gets the identifier of the extension.
+ *
+ * @return The extension id in the form {@code <groupId>:<artifactId>:<version>}, never {@code null}.
+ */
+ public String getId() {
+ return (getGroupId() == null ? "[unknown-group-id]" : getGroupId())
+ + ":" + (getArtifactId() == null ? "[unknown-artifact-id]" : getArtifactId())
+ + ":" + (getVersion() == null ? "[unknown-version]" : getVersion());
+ }
+ ]]>
+ </code>
+ </codeSegment>
+ </codeSegments>
+ </class>
+
+ <class xml.tagName="alias">
+ <name>Alias</name>
+ <description>A named build alias. When the first non-flag argument matches the alias name,
+ it is replaced by the alias expansion. Supports both inline text (shorthand, whitespace-separated)
Review Comment:
⚠️ **[3rd occurrence — still unaddressed]** Same incorrect description on the `Alias` class: *"When the first non-flag argument matches the alias name, it is replaced by the alias expansion."*
```suggestion
<description>A named build alias. Every argument on the command line is checked against alias names;
when a match is found it is replaced in-place by the alias expansion tokens. Supports both inline
text (shorthand, whitespace-separated) and structured <arg> elements for values that contain spaces.</description>
```
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/MavenParser.java:
##########
@@ -76,6 +174,21 @@ protected MavenOptions parseMavenAtFileOptions(Path atFile) {
}
}
+ protected MavenOptions parseMavenReactorOptions(List<String> args) {
+ try {
+ MavenOptions options = parseArgs("reactor.xml", args);
+ if (options.goals().isPresent()) {
+ // <options> can only contain options, not goals/phases
+ throw new IllegalArgumentException("Unrecognized entries in reactor.xml <options>: "
+ + options.goals().get());
+ }
+ return options;
+ } catch (ParseException e) {
+ throw new IllegalArgumentException(
+ "Failed to parse arguments from reactor.xml <options>: " + e.getMessage(), e.getCause());
Review Comment:
⚠️ **[3rd occurrence — still unaddressed]** `e.getCause()` on a `ParseException` returns `null` — `ParseException` has no wrapped cause. The `IllegalArgumentException` is therefore constructed with a `null` cause, silently dropping the original exception from the stack trace.
The same pattern appears at lines 160, 171, and 206 (pre-existing), but this is new code added by this PR. If the intent is to match the pre-existing style, add a comment explaining why `e.getCause()` is preferred over `e`. If it's unintentional:
```suggestion
"Failed to parse arguments from reactor.xml <options>: " + e.getMessage(), e);
```
##########
api/maven-api-spi/src/main/java/org/apache/maven/api/spi/LifecycleProcessor.java:
##########
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.api.spi;
+
+import org.apache.maven.api.Lifecycle;
+import org.apache.maven.api.annotations.Consumer;
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Nonnull;
+import org.apache.maven.api.di.Named;
+
+/**
+ * SPI for transforming Maven lifecycles at session initialisation time.
+ *
+ * <p>A {@code LifecycleProcessor} receives each built-in (or extension-contributed) lifecycle
+ * and may return a modified version of it — for example to inject additional phases into the
+ * lifecycle DAG. Multiple processors are applied in order; each one receives the output of the
+ * previous one.
+ *
+ * <p>Processors are called once per lifecycle per session, after the base lifecycles have been
+ * assembled by the {@link LifecycleProvider}s, so they have access to full session context.
+ *
+ * <p>Example — injecting a {@code docker-push} phase after {@code deploy}:
+ * <pre>{@code
+ * @Named
+ * public class DockerLifecycleProcessor implements LifecycleProcessor {
+ * public Lifecycle process(Lifecycle lifecycle) {
+ * if (!Lifecycle.DEFAULT.equals(lifecycle.id())) return lifecycle;
+ * return LifecycleProcessor.withInjectedPhase(lifecycle,
+ * "docker-push", "deploy", null);
Review Comment:
⚠️ **[2nd occurrence — still unaddressed]** The Javadoc example calls `LifecycleProcessor.withInjectedPhase(...)`, a static helper that does not exist on this interface (or anywhere in the API). Anyone copying this example gets a compile error.
```suggestion
* return new PhaseEnrichedLifecycle(lifecycle,
* List.of(new PhaseEnrichedLifecycle.InjectedPhase("docker-push", null, "deploy", null)));
```
##########
api/maven-api-spi/src/main/java/org/apache/maven/api/spi/LifecycleProcessor.java:
##########
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.api.spi;
+
+import org.apache.maven.api.Lifecycle;
+import org.apache.maven.api.annotations.Consumer;
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Nonnull;
+import org.apache.maven.api.di.Named;
+
+/**
+ * SPI for transforming Maven lifecycles at session initialisation time.
+ *
+ * <p>A {@code LifecycleProcessor} receives each built-in (or extension-contributed) lifecycle
+ * and may return a modified version of it — for example to inject additional phases into the
+ * lifecycle DAG. Multiple processors are applied in order; each one receives the output of the
+ * previous one.
+ *
+ * <p>Processors are called once per lifecycle per session, after the base lifecycles have been
+ * assembled by the {@link LifecycleProvider}s, so they have access to full session context.
+ *
Review Comment:
⚠️ **[2nd occurrence — still unaddressed]** *"Processors are called once per lifecycle per session"* is factually incorrect. `DefaultLifecycles.getPhaseToLifecycleMap()` contains an explicit comment: *"Lifecycles cannot be cached as extensions might add custom lifecycles later in the execution."* The processor chain is invoked on every call to `getPhaseToLifecycleMap()`, which happens per mojo execution in `DefaultLifecycleExecutionPlanCalculator` — not once per session.
```suggestion
* <p>Processors are called each time the lifecycle phase map is rebuilt (potentially multiple
* times per build). Implementations must be stateless and idempotent — the same lifecycle
* input must always produce the same output. They receive the output of the previous processor
* in the chain.
```
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] reactor.xml: named build aliases and lifecycle execution filter [maven]
Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
gnodet-bot commented on code in PR #13249:
URL: https://github.com/apache/maven/pull/13249#discussion_r4093856302
##########
impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/filter/CoordinatePredicate.java:
##########
@@ -0,0 +1,241 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.lifecycle.internal.filter;
+
+import org.apache.maven.api.MojoExecution;
+import org.apache.maven.api.plugin.descriptor.PluginDescriptor;
+
+/**
+ * A {@link FilterPredicate} that matches {@link MojoExecution}s by plugin coordinate or prefix.
+ *
+ * <h2>Syntax: {@code ([G[:A]]|P)[:v][:g[@e]]}</h2>
+ *
+ * <p>The {@code @} separator for execution ID is compatible with Maven's existing
+ * {@code plugin:version:goal@executionId} notation used in
+ * {@code DefaultLifecycleExecutionPlanCalculator} for goal tasks.
+ *
+ * <p>Matching uses {@link MojoExecution#getDescriptor()} for goal-level fields and
+ * {@link MojoExecution#getPlugin()} for plugin-level coordinates (groupId, artifactId, version,
+ * goal prefix).
+ *
+ * <p>Forms:
+ * <ul>
+ * <li>{@code *} — matches every mojo execution</li>
+ * <li>{@code :A} — any groupId, specific artifactId (e.g. {@code :maven-enforcer-plugin})</li>
+ * <li>{@code G:A} — exact groupId:artifactId (e.g. {@code org.apache.maven.plugins:maven-enforcer-plugin})</li>
+ * <li>{@code P} — plugin prefix (e.g. {@code enforcer}), resolved against
+ * {@link MojoExecution#getMojoDescriptor()} goal prefix</li>
+ * <li>{@code P:v:g} — prefix + version + goal</li>
+ * <li>{@code P:v:g@e} — prefix + version + goal + executionId</li>
+ * </ul>
+ *
+ * <p>When any field is {@code null} or not specified, it is treated as a wildcard (matches any value).
+ *
+ * @since 4.1.0
+ */
+public class CoordinatePredicate implements FilterPredicate {
+
+ /** Wildcard token — matches any value. */
+ private static final String ANY = null;
+
+ private final boolean matchAll;
+ private final String groupId; // null = any, non-null = exact match
+ private final String artifactId; // null = any, non-null = exact match
+ private final String prefix; // null = not used, non-null = match by goal prefix
+ private final String version; // null = any
+ private final String goal; // null = any
+ private final String executionId; // null = any
+
+ /** Wildcard predicate — matches everything. */
+ public static final CoordinatePredicate MATCH_ALL = new CoordinatePredicate();
+
+ private CoordinatePredicate() {
+ this.matchAll = true;
+ this.groupId = ANY;
+ this.artifactId = ANY;
+ this.prefix = ANY;
+ this.version = ANY;
+ this.goal = ANY;
+ this.executionId = ANY;
+ }
+
+ private CoordinatePredicate(
+ String groupId, String artifactId, String prefix, String version, String goal, String executionId) {
+ this.matchAll = false;
+ this.groupId = groupId;
+ this.artifactId = artifactId;
+ this.prefix = prefix;
+ this.version = version;
+ this.goal = goal;
+ this.executionId = executionId;
+ }
+
+ /**
+ * Parses a coordinate predicate from a string token.
+ *
+ * <p>Supported forms:
+ * <ul>
+ * <li>{@code *} — match all</li>
+ * <li>{@code :A} — by artifactId only</li>
+ * <li>{@code G:A} — by groupId:artifactId (token contains {@code :} after first char)</li>
+ * <li>{@code P} — by prefix</li>
+ * <li>{@code P:v:g} — by prefix + version + goal</li>
+ * <li>{@code P:v:g@e} — by prefix + version + goal + executionId</li>
+ * </ul>
+ *
+ * @param token the filter expression token (not {@code null}, not blank)
+ * @return the parsed predicate
+ */
+ public static CoordinatePredicate parse(String token) {
+ if ("*".equals(token)) {
+ return MATCH_ALL;
+ }
+
+ if (token.startsWith(":")) {
+ // :A form — any groupId, specific artifactId, optional :v:g[@e]
+ // e.g. ":maven-enforcer-plugin" or ":maven-enforcer-plugin:3.0.0:enforce@enforce-id"
+ String rest = token.substring(1); // remove leading ':'
+ String[] parts = rest.split(":", 3);
+ String artifactId = emptyToNull(parts[0]);
+ String version = parts.length > 1 ? emptyToNull(parts[1]) : null;
+ String goalAndExec = parts.length > 2 ? parts[2] : null;
+ String[] ge = splitGoalExecution(goalAndExec);
+ return new CoordinatePredicate(ANY, artifactId, ANY, version, ge[0], ge[1]);
+ }
+
+ // Try to detect G:A form: the token contains ':' AND the part before the first ':' looks like a
+ // groupId (contains a '.' suggesting it's a Java package name like org.apache.maven).
+ // This distinguishes "org.apache.maven.plugins:maven-enforcer-plugin" from "enforcer:3.1.0:enforce".
+ int firstColon = token.indexOf(':');
+ if (firstColon > 0 && token.substring(0, firstColon).contains(".")) {
Review Comment:
⚠️ **[NOT ADDRESSED] G:A detection heuristic is fragile — silent misclassification for dotless groupIds and dotted prefixes**
Still using `token.substring(0, firstColon).contains(".")` to distinguish `G:A` from `P:v:g`. Two silent failure cases remain:
1. **GroupId without a dot** — `commons-io:commons-io` has no dot before the colon → misrouted to prefix-mode → never matches, filter silently does nothing.
2. **Prefix containing a dot** — `io.smallrye:enforce` → misrouted to G:A mode when the user intended prefix-based matching.
Neither case produces an error; the filter expression is accepted and silently ignored.
The most reliable fix is to require the explicit `:A` prefix for artifact-only matching and treat any `token` containing `:` but not starting with `:` as ambiguous — resolved by colon count:
- 1 colon, no leading `:` → G:A (groupId always documented as required to contain `.`; document this constraint clearly)
- 2+ colons → P:v:g or G:A:v:g
At minimum, add a test for `commons-io:commons-io` to expose the current misrouting, and document the groupId-must-contain-dot constraint in the Javadoc.
##########
impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/filter/MojoExecutionFilter.java:
##########
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.lifecycle.internal.filter;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.apache.maven.api.Constants;
+import org.apache.maven.api.MojoExecution;
+import org.apache.maven.internal.impl.DefaultMojoExecution;
+
+/**
+ * Parses the {@code maven.lifecycle.filter} user property value into a list of {@link FilterPredicate}s,
+ * and applies them at mojo execution time in {@code MojoExecutor}.
+ *
+ * <p>The property value is a comma-separated list of predicates, OR-ed together:
+ * a mojo execution matching <em>any</em> predicate is skipped (a {@code MojoSkipped} event is fired).
+ *
+ * <p>Supported predicate forms:
+ * <ul>
+ * <li>{@code *} — skip all mojo executions</li>
+ * <li>{@code :A} — skip by artifactId (e.g. {@code :maven-enforcer-plugin})</li>
+ * <li>{@code G:A} — skip by groupId:artifactId</li>
+ * <li>{@code P} — skip by plugin prefix (e.g. {@code enforcer})</li>
+ * <li>{@code P:v:g} — skip by prefix:version:goal</li>
+ * <li>{@code P:v:g@e} — skip by prefix:version:goal@executionId</li>
+ * <li>{@code phase(name)} — skip all mojos bound to the named phase</li>
+ * </ul>
+ *
+ * @since 4.1.0
+ */
+public class MojoExecutionFilter {
+
+ /** The name of the user property that activates the filter. */
+ public static final String PROPERTY_NAME = Constants.MAVEN_LIFECYCLE_FILTER;
+
+ private MojoExecutionFilter() {
+ // utility class
+ }
+
+ /**
+ * Parses a filter expression string into a list of {@link FilterPredicate}s.
+ *
+ * @param expression the comma-separated filter expression (may be {@code null} or blank)
+ * @return list of parsed predicates; empty if the expression is absent or blank
+ */
+ public static List<FilterPredicate> parse(String expression) {
+ if (expression == null || expression.isBlank()) {
+ return List.of();
+ }
+ List<FilterPredicate> predicates = new ArrayList<>();
+ for (String token : expression.split(",")) {
+ token = token.strip();
+ if (token.isEmpty()) {
+ continue;
+ }
+ predicates.add(parseToken(token));
+ }
+ return List.copyOf(predicates);
+ }
+
+ private static FilterPredicate parseToken(String token) {
+ if (token.startsWith("phase(") && token.endsWith(")")) {
+ String phaseName = token.substring("phase(".length(), token.length() - 1);
+ return new PhasePredicate(phaseName);
Review Comment:
⚠️ **[NOT ADDRESSED] `phase()` with empty parens silently creates a non-matching predicate**
`phaseName` is `""` when the user writes `phase()`. This is passed unchecked to `new PhasePredicate("")`, which stores it and evaluates `.equals(execution.getLifecyclePhase())` — always `false` since no mojo has an empty phase name. No error is raised; the filter silently does nothing.
Add a blank check before constructing `PhasePredicate`:
```suggestion
String phaseName = token.substring("phase(".length(), token.length() - 1);
if (phaseName.isBlank()) {
throw new IllegalArgumentException("phase() predicate requires a non-blank phase name");
}
return new PhasePredicate(phaseName);
```
Also add a test:
```java
@Test
void emptyPhaseParensThrows() {
assertThrows(IllegalArgumentException.class, () -> MojoExecutionFilter.parse("phase()"));
}
```
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org