gnodet commented on PR #13628: URL: https://github.com/apache/maven/pull/13628#issuecomment-6271844747
Thanks for the contribution! Beyond the specific bugs already flagged in the review, I'd like to raise a broader design concern about the performance implications of this approach. **The core problem: eager transitive resolution at graph-build time** The `collectTransitiveReactorDependencies()` method in `DefaultGraphBuilder` calls `repositorySystem.collectDependencies()` for every project that has external dependencies. This triggers a full transitive dependency resolution — including remote metadata fetches and POM downloads — before the build even starts. On a large multi-module project with many external dependencies, this can add significant overhead to every build invocation, even incremental ones. This is the exact kind of work the graph builder is designed to avoid: it currently only needs direct dependency declarations (from the POM), not the fully resolved graph. **Why the fix is harder than it looks** The root issue (MNG-7392) is real and worth fixing: if module `A` depends on external library `lib`, and `lib` transitively depends on reactor module `B`, then anything depending on `A` must be built after `B`. The current graph builder misses this edge because it only looks at direct deps. But the correct fix shouldn't require resolving the full transitive closure upfront. A few alternative directions worth exploring: 1. **Lazy / on-demand resolution**: only resolve transitively for projects that are actually requested in the current build (e.g. `-pl` subset), not all projects. 2. **Local-cache-only resolution**: limit the transitive walk to POMs already present in the local repository. If a POM isn't cached, we can't have a reactor dependency through it anyway (it was never built in this workspace). This avoids all remote calls. 3. **Explicit declaration**: require the user to declare the ordering dependency explicitly via `<dependency>` on the reactor module (which is already the recommended workaround for MNG-7392). This keeps graph building O(direct deps) and makes the dependency visible. 4. **Post-resolution fixup**: detect the missing ordering after the first resolution phase and re-sort, accepting that the first build may fail with a confusing error but subsequent ones work. Option 2 (local-cache-only) seems the most practical: it's a bounded cost, doesn't regress cold-build performance, and covers the common case where the reactor has been partially built before. I'd encourage thinking about the performance contract of graph building before going further with this approach. -- 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]
