gnodet opened a new pull request, #2014:
URL: https://github.com/apache/maven-resolver/pull/2014

   ## Summary
   
   Fixes #2013 — BfDependencyCollector pool cache hit/miss changes graph 
structure (cache-transparency violation).
   
   **Root cause:** `TransitiveDependencyManager.deriveChildManager()` always 
created a new instance (with a unique `path` field and incrementing `depth`), 
making every pool key unique. The BF collector's pool cache includes the 
`DependencyManager` in its `GraphKey`, so distinct-but-semantically-equal 
managers caused pool misses. On a miss, the GACE-based skipper was consulted 
and could prune a node's children to zero — even though the same node had 
children when served from the cache via a different traversal path.
   
   **Fix:** `AbstractDependencyManager.deriveChildManager()` now returns `this` 
when:
   1. No new management data (versions, scopes, optionals, local paths, 
exclusions) was collected at the current depth, AND
   2. Management is already being applied (`depth >= applyFrom` — the 
`isApplied()` guard)
   
   The `isApplied()` guard is critical: at `depth < applyFrom`, returning 
`this` would freeze the depth counter and prevent `isApplied()` from ever 
returning true for descendants, breaking management rule application.
   
   This optimization addresses the common case for transitive dependencies 
whose POMs do not declare `<dependencyManagement>`. `ClassicDependencyManager` 
(Maven 3.x) was not affected because its `deriveUntil=2` caused `isDerived()` 
to return `this` at depth ≥ 2.
   
   ## Changes
   
   - **`AbstractDependencyManager.deriveChildManager()`** — added 
instance-reuse optimization before the `newInstance()` call
   - **`DependencyManagerTest`** — new unit test 
`testDeriveChildManagerReusesInstanceWhenNoNewManagementData()` verifying the 
optimization with `assertSame`/`assertNotSame` at different depths
   - **`BfWithSkipperDependencyCollectorTest`** — new integration test 
`testPoolCacheTransparencyWithTransitiveDependencyManager()` with a 
diamond-like graph (`root → b → c → d` and `root → b-alt → c → d`) verifying 
that `c` under `b-alt` retains its children
   
   ## Test plan
   
   - [x] Unit test: 
`DependencyManagerTest.testDeriveChildManagerReusesInstanceWhenNoNewManagementData`
 — verifies instance reuse at depth ≥ applyFrom with no management data, and 
new instance creation otherwise
   - [x] Integration test: 
`BfWithSkipperDependencyCollectorTest.testPoolCacheTransparencyWithTransitiveDependencyManager`
 — end-to-end test demonstrating the bug is fixed
   - [x] All existing tests pass (`maven-resolver-util`: 447 tests, 
`maven-resolver-impl`: 468 tests)
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)


-- 
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]

Reply via email to