gnodet opened a new issue, #13763: URL: https://github.com/apache/maven/issues/13763
## Summary Legacy plugins using `aggregator=true` should automatically benefit from the concurrent builder's DAG-based ordering without requiring source changes. The concurrent builder already has all the infrastructure needed (`ChildrenPointer` links, `before:`/`after:` `BuildStep` triples); it simply doesn't consult `isAggregator()` when placing mojos into the build plan. ## Problem Today, the concurrent builder places an `aggregator=true` mojo into its declared phase's `BuildStep` with no cross-project ordering constraints. This means: - An aggregator mojo bound to `package` may run **before** child modules have completed their `package` phase - The original `aggregatorLock` serialization (from the `-T` builder) is gone in the concurrent builder, but nothing replaces the ordering guarantee it provided - Plugin authors must manually rebind their mojos to `after:all` or `after:<phase>` to get correct behavior with the concurrent builder All the Apache Maven aggregator plugins (javadoc:aggregate, dependency:analyze-dep-mgt, site:stage, etc.) use `aggregator=true` and will not be migrated overnight. Until they are, users of the concurrent builder may get incorrect results from aggregator goals. ## Proposed Solution In `BuildPlanExecutor.calculateMojoExecutions()` (~line 940), when a `MojoExecution` has `mojoDescriptor.isAggregator() == true`: 1. **Rebind the mojo** so that it runs after all child projects have completed the declared phase. Semantically, an aggregator bound to `package` should wait for all children's `package` to finish — i.e., equivalent to `after:<phase>` with a `CHILDREN` link, **not** `after:all` (which would wait for the entire lifecycle). 2. **Restrict execution to the root project** — aggregator mojos should run once on the root, not on every module. ### Why this is feasible now (not a 5.x item) The mechanism already exists: - `DefaultLifecycleRegistry` (lines 453-486) defines the `ALL → children(ALL) → EACH → [phases]` structure with `ChildrenPointer` links - `BuildPlanExecutor` already constructs `before:X`, `X`, `after:X` `BuildStep` triples for every phase (line 1036+) - The `ChildrenPointer` link type (processed at line 1106) already creates cross-project DAG edges - No new concepts are needed — this is wiring an existing flag (`isAggregator()`) to existing infrastructure (`ChildrenPointer` links and `after:` steps) ### Semantic precision An aggregator bound to `package` should **not** be mapped to `after:all`. It should be mapped to a step that depends on all children completing `package`. This is more precise and preserves the original intent: the aggregator needs the outputs of a specific phase, not the entire lifecycle. ## Testing - Existing aggregator integration tests (`MavenITmng4618AggregatorBuiltAfterModulesTest`, `MavenITmng3716AggregatorForkingTest`, `MavenITmng1073AggregatorForksReactorTest`) should be extended to run with `-b concurrent` - New integration test: multi-module project with an aggregator mojo bound to `package`, verify it runs after all children's `package` phase with the concurrent builder ## Context - The concurrent builder's `MojoExecutor` already skips `aggregatorLock` (`useProjectLock()` returns `false`) - `BuildPlanExecutor` line 795 handles aggregator in `computeForkPlan()` but not in the main build plan - Related: #8718 (aggregator goals umbrella), #8291 (MNG-7163 — aggregator re-executes child goals) _Claude Code on behalf of Guillaume Nodet_ -- 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]
