Fix dependency-reduced-pom exclusion loss for classifier-distinct duplicate deps (Maven 4)

[ascheman]

What

Fixes the generated dependency-reduced-pom.xml dropping an <exclusion> on one of two classifier-distinct duplicate variants of a dependency under Maven 4 (e.g. b:0.2 keeps the exclusion, b:0.2:alt loses it).

Why

ShadeMojo.updateExcludesInDeps() walks a single conflict-resolved collect graph. Under Maven 4 the resolver prunes the duplicate transitive node (omitted for duplicate) under one variant, so the exclusion is re-attached to only that variant. This is stable, intended resolver behaviour (not a resolver bug); shade's own dependency keying is already classifier-aware.

Fix

Collect that graph with verbose conflict resolution (ConflictResolver.CONFIG_PROP_VERBOSE = STANDARD) on a copied session, so the omitted-duplicate node is retained as a marker; the existing walk then re-attaches the exclusion to both variants. Localized to the single collectDependencies call; no behaviour change on Maven 3.

Test

Covered by the existing ITs dep-reduced-pom-exclusions and MSHADE-467_parallel-dependency-reduced-pom (red on Maven 4 / green on Maven 3 before; green on both after). Full -P run-its verified locally: 83/0 on both Maven 4.0.0-SNAPSHOT and Maven 3.9.16.

Closes #819

[ascheman]

Note on Maven 4 coverage: this repo's PR matrix runs Maven 3.9.16 only, so the green checks here confirm there is no Maven 3 regression but do not exercise the Maven 4 path this fixes.

For the Maven 4 evidence I ran the full -P run-its suite against the latest Apache Maven 4.0.0-SNAPSHOT distribution on a fork branch (informational job that downloads the snapshot dist and runs the ITs):

• pre-fix baseline (red): Passed: 81, Failed: 2 — dep-reduced-pom-exclusions, MSHADE-467_parallel-dependency-reduced-pom → https://github.com/aschemaven/maven-shade-plugin/actions/runs/28367058534
• with this fix (green): Passed: 83, Failed: 0 → https://github.com/aschemaven/maven-shade-plugin/actions/runs/28402781798

Locally the suite is also 83/0 on both Maven 4.0.0-SNAPSHOT and Maven 3.9.16.

[hboutemy]

one aspect surprises me: I thought that the change in Maven 4 was tied to Resolver 2
then I tested with Maven 3.10.0-RC1: 3.10 does not suffer from the same issue as Maven 4.0

is Maven 4.0 using Resolver 2 not exactly the same way as Maven 3.10 on these conflict-resolved dependencies?

@cstamas having your review on this PR would help, please, to be sure that analysis and update to m-shade-p is completely understood and accepted by people who really understand (I don't really understand myself :) )

[cstamas]

one aspect surprises me: I thought that the change in Maven 4 was tied to Resolver 2 then I tested with Maven 3.10.0-RC1: 3.10 does not suffer from the same issue as Maven 4.0

is Maven 4.0 using Resolver 2 not exactly the same way as Maven 3.10 on these conflict-resolved dependencies?

Exactly the same issue in my eye as well. IF there are differences seen between Resolver 1 and Resolver 2, I would like to understand them better.

@ascheman can you create some simple reproducer for me to play with it?

[ascheman]

@cstamas here you go — standalone reproducer (stock shade 3.6.0, no plugin build needed): https://github.com/aschemaven/mshade-819-reproducer

./run.sh <maven-home> builds a tiny project (test -> a (excl c); a -> b + b:alt; b -> c, artifacts in a bundled file repo) and prints which deps in the generated dependency-reduced-pom.xml carry the exclusion. Results here:

Maven	b	b:alt
3.9.16 (Resolver 1.x)	1	1
3.10.0-rc-1 (Resolver 2.0.20)	1	1
4.0.0-rc-5	1	0
4.0.x-SNAPSHOT (e1f82346)	1	0

So @hboutemy's observation holds: same Resolver 2 line, different outcome — the delta is on the Maven side, not the resolver version. Two more data points from the reproducer:

• 4.0.0-rc-5 with -Daether.conflictResolver.verbose=true → b:alt gets its exclusion back. That is effectively what this PR does, scoped to the single collectDependencies call in updateExcludesInDeps instead of the whole session.
• Both 3.10.0-rc-1 and 4.0.0-rc-5 use BfDependencyCollector (per -X), so it is not the collector implementation either.

What I don't understand myself yet: why 3.10's collected graph retains the duplicate c node under b:alt with verbose unset while 4.0's does not — that difference is exactly what the reproducer should let you dig into.

On test coverage: the existing ITs (dep-reduced-pom-exclusions, MSHADE-467_parallel-dependency-reduced-pom) already cover this scenario and go red/green accordingly — once #810 enables the Maven 4 CI they become the permanent guard. The reproducer just takes the shade build out of the loop for bisecting the Maven side.

[gnodet]

I independently verified PR #820 against Maven 4.0.0-rc-5 and can confirm it fixes the two remaining failures from #810.

Test environment:

• Maven 4.0.0-rc-5 (fb3ecaef)
• Java 21.0.11 (Eclipse Adoptium)
• Linux amd64

Results with this PR applied:

Test run	Maven	Passed	Failed	Skipped
Target ITs only	4.0.0-rc-5	2	0	0
Target ITs only	3.9.16	2	0	0
Full IT suite	4.0.0-rc-5	83	0	1

The single skip is the pre-existing MSHADE-185 (unrelated).

Both dep-reduced-pom-exclusions and MSHADE-467_parallel-dependency-reduced-pom pass cleanly on Maven 4, and there is no regression on Maven 3.9.16.

[gnodet]

Root Cause Analysis: Why Maven 4 Prunes Differently Than Maven 3.10

@cstamas @hboutemy — I dug into why Maven 3.10.0-RC1 (Resolver 2.0.20) retains the duplicate c node under b:alt while Maven 4.0.0-rc-5 (same Resolver 2.0.20) prunes it. The short answer: it's the DependencyManager, not the ConflictResolver.

The Configuration Difference

The pivotal config property is maven.resolver.dependencyManagerTransitivity:

Maven Version	Default Value	DependencyManager Used
3.9.x (Resolver 1.x)	n/a	ClassicDependencyManager
3.10.0-rc-1 (Resolver 2.0.20)	false	ClassicDependencyManager(deriveUntil=2, applyFrom=2)
4.0.0-rc-5 (Resolver 2.0.20)	true	TransitiveDependencyManager(deriveUntil=MAX_VALUE, applyFrom=2)

Maven 3.10's DefaultRepositorySystemSessionFactory defaults dependencyManagerTransitivity to false, keeping ClassicDependencyManager. Maven 4's version defaults it to !mavenMaven3Personality = true, switching to TransitiveDependencyManager.

How This Causes the Bug

In BfDependencyCollector.doRecurse(), the collector builds a pool cache key from (artifact, repos, selector, manager, traverser, filter):

Object key = args.pool.toKey(artifact, childRepos, childSelector,
                              childManager, childTraverser, childFilter);
List<DependencyNode> children = args.pool.getChildren(key);
if (children == null) {
    boolean skipResolution = args.skipper.skipResolution(child, parentContext.parents);
    // ... resolve and cache
} else {
    child.setChildren(children);  // pool hit → skipper never consulted
}

Maven 3.10 (ClassicDependencyManager): At depth ≥ 2, deriveChildManager() returns this (same instance). So the pool key for c under b equals the pool key for c under b:alt → cache hit. The cached children are reused directly; the GACE skipper is never consulted. Both c nodes get populated subtrees, and ConflictResolver (even in default Verbosity.NONE) sees both and retains both.

Maven 4 (TransitiveDependencyManager): At every depth, deriveChildManager() creates a new manager instance incorporating managed dependencies from the current node. The pool key for c under b ≠ pool key for c under b:alt → cache miss. The GACE skipper is consulted on the second occurrence and marks c under b:alt as skippedAsDuplicate. That c node ends up with no children, and ConflictResolver (in Verbosity.NONE) prunes it entirely as a conflict loser. The shade plugin's exclusion walk never sees c under b:alt, so the exclusion is lost.

Conclusion

This is a deliberate Maven 4 design decision — TransitiveDependencyManager is the correct default for Maven 4's dependency model (dependency management applies transitively at all depths). The different pool-key behavior is a side effect, not a bug. The shade plugin genuinely needs to see all transitive occurrences regardless of conflict resolution, and CONFIG_PROP_VERBOSE = STANDARD is the documented mechanism for that use case. This PR's approach is the correct fix on the plugin side.

---

This analysis was generated by an AI agent and may contain inaccuracies. Please verify before relying on it.

[gnodet]

Approve

Reviewed with two independent specialized agents (code review + software architecture). Both found zero blocking issues.

What was checked

• Session copy safety: DefaultRepositorySystemSession copy constructor creates an isolated config map; original session is never mutated. The Maven Resolver upgrading guide explicitly blesses this pattern for Mojos.
• Forward compatibility: The copy constructor is NOT deprecated in Resolver 2.x (only the no-arg constructor is). CONFIG_PROP_VERBOSE and Verbosity.STANDARD are stable, non-deprecated API available since Resolver 1.9.8.
• Side effects of verbose mode: Verbosity.STANDARD retains conflict-loser nodes as childless markers with valid getArtifact()/getDependency(). The downstream walk (lines 1315-1366) handles these correctly — the dependencyHasExclusion guard prevents duplicate exclusions, and the classifier-aware getId() correctly matches across variants.
• Root cause: Investigated and filed as a resolver-side issue (apache/maven-resolver#2013). The underlying cause is a cache-transparency violation in BfDependencyCollector where the pool cache and GACE skipper use different keys. This PR's workaround is correct and semantically appropriate — verbose mode is the right tool for a walk that needs complete transitive visibility.

Clean, minimal, well-documented fix. Nice work! 👏

---

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

[gnodet]

Verified: ConflictResolver.CONFIG_PROP_VERBOSE and ConflictResolver.Verbosity.STANDARD are stable, non-deprecated API present in both Resolver 1.9.x and 2.0.x. The copy constructor of DefaultRepositorySystemSession is also explicitly blessed for Mojo use in Resolver 2.x (only the no-arg constructor is deprecated).

The underlying root cause (a cache-transparency violation in BfDependencyCollector where pool cache hit/miss changes graph structure) has been filed as apache/maven-resolver#2013. Even if that is fixed, verbose mode is semantically the right choice for this walk.