Skip to content

Commit

Permalink
Cache DependencyState after substitution
Browse files Browse the repository at this point in the history
Failing to do that was breaking the contract of DependencyState
deduplication.

Issue #23521
  • Loading branch information
ljacomet committed Jan 27, 2023
1 parent 50c8d05 commit 54b911e
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 5 deletions.
Expand Up @@ -20,6 +20,7 @@
import org.gradle.api.artifacts.component.ComponentSelector;
import org.gradle.api.artifacts.component.ModuleComponentSelector;
import org.gradle.api.internal.artifacts.ComponentSelectorConverter;
import org.gradle.api.internal.artifacts.ivyservice.dependencysubstitution.DependencySubstitutionApplicator;
import org.gradle.api.internal.artifacts.ivyservice.resolveengine.result.ComponentSelectionDescriptorInternal;
import org.gradle.internal.Describables;
import org.gradle.internal.component.model.DefaultIvyArtifactName;
Expand All @@ -30,7 +31,10 @@
import org.gradle.internal.resolve.ModuleVersionResolveException;

import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.function.Function;
import java.util.stream.Collectors;

import static org.gradle.api.internal.artifacts.ivyservice.resolveengine.result.ComponentSelectionReasons.BY_ANCESTOR;
Expand All @@ -48,6 +52,7 @@ class DependencyState {
private ModuleIdentifier moduleIdentifier;
public ModuleVersionResolveException failure;
private boolean reasonsAlreadyAdded;
private Map<DependencySubstitutionApplicator.SubstitutionResult, DependencyState> substitutionResultMap;

DependencyState(DependencyMetadata dependency, ComponentSelectorConverter componentSelectorConverter) {
this(dependency, dependency.getSelector(), Collections.emptyList(), componentSelectorConverter);
Expand Down Expand Up @@ -194,4 +199,11 @@ public boolean equals(Object o) {
public int hashCode() {
return hashCode;
}

public DependencyState withSubstitution(DependencySubstitutionApplicator.SubstitutionResult substitutionResult, Function<DependencySubstitutionApplicator.SubstitutionResult, DependencyState> mappingFunction) {
if (substitutionResultMap == null) {
substitutionResultMap = new HashMap<>();
}
return substitutionResultMap.computeIfAbsent(substitutionResult, mappingFunction);
}
}
Expand Up @@ -634,11 +634,14 @@ static DependencyState maybeSubstitute(DependencyState dependencyState, Dependen

DependencySubstitutionInternal details = substitutionResult.getResult();
if (details != null && details.isUpdated()) {
ArtifactSelectionDetailsInternal artifactSelectionDetails = details.getArtifactSelectionDetails();
if (artifactSelectionDetails.isUpdated()) {
return dependencyState.withTargetAndArtifacts(details.getTarget(), artifactSelectionDetails.getTargetSelectors(), details.getRuleDescriptors());
}
return dependencyState.withTarget(details.getTarget(), details.getRuleDescriptors());
// This caching works because our substitutionResult are cached themselves
return dependencyState.withSubstitution(substitutionResult, result -> {
ArtifactSelectionDetailsInternal artifactSelectionDetails = details.getArtifactSelectionDetails();
if (artifactSelectionDetails.isUpdated()) {
return dependencyState.withTargetAndArtifacts(details.getTarget(), artifactSelectionDetails.getTargetSelectors(), details.getRuleDescriptors());
}
return dependencyState.withTarget(details.getTarget(), details.getRuleDescriptors());
});
}
return dependencyState;
}
Expand Down

0 comments on commit 54b911e

Please sign in to comment.