From 7794fc358ab17de2075a0af1a9b2dbd28377a7b5 Mon Sep 17 00:00:00 2001 From: Louis Jacomet Date: Fri, 5 Jun 2020 16:04:05 +0200 Subject: [PATCH] Improve tracking of unattached dependencies When restarting an edge, it is possible that the new state fails to compute the target node. In some cases, it is important to make sure the detached edge is effectively tracked as an unattached edge. Fixes #13251 Fixes #13316 --- .../JavaPlatformResolveIntegrationTest.groovy | 78 +++++++++++++++++++ .../graph/builder/EdgeState.java | 19 ++++- .../graph/builder/ModuleResolveState.java | 9 ++- .../graph/builder/NodeState.java | 10 +-- 4 files changed, 107 insertions(+), 9 deletions(-) diff --git a/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/platforms/JavaPlatformResolveIntegrationTest.groovy b/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/platforms/JavaPlatformResolveIntegrationTest.groovy index fa86e1875a35..0b5fe2dd8162 100755 --- a/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/platforms/JavaPlatformResolveIntegrationTest.groovy +++ b/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/platforms/JavaPlatformResolveIntegrationTest.groovy @@ -640,6 +640,84 @@ class JavaPlatformResolveIntegrationTest extends AbstractHttpDependencyResolutio platform << ["project(':platform')", "'org:other-platform:1.0'", "'org:bom-platform:1.0'"] } + def 'platform deselection / reselection does not cause orphan edges'() { + given: + def depExcluded = mavenHttpRepo.module('org.test', 'excluded', '1.0').publish() + def depA = mavenHttpRepo.module('org.test', 'depA', '1.0').publish() + def platform = mavenHttpRepo.module('org.test', 'platform', '1.0').withModuleMetadata().withoutDefaultVariants() + .withVariant('apiElements') { + useDefaultArtifacts = false + attribute(Usage.USAGE_ATTRIBUTE.name, Usage.JAVA_API) + attribute(Category.CATEGORY_ATTRIBUTE.name, Category.REGULAR_PLATFORM) + constraint('org.test', 'depA', '1.0') + constraint('org.test', 'excluded', '1.0') + } + .withVariant('runtimeElements') { + useDefaultArtifacts = false + attribute(Usage.USAGE_ATTRIBUTE.name, Usage.JAVA_RUNTIME) + attribute(Category.CATEGORY_ATTRIBUTE.name, Category.REGULAR_PLATFORM) + constraint('org.test', 'depA', '1.0') + constraint('org.test', 'excluded', '1.0') + }.publish() + def depC = mavenHttpRepo.module('org.test', 'depC', '1.0').withModuleMetadata() + .withVariant('runtime') { + dependsOn('org.test', 'platform', '1.0', null, [(Category.CATEGORY_ATTRIBUTE.name): Category.REGULAR_PLATFORM]) + }.publish() + def depB = mavenHttpRepo.module('org.test', 'depB', '1.0').dependsOn([exclusions: [[module: 'excluded']]], depC).publish() + def depF = mavenHttpRepo.module('org.test', 'depF', '1.0').dependsOn(depC).publish() + def depE = mavenHttpRepo.module('org.test', 'depE', '1.0').dependsOn(depF).publish() + def depD = mavenHttpRepo.module('org.test', 'depD', '1.0').dependsOn(depE).publish() + + depExcluded.allowAll() + depA.allowAll() + depB.allowAll() + depC.allowAll() + depD.allowAll() + depE.allowAll() + depF.allowAll() + platform.allowAll() + + buildFile << """ + configurations { + conf.dependencies.clear() + } + + dependencies { + conf 'org.test:depA' + conf 'org.test:depB:1.0' + conf 'org.test:depD:1.0' + } +""" + checkConfiguration("conf") + resolve.expectDefaultConfiguration("runtime") + + when: + succeeds 'checkDeps' + + then: + resolve.expectGraph { + root(":", "org.test:test:1.9") { + edge("org.test:depA", "org.test:depA:1.0") + module("org.test:depB:1.0") { + module("org.test:depC:1.0") { + module('org.test:platform:1.0') { + noArtifacts() + constraint('org.test:depA:1.0') + } + } + } + module('org.test:depD:1.0') { + module('org.test:depE:1.0') { + module('org.test:depF:1.0') { + module('org.test:depC:1.0') + } + } + } + } + } + } + + private void checkConfiguration(String configuration) { resolve = new ResolveTestFixture(buildFile, configuration) resolve.expectDefaultConfiguration("compile") diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/EdgeState.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/EdgeState.java index ec7d71993e66..4eea99c1491f 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/EdgeState.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/EdgeState.java @@ -66,6 +66,7 @@ class EdgeState implements DependencyGraphEdge { private ExcludeSpec cachedExclusions; private ResolvedVariantResult resolvedVariant; + private boolean unattached; EdgeState(NodeState from, DependencyState dependencyState, ExcludeSpec transitiveExclusions, ResolveState resolveState) { this.from = from; @@ -181,9 +182,13 @@ void failWith(Throwable err) { targetNodeSelectionFailure = new ModuleVersionResolveException(dependencyState.getRequested(), err); } - public void restart() { + public void restart(boolean checkUnattached) { if (from.isSelected()) { removeFromTargetConfigurations(); + // We now have corner cases that can lead to this restart not succeeding + if (checkUnattached && !isUnattached()) { + selector.getTargetModule().addUnattachedDependency(this); + } attachToTargetConfigurations(); } } @@ -430,4 +435,16 @@ public void updateTransitiveExcludes(ExcludeSpec newResolutionFilter) { targetNode.updateTransitiveExcludes(); } } + + public void markUnattached() { + this.unattached = true; + } + + public void markAttached() { + this.unattached = false; + } + + public boolean isUnattached() { + return unattached; + } } diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/ModuleResolveState.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/ModuleResolveState.java index 8eb1e6e2ecf3..6a3089067ecb 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/ModuleResolveState.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/ModuleResolveState.java @@ -262,20 +262,23 @@ private void doRestart(ComponentState selected) { private void restartUnattachedDependencies() { if (unattachedDependencies.size() == 1) { EdgeState singleDependency = unattachedDependencies.get(0); - singleDependency.restart(); + singleDependency.restart(false); } else { for (EdgeState dependency : new ArrayList<>(unattachedDependencies)) { - dependency.restart(); + dependency.restart(false); } } } public void addUnattachedDependency(EdgeState edge) { unattachedDependencies.add(edge); + edge.markUnattached(); } public void removeUnattachedDependency(EdgeState edge) { - unattachedDependencies.remove(edge); + if (unattachedDependencies.remove(edge)) { + edge.markAttached(); + } } public ComponentState getVersion(ModuleVersionIdentifier id, ComponentIdentifier componentIdentifier) { diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/NodeState.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/NodeState.java index 8cdc56bce164..fe73ae1537d6 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/NodeState.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/NodeState.java @@ -610,7 +610,7 @@ public boolean isSelected() { public void evict() { evicted = true; - restartIncomingEdges(); + restartIncomingEdges(false); } boolean shouldIncludedInGraphResult() { @@ -971,18 +971,18 @@ public void restart(ComponentState selected) { } } else { if (!incomingEdges.isEmpty()) { - restartIncomingEdges(); + restartIncomingEdges(true); } } } - private void restartIncomingEdges() { + private void restartIncomingEdges(boolean checkUnattached) { if (incomingEdges.size() == 1) { EdgeState singleEdge = incomingEdges.get(0); - singleEdge.restart(); + singleEdge.restart(checkUnattached); } else { for (EdgeState dependency : new ArrayList<>(incomingEdges)) { - dependency.restart(); + dependency.restart(checkUnattached); } } clearIncomingEdges();