[MRESOLVER-614] Do not apply depMgt onto itself (#588)

Changes: * make collection context immutable (and related changes in DF and BF collectors to handle that) * related changes for clic-clac in all dependency manager implementations, basically defer depMgt application to "level lower" "Clic-clac": do NOT factor in depMgt that arrived in derive method, but "put it aside for later". Essentially, if node introduces new depMgt entries, it should NOT apply those onto itself (own dependencies), but ONLY its children nodes. --- https://issues.apache.org/jira/browse/MRESOLVER-614
apache · Oct 24, 2024 · c19aa9f · c19aa9f
1 parent 0dbe8b0
commit c19aa9f
Show file tree

Hide file tree

Showing 9 changed files with 107 additions and 42 deletions.
diff --git a/...ain/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollectionContext.java b/...ain/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollectionContext.java
@@ -32,11 +32,11 @@ public final class DefaultDependencyCollectionContext implements DependencyColle
 
     private final RepositorySystemSession session;
 
-    private Artifact artifact;
+    private final Artifact artifact;
 
-    private Dependency dependency;
+    private final Dependency dependency;
 
-    private List<Dependency> managedDependencies;
+    private final List<Dependency> managedDependencies;
 
     public DefaultDependencyCollectionContext(
             RepositorySystemSession session,
@@ -49,26 +49,29 @@ public DefaultDependencyCollectionContext(
         this.managedDependencies = managedDependencies;
     }
 
+    @Override
     public RepositorySystemSession getSession() {
         return session;
     }
 
+    @Override
     public Artifact getArtifact() {
         return artifact;
     }
 
+    @Override
     public Dependency getDependency() {
         return dependency;
     }
 
+    @Override
     public List<Dependency> getManagedDependencies() {
         return managedDependencies;
     }
 
-    public void set(Dependency dependency, List<Dependency> managedDependencies) {
-        artifact = dependency.getArtifact();
-        this.dependency = dependency;
-        this.managedDependencies = managedDependencies;
+    public DefaultDependencyCollectionContext set(Dependency dependency, List<Dependency> managedDependencies) {
+        return new DefaultDependencyCollectionContext(
+                session, dependency.getArtifact(), dependency, managedDependencies);
     }
 
     @Override

diff --git a/...impl/src/main/java/org/eclipse/aether/internal/impl/collect/bf/BfDependencyCollector.java b/...impl/src/main/java/org/eclipse/aether/internal/impl/collect/bf/BfDependencyCollector.java
@@ -350,8 +350,10 @@ private void doRecurse(
             DefaultDependencyNode child,
             Results results,
             boolean disableVersionManagement) {
-        DefaultDependencyCollectionContext context = args.collectionContext;
-        context.set(parentContext.dependency, descriptorResult.getManagedDependencies());
+        DefaultDependencyCollectionContext context = args.collectionContext.get();
+        args.collectionContext.compareAndSet(
+                context, context.set(parentContext.dependency, descriptorResult.getManagedDependencies()));
+        context = args.collectionContext.get();
 
         DependencySelector childSelector =
                 parentContext.depSelector != null ? parentContext.depSelector.deriveChildSelector(context) : null;
@@ -579,7 +581,7 @@ static class Args {
 
         final Queue<DependencyProcessingContext> dependencyProcessingQueue = new ArrayDeque<>(128);
 
-        final DefaultDependencyCollectionContext collectionContext;
+        final AtomicReference<DefaultDependencyCollectionContext> collectionContext;
 
         final DefaultVersionFilterContext versionContext;
 
@@ -604,7 +606,7 @@ static class Args {
             this.ignoreRepos = session.isIgnoreArtifactDescriptorRepositories();
             this.premanagedState = ConfigUtils.getBoolean(session, false, DependencyManagerUtils.CONFIG_PROP_VERBOSE);
             this.pool = pool;
-            this.collectionContext = collectionContext;
+            this.collectionContext = new AtomicReference<>(collectionContext);
             this.versionContext = versionContext;
             this.skipper = skipper;
             this.resolver = resolver;

diff --git a/...impl/src/main/java/org/eclipse/aether/internal/impl/collect/df/DfDependencyCollector.java b/...impl/src/main/java/org/eclipse/aether/internal/impl/collect/df/DfDependencyCollector.java
@@ -323,8 +323,9 @@ private void doRecurse(
             Dependency d,
             ArtifactDescriptorResult descriptorResult,
             DefaultDependencyNode child) {
-        DefaultDependencyCollectionContext context = args.collectionContext;
-        context.set(d, descriptorResult.getManagedDependencies());
+        DefaultDependencyCollectionContext context = args.collectionContext.get();
+        args.collectionContext.compareAndSet(context, context.set(d, descriptorResult.getManagedDependencies()));
+        context = args.collectionContext.get();
 
         DependencySelector childSelector = depSelector != null ? depSelector.deriveChildSelector(context) : null;
         DependencyManager childManager = depManager != null ? depManager.deriveChildManager(context) : null;
@@ -386,7 +387,7 @@ static class Args {
 
         final NodeStack nodes;
 
-        final DefaultDependencyCollectionContext collectionContext;
+        final AtomicReference<DefaultDependencyCollectionContext> collectionContext;
 
         final DefaultVersionFilterContext versionContext;
 
@@ -407,7 +408,7 @@ static class Args {
             this.premanagedState = ConfigUtils.getBoolean(session, false, DependencyManagerUtils.CONFIG_PROP_VERBOSE);
             this.pool = pool;
             this.nodes = nodes;
-            this.collectionContext = collectionContext;
+            this.collectionContext = new AtomicReference<>(collectionContext);
             this.versionContext = versionContext;
             this.interruptedException = new AtomicReference<>(null);
         }

diff --git a/.../src/main/java/org/eclipse/aether/internal/test/util/TestDependencyCollectionContext.java b/.../src/main/java/org/eclipse/aether/internal/test/util/TestDependencyCollectionContext.java
@@ -48,18 +48,22 @@ final class TestDependencyCollectionContext implements DependencyCollectionConte
         this.managedDependencies = managedDependencies;
     }
 
+    @Override
     public RepositorySystemSession getSession() {
         return session;
     }
 
+    @Override
     public Artifact getArtifact() {
         return artifact;
     }
 
+    @Override
     public Dependency getDependency() {
         return dependency;
     }
 
+    @Override
     public List<Dependency> getManagedDependencies() {
         return managedDependencies;
     }

diff --git a/...r-util/src/main/java/org/eclipse/aether/util/graph/manager/AbstractDependencyManager.java b/...r-util/src/main/java/org/eclipse/aether/util/graph/manager/AbstractDependencyManager.java
@@ -61,6 +61,8 @@ public abstract class AbstractDependencyManager implements DependencyManager {
 
     protected final SystemDependencyScope systemDependencyScope;
 
+    protected final DependencyCollectionContext currentContext;
+
     private final int hashCode;
 
     protected AbstractDependencyManager(int deriveUntil, int applyFrom, ScopeManager scopeManager) {
@@ -75,7 +77,8 @@ protected AbstractDependencyManager(int deriveUntil, int applyFrom, ScopeManager
                 Collections.emptyMap(),
                 scopeManager != null
                         ? scopeManager.getSystemDependencyScope().orElse(null)
-                        : SystemDependencyScope.LEGACY);
+                        : SystemDependencyScope.LEGACY,
+                null);
     }
 
     @SuppressWarnings("checkstyle:ParameterNumber")
@@ -88,7 +91,8 @@ protected AbstractDependencyManager(
             Map<Object, Boolean> managedOptionals,
             Map<Object, String> managedLocalPaths,
             Map<Object, Collection<Exclusion>> managedExclusions,
-            SystemDependencyScope systemDependencyScope) {
+            SystemDependencyScope systemDependencyScope,
+            DependencyCollectionContext currentContext) {
         this.depth = depth;
         this.deriveUntil = deriveUntil;
         this.applyFrom = applyFrom;
@@ -99,6 +103,8 @@ protected AbstractDependencyManager(
         this.managedExclusions = requireNonNull(managedExclusions);
         // nullable: if using scope manager, but there is no system scope defined
         this.systemDependencyScope = systemDependencyScope;
+        // nullable until applicable, then "lock step" below
+        this.currentContext = currentContext;
 
         this.hashCode = Objects.hash(
                 depth,
@@ -116,22 +122,32 @@ protected abstract DependencyManager newInstance(
             Map<Object, String> managedScopes,
             Map<Object, Boolean> managedOptionals,
             Map<Object, String> managedLocalPaths,
-            Map<Object, Collection<Exclusion>> managedExclusions);
+            Map<Object, Collection<Exclusion>> managedExclusions,
+            DependencyCollectionContext currentContext);
 
     @Override
     public DependencyManager deriveChildManager(DependencyCollectionContext context) {
         requireNonNull(context, "context cannot be null");
-        if (depth >= deriveUntil) {
+        if (!isDerived()) {
             return this;
         }
 
+        if (!isApplied() || currentContext == null) {
+            return derive(context, context); // original behaviour
+        } else {
+            return derive(currentContext, context); // clic-clac: defer application to children; not onto own deps
+        }
+    }
+
+    protected DependencyManager derive(
+            DependencyCollectionContext currentContext, DependencyCollectionContext nextContext) {
         Map<Object, String> managedVersions = this.managedVersions;
         Map<Object, String> managedScopes = this.managedScopes;
         Map<Object, Boolean> managedOptionals = this.managedOptionals;
         Map<Object, String> managedLocalPaths = this.managedLocalPaths;
         Map<Object, Collection<Exclusion>> managedExclusions = this.managedExclusions;
 
-        for (Dependency managedDependency : context.getManagedDependencies()) {
+        for (Dependency managedDependency : currentContext.getManagedDependencies()) {
             Artifact artifact = managedDependency.getArtifact();
             Object key = new Key(artifact);
 
@@ -179,7 +195,8 @@ public DependencyManager deriveChildManager(DependencyCollectionContext context)
             }
         }
 
-        return newInstance(managedVersions, managedScopes, managedOptionals, managedLocalPaths, managedExclusions);
+        return newInstance(
+                managedVersions, managedScopes, managedOptionals, managedLocalPaths, managedExclusions, nextContext);
     }
 
     @Override
@@ -188,7 +205,7 @@ public DependencyManagement manageDependency(Dependency dependency) {
         DependencyManagement management = null;
         Object key = new Key(dependency.getArtifact());
 
-        if (depth >= applyFrom) {
+        if (isApplied()) {
             String version = managedVersions.get(key);
             if (version != null) {
                 management = new DependencyManagement();
@@ -249,6 +266,20 @@ public DependencyManagement manageDependency(Dependency dependency) {
         return management;
     }
 
+    /**
+     * Returns {@code true} if current context should be factored in (collected/derived).
+     */
+    protected boolean isDerived() {
+        return depth < deriveUntil;
+    }
+
+    /**
+     * Returns {@code true} if current dependency should be managed according to so far collected/derived rules.
+     */
+    protected boolean isApplied() {
+        return depth >= applyFrom;
+    }
+
     @Override
     public boolean equals(Object obj) {
         if (this == obj) {

diff --git a/...er-util/src/main/java/org/eclipse/aether/util/graph/manager/ClassicDependencyManager.java b/...er-util/src/main/java/org/eclipse/aether/util/graph/manager/ClassicDependencyManager.java
@@ -70,7 +70,8 @@ private ClassicDependencyManager(
             Map<Object, Boolean> managedOptionals,
             Map<Object, String> managedLocalPaths,
             Map<Object, Collection<Exclusion>> managedExclusions,
-            SystemDependencyScope systemDependencyScope) {
+            SystemDependencyScope systemDependencyScope,
+            DependencyCollectionContext currentContext) {
         super(
                 depth,
                 deriveUntil,
@@ -80,7 +81,8 @@ private ClassicDependencyManager(
                 managedOptionals,
                 managedLocalPaths,
                 managedExclusions,
-                systemDependencyScope);
+                systemDependencyScope,
+                currentContext);
     }
 
     @Override
@@ -89,7 +91,13 @@ public DependencyManager deriveChildManager(DependencyCollectionContext context)
         // Removing this IF makes one IT fail here (read comment above):
         // https://github.com/apache/maven-integration-testing/blob/b4e8fd52b99a058336f9c7c5ec44fdbc1427759c/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng4720DependencyManagementExclusionMergeTest.java#L67
         if (depth == 1) {
-            return newInstance(managedVersions, managedScopes, managedOptionals, managedLocalPaths, managedExclusions);
+            return newInstance(
+                    managedVersions,
+                    managedScopes,
+                    managedOptionals,
+                    managedLocalPaths,
+                    managedExclusions,
+                    currentContext);
         }
         return super.deriveChildManager(context);
     }
@@ -100,7 +108,8 @@ protected DependencyManager newInstance(
             Map<Object, String> managedScopes,
             Map<Object, Boolean> managedOptionals,
             Map<Object, String> managedLocalPaths,
-            Map<Object, Collection<Exclusion>> managedExclusions) {
+            Map<Object, Collection<Exclusion>> managedExclusions,
+            DependencyCollectionContext currentContext) {
         return new ClassicDependencyManager(
                 depth + 1,
                 deriveUntil,
@@ -110,6 +119,7 @@ protected DependencyManager newInstance(
                 managedOptionals,
                 managedLocalPaths,
                 managedExclusions,
-                systemDependencyScope);
+                systemDependencyScope,
+                currentContext);
     }
 }
diff --git a/...er-util/src/main/java/org/eclipse/aether/util/graph/manager/DefaultDependencyManager.java b/...er-util/src/main/java/org/eclipse/aether/util/graph/manager/DefaultDependencyManager.java
@@ -21,6 +21,7 @@
 import java.util.Collection;
 import java.util.Map;
 
+import org.eclipse.aether.collection.DependencyCollectionContext;
 import org.eclipse.aether.collection.DependencyManager;
 import org.eclipse.aether.graph.Exclusion;
 import org.eclipse.aether.scope.ScopeManager;
@@ -63,7 +64,8 @@ private DefaultDependencyManager(
             Map<Object, Boolean> managedOptionals,
             Map<Object, String> managedLocalPaths,
             Map<Object, Collection<Exclusion>> managedExclusions,
-            SystemDependencyScope systemDependencyScope) {
+            SystemDependencyScope systemDependencyScope,
+            DependencyCollectionContext currentContext) {
         super(
                 depth,
                 deriveUntil,
@@ -73,7 +75,8 @@ private DefaultDependencyManager(
                 managedOptionals,
                 managedLocalPaths,
                 managedExclusions,
-                systemDependencyScope);
+                systemDependencyScope,
+                currentContext);
     }
 
     @Override
@@ -82,7 +85,8 @@ protected DependencyManager newInstance(
             Map<Object, String> managedScopes,
             Map<Object, Boolean> managedOptionals,
             Map<Object, String> managedLocalPaths,
-            Map<Object, Collection<Exclusion>> managedExclusions) {
+            Map<Object, Collection<Exclusion>> managedExclusions,
+            DependencyCollectionContext currentContext) {
         return new DefaultDependencyManager(
                 depth + 1,
                 deriveUntil,
@@ -92,6 +96,7 @@ protected DependencyManager newInstance(
                 managedOptionals,
                 managedLocalPaths,
                 managedExclusions,
-                systemDependencyScope);
+                systemDependencyScope,
+                currentContext);
     }
 }
diff --git a/...util/src/main/java/org/eclipse/aether/util/graph/manager/TransitiveDependencyManager.java b/...util/src/main/java/org/eclipse/aether/util/graph/manager/TransitiveDependencyManager.java
@@ -21,6 +21,7 @@
 import java.util.Collection;
 import java.util.Map;
 
+import org.eclipse.aether.collection.DependencyCollectionContext;
 import org.eclipse.aether.collection.DependencyManager;
 import org.eclipse.aether.graph.Exclusion;
 import org.eclipse.aether.scope.ScopeManager;
@@ -60,7 +61,8 @@ private TransitiveDependencyManager(
             Map<Object, Boolean> managedOptionals,
             Map<Object, String> managedLocalPaths,
             Map<Object, Collection<Exclusion>> managedExclusions,
-            SystemDependencyScope systemDependencyScope) {
+            SystemDependencyScope systemDependencyScope,
+            DependencyCollectionContext currentContext) {
         super(
                 depth,
                 deriveUntil,
@@ -70,7 +72,8 @@ private TransitiveDependencyManager(
                 managedOptionals,
                 managedLocalPaths,
                 managedExclusions,
-                systemDependencyScope);
+                systemDependencyScope,
+                currentContext);
     }
 
     @Override
@@ -79,7 +82,8 @@ protected DependencyManager newInstance(
             Map<Object, String> managedScopes,
             Map<Object, Boolean> managedOptionals,
             Map<Object, String> managedLocalPaths,
-            Map<Object, Collection<Exclusion>> managedExclusions) {
+            Map<Object, Collection<Exclusion>> managedExclusions,
+            DependencyCollectionContext currentContext) {
         return new TransitiveDependencyManager(
                 depth + 1,
                 deriveUntil,
@@ -89,6 +93,7 @@ protected DependencyManager newInstance(
                 managedOptionals,
                 managedLocalPaths,
                 managedExclusions,
-                systemDependencyScope);
+                systemDependencyScope,
+                currentContext);
     }
 }