Remember the full set of constraints instead of non activated

Depending on the order, it may be that a constraint is no longer pending, but would still participate in selection and choose a different version.
gradle · May 8, 2019 · 123c41b · 123c41b
1 parent 56b59cf
commit 123c41b
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 16 deletions.
diff --git a/...egTest/groovy/org/gradle/integtests/resolve/alignment/ForcingPlatformAlignmentTest.groovy b/...egTest/groovy/org/gradle/integtests/resolve/alignment/ForcingPlatformAlignmentTest.groovy
@@ -21,7 +21,6 @@ import org.gradle.integtests.fixtures.RequiredFeatures
 import org.gradle.integtests.fixtures.executer.GradleContextualExecuter
 import org.gradle.integtests.fixtures.publish.RemoteRepositorySpec
 import org.gradle.test.fixtures.server.http.MavenHttpModule
-import spock.lang.Ignore
 import spock.lang.IgnoreIf
 import spock.lang.Issue
 import spock.lang.Unroll
@@ -803,9 +802,9 @@ include 'other'
     @Unroll("can force a published platform version by forcing the platform itself via a dependency (#descriptor)")
     @RequiredFeatures([
             @RequiredFeature(feature = GradleMetadataResolveRunner.REPOSITORY_TYPE, value = "maven"),
-            @RequiredFeature(feature = GradleMetadataResolveRunner.EXPERIMENTAL_RESOLVE_BEHAVIOR, value = "true")
+            @RequiredFeature(feature = GradleMetadataResolveRunner.EXPERIMENTAL_RESOLVE_BEHAVIOR, value = "true"),
+            @RequiredFeature(feature = GradleMetadataResolveRunner.GRADLE_METADATA, value = "false"),
     ])
-    @Ignore
     def "can force a published platform version by forcing the platform itself via a dependency"() {
         repository {
             ['2.7.9', '2.9.4', '2.9.4.1'].each { v ->

diff --git a/...a/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/NodeState.java b/...a/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/NodeState.java
@@ -90,7 +90,7 @@ public boolean isSatisfiedBy(EdgeState edge) {
     private Set<ModuleIdentifier> upcomingNoLongerPendingConstraints;
     private boolean virtualPlatformNeedsRefresh;
     private Set<EdgeState> edgesToRecompute;
-    private Multimap<ModuleIdentifier, DependencyState> potentiallyActivatedConstraints;
+    private Multimap<ModuleIdentifier, DependencyState> moduleIdToConstraints;
 
     // caches
     private Map<DependencyMetadata, DependencyState> dependencyStateCache = Maps.newHashMap();
@@ -269,7 +269,7 @@ public void visitOutgoingDependencies(Collection<EdgeState> discoveredEdges) {
             removeOutgoingEdges();
             upcomingNoLongerPendingConstraints = null;
             edgesToRecompute = null;
-            potentiallyActivatedConstraints = null;
+            moduleIdToConstraints = null;
         }
 
         visitDependencies(resolutionFilter, discoveredEdges);
@@ -339,12 +339,7 @@ private void visitDependencies(ExcludeSpec resolutionFilter, Collection<EdgeStat
             for (DependencyState dependencyState : dependencies(resolutionFilter)) {
                 dependencyState = maybeSubstitute(dependencyState, resolveState.getDependencySubstitutionApplicator());
                 PendingDependenciesVisitor.PendingState pendingState = pendingDepsVisitor.maybeAddAsPendingDependency(this, dependencyState);
-                if (pendingState.isPending()) {
-                    if (potentiallyActivatedConstraints == null) {
-                        potentiallyActivatedConstraints = ArrayListMultimap.create();
-                    }
-                    potentiallyActivatedConstraints.put(dependencyState.getModuleIdentifier(), dependencyState);
-                } else {
+                if (!pendingState.isPending()) {
                     createAndLinkEdgeState(dependencyState, discoveredEdges, resolutionFilter, pendingState == PendingDependenciesVisitor.PendingState.NOT_PENDING_ACTIVATING);
                 }
             }
@@ -357,6 +352,12 @@ private void visitDependencies(ExcludeSpec resolutionFilter, Collection<EdgeStat
         }
     }
 
+    private void initializeModuleIdToConstraintMap(int size) {
+        if (moduleIdToConstraints == null) {
+            moduleIdToConstraints = ArrayListMultimap.create(size, 1);
+        }
+    }
+
     private List<? extends DependencyMetadata> dependencies() {
         if (dependenciesMayChange) {
             cachedDependencyStates = null;
@@ -387,9 +388,15 @@ private List<DependencyState> cacheFilteredDependencyStates(ExcludeSpec spec, Li
     }
 
     private List<DependencyState> cacheDependencyStates(List<? extends DependencyMetadata> dependencies) {
-        List<DependencyState> tmp = Lists.newArrayListWithCapacity(dependencies.size());
+        int size = dependencies.size();
+        List<DependencyState> tmp = Lists.newArrayListWithCapacity(size);
         for (DependencyMetadata dependency : dependencies) {
-            tmp.add(cachedDependencyStateFor(dependency));
+            DependencyState ds = cachedDependencyStateFor(dependency);
+            if (dependency.isConstraint()) {
+                initializeModuleIdToConstraintMap(size);
+                moduleIdToConstraints.put(ds.getModuleIdentifier(), ds);
+            }
+            tmp.add(ds);
         }
         return tmp;
     }
@@ -411,17 +418,16 @@ private void createAndLinkEdgeState(DependencyState dependencyState, Collection<
      * in upcomingNoLongerPendingConstraints
      */
     private void visitAdditionalConstraints(Collection<EdgeState> discoveredEdges) {
-        if (potentiallyActivatedConstraints == null) {
+        if (moduleIdToConstraints == null) {
             return;
         }
         for (ModuleIdentifier module : upcomingNoLongerPendingConstraints) {
-            Collection<DependencyState> dependencyStates = potentiallyActivatedConstraints.get(module);
+            Collection<DependencyState> dependencyStates = moduleIdToConstraints.get(module);
             if (!dependencyStates.isEmpty()) {
                 for (DependencyState dependencyState : dependencyStates) {
                     dependencyState = maybeSubstitute(dependencyState, resolveState.getDependencySubstitutionApplicator());
                     createAndLinkEdgeState(dependencyState, discoveredEdges, previousTraversalExclusions, false);
                 }
-                potentiallyActivatedConstraints.removeAll(module);
             }
         }
         upcomingNoLongerPendingConstraints = null;