From 37a7d7cdb3ea9cfab250cc2d63e0907fffcfa058 Mon Sep 17 00:00:00 2001 From: Dan Sanduleac Date: Thu, 4 Jul 2019 21:02:48 +0100 Subject: [PATCH 1/4] Failing test Signed-off-by: Dan Sanduleac --- .../alignment/AlignmentIntegrationTest.groovy | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/alignment/AlignmentIntegrationTest.groovy b/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/alignment/AlignmentIntegrationTest.groovy index 88bb5dbf5dbf..77b8f75b3f24 100644 --- a/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/alignment/AlignmentIntegrationTest.groovy +++ b/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/alignment/AlignmentIntegrationTest.groovy @@ -1089,4 +1089,65 @@ class AlignmentIntegrationTest extends AbstractAlignmentSpec { virtualConfiguration("org:platform:1.1") } } + + @RequiredFeatures([ + // We only need to test one flavor + @RequiredFeature(feature = GradleMetadataResolveRunner.GRADLE_METADATA, value = "true"), + @RequiredFeature(feature = GradleMetadataResolveRunner.REPOSITORY_TYPE, value = "maven") + ]) + def "should manage to realign through two conflicts"() { + repository { + path 'start:start:1.0 -> foo:1.0' + + path 'foo:1.0 -> bar:1.0' + path 'bar:1.0 -> baz:1.0' + + path 'foo:1.1 -> bar:1.1' + path 'bar:1.1 -> baz:1.1' + + 'org:baz:1.0'() + 'org:baz:1.1'() + } + + given: + buildFile << ''' + dependencies { + constraints { + conf platform("org:platform:1.1") + } + + conf 'start:start:1.0' + } + ''' + + and: + "align the 'org' group only"() + + when: + expectAlignment { + module('start') group('start') alignsTo('1.0') + module('foo') tries('1.0') alignsTo('1.1') byVirtualPlatform() + module('bar') tries('1.0') alignsTo('1.1') byVirtualPlatform() + module('baz') tries('1.0') alignsTo('1.1') byVirtualPlatform() + } + run ':checkDeps', 'dependencyInsight', '--configuration', 'conf', '--dependency', 'baz' + + then: + resolve.expectGraph { + root(":", ":test:") { + module("start:start:1.0") { + edge("org:foo:1.0", "org:foo:1.1") { + byConstraint("belongs to platform org:platform:1.1") + edge("org:bar:1.0", "org:bar:1.1") { + byConstraint("belongs to platform org:platform:1.1") + edge("org:baz:1.0", "org:baz:1.1") { + byConstraint("belongs to platform org:platform:1.1") + } + } + } + } + } + virtualConfiguration("org:platform:1.1") + } + } } From 7b3c53cb806a035dc8c45e98bf327f2935b9986b Mon Sep 17 00:00:00 2001 From: Dan Sanduleac Date: Thu, 4 Jul 2019 21:04:29 +0100 Subject: [PATCH 2/4] Simplify test Signed-off-by: Dan Sanduleac --- .../alignment/AlignmentIntegrationTest.groovy | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/alignment/AlignmentIntegrationTest.groovy b/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/alignment/AlignmentIntegrationTest.groovy index 77b8f75b3f24..b8a5ad3972c5 100644 --- a/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/alignment/AlignmentIntegrationTest.groovy +++ b/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/alignment/AlignmentIntegrationTest.groovy @@ -1100,13 +1100,10 @@ class AlignmentIntegrationTest extends AbstractAlignmentSpec { path 'start:start:1.0 -> foo:1.0' path 'foo:1.0 -> bar:1.0' - path 'bar:1.0 -> baz:1.0' - path 'foo:1.1 -> bar:1.1' - path 'bar:1.1 -> baz:1.1' - 'org:baz:1.0'() - 'org:baz:1.1'() + 'org:bar:1.0'() + 'org:bar:1.1'() } given: @@ -1128,9 +1125,8 @@ class AlignmentIntegrationTest extends AbstractAlignmentSpec { module('start') group('start') alignsTo('1.0') module('foo') tries('1.0') alignsTo('1.1') byVirtualPlatform() module('bar') tries('1.0') alignsTo('1.1') byVirtualPlatform() - module('baz') tries('1.0') alignsTo('1.1') byVirtualPlatform() } - run ':checkDeps', 'dependencyInsight', '--configuration', 'conf', '--dependency', 'baz' + run ':checkDeps', 'dependencyInsight', '--configuration', 'conf', '--dependency', 'bar' then: resolve.expectGraph { @@ -1140,9 +1136,6 @@ class AlignmentIntegrationTest extends AbstractAlignmentSpec { byConstraint("belongs to platform org:platform:1.1") edge("org:bar:1.0", "org:bar:1.1") { byConstraint("belongs to platform org:platform:1.1") - edge("org:baz:1.0", "org:baz:1.1") { - byConstraint("belongs to platform org:platform:1.1") - } } } } From f9cf8154032a8db8341234ee2aa99400e1b35a6c Mon Sep 17 00:00:00 2001 From: Dan Sanduleac Date: Thu, 4 Jul 2019 21:10:31 +0100 Subject: [PATCH 3/4] fix expectation Signed-off-by: Dan Sanduleac --- .../resolve/alignment/AlignmentIntegrationTest.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/alignment/AlignmentIntegrationTest.groovy b/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/alignment/AlignmentIntegrationTest.groovy index b8a5ad3972c5..4ae6c907d4fa 100644 --- a/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/alignment/AlignmentIntegrationTest.groovy +++ b/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/alignment/AlignmentIntegrationTest.groovy @@ -1134,7 +1134,7 @@ class AlignmentIntegrationTest extends AbstractAlignmentSpec { module("start:start:1.0") { edge("org:foo:1.0", "org:foo:1.1") { byConstraint("belongs to platform org:platform:1.1") - edge("org:bar:1.0", "org:bar:1.1") { + module("org:bar:1.1") { byConstraint("belongs to platform org:platform:1.1") } } From 79c71405140f67b3b608b537ad90924fba1becf8 Mon Sep 17 00:00:00 2001 From: Louis Jacomet Date: Fri, 5 Jul 2019 14:54:20 +0200 Subject: [PATCH 4/4] Special case for deferred selector When a dependency activates a pending constraint, do not defer selection if the pending constraint comes from a virtual platform. Because of their special aspects, virtual platforms really need to be handled in line each time. Fixes #9882 --- .../builder/DefaultPendingDependenciesVisitor.java | 2 +- .../graph/builder/PendingDependencies.java | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/DefaultPendingDependenciesVisitor.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/DefaultPendingDependenciesVisitor.java index 6d2030b41783..2c174a8a534d 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/DefaultPendingDependenciesVisitor.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/DefaultPendingDependenciesVisitor.java @@ -68,7 +68,7 @@ private boolean markNoLongerPending(PendingDependencies pendingDependencies) { noLongerPending = Lists.newLinkedList(); } noLongerPending.add(pendingDependencies); - activatedPending = true; + activatedPending = pendingDependencies.shouldReportActivatePending(); } pendingDependencies.increaseHardEdgeCount(); return activatedPending; diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/PendingDependencies.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/PendingDependencies.java index 5d2a0e2544a7..1f2a70ae9774 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/PendingDependencies.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/PendingDependencies.java @@ -24,11 +24,13 @@ public class PendingDependencies { private final ModuleIdentifier moduleIdentifier; private final Set affectedComponents; private int hardEdges; + private boolean reportActivePending; PendingDependencies(ModuleIdentifier moduleIdentifier) { this.moduleIdentifier = moduleIdentifier; this.affectedComponents = Sets.newLinkedHashSet(); this.hardEdges = 0; + this.reportActivePending = true; } ModuleIdentifier getModuleIdentifier() { @@ -40,6 +42,9 @@ void addNode(NodeState state) { throw new IllegalStateException("Cannot add a pending node for a dependency which is not pending"); } affectedComponents.add(state); + if (state.getComponent().getModule().isVirtualPlatform()) { + reportActivePending = false; + } } void turnIntoHardDependencies() { @@ -47,6 +52,7 @@ void turnIntoHardDependencies() { affectedComponent.prepareForConstraintNoLongerPending(moduleIdentifier); } affectedComponents.clear(); + reportActivePending = true; } public boolean isPending() { @@ -65,4 +71,8 @@ void decreaseHardEdgeCount() { assert hardEdges > 0 : "Cannot remove a hard edge when none recorded"; hardEdges--; } + + public boolean shouldReportActivatePending() { + return reportActivePending; + } }