Skip to content

Commit 530ccb7

Browse files
committed
Fix for transitive depManager (apache#1589)
Do not collect management data for scope and optional transitively, as those are "inherited" in graph (contextualized from parent). Changes: * fixed transitive manager to properly handle "scope" and "optional" (only from root) * removed "transitive" capability from classic manager (added in 2.0.x), it was a mistake (along with UT) * documented in Javadoc what each do and how * fixed suppliers to supply what is really expected
1 parent e8169b1 commit 530ccb7

File tree

8 files changed

+404
-124
lines changed

8 files changed

+404
-124
lines changed

maven-resolver-supplier-mvn3/src/main/java/org/eclipse/aether/supplier/SessionBuilderSupplier.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ protected DependencyTraverser getDependencyTraverser() {
9393
}
9494

9595
protected DependencyManager getDependencyManager() {
96-
return new ClassicDependencyManager(false, scopeManager);
96+
return new ClassicDependencyManager(scopeManager);
9797
}
9898

9999
protected DependencySelector getDependencySelector() {

maven-resolver-supplier-mvn4/src/main/java/org/eclipse/aether/supplier/SessionBuilderSupplier.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import org.eclipse.aether.resolution.ArtifactDescriptorPolicy;
4848
import org.eclipse.aether.util.artifact.DefaultArtifactTypeRegistry;
4949
import org.eclipse.aether.util.graph.manager.ClassicDependencyManager;
50+
import org.eclipse.aether.util.graph.manager.TransitiveDependencyManager;
5051
import org.eclipse.aether.util.graph.selector.AndDependencySelector;
5152
import org.eclipse.aether.util.graph.selector.ExclusionDependencySelector;
5253
import org.eclipse.aether.util.graph.transformer.ChainedDependencyGraphTransformer;
@@ -69,7 +70,7 @@ public class SessionBuilderSupplier {
6970
protected final InternalScopeManager scopeManager;
7071

7172
public SessionBuilderSupplier(RepositorySystem repositorySystem) {
72-
this.repositorySystem = (RepositorySystem) Objects.requireNonNull(repositorySystem);
73+
this.repositorySystem = Objects.requireNonNull(repositorySystem);
7374
this.scopeManager = new ScopeManagerImpl(Maven4ScopeManagerConfiguration.INSTANCE);
7475
}
7576

@@ -93,7 +94,9 @@ protected DependencyManager getDependencyManager() {
9394
}
9495

9596
public DependencyManager getDependencyManager(boolean transitive) {
96-
return new ClassicDependencyManager(transitive, this.getScopeManager());
97+
return transitive
98+
? new TransitiveDependencyManager(this.getScopeManager())
99+
: new ClassicDependencyManager(this.scopeManager);
97100
}
98101

99102
protected DependencySelector getDependencySelector() {

maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/manager/AbstractDependencyManager.java

Lines changed: 143 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -36,57 +36,116 @@
3636
import static java.util.Objects.requireNonNull;
3737

3838
/**
39-
* A dependency manager support class.
39+
* A dependency manager support class for Maven-specific dependency graph management.
40+
*
41+
* <h2>Overview</h2>
4042
* <p>
41-
* This implementation is Maven specific, as it works hand-in-hand along with Maven ModelBuilder. While model builder
42-
* handles dependency management in the context of single POM (inheritance, imports, etc.), this implementation carries
43-
* in-lineage modifications based on previously recorded dependency management rules sourced from ascendants while
44-
* building the dependency graph. Root sourced management rules are special, in a way they are always applied, while
45-
* en-route collected ones are carefully applied to proper descendants only, to not override work done by model
46-
* builder already.
43+
* This implementation works in conjunction with Maven ModelBuilder to handle dependency
44+
* management across the dependency graph. While ModelBuilder manages dependencies within
45+
* a single POM context (inheritance, imports), this class applies lineage-based modifications
46+
* based on previously recorded dependency management rules sourced from ancestors while
47+
* building the dependency graph. Root-sourced management rules are special, in that they are
48+
* always applied, while rules collected during traversal are carefully applied to proper
49+
* descendants only, to not override work done by ModelBuilder already.
50+
* </p>
51+
*
52+
* <h2>Managed Properties</h2>
53+
* <ul>
54+
* <li><strong>Version & Scope:</strong> Handled by ModelBuilder for own dependency management
55+
* (think "effective POM"). This implementation ensures these are not applied to the same
56+
* node that provided the rules, to not override ModelBuilder's work.</li>
57+
* <li><strong>Optional:</strong> Not handled by ModelBuilder; managed here.</li>
58+
* <li><strong>System Paths:</strong> Aligned across the entire graph, ensuring the same
59+
* system path is used by the same dependency.</li>
60+
* <li><strong>Exclusions:</strong> Always applied as additional information (not effective
61+
* or applied in the same POM).</li>
62+
* </ul>
63+
*
64+
* <h2>Depth-Based Rule Application</h2>
4765
* <p>
48-
* Details: Model builder handles version, scope from own dependency management (think "effective POM"). On the other
49-
* hand it does not handle optional for example. System paths are aligned across whole graph, making sure there is
50-
* same system path used by same dependency. Finally, exclusions (exclusions are additional information not effective
51-
* or applied in same POM) are always applied. This implementation makes sure, that version and scope are not applied
52-
* onto same node that actually provided the rules, to no override work that ModelBuilder did. It achieves this goal
53-
* by tracking "depth" for each collected rule and ignoring rules coming from same depth as processed dependency node is.
66+
* This implementation achieves proper rule application by tracking "depth" for each collected
67+
* rule and ignoring rules coming from the same depth as the processed dependency node.
68+
* </p>
69+
* <ul>
70+
* <li><strong>Depth 0:</strong> Factory instance created during session initialization and
71+
* parameterized. Collection begins with "derive" operation using root context.</li>
72+
* <li><strong>Depth 1:</strong> Special case for "version", "scope" and "optional" properties.
73+
* At this level, "apply onto itself" ensures root-defined rules are applied to first-level
74+
* siblings (which, if managed by ModelBuilder, will be the same, making this a no-op).</li>
75+
* <li><strong>Depth > 1:</strong> "Apply onto itself" is not in effect; only "apply below" is used.</li>
76+
* </ul>
77+
*
78+
* <h2>Rule Precedence</h2>
5479
* <p>
55-
* Note for future: the field {@code managedLocalPaths} is <em>intentionally left out of hash/equals</em>, with
56-
* reason explained above.
80+
* Rules are keyed by dependency management entry coordinates (GACE: Group, Artifact, Classifier,
81+
* Extension - see {@link Key}) and are recorded only if a rule for the same key did not exist
82+
* previously. This implements the "nearer (to root) management wins" rule, while root management
83+
* overrides all.
84+
* </p>
85+
*
86+
* <h2>Managed Bits and Graph Transformations</h2>
5787
* <p>
58-
* Implementation note for all managers extending this class: this class maintains "path" (list of parent managers)
59-
* and "depth". Depth {@code 0} is basically used as "factory" on session; is the instance created during session
60-
* creation and is usually empty (just parameterized). Depth 1 is the current collection "root", depth 2
61-
* are direct dependencies, depth 3 first level of transitive dependencies of direct dependencies and so on. Hence, on
62-
* depth 1 (the collection root, initialized with management possibly as well) parent will be always the empty "factory"
63-
* instance, and we need special handling: "apply onto itself". This does not stand on depth > 1.
88+
* When a {@link org.eclipse.aether.graph.DependencyNode} becomes "managed" by any property
89+
* provided from this manager, {@link org.eclipse.aether.graph.DependencyNode#getManagedBits()}
90+
* will carry this information for the given property. Later graph transformations will abstain
91+
* from modifying these properties of marked nodes (assuming the node already has the property
92+
* set to what it should have). Sometimes this is unwanted, especially for properties that need
93+
* to be inherited in the graph (values derived from parent-child context of the actual node,
94+
* like "scope" or "optional").
95+
* </p>
96+
*
97+
* <h2>Implementation Notes</h2>
98+
* <ul>
99+
* <li>This class maintains a "path" (list of parent managers) and "depth".</li>
100+
* <li>The field {@code managedLocalPaths} is <em>intentionally left out of hash/equals</em>.</li>
101+
* <li>Each dependency "derives" an instance with its own context to process second-level
102+
* dependencies and so on.</li>
103+
* </ul>
64104
*
65105
* @since 2.0.0
66106
*/
67107
public abstract class AbstractDependencyManager implements DependencyManager {
108+
/** The path of parent managers from root to current level. */
68109
protected final ArrayList<AbstractDependencyManager> path;
69110

111+
/** The current depth in the dependency graph (0 = factory, 1 = root, 2+ = descendants). */
70112
protected final int depth;
71113

114+
/** Maximum depth for rule derivation (exclusive). */
72115
protected final int deriveUntil;
73116

117+
/** Minimum depth for rule application (inclusive). */
74118
protected final int applyFrom;
75119

120+
/** Managed version rules keyed by dependency coordinates. */
76121
protected final MMap<Key, String> managedVersions;
77122

123+
/** Managed scope rules keyed by dependency coordinates. */
78124
protected final MMap<Key, String> managedScopes;
79125

126+
/** Managed optional flags keyed by dependency coordinates. */
80127
protected final MMap<Key, Boolean> managedOptionals;
81128

129+
/** Managed local paths for system dependencies (intentionally excluded from equals/hashCode). */
82130
protected final MMap<Key, String> managedLocalPaths;
83131

132+
/** Managed exclusions keyed by dependency coordinates. */
84133
protected final MMap<Key, Holder<Collection<Exclusion>>> managedExclusions;
85134

135+
/** System dependency scope handler, may be null if no system scope is defined. */
86136
protected final SystemDependencyScope systemDependencyScope;
87137

138+
/** Pre-computed hash code (excludes managedLocalPaths). */
88139
private final int hashCode;
89140

141+
/**
142+
* Creates a new dependency manager with the specified derivation and application parameters.
143+
*
144+
* @param deriveUntil the maximum depth for rule derivation (exclusive), must be >= 0
145+
* @param applyFrom the minimum depth for rule application (inclusive), must be >= 0
146+
* @param scopeManager the scope manager for handling system dependencies, may be null
147+
* @throws IllegalArgumentException if deriveUntil or applyFrom are negative
148+
*/
90149
protected AbstractDependencyManager(int deriveUntil, int applyFrom, ScopeManager scopeManager) {
91150
this(
92151
new ArrayList<>(),
@@ -210,6 +269,13 @@ private boolean containsManagedLocalPath(Key key) {
210269
return managedLocalPaths != null && managedLocalPaths.containsKey(key);
211270
}
212271

272+
/**
273+
* Gets the managed local path for system dependencies.
274+
* Note: Local paths don't follow the depth=1 special rule like versions/scopes.
275+
*
276+
* @param key the dependency key
277+
* @return the managed local path, or null if not managed
278+
*/
213279
private String getManagedLocalPath(Key key) {
214280
for (AbstractDependencyManager ancestor : path) {
215281
if (ancestor.managedLocalPaths != null && ancestor.managedLocalPaths.containsKey(key)) {
@@ -223,7 +289,12 @@ private String getManagedLocalPath(Key key) {
223289
}
224290

225291
/**
226-
* Merges all way down.
292+
* Merges exclusions from all levels in the dependency path.
293+
* Unlike other managed properties, exclusions are accumulated additively
294+
* from root to current level throughout the entire dependency path.
295+
*
296+
* @param key the dependency key
297+
* @return merged collection of exclusions, or null if none exist
227298
*/
228299
private Collection<Exclusion> getManagedExclusions(Key key) {
229300
ArrayList<Exclusion> result = new ArrayList<>();
@@ -263,20 +334,22 @@ public DependencyManager deriveChildManager(DependencyCollectionContext context)
263334
managedVersions.put(key, version);
264335
}
265336

266-
String scope = managedDependency.getScope();
267-
if (!scope.isEmpty() && !containsManagedScope(key)) {
268-
if (managedScopes == null) {
269-
managedScopes = MMap.emptyNotDone();
337+
if (isInheritedDerived()) {
338+
String scope = managedDependency.getScope();
339+
if (!scope.isEmpty() && !containsManagedScope(key)) {
340+
if (managedScopes == null) {
341+
managedScopes = MMap.emptyNotDone();
342+
}
343+
managedScopes.put(key, scope);
270344
}
271-
managedScopes.put(key, scope);
272-
}
273345

274-
Boolean optional = managedDependency.getOptional();
275-
if (optional != null && !containsManagedOptional(key)) {
276-
if (managedOptionals == null) {
277-
managedOptionals = MMap.emptyNotDone();
346+
Boolean optional = managedDependency.getOptional();
347+
if (optional != null && !containsManagedOptional(key)) {
348+
if (managedOptionals == null) {
349+
managedOptionals = MMap.emptyNotDone();
350+
}
351+
managedOptionals.put(key, optional);
278352
}
279-
managedOptionals.put(key, optional);
280353
}
281354

282355
String localPath = systemDependencyScope == null
@@ -401,6 +474,34 @@ protected boolean isDerived() {
401474
return depth < deriveUntil;
402475
}
403476

477+
/**
478+
* Manages dependency properties including "version", "scope", "optional", "local path", and "exclusions".
479+
* <p>
480+
* Property management behavior:
481+
* <ul>
482+
* <li><strong>Version:</strong> Follows {@link #isDerived()} pattern. Management is applied only at higher
483+
* levels to avoid interference with the model builder.</li>
484+
* <li><strong>Scope:</strong> Derived from root only due to inheritance in dependency graphs. Special handling
485+
* for "system" scope to align artifact paths.</li>
486+
* <li><strong>Optional:</strong> Derived from root only due to inheritance in dependency graphs.</li>
487+
* <li><strong>Local path:</strong> Managed only when scope is or was set to "system" to ensure consistent
488+
* artifact path alignment.</li>
489+
* <li><strong>Exclusions:</strong> Accumulated additively from root to current level throughout the entire
490+
* dependency path.</li>
491+
* </ul>
492+
* <p>
493+
* <strong>Inheritance handling:</strong> Since "scope" and "optional" properties inherit through dependency
494+
* graphs (beyond model builder scope), they are derived only from the root node. The actual manager
495+
* implementation determines specific handling behavior.
496+
* <p>
497+
* <strong>Default behavior:</strong> Defaults to {@link #isDerived()} to maintain compatibility with
498+
* "classic" behavior (equivalent to {@code deriveUntil=2}). For custom transitivity management, override
499+
* this method or ensure inherited managed properties are handled during graph transformation.
500+
*/
501+
protected boolean isInheritedDerived() {
502+
return isDerived();
503+
}
504+
404505
/**
405506
* Returns {@code true} if current dependency should be managed according to so far collected/derived rules.
406507
*/
@@ -431,10 +532,19 @@ public int hashCode() {
431532
return hashCode;
432533
}
433534

535+
/**
536+
* Key class for dependency management rules based on GACE coordinates.
537+
* GACE = Group, Artifact, Classifier, Extension (excludes version for management purposes).
538+
*/
434539
protected static class Key {
435540
private final Artifact artifact;
436541
private final int hashCode;
437542

543+
/**
544+
* Creates a new key from the given artifact's GACE coordinates.
545+
*
546+
* @param artifact the artifact to create a key for
547+
*/
438548
Key(Artifact artifact) {
439549
this.artifact = artifact;
440550
this.hashCode = Objects.hash(

0 commit comments

Comments
 (0)