From 5d7efbd63a0d5e9e47f4ece15cbd8b8355ac0c79 Mon Sep 17 00:00:00 2001 From: Louis Jacomet Date: Wed, 15 Jul 2020 11:14:31 +0200 Subject: [PATCH] Stop accepting selection changes Prior to this change, in some corner cases, dependency graph building would never terminate, with selection of some modules alternating repeatedly. With these changes, we now ignore selection changes after a high threshold. This prevents unstable graph to loop in resolution for ever. Fixes #13450 --- .../graph/builder/DependencyGraphBuilder.java | 2 +- .../graph/builder/ModuleResolveState.java | 40 ++++++++++++++++++- .../graph/builder/ResolveState.java | 8 +++- 3 files changed, 45 insertions(+), 5 deletions(-) diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/DependencyGraphBuilder.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/DependencyGraphBuilder.java index 68fd5bcc8b08..b3ef145eb138 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/DependencyGraphBuilder.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/DependencyGraphBuilder.java @@ -139,7 +139,7 @@ public void resolve(final ResolveContext resolveContext, final DependencyGraphVi int graphSize = estimateSize(resolveContext); ResolutionStrategyInternal resolutionStrategy = resolveContext.getResolutionStrategy(); - final ResolveState resolveState = new ResolveState(idGenerator, rootModule, resolveContext.getName(), idResolver, metaDataResolver, edgeFilter, attributesSchema, moduleExclusions, componentSelectorConverter, attributesFactory, dependencySubstitutionApplicator, versionSelectorScheme, versionComparator, versionParser, moduleConflictHandler.getResolver(), graphSize); + final ResolveState resolveState = new ResolveState(idGenerator, rootModule, resolveContext.getName(), idResolver, metaDataResolver, edgeFilter, attributesSchema, moduleExclusions, componentSelectorConverter, attributesFactory, dependencySubstitutionApplicator, versionSelectorScheme, versionComparator, versionParser, moduleConflictHandler.getResolver(), graphSize, resolveContext.getResolutionStrategy().getConflictResolution()); Map componentIdentifierCache = Maps.newHashMapWithExpectedSize(graphSize / 2); traverseGraph(resolveState, componentIdentifierCache); 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 581a12c1f7a6..29a35c6784f4 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 @@ -23,7 +23,9 @@ import org.gradle.api.artifacts.component.ComponentIdentifier; import org.gradle.api.artifacts.component.ComponentSelector; import org.gradle.api.artifacts.component.ModuleComponentIdentifier; +import org.gradle.api.artifacts.component.ProjectComponentIdentifier; import org.gradle.api.attributes.AttributeContainer; +import org.gradle.api.internal.artifacts.configurations.ConflictResolution; import org.gradle.api.internal.artifacts.ivyservice.ivyresolve.strategy.Version; import org.gradle.api.internal.artifacts.ivyservice.ivyresolve.strategy.VersionParser; import org.gradle.api.internal.artifacts.ivyservice.resolveengine.graph.conflicts.CandidateModule; @@ -36,6 +38,8 @@ import org.gradle.internal.component.model.ForcingDependencyMetadata; import org.gradle.internal.id.IdGenerator; import org.gradle.internal.resolve.resolver.ComponentMetaDataResolver; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import javax.annotation.Nullable; import java.util.ArrayList; @@ -52,12 +56,16 @@ * Resolution state for a given module. */ class ModuleResolveState implements CandidateModule { + private static final Logger LOGGER = LoggerFactory.getLogger(ModuleResolveState.class); + private static final int MAX_SELECTION_CHANGE = 1000; + private final ComponentMetaDataResolver metaDataResolver; private final IdGenerator idGenerator; private final ModuleIdentifier id; private final List unattachedDependencies = new LinkedList<>(); private final Map versions = new LinkedHashMap<>(); private final ModuleSelectors selectors; + private final ConflictResolution conflictResolution; private final ImmutableAttributesFactory attributesFactory; private final Comparator versionComparator; private final VersionParser versionParser; @@ -74,6 +82,7 @@ class ModuleResolveState implements CandidateModule { private Set platformOwners; private boolean replaced = false; private boolean changingSelection; + private int selectionChangedCounter; ModuleResolveState(IdGenerator idGenerator, ModuleIdentifier id, @@ -82,7 +91,9 @@ class ModuleResolveState implements CandidateModule { Comparator versionComparator, VersionParser versionParser, SelectorStateResolver selectorStateResolver, - ResolveOptimizations resolveOptimizations, boolean rootModule) { + ResolveOptimizations resolveOptimizations, + boolean rootModule, + ConflictResolution conflictResolution) { this.idGenerator = idGenerator; this.id = id; this.metaDataResolver = metaDataResolver; @@ -93,7 +104,8 @@ class ModuleResolveState implements CandidateModule { this.rootModule = rootModule; this.pendingDependencies = new PendingDependencies(id); this.selectorStateResolver = selectorStateResolver; - selectors = new ModuleSelectors(versionComparator); + this.selectors = new ModuleSelectors<>(versionComparator); + this.conflictResolution = conflictResolution; } void setSelectorStateResolver(SelectorStateResolver selectorStateResolver) { @@ -411,10 +423,34 @@ public void maybeUpdateSelection() { if (selected == null) { select(newSelected); } else if (newSelected != selected) { + if (++selectionChangedCounter > MAX_SELECTION_CHANGE) { + // Let's ignore modules that are changing selection way too much, by keeping the highest version + if (maybeSkipSelectionChange(newSelected)) { + return; + } + } changeSelection(newSelected); } } + private boolean maybeSkipSelectionChange(ComponentState newSelected) { + if (selectionChangedCounter == MAX_SELECTION_CHANGE + 1) { + LOGGER.warn("The dependency resolution engine wasn't able to find a version of module {} which satisfied all requirements because the graph wasn't stable enough. " + + "The highest version was selected in order to stabilize selection.\n" + + "Features available in a stable graph like version alignment are not guaranteed in this case.", id); + } + boolean newSelectedIsProject = false; + if (conflictResolution == ConflictResolution.preferProjectModules) { + if (newSelected.getComponentId() instanceof ProjectComponentIdentifier) { + // Keep the project selected + newSelectedIsProject = true; + } + } + Version newVersion = versionParser.transform(newSelected.getVersion()); + Version currentVersion = versionParser.transform(selected.getVersion()); + return !newSelectedIsProject && versionComparator.compare(newVersion, currentVersion) <= 0; + } + void maybeCreateVirtualMetadata(ResolveState resolveState) { for (ComponentState componentState : versions.values()) { if (componentState.getMetadata() == null) { diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/ResolveState.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/ResolveState.java index 379dead585eb..5ccc59e08261 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/ResolveState.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/ResolveState.java @@ -26,6 +26,7 @@ import org.gradle.api.internal.artifacts.ComponentSelectorConverter; import org.gradle.api.internal.artifacts.ResolvedConfigurationIdentifier; import org.gradle.api.internal.artifacts.ResolvedVersionConstraint; +import org.gradle.api.internal.artifacts.configurations.ConflictResolution; import org.gradle.api.internal.artifacts.dependencies.DefaultResolvedVersionConstraint; import org.gradle.api.internal.artifacts.ivyservice.dependencysubstitution.DependencySubstitutionApplicator; import org.gradle.api.internal.artifacts.ivyservice.ivyresolve.strategy.Version; @@ -70,6 +71,7 @@ class ResolveState implements ComponentStateFactory { private final DependencyToComponentIdResolver idResolver; private final ComponentMetaDataResolver metaDataResolver; private final Deque queue; + private final ConflictResolution conflictResolution; private final AttributesSchemaInternal attributesSchema; private final ModuleExclusions moduleExclusions; private final DeselectVersionAction deselectVersionAction = new DeselectVersionAction(this); @@ -100,7 +102,8 @@ public ResolveState(IdGenerator idGenerator, Comparator versionComparator, VersionParser versionParser, ModuleConflictResolver conflictResolver, - int graphSize) { + int graphSize, + ConflictResolution conflictResolution) { this.idGenerator = idGenerator; this.idResolver = idResolver; this.metaDataResolver = metaDataResolver; @@ -117,6 +120,7 @@ public ResolveState(IdGenerator idGenerator, this.nodes = new LinkedHashMap<>(3 * graphSize / 2); this.selectors = new LinkedHashMap<>(5 * graphSize / 2); this.queue = new ArrayDeque<>(graphSize); + this.conflictResolution = conflictResolution; this.resolveOptimizations = new ResolveOptimizations(); this.attributeDesugaring = new AttributeDesugaring(attributesFactory); // Create root module @@ -149,7 +153,7 @@ public ModuleResolveState getModule(ModuleIdentifier id) { } private ModuleResolveState getModule(ModuleIdentifier id, boolean rootModule) { - return modules.computeIfAbsent(id, mid -> new ModuleResolveState(idGenerator, id, metaDataResolver, attributesFactory, versionComparator, versionParser, selectorStateResolver, resolveOptimizations, rootModule)); + return modules.computeIfAbsent(id, mid -> new ModuleResolveState(idGenerator, id, metaDataResolver, attributesFactory, versionComparator, versionParser, selectorStateResolver, resolveOptimizations, rootModule, conflictResolution)); } @Override