gnodet-bot commented on code in PR #13330:
URL: https://github.com/apache/maven/pull/13330#discussion_r4175849937
##########
compat/maven-plugin-api/src/main/java/org/apache/maven/plugin/descriptor/MojoDescriptor.java:
##########
@@ -539,16 +539,18 @@ public void setExecuteLifecycle(String executeLifecycle) {
}
/**
- * @param aggregator <code>true</code> if the Mojo uses the Maven project
and its child modules,
- * <code>false</code> otherwise.
+ * @param aggregator <code>true</code> if the Mojo operates across the
reactor project and its subprojects,
+ * <code>false</code> otherwise.
*/
public void setAggregator(boolean aggregator) {
this.aggregator = aggregator;
}
/**
- * @return <code>true</code> if the Mojo uses the Maven project and its
child modules,
- * <code>false</code> otherwise.
+ * Indicates whether the Mojo operates as an aggregator across the Maven
reactor and its child modules.
+ *
+ * @return <code>true</code> if the Mojo operates across the Maven reactor
and its child modules,
+ * <code>false</code> otherwise.
Review Comment:
💡 `setAggregator` (line 542) uses "subprojects" but `isAggregator` uses
"child modules" — inconsistent within the same class in the same PR.
```suggestion
* Indicates whether the Mojo operates as an aggregator across the Maven
reactor and its subprojects.
*
* @return <code>true</code> if the Mojo operates across the Maven
reactor and its subprojects,
* <code>false</code> otherwise.
```
##########
src/site/markdown/aggregator-goals.md:
##########
@@ -0,0 +1,124 @@
+<!--
+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.
+-->
+# Aggregator Mojos and Reactor Lifecycle Specification
+
+## Overview
+
+Aggregation was introduced early in Maven 2
([MNG-250](https://issues.apache.org/jira/browse/MNG-250)) to enable plugins to
operate across an entire multi-module build reactor rather than on a single
isolated module. It is exposed to plugin developers via the `@Mojo(aggregator =
true)` annotation (or `@aggregator` in JavaDoc tag format).
+
+This document analyzes the current behavior, outlines historical shortcomings,
and establishes a target design specification for refactoring aggregator goals
in Maven ([MNG-7991](https://issues.apache.org/jira/browse/MNG-7991)).
+
+---
+
+## 1. Current Aggregator Behavior
+
+Maven treats aggregator Mojos differently depending on whether they are
invoked directly via the Command Line Interface (CLI) or bound to a build
lifecycle phase in a POM.
+
+### 1.1 CLI Invocation (`mvn plugin:goal`)
+When an aggregating goal is invoked from the command line:
+1. **Task Segment Classification**: `DefaultLifecycleTaskSegmentCalculator`
inspects the Mojo descriptor:
+ ```java
+ boolean aggregating = mojoDescriptor.isAggregator() ||
!mojoDescriptor.isProjectRequired();
+ ```
+2. **Aggregating Task Segment**: A distinct aggregating `TaskSegment` is
created.
+3. **Execution on Root Project Only**: The lifecycle execution engine
(`LifecycleStarter` / `ConcurrentLifecycleStarter`) executes this task segment
exclusively on the top-level project (`session.getTopLevelProject()`).
Submodules in the reactor are skipped for this goal.
Review Comment:
⚠️ **Inaccurate class attribution.** The root-project-only filtering is
handled by `BuildListCalculator` (line 48: `if (taskSegment.isAggregating()) {
projects = Collections.singletonList(rootProject); }`) and `BuildPlanExecutor`
(line 246-248, same logic). `LifecycleStarter` / `ConcurrentLifecycleStarter`
are entry points that delegate downward — they don't perform the aggregation
filtering themselves.
```suggestion
3. **Execution on Root Project Only**: `BuildListCalculator` (and
`BuildPlanExecutor` in the concurrent path) restricts aggregating task segments
to the top-level project (`session.getTopLevelProject()`). Submodules in the
reactor are skipped for this goal.
```
##########
api/maven-api-core/src/main/java/org/apache/maven/api/plugin/annotations/Mojo.java:
##########
@@ -76,8 +76,17 @@
boolean projectRequired() default true;
/**
- * if the Mojo uses the Maven project and its subprojects.
- * @return uses the Maven project and its subprojects
+ * Indicates whether the Mojo operates as an aggregator across the Maven
reactor and its subprojects.
+ * <p>
+ * <b>Execution semantics:</b>
+ * <ul>
+ * <li>When invoked directly via CLI (e.g. {@code mvn plugin:goal}), an
aggregator Mojo executes
+ * only once on the top-level project of the reactor, after
preceding lifecycle phases complete.</li>
+ * <li>When bound to a lifecycle phase, the goal executes during that
phase for each module in the
+ * reactor unless explicitly configured with {@code
<inherited>false</inherited>}.</li>
Review Comment:
💡 "module" here is inconsistent with the rest of this Javadoc (lines 79, 89)
which uses "subprojects" per MNG-8210.
```suggestion
* <li>When bound to a lifecycle phase, the goal executes during that
phase for each subproject in the
* reactor unless explicitly configured with {@code
<inherited>false</inherited>}.</li>
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
