From 1b720bf48eec0fe7ef281d787d0cadef524d53bf 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 007a8fe5b94b..30e0fba3f601 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 5b10bdbb5465..3580503ec9d3 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 @@ -615,7 +615,7 @@ public boolean isSelected() { public void evict() { evicted = true; - restartIncomingEdges(); + restartIncomingEdges(false); } boolean shouldIncludedInGraphResult() { @@ -976,18 +976,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();