gnodet commented on PR #13330: URL: https://github.com/apache/maven/pull/13330#issuecomment-6203287317
Thanks for the thorough documentation @Tarun4201! The current behavior sections (1 and 2) are excellent and should be kept as-is. Following the merge of #13532 (concurrent builder merge fixes) and a deeper analysis of what `aggregator` actually controls at runtime, I think sections 3.4 and 3.5 should be updated to frame `before:`/`after:` phases as the **replacement path** for `aggregator`, not just an alternative. The analysis also shows that forked lifecycles (`@Execute`) can be deprecated entirely — no existing plugin needs fork isolation, only ordering guarantees. Here's a suggested rewrite for sections 3.4 and 3.5, plus a new section 3.6: --- ### 3.4 Decomposing `aggregator = true` The `aggregator` flag is a **compound attribute** that bundles four distinct runtime behaviors. Maven 4 already provides orthogonal replacements for each: | Behavior | What `aggregator=true` does | Maven 4 replacement | Status | | :--- | :--- | :--- | :--- | | **Execution scope** | Run once on the root project, not per-module | Bind to `all` phase (`before:all`, `after:all`) | Available | | **Dependency resolution scope** | Resolve dependencies for all reactor projects | `@Resolution` annotation (per-field, per-project injection in `DefaultMavenPluginManager`) | Available | | **Exclusive locking** | Acquire reactor-wide write lock in `-T` builder | DAG edges enforce ordering in the concurrent builder — no lock needed | Available | | **Fork scope** | Forked lifecycles include all subprojects | Deprecated — see section 3.5 | Planned | This decomposition means new Maven 4 plugins should **not** use `@Mojo(aggregator = true)`. Instead: - Bind the goal to `after:all` (or `before:all`) for reactor-scoped execution. - Use `@Resolution` fields for dependency injection, which is already orthogonal to aggregator. - Express ordering constraints via `@After` / `AfterLink` with `PROJECT`, `DEPENDENCIES`, or `CHILDREN` pointers. For backward compatibility, existing plugin descriptors declaring `aggregator=true` will continue to work — the runtime maps the flag to the equivalent DAG constraints internally. ### 3.5 Deprecating Forked Lifecycles (`@Execute`) Forked lifecycles (`@Execute(phase = ...)`) let a mojo fork a lifecycle phase in a separate execution context before running. Analysis of all Apache Maven plugins using `@Execute` shows that **none require fork isolation** — every use is purely an ordering guarantee: | Plugin | `@Execute` target | Actual need | | :--- | :--- | :--- | | maven-javadoc-plugin | `@Execute(phase = COMPILE)` | Compiled classes must exist | | maven-source-plugin | `@Execute(phase = GENERATE_SOURCES)` | Generated sources must exist | | maven-dependency-plugin | `@Execute(phase = TEST_COMPILE)` | Classes compiled for analysis | | maven-pmd-plugin | `@Execute(phase = TEST_COMPILE)`, `@Execute(goal = "pmd")` | Classes compiled; PMD report generated | | maven-surefire (report) | `@Execute(lifecycle = "surefire", phase = TEST)` | Test results must exist | Every case reduces to "phase X must have completed before I run" — a DAG edge, not an isolated execution context. The `-no-fork` mojo variants (`javadoc-no-fork`, `aggregate-no-fork`, `jxr-no-fork`) exist only to opt out of unwanted forking, confirming that the fork is the wrong abstraction. The Maven 4 replacement is `@After` constraints: ```java // Instead of @Execute(phase = LifecyclePhase.COMPILE) @After(phase = "compile", scope = AfterLink.Pointer.PROJECT) // Instead of @Execute(goal = "pmd") @After(goal = "pmd:pmd") ``` The `computeForkPlan()` machinery, `$plan$` sentinel detection, and `getExecutionProject()` API can be deprecated for v4 mojos. Maven 3 plugin descriptors declaring `@Execute` will be translated into equivalent `@After` constraints at descriptor load time. ### 3.6 Migration Roadmap | Phase | Action | | :--- | :--- | | **Maven 4.0** | `before:`/`after:` phases available; `aggregator` and `@Execute` still supported | | **Maven 4.x** | Document `aggregator` and `@Execute` as deprecated for new plugins; provide migration guide | | **Maven 5.x** | Remove forked lifecycle machinery; `aggregator` mapped to phase binding internally | --- The related issues table (section 4) should also add: - [#13529](https://github.com/apache/maven/issues/13529) — Concurrent builder drops later lifecycle phases when merging overlapping task segments (fixed in #13532) _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]
