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 0b5fe2dd8162..4784e3248844 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 @@ -717,6 +717,132 @@ class JavaPlatformResolveIntegrationTest extends AbstractHttpDependencyResolutio } } + def 'platform deselection does not cause orphan edges'() { + given: + 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.1') + constraint('org.test', 'depB', '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.1') + constraint('org.test', 'depB', '1.0') + }.publish() + def depA = mavenHttpRepo.module('org.test', 'depA', '1.0').withModuleMetadata() + .withVariant('runtime') { + dependsOn('org.test', 'platform', '1.0', null, [(Category.CATEGORY_ATTRIBUTE.name): Category.REGULAR_PLATFORM]) + dependsOn('org.test', 'depB', '1.0') + }.publish() + def depA11 = mavenHttpRepo.module('org.test', 'depA', '1.1').withModuleMetadata() + .withVariant('runtime') { + dependsOn('org.test', 'platform', '1.0', null, [(Category.CATEGORY_ATTRIBUTE.name): Category.REGULAR_PLATFORM]) + dependsOn('org.test', 'depB', '1.0') + }.publish() + def depB = mavenHttpRepo.module('org.test', 'depB', '1.0').withModuleMetadata() + .withVariant('runtime') { + dependsOn('org.test', 'platform', '1.0', null, [(Category.CATEGORY_ATTRIBUTE.name): Category.REGULAR_PLATFORM]) + }.publish() + + def otherPlatform10 = mavenHttpRepo.module('org.test', 'otherPlatform', '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', 'depD', '1.0') + constraint('org.test', 'test', '1.9') + } + .withVariant('runtimeElements') { + useDefaultArtifacts = false + attribute(Usage.USAGE_ATTRIBUTE.name, Usage.JAVA_RUNTIME) + attribute(Category.CATEGORY_ATTRIBUTE.name, Category.REGULAR_PLATFORM) + constraint('org.test', 'depD', '1.0') + constraint('org.test', 'test', '1.9') + }.publish() + def otherPlatform11 = mavenHttpRepo.module('org.test', 'otherPlatform', '1.1').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', 'depD', '1.0') + constraint('org.test', 'test', '1.9') + } + .withVariant('runtimeElements') { + useDefaultArtifacts = false + attribute(Usage.USAGE_ATTRIBUTE.name, Usage.JAVA_RUNTIME) + attribute(Category.CATEGORY_ATTRIBUTE.name, Category.REGULAR_PLATFORM) + constraint('org.test', 'depD', '1.0') + constraint('org.test', 'test', '1.9') + }.publish() + def depE = mavenHttpRepo.module('org.test', 'depE', '1.0').withModuleMetadata() + .withVariant('runtime') { + dependsOn('org.test', 'otherPlatform', '1.1', null, [(Category.CATEGORY_ATTRIBUTE.name): Category.REGULAR_PLATFORM]) + }.publish() + def depD = mavenHttpRepo.module('org.test', 'depD', '1.0').dependsOn(depE).publish() + def depC = mavenHttpRepo.module('org.test', 'depC', '1.0').dependsOn(depD).publish() + + depA.allowAll() + depA11.allowAll() + depB.allowAll() + depC.allowAll() + depD.allowAll() + depE.allowAll() + platform.allowAll() + otherPlatform10.allowAll() + otherPlatform11.allowAll() + + buildFile << """ + configurations { + conf.dependencies.clear() + } + + dependencies { + conf 'org.test:depA:1.0' + conf platform('org.test:otherPlatform:1.0') + conf 'org.test:depC:1.0' + } +""" + checkConfiguration("conf") + resolve.expectDefaultConfiguration("runtime") + + when: + succeeds 'checkDeps' + + then: + resolve.expectGraph { + root(":", "org.test:test:1.9") { + edge('org.test:depA:1.0', 'org.test:depA:1.1') { + module('org.test:platform:1.0') { + noArtifacts() + constraint('org.test:depA:1.1') + constraint('org.test:depB:1.0') + } + module('org.test:depB:1.0') { + module('org.test:platform:1.0') + } + } + edge("org.test:otherPlatform:1.0", "org.test:otherPlatform:1.1") { + noArtifacts() + constraint('org.test:depD:1.0') + } + module("org.test:depC:1.0") { + module('org.test:depD:1.0') { + module('org.test:depE:1.0') { + module('org.test:otherPlatform:1.1') { + noArtifacts() + } + } + } + } + } + } + } + private void checkConfiguration(String configuration) { resolve = new ResolveTestFixture(buildFile, configuration) 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 2c174a8a534d..198d5a12b70a 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 @@ -65,7 +65,7 @@ private boolean markNoLongerPending(PendingDependencies pendingDependencies) { boolean activatedPending = false; if (pendingDependencies.hasPendingComponents()) { if (noLongerPending == null) { - noLongerPending = Lists.newLinkedList(); + noLongerPending = Lists.newArrayList(); } noLongerPending.add(pendingDependencies); activatedPending = pendingDependencies.shouldReportActivatePending(); 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 6a3089067ecb..ed930159eaa5 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 @@ -391,6 +391,10 @@ void addPendingNode(NodeState node) { pendingDependencies.addNode(node); } + void removePendingNode(NodeState nodeState) { + pendingDependencies.removeNode(nodeState); + } + public void maybeUpdateSelection() { if (replaced) { // Never update selection for a replaced module 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 3580503ec9d3..b3f17dc30fa8 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 @@ -235,23 +235,11 @@ public void visitOutgoingDependencies(Collection discoveredEdges) { if (!component.isSelected()) { LOGGER.debug("version for {} is not selected. ignoring.", this); - if (upcomingNoLongerPendingConstraints != null) { - for (ModuleIdentifier identifier : upcomingNoLongerPendingConstraints) { - ModuleResolveState module = resolveState.getModule(identifier); - for (EdgeState unattachedDependency : module.getUnattachedDependencies()) { - if (!unattachedDependency.getSelector().isResolved()) { - // Unresolved - we have a selector that was deferred but the constraint has been removed in between - NodeState from = unattachedDependency.getFrom(); - from.prepareToRecomputeEdge(unattachedDependency); - } - } - } - upcomingNoLongerPendingConstraints = null; - } + cleanupConstraints(); return; } - // Check if there are any transitive incoming edges at all. Don't traverse if not. + // Check if there are any transitive incoming edges at all. Don't traverse if not. if (transitiveEdgeCount == 0 && !isRoot()) { handleNonTransitiveNode(discoveredEdges); return; @@ -289,6 +277,42 @@ public void visitOutgoingDependencies(Collection discoveredEdges) { visitOwners(discoveredEdges); } + /* + * When a node exits the graph, its constraints need to be cleaned up. + * This means: + * * Rescheduling any deferred selection impacted by a constraint coming from this node + * * Making sure we no longer are registered as pending interest on nodes pointed by constraints + */ + private void cleanupConstraints() { + // This part covers constraint that were taken into account between a selection being deferred and this node being scheduled for traversal + if (upcomingNoLongerPendingConstraints != null) { + for (ModuleIdentifier identifier : upcomingNoLongerPendingConstraints) { + ModuleResolveState module = resolveState.getModule(identifier); + for (EdgeState unattachedDependency : module.getUnattachedDependencies()) { + if (!unattachedDependency.getSelector().isResolved()) { + // Unresolved - we have a selector that was deferred but the constraint has been removed in between + NodeState from = unattachedDependency.getFrom(); + from.prepareToRecomputeEdge(unattachedDependency); + } + } + } + upcomingNoLongerPendingConstraints = null; + } + // This part covers constraint that might be triggered in the future if the node they point gains a real edge + if (cachedFilteredDependencyStates != null && !cachedFilteredDependencyStates.isEmpty()) { + // We may have registered this node as pending if it had constraints. + // Let's clear that state since it is no longer part of selection + for (DependencyState dependencyState : cachedFilteredDependencyStates) { + if (dependencyState.getDependency().isConstraint()) { + ModuleResolveState targetModule = resolveState.getModule(dependencyState.getModuleIdentifier()); + if (targetModule.isPending()) { + targetModule.removePendingNode(this); + } + } + } + } + } + private boolean excludesSameDependenciesAsPreviousTraversal(ExcludeSpec newResolutionFilter) { List oldStates = cachedFilteredDependencyStates; if (previousTraversalExclusions == null || oldStates == null) { @@ -361,6 +385,7 @@ private boolean isVirtualPlatformNeedsRefresh() { * @param discoveredEdges In/Out parameter collecting dependencies or platforms */ private void handleNonTransitiveNode(Collection discoveredEdges) { + cleanupConstraints(); // If node was previously traversed, need to remove outgoing edges. if (previousTraversalExclusions != null) { removeOutgoingEdges(); @@ -601,10 +626,10 @@ void addIncomingEdge(EdgeState dependencyEdge) { void removeIncomingEdge(EdgeState dependencyEdge) { if (incomingEdges.remove(dependencyEdge)) { incomingHash -= dependencyEdge.hashCode(); - resolveState.onFewerSelected(this); if (dependencyEdge.isTransitive()) { transitiveEdgeCount--; } + resolveState.onFewerSelected(this); } } 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 5ddba1a65bce..d8ece3835e27 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 @@ -33,16 +33,24 @@ public class PendingDependencies { this.reportActivePending = true; } - void addNode(NodeState state) { + void addNode(NodeState nodeState) { if (hardEdges != 0) { throw new IllegalStateException("Cannot add a pending node for a dependency which is not pending"); } - affectedComponents.add(state); - if (state.getComponent().getModule().isVirtualPlatform()) { + affectedComponents.add(nodeState); + if (nodeState.getComponent().getModule().isVirtualPlatform()) { reportActivePending = false; } } + public void removeNode(NodeState nodeState) { + if (hardEdges != 0) { + throw new IllegalStateException("Cannot remove a pending node for a dependency which is not pending"); + } + boolean removed = affectedComponents.remove(nodeState); + assert removed : "Removed a pending node that was not registered"; + } + void turnIntoHardDependencies() { for (NodeState affectedComponent : affectedComponents) { affectedComponent.prepareForConstraintNoLongerPending(moduleIdentifier); @@ -71,4 +79,5 @@ void decreaseHardEdgeCount() { public boolean shouldReportActivatePending() { return reportActivePending; } + }