From 03903bbe99759efed769bf9d800d7b98d635e25d Mon Sep 17 00:00:00 2001 From: Jendrik Johannes Date: Wed, 9 Oct 2019 20:18:15 +0200 Subject: [PATCH 01/19] Remove private 'firstSeenDependency' field This is just a shortcut to a final filed in 'dependencyState'. Sometimes it was used and sometimes 'dependencyState.getDepenedency()' was used which is always the same value. This makes this complex code a bit easier to comprehend. --- .../resolveengine/graph/builder/SelectorState.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/SelectorState.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/SelectorState.java index 01d33fc4e750..714fa2cd0166 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/SelectorState.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/SelectorState.java @@ -59,7 +59,6 @@ class SelectorState implements DependencyGraphSelector, ResolvableSelectorState { private final Long id; private final DependencyState dependencyState; - private final DependencyMetadata firstSeenDependency; private final DependencyToComponentIdResolver resolver; private final ResolvedVersionConstraint versionConstraint; private final List dependencyReasons = Lists.newArrayListWithExpectedSize(4); @@ -92,10 +91,9 @@ class SelectorState implements DependencyGraphSelector, ResolvableSelectorState } update(dependencyState); this.dependencyState = dependencyState; - this.firstSeenDependency = dependencyState.getDependency(); this.versionConstraint = versionByAncestor ? resolveState.resolveVersionConstraint(DefaultImmutableVersionConstraint.of()) : - resolveState.resolveVersionConstraint(firstSeenDependency.getSelector()); + resolveState.resolveVersionConstraint(dependencyState.getDependency().getSelector()); this.isProjectSelector = getSelector() instanceof ProjectComponentSelector; this.attributeDesugaring = resolveState.getAttributeDesugaring(); } @@ -133,7 +131,7 @@ public Long getResultId() { @Override public String toString() { - return firstSeenDependency.toString(); + return dependencyState.getDependency().toString(); } @Override @@ -181,7 +179,7 @@ private ComponentIdResolveResult resolve(VersionSelector selector, VersionSelect if (dependencyState.failure != null) { idResolveResult.failed(dependencyState.failure); } else { - resolver.resolve(firstSeenDependency, selector, rejector, idResolveResult); + resolver.resolve(dependencyState.getDependency(), selector, rejector, idResolveResult); } if (idResolveResult.getFailure() != null) { @@ -291,7 +289,7 @@ public void addReasonsForSelector(ComponentSelectionReasonInternal selectionReas } public DependencyMetadata getDependencyMetadata() { - return firstSeenDependency; + return dependencyState.getDependency(); } @Override From a71cc911bd505f0f0b60c602fab8d42a9c3d49d1 Mon Sep 17 00:00:00 2001 From: Jendrik Johannes Date: Wed, 9 Oct 2019 22:59:05 +0200 Subject: [PATCH 02/19] Track first dependency artifact in SelectorState and ModuleSelectors This information needs to be preserved to make sure that an artifact that acts as metadata for itself (metadataSources = artifact()) is found during the early resolution phase where we search for modules in repositories. --- .../graph/builder/ModuleSelectors.java | 10 ++++++++++ .../resolveengine/graph/builder/SelectorState.java | 14 ++++++++++++++ .../graph/selectors/ResolvableSelectorState.java | 3 +++ .../graph/selectors/TestModuleSelectorState.java | 9 +++++++++ .../selectors/TestProjectSelectorState.groovy | 14 ++++++++++---- 5 files changed, 46 insertions(+), 4 deletions(-) diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/ModuleSelectors.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/ModuleSelectors.java index cc438a07f1ce..aa81a6a6054b 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/ModuleSelectors.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/ModuleSelectors.java @@ -26,6 +26,7 @@ import org.gradle.api.internal.artifacts.ivyservice.ivyresolve.strategy.VersionSelector; import org.gradle.api.internal.artifacts.ivyservice.resolveengine.graph.selectors.ResolvableSelectorState; import org.gradle.internal.Cast; +import org.gradle.internal.component.model.IvyArtifactName; import java.util.Collections; import java.util.Comparator; @@ -172,4 +173,13 @@ public T first() { } return selectors.get(0); } + + public IvyArtifactName getFirstDependencyArtifact() { + for (T selector: selectors) { + if (selector.getFirstDependencyArtifact() != null) { + return selector.getFirstDependencyArtifact(); + } + } + return null; + } } diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/SelectorState.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/SelectorState.java index 714fa2cd0166..67578478798a 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/SelectorState.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/SelectorState.java @@ -17,6 +17,7 @@ package org.gradle.api.internal.artifacts.ivyservice.resolveengine.graph.builder; import com.google.common.base.Joiner; +import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; import org.gradle.api.Describable; import org.gradle.api.artifacts.ModuleIdentifier; @@ -33,6 +34,7 @@ import org.gradle.api.internal.artifacts.ivyservice.resolveengine.result.ComponentSelectionReasons; import org.gradle.api.internal.attributes.AttributeDesugaring; import org.gradle.internal.component.model.DependencyMetadata; +import org.gradle.internal.component.model.IvyArtifactName; import org.gradle.internal.logging.text.TreeFormatter; import org.gradle.internal.resolve.ModuleVersionResolveException; import org.gradle.internal.resolve.RejectedByAttributesVersion; @@ -73,6 +75,7 @@ class SelectorState implements DependencyGraphSelector, ResolvableSelectorState private boolean forced; private boolean softForced; private boolean fromLock; + private IvyArtifactName firstDependencyArtifact; // An internal counter used to track the number of outgoing edges // that use this selector. Since a module resolve state tracks all selectors @@ -292,6 +295,11 @@ public DependencyMetadata getDependencyMetadata() { return dependencyState.getDependency(); } + @Override + public IvyArtifactName getFirstDependencyArtifact() { + return firstDependencyArtifact; + } + @Override public ResolvedVersionConstraint getVersionConstraint() { return versionConstraint; @@ -336,6 +344,12 @@ public void update(DependencyState dependencyState) { resolved = false; // when a selector changes from non lock to lock, we must reselect } dependencyState.addSelectionReasons(dependencyReasons); + if (firstDependencyArtifact == null) { + List artifacts = dependencyState.getDependency().getArtifacts(); + if (!artifacts.isEmpty()) { + firstDependencyArtifact = artifacts.get(0); + } + } } } diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/selectors/ResolvableSelectorState.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/selectors/ResolvableSelectorState.java index 8ba95d1ac099..10b675398d43 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/selectors/ResolvableSelectorState.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/selectors/ResolvableSelectorState.java @@ -19,6 +19,7 @@ import org.gradle.api.artifacts.component.ProjectComponentSelector; import org.gradle.api.internal.artifacts.ResolvedVersionConstraint; import org.gradle.api.internal.artifacts.ivyservice.ivyresolve.strategy.VersionSelector; +import org.gradle.internal.component.model.IvyArtifactName; import org.gradle.internal.resolve.ModuleVersionResolveException; import org.gradle.internal.resolve.result.ComponentIdResolveResult; @@ -66,6 +67,8 @@ public interface ResolvableSelectorState { boolean hasStrongOpinion(); + IvyArtifactName getFirstDependencyArtifact(); + default boolean isProject() { return getSelector() instanceof ProjectComponentSelector; } diff --git a/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/selectors/TestModuleSelectorState.java b/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/selectors/TestModuleSelectorState.java index 0a8dee82c6dd..29725b90582a 100644 --- a/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/selectors/TestModuleSelectorState.java +++ b/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/selectors/TestModuleSelectorState.java @@ -24,12 +24,16 @@ import org.gradle.api.internal.artifacts.ivyservice.ivyresolve.strategy.VersionParser; import org.gradle.api.internal.artifacts.ivyservice.ivyresolve.strategy.VersionSelector; import org.gradle.api.internal.artifacts.ivyservice.ivyresolve.strategy.VersionSelectorScheme; +import org.gradle.internal.component.model.IvyArtifactName; import org.gradle.internal.resolve.ModuleVersionResolveException; import org.gradle.internal.resolve.resolver.DependencyToComponentIdResolver; import org.gradle.internal.resolve.result.BuildableComponentIdResolveResult; import org.gradle.internal.resolve.result.ComponentIdResolveResult; import org.gradle.internal.resolve.result.DefaultBuildableComponentIdResolveResult; +import java.util.Collections; +import java.util.List; + public class TestModuleSelectorState implements ResolvableSelectorState { private static final VersionParser VERSION_PARSER = new VersionParser(); @@ -117,4 +121,9 @@ public boolean isFromLock() { public boolean hasStrongOpinion() { return false; } + + @Override + public IvyArtifactName getFirstDependencyArtifact() { + return null; + } } diff --git a/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/selectors/TestProjectSelectorState.groovy b/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/selectors/TestProjectSelectorState.groovy index 1fd545cb65f6..fe85053826b9 100644 --- a/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/selectors/TestProjectSelectorState.groovy +++ b/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/selectors/TestProjectSelectorState.groovy @@ -22,6 +22,7 @@ import org.gradle.api.internal.artifacts.DefaultModuleVersionIdentifier import org.gradle.api.internal.artifacts.ResolvedVersionConstraint import org.gradle.api.internal.artifacts.ivyservice.ivyresolve.strategy.VersionSelector import org.gradle.internal.component.local.model.DefaultProjectComponentSelector +import org.gradle.internal.component.model.IvyArtifactName import org.gradle.internal.resolve.ModuleVersionResolveException import org.gradle.internal.resolve.result.ComponentIdResolveResult import org.gradle.internal.resolve.result.DefaultBuildableComponentIdResolveResult @@ -59,16 +60,16 @@ class TestProjectSelectorState implements ResolvableSelectorState { @Override void failed(ModuleVersionResolveException failure) { - throw new UnsupportedOperationException("To be implemented"); + throw new UnsupportedOperationException("To be implemented") } @Override - public void markResolved() { + void markResolved() { } @Override - public boolean isForce() { - return false; + boolean isForce() { + return false } @Override @@ -85,5 +86,10 @@ class TestProjectSelectorState implements ResolvableSelectorState { boolean hasStrongOpinion() { return false } + + @Override + IvyArtifactName getFirstDependencyArtifact() { + return null + } } From adf28d2d0aa752b8ec3bf7071b7e26d844198a43 Mon Sep 17 00:00:00 2001 From: Jendrik Johannes Date: Wed, 9 Oct 2019 22:59:35 +0200 Subject: [PATCH 03/19] Reuse one object for empty ComponentOverrideMetadata --- .../ExternalResourceResolverDescriptorParseContext.java | 2 +- .../component/model/DefaultComponentOverrideMetadata.java | 6 ++++-- .../projectmodule/ProjectDependencyResolverTest.groovy | 2 +- .../artifacts/repositories/resolver/IvyResolverTest.groovy | 4 ++-- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/repositories/resolver/ExternalResourceResolverDescriptorParseContext.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/repositories/resolver/ExternalResourceResolverDescriptorParseContext.java index 4bba2e0957e3..5ab32a12f8a9 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/repositories/resolver/ExternalResourceResolverDescriptorParseContext.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/repositories/resolver/ExternalResourceResolverDescriptorParseContext.java @@ -68,7 +68,7 @@ public LocallyAvailableExternalResource getMetaDataArtifact(ModuleDependencyMeta private File resolveMetaDataArtifactFile(ModuleComponentIdentifier moduleComponentIdentifier, ComponentMetaDataResolver componentResolver, ArtifactResolver artifactResolver, ArtifactType artifactType) { BuildableComponentResolveResult moduleVersionResolveResult = new DefaultBuildableComponentResolveResult(); - componentResolver.resolve(moduleComponentIdentifier, new DefaultComponentOverrideMetadata(), moduleVersionResolveResult); + componentResolver.resolve(moduleComponentIdentifier, DefaultComponentOverrideMetadata.EMPTY, moduleVersionResolveResult); BuildableArtifactSetResolveResult moduleArtifactsResolveResult = new DefaultBuildableArtifactSetResolveResult(); artifactResolver.resolveArtifactsWithType(moduleVersionResolveResult.getMetadata(), artifactType, moduleArtifactsResolveResult); diff --git a/subprojects/dependency-management/src/main/java/org/gradle/internal/component/model/DefaultComponentOverrideMetadata.java b/subprojects/dependency-management/src/main/java/org/gradle/internal/component/model/DefaultComponentOverrideMetadata.java index a90fa3cff13f..4265133b37c4 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/internal/component/model/DefaultComponentOverrideMetadata.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/internal/component/model/DefaultComponentOverrideMetadata.java @@ -25,6 +25,8 @@ import java.util.List; public class DefaultComponentOverrideMetadata implements ComponentOverrideMetadata { + public static final ComponentOverrideMetadata EMPTY = new DefaultComponentOverrideMetadata(); + private final boolean changing; private final List artifacts; private final ClientModule clientModule; @@ -33,8 +35,8 @@ public static ComponentOverrideMetadata forDependency(DependencyMetadata depende return new DefaultComponentOverrideMetadata(dependencyMetadata.isChanging(), dependencyMetadata.getArtifacts(), extractClientModule(dependencyMetadata)); } - public DefaultComponentOverrideMetadata() { - this(false, Collections.emptyList(), null); + private DefaultComponentOverrideMetadata() { + this(false, Collections.emptyList(), null); } private DefaultComponentOverrideMetadata(boolean changing, List artifacts, ClientModule clientModule) { diff --git a/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/projectmodule/ProjectDependencyResolverTest.groovy b/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/projectmodule/ProjectDependencyResolverTest.groovy index 28dee823f59a..781a26da947f 100644 --- a/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/projectmodule/ProjectDependencyResolverTest.groovy +++ b/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/projectmodule/ProjectDependencyResolverTest.groovy @@ -71,7 +71,7 @@ class ProjectDependencyResolverTest extends Specification { def projectComponentId = newProjectId(":projectPath") when: - resolver.resolve(projectComponentId, new DefaultComponentOverrideMetadata(), result) + resolver.resolve(projectComponentId, DefaultComponentOverrideMetadata.EMPTY, result) then: 1 * registry.getComponent(projectComponentId) >> componentMetaData diff --git a/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/repositories/resolver/IvyResolverTest.groovy b/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/repositories/resolver/IvyResolverTest.groovy index 3b25186f397b..5d20c3d4eeed 100644 --- a/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/repositories/resolver/IvyResolverTest.groovy +++ b/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/repositories/resolver/IvyResolverTest.groovy @@ -75,7 +75,7 @@ class IvyResolverTest extends Specification { @Unroll def "remote access fails directly for module id #moduleId with layout #layoutPattern"() { given: - def overrideMetadata = new DefaultComponentOverrideMetadata() + def overrideMetadata = DefaultComponentOverrideMetadata.EMPTY def result = new DefaultBuildableModuleComponentMetaDataResolveResult() when: @@ -105,7 +105,7 @@ class IvyResolverTest extends Specification { @Unroll def "remote access attempts to access metadata for id #moduleId with layout #layoutPattern"() { given: - def overrideMetadata = new DefaultComponentOverrideMetadata() + def overrideMetadata = DefaultComponentOverrideMetadata.EMPTY def result = new DefaultBuildableModuleComponentMetaDataResolveResult() when: From f515b93488aa784733d3d4ba04fe413c8f9a3934 Mon Sep 17 00:00:00 2001 From: Jendrik Johannes Date: Wed, 9 Oct 2019 23:11:08 +0200 Subject: [PATCH 04/19] Use the first found dependency artifact for override metadata Later in the resolution, we already combine all artifacts defined as 'dependency artifacts' on incoming edges. If a component does not have metadata, we need at least information about one artifact early when we look for an artifact (instead of a metadata file). --- .../ivyresolve/DynamicVersionResolver.java | 4 +++- .../ComponentResolutionState.java | 3 --- .../graph/builder/ComponentState.java | 19 +++++++++---------- .../graph/builder/ModuleResolveState.java | 1 + .../graph/builder/PotentialEdge.java | 2 -- .../SelectorStateResolverResults.java | 1 - .../query/DefaultArtifactResolutionQuery.java | 2 +- .../DefaultComponentOverrideMetadata.java | 12 ++++++------ .../AbstractConflictResolverTest.groovy | 6 ------ .../TestComponentResolutionState.java | 5 ----- 10 files changed, 20 insertions(+), 35 deletions(-) diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/ivyresolve/DynamicVersionResolver.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/ivyresolve/DynamicVersionResolver.java index d4edecbf4869..3363e69fc982 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/ivyresolve/DynamicVersionResolver.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/ivyresolve/DynamicVersionResolver.java @@ -41,6 +41,7 @@ import org.gradle.internal.component.external.model.ModuleDependencyMetadata; import org.gradle.internal.component.model.DefaultComponentOverrideMetadata; import org.gradle.internal.component.model.DependencyMetadata; +import org.gradle.internal.component.model.IvyArtifactName; import org.gradle.internal.resolve.ModuleVersionNotFoundException; import org.gradle.internal.resolve.ModuleVersionResolveException; import org.gradle.internal.resolve.RejectedByAttributesVersion; @@ -477,7 +478,8 @@ public CachePolicy getCachePolicy() { private void process(ModuleComponentRepositoryAccess access, DefaultBuildableModuleComponentMetaDataResolveResult result) { DependencyMetadata dependency = dependencyMetadata.withRequestedVersion(new DefaultImmutableVersionConstraint(version.getSource())); - access.resolveComponentMetaData(identifier, DefaultComponentOverrideMetadata.forDependency(dependency), result); + IvyArtifactName firstArtifact = dependency.getArtifacts().isEmpty() ? null : dependency.getArtifacts().get(0); + access.resolveComponentMetaData(identifier, DefaultComponentOverrideMetadata.forDependency(dependency, firstArtifact), result); attemptCollector.execute(result); } diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/ComponentResolutionState.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/ComponentResolutionState.java index 8111daa1c59c..cad3549266a0 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/ComponentResolutionState.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/ComponentResolutionState.java @@ -19,7 +19,6 @@ import org.gradle.api.artifacts.component.ComponentIdentifier; import org.gradle.api.internal.artifacts.ivyservice.ivyresolve.StringVersioned; import org.gradle.api.internal.artifacts.ivyservice.resolveengine.graph.builder.VirtualPlatformState; -import org.gradle.api.internal.artifacts.ivyservice.resolveengine.graph.selectors.ResolvableSelectorState; import org.gradle.api.internal.artifacts.ivyservice.resolveengine.result.ComponentSelectionDescriptorInternal; import org.gradle.internal.component.model.ComponentResolveMetadata; @@ -31,8 +30,6 @@ public interface ComponentResolutionState extends StringVersioned { ModuleVersionIdentifier getId(); - void selectedBy(ResolvableSelectorState selectorState); - /** * Returns the meta-data for the component. Resolves if not already resolved. * diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/ComponentState.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/ComponentState.java index cce3a617eb4f..0cb78f4b9444 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/ComponentState.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/ComponentState.java @@ -28,7 +28,6 @@ import org.gradle.api.internal.artifacts.ivyservice.resolveengine.ComponentResolutionState; import org.gradle.api.internal.artifacts.ivyservice.resolveengine.graph.DependencyGraphComponent; import org.gradle.api.internal.artifacts.ivyservice.resolveengine.graph.conflicts.VersionConflictResolutionDetails; -import org.gradle.api.internal.artifacts.ivyservice.resolveengine.graph.selectors.ResolvableSelectorState; import org.gradle.api.internal.artifacts.ivyservice.resolveengine.result.ComponentSelectionDescriptorInternal; import org.gradle.api.internal.artifacts.ivyservice.resolveengine.result.ComponentSelectionReasonInternal; import org.gradle.api.internal.artifacts.ivyservice.resolveengine.result.ComponentSelectionReasons; @@ -66,7 +65,7 @@ public class ComponentState implements ComponentResolutionState, DependencyGraph private ComponentSelectionState state = ComponentSelectionState.Selectable; private ModuleVersionResolveException metadataResolveFailure; - private SelectorState firstSelectedBy; + private ModuleSelectors selectors; private DependencyGraphBuilder.VisitState visitState = DependencyGraphBuilder.VisitState.NotSeen; private boolean rejected; @@ -165,11 +164,8 @@ public void restartIncomingEdges(ComponentState selected) { } } - @Override - public void selectedBy(ResolvableSelectorState selectorState) { - if (firstSelectedBy == null) { - firstSelectedBy = (SelectorState) selectorState; - } + public void setSelectors(ModuleSelectors selectors) { + this.selectors = selectors; } /** @@ -186,9 +182,12 @@ public void resolve() { return; } - // Any metadata overrides (e.g classifier/artifacts/client-module) will be taken from the first dependency that referenced this component - ComponentOverrideMetadata componentOverrideMetadata = DefaultComponentOverrideMetadata.forDependency(firstSelectedBy.getDependencyMetadata()); - + ComponentOverrideMetadata componentOverrideMetadata; + if (selectors != null && selectors.size() > 0) { + componentOverrideMetadata = DefaultComponentOverrideMetadata.forDependency(selectors.first().getDependencyMetadata(), selectors.getFirstDependencyArtifact()); + } else { + componentOverrideMetadata = DefaultComponentOverrideMetadata.EMPTY; + } DefaultBuildableComponentResolveResult result = new DefaultBuildableComponentResolveResult(); if (tryResolveVirtualPlatform()) { return; 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 a8aed8770094..37b008efc240 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 @@ -370,6 +370,7 @@ public boolean maybeUpdateSelection() { return false; } ComponentState newSelected = selectorStateResolver.selectBest(getId(), selectors); + newSelected.setSelectors(selectors); if (selected == null) { select(newSelected); return true; diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/PotentialEdge.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/PotentialEdge.java index 598854b399ad..623db9fc9ab3 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/PotentialEdge.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/PotentialEdge.java @@ -54,8 +54,6 @@ static PotentialEdge of(ResolveState resolveState, NodeState from, ModuleCompone edge.computeSelector(); ModuleVersionIdentifier toModuleVersionId = DefaultModuleVersionIdentifier.newId(toSelector.getModuleIdentifier(), toSelector.getVersion()); ComponentState version = resolveState.getModule(toSelector.getModuleIdentifier()).getVersion(toModuleVersionId, toComponent); - SelectorState selector = edge.getSelector(); - version.selectedBy(selector); // We need to check if the target version exists. For this, we have to try to get metadata for the aligned version. // If it's there, it means we can align, otherwise, we must NOT add the edge, or resolution would fail ComponentResolveMetadata metadata = version.getMetadata(); diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/selectors/SelectorStateResolverResults.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/selectors/SelectorStateResolverResults.java index 899501aa493d..5784f46a549a 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/selectors/SelectorStateResolverResults.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/selectors/SelectorStateResolverResults.java @@ -107,7 +107,6 @@ private boolean hasSoftForce() { public static T componentForIdResolveResult(ComponentStateFactory componentFactory, ComponentIdResolveResult idResolveResult, ResolvableSelectorState selector) { T component = componentFactory.getRevision(idResolveResult.getId(), idResolveResult.getModuleVersionId(), idResolveResult.getMetadata()); - component.selectedBy(selector); if (idResolveResult.isRejected()) { component.reject(); } diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/query/DefaultArtifactResolutionQuery.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/query/DefaultArtifactResolutionQuery.java index 5fc56677327a..ec3af0d264c9 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/query/DefaultArtifactResolutionQuery.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/query/DefaultArtifactResolutionQuery.java @@ -169,7 +169,7 @@ private ComponentIdentifier validateComponentIdentifier(ComponentIdentifier comp private ComponentArtifactsResult buildComponentResult(ComponentIdentifier componentId, ComponentMetaDataResolver componentMetaDataResolver, ArtifactResolver artifactResolver) { BuildableComponentResolveResult moduleResolveResult = new DefaultBuildableComponentResolveResult(); - componentMetaDataResolver.resolve(componentId, new DefaultComponentOverrideMetadata(), moduleResolveResult); + componentMetaDataResolver.resolve(componentId, DefaultComponentOverrideMetadata.EMPTY, moduleResolveResult); ComponentResolveMetadata component = moduleResolveResult.getMetadata(); DefaultComponentArtifactsResult componentResult = new DefaultComponentArtifactsResult(component.getId()); for (Class artifactType : artifactTypes) { diff --git a/subprojects/dependency-management/src/main/java/org/gradle/internal/component/model/DefaultComponentOverrideMetadata.java b/subprojects/dependency-management/src/main/java/org/gradle/internal/component/model/DefaultComponentOverrideMetadata.java index 4265133b37c4..0f7deeb02e51 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/internal/component/model/DefaultComponentOverrideMetadata.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/internal/component/model/DefaultComponentOverrideMetadata.java @@ -25,23 +25,23 @@ import java.util.List; public class DefaultComponentOverrideMetadata implements ComponentOverrideMetadata { - public static final ComponentOverrideMetadata EMPTY = new DefaultComponentOverrideMetadata(); + public static final ComponentOverrideMetadata EMPTY = new DefaultComponentOverrideMetadata(false, (IvyArtifactName) null, null); private final boolean changing; private final List artifacts; private final ClientModule clientModule; - public static ComponentOverrideMetadata forDependency(DependencyMetadata dependencyMetadata) { - return new DefaultComponentOverrideMetadata(dependencyMetadata.isChanging(), dependencyMetadata.getArtifacts(), extractClientModule(dependencyMetadata)); + public static ComponentOverrideMetadata forDependency(DependencyMetadata dependencyMetadata, IvyArtifactName mainArtifact) { + return new DefaultComponentOverrideMetadata(dependencyMetadata.isChanging(), mainArtifact, extractClientModule(dependencyMetadata)); } - private DefaultComponentOverrideMetadata() { - this(false, Collections.emptyList(), null); + private DefaultComponentOverrideMetadata(boolean changing, IvyArtifactName artifact, ClientModule clientModule) { + this(changing, artifact == null ? Collections.emptyList() : ImmutableList.of(artifact), clientModule); } private DefaultComponentOverrideMetadata(boolean changing, List artifacts, ClientModule clientModule) { this.changing = changing; - this.artifacts = ImmutableList.copyOf(artifacts); + this.artifacts = artifacts; this.clientModule = clientModule; } diff --git a/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/resolveengine/AbstractConflictResolverTest.groovy b/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/resolveengine/AbstractConflictResolverTest.groovy index 3e0099ed0bbb..c63105a4d1ea 100644 --- a/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/resolveengine/AbstractConflictResolverTest.groovy +++ b/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/resolveengine/AbstractConflictResolverTest.groovy @@ -23,7 +23,6 @@ import org.gradle.api.internal.artifacts.DefaultModuleVersionIdentifier import org.gradle.api.internal.artifacts.dependencies.DefaultMutableVersionConstraint import org.gradle.api.internal.artifacts.ivyservice.resolveengine.graph.builder.VirtualPlatformState import org.gradle.api.internal.artifacts.ivyservice.resolveengine.graph.conflicts.DefaultConflictResolverDetails -import org.gradle.api.internal.artifacts.ivyservice.resolveengine.graph.selectors.ResolvableSelectorState import org.gradle.api.internal.artifacts.ivyservice.resolveengine.result.ComponentSelectionDescriptorInternal import org.gradle.internal.component.external.model.DefaultModuleComponentIdentifier import org.gradle.internal.component.model.ComponentResolveMetadata @@ -123,11 +122,6 @@ abstract class AbstractConflictResolverTest extends Specification { String toString() { id } - @Override - void selectedBy(ResolvableSelectorState selectorState) { - - } - @Override Set getPlatformOwners() { Collections.emptySet() diff --git a/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/selectors/TestComponentResolutionState.java b/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/selectors/TestComponentResolutionState.java index 5d4549fc0263..e7ab368a2228 100644 --- a/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/selectors/TestComponentResolutionState.java +++ b/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/selectors/TestComponentResolutionState.java @@ -57,11 +57,6 @@ public ModuleVersionIdentifier getId() { return id; } - @Override - public void selectedBy(ResolvableSelectorState selectorState) { - - } - @Nullable @Override public ComponentResolveMetadata getMetadata() { From 0395f2e512d06ef1fd9131522b08f0b55218b973 Mon Sep 17 00:00:00 2001 From: Jendrik Johannes Date: Wed, 9 Oct 2019 22:16:42 +0200 Subject: [PATCH 05/19] Support artifacts with different names in maven module fixture Such artifacts can actually be addressed by 'dependency artifacts' through the 'artifact { }' notation inside a dependency declaration; or in Gradle Module Metadata. --- .../gradle/test/fixtures/maven/AbstractMavenModule.groovy | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/maven/AbstractMavenModule.groovy b/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/maven/AbstractMavenModule.groovy index d1da9c5f3db2..288077e5e4ee 100644 --- a/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/maven/AbstractMavenModule.groovy +++ b/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/maven/AbstractMavenModule.groovy @@ -421,12 +421,13 @@ abstract class AbstractMavenModule extends AbstractModule implements MavenModule ModuleArtifact getArtifact(Map options) { def artifact = toArtifact(options) def suffix = (artifact.classifier ? "-${artifact.classifier}" : "") + (artifact.type ? ".${artifact.type}" : "") + def artifactName = artifact.name ? artifact.name : artifactId return new ModuleArtifact() { String getFileName() { if (version.endsWith("-SNAPSHOT") && !metaDataFile.exists() && uniqueSnapshots) { - return "${artifactId}-${version}${suffix}" + return "${artifactName}-${version}${suffix}" } else { - return "$artifactId-${publishArtifactVersion}${suffix}" + return "$artifactName-${publishArtifactVersion}${suffix}" } } @@ -450,7 +451,7 @@ abstract class AbstractMavenModule extends AbstractModule implements MavenModule protected Map toArtifact(Map options) { options = new HashMap(options) - def artifact = [type: options.containsKey('type') ? options.remove('type') : type, classifier: options.remove('classifier') ?: null] + def artifact = [type: options.containsKey('type') ? options.remove('type') : type, classifier: options.remove('classifier') ?: null, name: options.containsKey('name') ? options.remove('name') : null] assert options.isEmpty(): "Unknown options : ${options.keySet()}" return artifact } From 9369a1b5915ba052b24bf8ba49438a1acfabae56 Mon Sep 17 00:00:00 2001 From: Jendrik Johannes Date: Wed, 9 Oct 2019 22:24:24 +0200 Subject: [PATCH 06/19] Use a linked hash set for dependency artifacts The order can make a difference in repository selection, when checking if an artifact-only-component containing the artifacts exists in a repository. In absence of a metadata file, we initially test for the existence of one artifact to decide if a repository contains the corresponding artifact-only-component. This is always the first artifact in the set (which is internally converted into a list). This change makes sure that the first artifact is always the same for a build that does not change. --- .../artifacts/dependencies/AbstractModuleDependency.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/subprojects/core/src/main/java/org/gradle/api/internal/artifacts/dependencies/AbstractModuleDependency.java b/subprojects/core/src/main/java/org/gradle/api/internal/artifacts/dependencies/AbstractModuleDependency.java index fb6acbd5226a..2cbfae2be9d6 100644 --- a/subprojects/core/src/main/java/org/gradle/api/internal/artifacts/dependencies/AbstractModuleDependency.java +++ b/subprojects/core/src/main/java/org/gradle/api/internal/artifacts/dependencies/AbstractModuleDependency.java @@ -36,7 +36,7 @@ import javax.annotation.Nullable; import java.util.Collections; -import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -49,7 +49,7 @@ public abstract class AbstractModuleDependency extends AbstractDependency implem private ImmutableAttributesFactory attributesFactory; private NotationParser capabilityNotationParser; private DefaultExcludeRuleContainer excludeRuleContainer = new DefaultExcludeRuleContainer(); - private Set artifacts = new HashSet(); + private Set artifacts = new LinkedHashSet<>(); private ImmutableActionSet onMutate = ImmutableActionSet.empty(); private AttributeContainerInternal attributes; private ModuleDependencyCapabilitiesInternal moduleDependencyCapabilities; @@ -142,7 +142,7 @@ public DependencyArtifact artifact(Action configureA protected void copyTo(AbstractModuleDependency target) { super.copyTo(target); - target.setArtifacts(new HashSet(getArtifacts())); + target.setArtifacts(new LinkedHashSet<>(getArtifacts())); target.setExcludeRuleContainer(new DefaultExcludeRuleContainer(getExcludeRules())); target.setTransitive(isTransitive()); if (attributes != null) { From 146c6f9a0a1d60b374be4981cff24d55a0c47e6e Mon Sep 17 00:00:00 2001 From: Jendrik Johannes Date: Wed, 9 Oct 2019 22:24:54 +0200 Subject: [PATCH 07/19] Avoid copying an already immutable list --- .../component/model/LocalComponentDependencyMetadata.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/subprojects/dependency-management/src/main/java/org/gradle/internal/component/model/LocalComponentDependencyMetadata.java b/subprojects/dependency-management/src/main/java/org/gradle/internal/component/model/LocalComponentDependencyMetadata.java index 8a3a8a0b4418..eaf3fcec18e3 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/internal/component/model/LocalComponentDependencyMetadata.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/internal/component/model/LocalComponentDependencyMetadata.java @@ -96,7 +96,7 @@ public LocalComponentDependencyMetadata(ComponentIdentifier componentId, } private static List asImmutable(List artifactNames) { - return artifactNames.isEmpty() ? Collections.emptyList() : ImmutableList.copyOf(artifactNames); + return artifactNames.isEmpty() ? Collections.emptyList() : artifactNames instanceof ImmutableList ? artifactNames : ImmutableList.copyOf(artifactNames); } @Override From 2738256301b8ec2886e003b49fac835ec1645791 Mon Sep 17 00:00:00 2001 From: Jendrik Johannes Date: Wed, 9 Oct 2019 22:35:25 +0200 Subject: [PATCH 08/19] Add integration test for dependency artifacts in multiple declarations This test reproduces https://github.com/gradle/gradle/issues/10948 and other variations of it. --- ...encyArtifactsResolveIntegrationTest.groovy | 139 ++++++++++++++++++ 1 file changed, 139 insertions(+) create mode 100644 subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/artifacts/DependencyArtifactsResolveIntegrationTest.groovy diff --git a/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/artifacts/DependencyArtifactsResolveIntegrationTest.groovy b/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/artifacts/DependencyArtifactsResolveIntegrationTest.groovy new file mode 100644 index 000000000000..68dfcb41f82c --- /dev/null +++ b/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/artifacts/DependencyArtifactsResolveIntegrationTest.groovy @@ -0,0 +1,139 @@ +/* + * Copyright 2019 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.gradle.integtests.resolve.artifacts + +import org.gradle.integtests.fixtures.GradleMetadataResolveRunner +import org.gradle.integtests.fixtures.RequiredFeature +import org.gradle.integtests.fixtures.RequiredFeatures +import org.gradle.integtests.resolve.AbstractModuleDependencyResolveTest +import spock.lang.Unroll + +/** + * There is more test coverage for 'dependency artifacts' in ArtifactDependenciesIntegrationTest (old test setup). + */ +@RequiredFeatures( + // This test bypasses all metadata using 'artifact()' metadata sources. It is sufficient to test with one metadata setup. + @RequiredFeature(feature = GradleMetadataResolveRunner.GRADLE_METADATA, value = "true") +) +class DependencyArtifactsResolveIntegrationTest extends AbstractModuleDependencyResolveTest { + + def setup() { + resolve.expectDefaultConfiguration(useMaven() ? 'runtime' : 'default') + buildFile << """ + repositories.all { + metadataSources { + artifact() //sss + } + } + """ + } + + def "can combine artifact notation and constraints"() { + given: + repository { + 'org:foo:1.0' { + withModule { + undeclaredArtifact(type: 'distribution-tgz') + } + } + } + + buildFile << """ + dependencies { + conf('org:foo@distribution-tgz') + + constraints { + conf('org:foo:1.0') + } + } + """ + + when: + repositoryInteractions { + 'org:foo:1.0' { + expectHeadArtifact(type: 'distribution-tgz') // test for the artifact when we would usually download metadata + expectHeadArtifact(type: 'distribution-tgz') // test for the artifact before actually retrieving it + expectGetArtifact(type: 'distribution-tgz') + } + } + succeeds 'checkDeps' + + then: + resolve.expectGraph { + root(":", ":test:") { + edge('org:foo', 'org:foo:1.0') { + artifact(type: 'distribution-tgz') + } + constraint('org:foo:1.0') + } + } + } + + @Unroll + def "The first artifact is used as replacement for metadata if multiple artifacts are declared using #declaration"() { + given: + repository { + 'org:foo:1.0' { + withModule { + undeclaredArtifact(name: artifactName, type: 'distribution-tgz') + undeclaredArtifact(name: artifactName, type: 'zip') + } + } + } + + buildFile << """ + dependencies { + $declaration + constraints { + conf('org:foo:1.0') + } + } + """ + + when: + repositoryInteractions { + 'org:foo:1.0' { + (0..declarationCount).each { + // These happen in parallel when Gradle downloads metadata. + // In this cases "downloading metadata" means testing for the artifact. + // Each declaration is treated separately with it's own "consumer provided" metadata + expectHeadArtifact(name: artifactName, type: 'distribution-tgz') + } + expectGetArtifact(name: artifactName, type: 'distribution-tgz') + expectGetArtifact(name: artifactName, type: 'zip') + } + } + succeeds 'checkDeps' + + then: + resolve.expectGraph { + root(":", ":test:") { + edge('org:foo', 'org:foo:1.0') { + artifact(name: artifactName, type: 'distribution-tgz') + artifact(name: artifactName, type: 'zip') + } + constraint('org:foo:1.0') + } + } + + where: + notation | artifactName | declarationCount | declaration + 'multiple dependency declarations (AT notation)' | 'foo' | 2 | "conf('org:foo@distribution-tgz'); conf('org:foo@zip')" + 'multiple dependency declarations (artifact notation)' | 'bar' | 2 | "conf('org:foo') { artifact { name = 'bar'; type = 'distribution-tgz' } }; conf('org:foo') { artifact { name = 'bar'; type = 'zip' } }" + 'multiple artifact declaration' | 'bar' | 1 | "conf('org:foo') { artifact { name = 'bar'; type = 'distribution-tgz' }; artifact { name = 'bar'; type = 'zip' } }" + } +} From 550ce00e2e4d637388292e407bf531eac1827d82 Mon Sep 17 00:00:00 2001 From: Jendrik Johannes Date: Wed, 9 Oct 2019 23:29:35 +0200 Subject: [PATCH 09/19] Make http server fixture's handle() thread safe As stated in the here implemented 'ServerWithExpectations' fixture: "handlers, as well as failures, need to be thread-safe" This concrete case was working most of the time since usually there are not more than one expectations for the same request. But if the same request is expected several times, and the requests are received in parallel (as it is the case for metadata download), the handle() method behaved flaky - by not doing reading and updating of the 'run' flag as an atomic operation. --- .../org/gradle/test/fixtures/server/ExpectOne.groovy | 8 +++++++- .../gradle/test/fixtures/server/http/HttpServer.groovy | 3 +-- .../gradle/test/fixtures/server/sftp/SFTPServer.groovy | 2 +- .../integtests/resource/gcs/fixtures/GcsServer.groovy | 2 +- .../integtests/resource/s3/fixtures/S3Server.groovy | 2 +- 5 files changed, 11 insertions(+), 6 deletions(-) diff --git a/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/server/ExpectOne.groovy b/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/server/ExpectOne.groovy index 0534688000e6..7bd9f7c5dacf 100644 --- a/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/server/ExpectOne.groovy +++ b/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/server/ExpectOne.groovy @@ -16,8 +16,14 @@ package org.gradle.test.fixtures.server +import java.util.concurrent.atomic.AtomicBoolean + abstract class ExpectOne implements ServerExpectation { - boolean run + AtomicBoolean atomicRun = new AtomicBoolean() + + boolean isRun() { + return atomicRun.get() + } void assertMet() { if (!run) { diff --git a/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/server/http/HttpServer.groovy b/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/server/http/HttpServer.groovy index c0a71d0f67e1..1a7dd5bf0e12 100755 --- a/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/server/http/HttpServer.groovy +++ b/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/server/http/HttpServer.groovy @@ -624,10 +624,9 @@ class HttpServer extends ServerWithExpectations implements HttpServerFixture { expectations << expectation add(path, matchPrefix, methods, new AbstractHandler() { void handle(String target, HttpServletRequest request, HttpServletResponse response, int dispatch) { - if (expectation.run) { + if (expectation.atomicRun.getAndSet(true)) { return } - expectation.run = true action.handle(request, response) request.handled = true } diff --git a/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/server/sftp/SFTPServer.groovy b/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/server/sftp/SFTPServer.groovy index 7c8382371965..b7448a9b0378 100644 --- a/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/server/sftp/SFTPServer.groovy +++ b/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/server/sftp/SFTPServer.groovy @@ -382,7 +382,7 @@ class SFTPServer extends ServerWithExpectations implements RepositoryServer { boolean matches(Buffer buffer, int type, int id) { if (!run && type == expectedType) { int originalBufferPosition = buffer.rpos() - run = bufferMatches(buffer, id) + atomicRun.set(bufferMatches(buffer, id)) buffer.rpos(originalBufferPosition) return run } else { diff --git a/subprojects/resources-gcs/src/integTest/groovy/org/gradle/integtests/resource/gcs/fixtures/GcsServer.groovy b/subprojects/resources-gcs/src/integTest/groovy/org/gradle/integtests/resource/gcs/fixtures/GcsServer.groovy index 526ec99460ef..8b7ec11fb20c 100644 --- a/subprojects/resources-gcs/src/integTest/groovy/org/gradle/integtests/resource/gcs/fixtures/GcsServer.groovy +++ b/subprojects/resources-gcs/src/integTest/groovy/org/gradle/integtests/resource/gcs/fixtures/GcsServer.groovy @@ -456,7 +456,7 @@ class GcsServer extends HttpServer implements RepositoryServer { return } if (!((Request) request).isHandled()) { - expectation.run = true + expectation.atomicRun.set(true) action.handle(request, response) ((Request) request).setHandled(true) } diff --git a/subprojects/resources-s3/src/integTest/groovy/org/gradle/integtests/resource/s3/fixtures/S3Server.groovy b/subprojects/resources-s3/src/integTest/groovy/org/gradle/integtests/resource/s3/fixtures/S3Server.groovy index b48685cdaf21..33e4b86cb6b1 100644 --- a/subprojects/resources-s3/src/integTest/groovy/org/gradle/integtests/resource/s3/fixtures/S3Server.groovy +++ b/subprojects/resources-s3/src/integTest/groovy/org/gradle/integtests/resource/s3/fixtures/S3Server.groovy @@ -480,7 +480,7 @@ class S3Server extends HttpServer implements RepositoryServer { return } if (!((Request) request).isHandled()) { - expectation.run = true + expectation.atomicRun.set(true) action.handle(request, response) ((Request) request).setHandled(true) } From b27917181572862e427344b38fd20351be26ff3d Mon Sep 17 00:00:00 2001 From: Jendrik Johannes Date: Wed, 9 Oct 2019 23:37:13 +0200 Subject: [PATCH 10/19] Remove unused imports --- .../ivyservice/resolveengine/graph/builder/SelectorState.java | 1 - .../resolveengine/graph/selectors/TestModuleSelectorState.java | 3 --- 2 files changed, 4 deletions(-) diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/SelectorState.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/SelectorState.java index 67578478798a..b1a63e571be6 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/SelectorState.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/SelectorState.java @@ -17,7 +17,6 @@ package org.gradle.api.internal.artifacts.ivyservice.resolveengine.graph.builder; import com.google.common.base.Joiner; -import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; import org.gradle.api.Describable; import org.gradle.api.artifacts.ModuleIdentifier; diff --git a/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/selectors/TestModuleSelectorState.java b/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/selectors/TestModuleSelectorState.java index 29725b90582a..433e7a0e6190 100644 --- a/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/selectors/TestModuleSelectorState.java +++ b/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/selectors/TestModuleSelectorState.java @@ -31,9 +31,6 @@ import org.gradle.internal.resolve.result.ComponentIdResolveResult; import org.gradle.internal.resolve.result.DefaultBuildableComponentIdResolveResult; -import java.util.Collections; -import java.util.List; - public class TestModuleSelectorState implements ResolvableSelectorState { private static final VersionParser VERSION_PARSER = new VersionParser(); From 32c2d7dcfc9267ef6caf4366d6fe3ab9b6792bd7 Mon Sep 17 00:00:00 2001 From: Jendrik Johannes Date: Thu, 10 Oct 2019 00:13:03 +0200 Subject: [PATCH 11/19] Do not expect an exact number of HEAD requests This seems to be dependent on timing. If one request starts after another did already finish, it can take the result from cache. --- ...encyArtifactsResolveIntegrationTest.groovy | 20 +++++++++---------- .../fixtures/publish/ModuleVersionSpec.groovy | 4 ++++ 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/artifacts/DependencyArtifactsResolveIntegrationTest.groovy b/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/artifacts/DependencyArtifactsResolveIntegrationTest.groovy index 68dfcb41f82c..ec02a612a3e0 100644 --- a/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/artifacts/DependencyArtifactsResolveIntegrationTest.groovy +++ b/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/artifacts/DependencyArtifactsResolveIntegrationTest.groovy @@ -107,13 +107,11 @@ class DependencyArtifactsResolveIntegrationTest extends AbstractModuleDependency when: repositoryInteractions { 'org:foo:1.0' { - (0..declarationCount).each { - // These happen in parallel when Gradle downloads metadata. - // In this cases "downloading metadata" means testing for the artifact. - // Each declaration is treated separately with it's own "consumer provided" metadata - expectHeadArtifact(name: artifactName, type: 'distribution-tgz') - } - expectGetArtifact(name: artifactName, type: 'distribution-tgz') + // HEAD requests happen in parallel when Gradle downloads metadata. + // In this cases "downloading metadata" means testing for the artifact. + // Each declaration is treated separately with it's own "consumer provided" metadata + // Depending on the timing, this can lead to multiple parallel requests (one for each declaration) + maybeHeadOrGetArtifact(name: artifactName, type: 'distribution-tgz') expectGetArtifact(name: artifactName, type: 'zip') } } @@ -131,9 +129,9 @@ class DependencyArtifactsResolveIntegrationTest extends AbstractModuleDependency } where: - notation | artifactName | declarationCount | declaration - 'multiple dependency declarations (AT notation)' | 'foo' | 2 | "conf('org:foo@distribution-tgz'); conf('org:foo@zip')" - 'multiple dependency declarations (artifact notation)' | 'bar' | 2 | "conf('org:foo') { artifact { name = 'bar'; type = 'distribution-tgz' } }; conf('org:foo') { artifact { name = 'bar'; type = 'zip' } }" - 'multiple artifact declaration' | 'bar' | 1 | "conf('org:foo') { artifact { name = 'bar'; type = 'distribution-tgz' }; artifact { name = 'bar'; type = 'zip' } }" + notation | artifactName | declaration + 'multiple dependency declarations (AT notation)' | 'foo' | "conf('org:foo@distribution-tgz'); conf('org:foo@zip')" + 'multiple dependency declarations (artifact notation)' | 'bar' | "conf('org:foo') { artifact { name = 'bar'; type = 'distribution-tgz' } }; conf('org:foo') { artifact { name = 'bar'; type = 'zip' } }" + 'multiple artifact declaration' | 'bar' | "conf('org:foo') { artifact { name = 'bar'; type = 'distribution-tgz' }; artifact { name = 'bar'; type = 'zip' } }" } } diff --git a/subprojects/dependency-management/src/testFixtures/groovy/org/gradle/integtests/fixtures/publish/ModuleVersionSpec.groovy b/subprojects/dependency-management/src/testFixtures/groovy/org/gradle/integtests/fixtures/publish/ModuleVersionSpec.groovy index d912946578d8..2bb99f92ded1 100644 --- a/subprojects/dependency-management/src/testFixtures/groovy/org/gradle/integtests/fixtures/publish/ModuleVersionSpec.groovy +++ b/subprojects/dependency-management/src/testFixtures/groovy/org/gradle/integtests/fixtures/publish/ModuleVersionSpec.groovy @@ -105,6 +105,10 @@ class ModuleVersionSpec { expectGetArtifact << new ArtifactExpectation(InteractionExpectation.HEAD_MISSING, artifact) } + void maybeHeadOrGetArtifact(Map artifact) { + expectGetArtifact << new ArtifactExpectation(InteractionExpectation.MAYBE, artifact) + } + void maybeGetMetadata() { expectGetMetadata << InteractionExpectation.MAYBE } From d4fe862d4f449c61149ef945e53431cf1fd8daa4 Mon Sep 17 00:00:00 2001 From: Jendrik Johannes Date: Thu, 10 Oct 2019 09:59:17 +0200 Subject: [PATCH 12/19] Remove duplicated 'isKeyEquals' check --- .../dependencies/AbstractExternalModuleDependency.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/dependencies/AbstractExternalModuleDependency.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/dependencies/AbstractExternalModuleDependency.java index 9221dca7a584..62f1c8cb5abc 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/dependencies/AbstractExternalModuleDependency.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/dependencies/AbstractExternalModuleDependency.java @@ -49,7 +49,7 @@ protected void copyTo(AbstractExternalModuleDependency target) { } protected boolean isContentEqualsFor(ExternalModuleDependency dependencyRhs) { - if (!isKeyEquals(dependencyRhs) || !isCommonContentEquals(dependencyRhs)) { + if (!isCommonContentEquals(dependencyRhs)) { return false; } return force == dependencyRhs.isForce() && changing == dependencyRhs.isChanging(); From de1fa3f0159d3f499a8e9993b6bdbb2ed44c6b69 Mon Sep 17 00:00:00 2001 From: Jendrik Johannes Date: Thu, 10 Oct 2019 10:31:54 +0200 Subject: [PATCH 13/19] Fix equals() of client module --- .../dependencies/ClientModuleDependencySpec.groovy | 4 ++-- .../artifacts/dependencies/DefaultClientModule.java | 8 ++++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/subprojects/core/src/test/groovy/org/gradle/api/internal/artifacts/dependencies/ClientModuleDependencySpec.groovy b/subprojects/core/src/test/groovy/org/gradle/api/internal/artifacts/dependencies/ClientModuleDependencySpec.groovy index 25b49e836845..edd9e081c7da 100644 --- a/subprojects/core/src/test/groovy/org/gradle/api/internal/artifacts/dependencies/ClientModuleDependencySpec.groovy +++ b/subprojects/core/src/test/groovy/org/gradle/api/internal/artifacts/dependencies/ClientModuleDependencySpec.groovy @@ -27,14 +27,14 @@ class ClientModuleDependencySpec extends AbstractModuleDependencySpec { new DefaultClientModule(group, name, version, configuration) } - def "equals but content not equal with different module dependencies"() { + def "not equal with different module dependencies"() { when: def dep1 = createDependency("group1", "name1", "version1", null) def dep2 = createDependency("group1", "name1", "version1", null) dep2.addDependency(Mock(ModuleDependency)) then: - dep1 == dep2 + dep1 != dep2 !dep1.contentEquals(dep2) !dep2.contentEquals(dep1) } diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/dependencies/DefaultClientModule.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/dependencies/DefaultClientModule.java index acd01cc71cce..543dd0f73916 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/dependencies/DefaultClientModule.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/dependencies/DefaultClientModule.java @@ -64,6 +64,14 @@ public ClientModule copy() { return copiedClientModule; } + @Override + public boolean equals(Object o) { + if (!(o instanceof Dependency)) { + return false; + } + return contentEquals((Dependency) o); + } + @Override public boolean contentEquals(Dependency dependency) { if (this == dependency) { From 17bba050f3faeced293f44d1d4e48d6616d140d8 Mon Sep 17 00:00:00 2001 From: Jendrik Johannes Date: Thu, 10 Oct 2019 10:55:09 +0200 Subject: [PATCH 14/19] Track 'changing' and 'client module' information for override metadata Although these are edge cases, it leads to more consistency and makes the behavior less dependent on order which may change unexpectedly through internal optimisations. --- .../ivyresolve/DynamicVersionResolver.java | 2 +- .../graph/builder/ComponentState.java | 5 ++- .../graph/builder/SelectorState.java | 40 ++++++++++++++++--- .../selectors/ResolvableSelectorState.java | 5 +++ .../DefaultComponentOverrideMetadata.java | 9 +++-- .../selectors/TestModuleSelectorState.java | 11 +++++ .../selectors/TestProjectSelectorState.groovy | 11 +++++ 7 files changed, 73 insertions(+), 10 deletions(-) diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/ivyresolve/DynamicVersionResolver.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/ivyresolve/DynamicVersionResolver.java index 3363e69fc982..35d160445902 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/ivyresolve/DynamicVersionResolver.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/ivyresolve/DynamicVersionResolver.java @@ -479,7 +479,7 @@ public CachePolicy getCachePolicy() { private void process(ModuleComponentRepositoryAccess access, DefaultBuildableModuleComponentMetaDataResolveResult result) { DependencyMetadata dependency = dependencyMetadata.withRequestedVersion(new DefaultImmutableVersionConstraint(version.getSource())); IvyArtifactName firstArtifact = dependency.getArtifacts().isEmpty() ? null : dependency.getArtifacts().get(0); - access.resolveComponentMetaData(identifier, DefaultComponentOverrideMetadata.forDependency(dependency, firstArtifact), result); + access.resolveComponentMetaData(identifier, DefaultComponentOverrideMetadata.forDependency(dependency.isChanging(), firstArtifact, DefaultComponentOverrideMetadata.extractClientModule(dependency)), result); attemptCollector.execute(result); } diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/ComponentState.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/ComponentState.java index 0cb78f4b9444..a733a72620ac 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/ComponentState.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/ComponentState.java @@ -184,7 +184,10 @@ public void resolve() { ComponentOverrideMetadata componentOverrideMetadata; if (selectors != null && selectors.size() > 0) { - componentOverrideMetadata = DefaultComponentOverrideMetadata.forDependency(selectors.first().getDependencyMetadata(), selectors.getFirstDependencyArtifact()); + // Taking the first selector here to determine the 'changing' status and 'client module' is our best bet to get the selector that will most likely be chosen in the end. + // As selectors are sorted accordingly (see ModuleSelectors.SELECTOR_COMPARATOR). + SelectorState firstSelector = selectors.first(); + componentOverrideMetadata = DefaultComponentOverrideMetadata.forDependency(firstSelector.isChanging(), selectors.getFirstDependencyArtifact(), firstSelector.getClientModule()); } else { componentOverrideMetadata = DefaultComponentOverrideMetadata.EMPTY; } diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/SelectorState.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/SelectorState.java index b1a63e571be6..7874438d0c28 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/SelectorState.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/SelectorState.java @@ -19,6 +19,8 @@ import com.google.common.base.Joiner; import com.google.common.collect.Lists; import org.gradle.api.Describable; +import org.gradle.api.InvalidUserDataException; +import org.gradle.api.artifacts.ClientModule; import org.gradle.api.artifacts.ModuleIdentifier; import org.gradle.api.artifacts.component.ComponentSelector; import org.gradle.api.artifacts.component.ProjectComponentSelector; @@ -32,6 +34,7 @@ import org.gradle.api.internal.artifacts.ivyservice.resolveengine.result.ComponentSelectionReasonInternal; import org.gradle.api.internal.artifacts.ivyservice.resolveengine.result.ComponentSelectionReasons; import org.gradle.api.internal.attributes.AttributeDesugaring; +import org.gradle.internal.component.model.DefaultComponentOverrideMetadata; import org.gradle.internal.component.model.DependencyMetadata; import org.gradle.internal.component.model.IvyArtifactName; import org.gradle.internal.logging.text.TreeFormatter; @@ -74,7 +77,11 @@ class SelectorState implements DependencyGraphSelector, ResolvableSelectorState private boolean forced; private boolean softForced; private boolean fromLock; + + // The following state needs to be tracked to consistently construct `` independent of the order dependencies are vistited private IvyArtifactName firstDependencyArtifact; + private ClientModule clientModule; + private boolean changing; // An internal counter used to track the number of outgoing edges // that use this selector. Since a module resolve state tracks all selectors @@ -299,6 +306,16 @@ public IvyArtifactName getFirstDependencyArtifact() { return firstDependencyArtifact; } + @Override + public ClientModule getClientModule() { + return clientModule; + } + + @Override + public boolean isChanging() { + return changing; + } + @Override public ResolvedVersionConstraint getVersionConstraint() { return versionConstraint; @@ -343,13 +360,26 @@ public void update(DependencyState dependencyState) { resolved = false; // when a selector changes from non lock to lock, we must reselect } dependencyState.addSelectionReasons(dependencyReasons); - if (firstDependencyArtifact == null) { - List artifacts = dependencyState.getDependency().getArtifacts(); - if (!artifacts.isEmpty()) { - firstDependencyArtifact = artifacts.get(0); - } + trackDetailsForOverrideMetadata(dependencyState); + } + } + + private void trackDetailsForOverrideMetadata(DependencyState dependencyState) { + if (firstDependencyArtifact == null) { + List artifacts = dependencyState.getDependency().getArtifacts(); + if (!artifacts.isEmpty()) { + firstDependencyArtifact = artifacts.get(0); + } + } + ClientModule nextClientModule = DefaultComponentOverrideMetadata.extractClientModule(dependencyState.getDependency()); + if (nextClientModule != null) { + if (clientModule == null) { + clientModule = nextClientModule; + } else { + throw new InvalidUserDataException(dependencyState.getDependency().getSelector().getDisplayName() + " has more than one client module definitions."); } } + changing = changing || dependencyState.getDependency().isChanging(); } private class UnmatchedVersionsReason implements Describable { diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/selectors/ResolvableSelectorState.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/selectors/ResolvableSelectorState.java index 10b675398d43..f83f2fcfdc60 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/selectors/ResolvableSelectorState.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/selectors/ResolvableSelectorState.java @@ -15,6 +15,7 @@ */ package org.gradle.api.internal.artifacts.ivyservice.resolveengine.graph.selectors; +import org.gradle.api.artifacts.ClientModule; import org.gradle.api.artifacts.component.ComponentSelector; import org.gradle.api.artifacts.component.ProjectComponentSelector; import org.gradle.api.internal.artifacts.ResolvedVersionConstraint; @@ -69,6 +70,10 @@ public interface ResolvableSelectorState { IvyArtifactName getFirstDependencyArtifact(); + ClientModule getClientModule(); + + boolean isChanging(); + default boolean isProject() { return getSelector() instanceof ProjectComponentSelector; } diff --git a/subprojects/dependency-management/src/main/java/org/gradle/internal/component/model/DefaultComponentOverrideMetadata.java b/subprojects/dependency-management/src/main/java/org/gradle/internal/component/model/DefaultComponentOverrideMetadata.java index 0f7deeb02e51..ab88bc454ee8 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/internal/component/model/DefaultComponentOverrideMetadata.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/internal/component/model/DefaultComponentOverrideMetadata.java @@ -31,8 +31,11 @@ public class DefaultComponentOverrideMetadata implements ComponentOverrideMetada private final List artifacts; private final ClientModule clientModule; - public static ComponentOverrideMetadata forDependency(DependencyMetadata dependencyMetadata, IvyArtifactName mainArtifact) { - return new DefaultComponentOverrideMetadata(dependencyMetadata.isChanging(), mainArtifact, extractClientModule(dependencyMetadata)); + public static ComponentOverrideMetadata forDependency(boolean changing, IvyArtifactName mainArtifact, ClientModule clientModule) { + if (!changing && mainArtifact == null && clientModule == null) { + return EMPTY; + } + return new DefaultComponentOverrideMetadata(changing, mainArtifact, clientModule); } private DefaultComponentOverrideMetadata(boolean changing, IvyArtifactName artifact, ClientModule clientModule) { @@ -45,7 +48,7 @@ private DefaultComponentOverrideMetadata(boolean changing, List this.clientModule = clientModule; } - private static ClientModule extractClientModule(DependencyMetadata dependencyMetadata) { + public static ClientModule extractClientModule(DependencyMetadata dependencyMetadata) { if (dependencyMetadata instanceof DslOriginDependencyMetadata) { Dependency source = ((DslOriginDependencyMetadata) dependencyMetadata).getSource(); if (source instanceof ClientModule) { diff --git a/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/selectors/TestModuleSelectorState.java b/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/selectors/TestModuleSelectorState.java index 433e7a0e6190..dd9ab274d4f6 100644 --- a/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/selectors/TestModuleSelectorState.java +++ b/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/selectors/TestModuleSelectorState.java @@ -15,6 +15,7 @@ */ package org.gradle.api.internal.artifacts.ivyservice.resolveengine.graph.selectors; +import org.gradle.api.artifacts.ClientModule; import org.gradle.api.artifacts.VersionConstraint; import org.gradle.api.artifacts.component.ComponentSelector; import org.gradle.api.internal.artifacts.ResolvedVersionConstraint; @@ -123,4 +124,14 @@ public boolean hasStrongOpinion() { public IvyArtifactName getFirstDependencyArtifact() { return null; } + + @Override + public ClientModule getClientModule() { + return null; + } + + @Override + public boolean isChanging() { + return false; + } } diff --git a/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/selectors/TestProjectSelectorState.groovy b/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/selectors/TestProjectSelectorState.groovy index fe85053826b9..f15281fdbc1a 100644 --- a/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/selectors/TestProjectSelectorState.groovy +++ b/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/selectors/TestProjectSelectorState.groovy @@ -16,6 +16,7 @@ package org.gradle.api.internal.artifacts.ivyservice.resolveengine.graph.selectors +import org.gradle.api.artifacts.ClientModule import org.gradle.api.artifacts.component.ComponentSelector import org.gradle.api.artifacts.component.ProjectComponentIdentifier import org.gradle.api.internal.artifacts.DefaultModuleVersionIdentifier @@ -91,5 +92,15 @@ class TestProjectSelectorState implements ResolvableSelectorState { IvyArtifactName getFirstDependencyArtifact() { return null } + + @Override + ClientModule getClientModule() { + return null + } + + @Override + boolean isChanging() { + return false + } } From 181205e7e05b4dc26a7526db64ffe6f3da1699d7 Mon Sep 17 00:00:00 2001 From: Jendrik Johannes Date: Thu, 10 Oct 2019 10:55:19 +0200 Subject: [PATCH 15/19] Add test coverage to pin down selector sorting behavior We now rely on the selector sorting when choosing a selector to compute override metadata. The sorting puts the most likely used selector first. --- ...rideMetadataResolveIntegrationTest.groovy} | 142 ++++++++++++++++-- 1 file changed, 129 insertions(+), 13 deletions(-) rename subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/{artifacts/DependencyArtifactsResolveIntegrationTest.groovy => override/ComponentOverrideMetadataResolveIntegrationTest.groovy} (59%) diff --git a/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/artifacts/DependencyArtifactsResolveIntegrationTest.groovy b/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/override/ComponentOverrideMetadataResolveIntegrationTest.groovy similarity index 59% rename from subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/artifacts/DependencyArtifactsResolveIntegrationTest.groovy rename to subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/override/ComponentOverrideMetadataResolveIntegrationTest.groovy index ec02a612a3e0..bed693fff80d 100644 --- a/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/artifacts/DependencyArtifactsResolveIntegrationTest.groovy +++ b/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/override/ComponentOverrideMetadataResolveIntegrationTest.groovy @@ -14,7 +14,7 @@ * limitations under the License. */ -package org.gradle.integtests.resolve.artifacts +package org.gradle.integtests.resolve.override import org.gradle.integtests.fixtures.GradleMetadataResolveRunner import org.gradle.integtests.fixtures.RequiredFeature @@ -29,20 +29,11 @@ import spock.lang.Unroll // This test bypasses all metadata using 'artifact()' metadata sources. It is sufficient to test with one metadata setup. @RequiredFeature(feature = GradleMetadataResolveRunner.GRADLE_METADATA, value = "true") ) -class DependencyArtifactsResolveIntegrationTest extends AbstractModuleDependencyResolveTest { +class ComponentOverrideMetadataResolveIntegrationTest extends AbstractModuleDependencyResolveTest { - def setup() { + def "can combine artifact notation and constraints"() { resolve.expectDefaultConfiguration(useMaven() ? 'runtime' : 'default') - buildFile << """ - repositories.all { - metadataSources { - artifact() //sss - } - } - """ - } - def "can combine artifact notation and constraints"() { given: repository { 'org:foo:1.0' { @@ -53,6 +44,9 @@ class DependencyArtifactsResolveIntegrationTest extends AbstractModuleDependency } buildFile << """ + repositories.all { + metadataSources { artifact() } + } dependencies { conf('org:foo@distribution-tgz') @@ -85,6 +79,8 @@ class DependencyArtifactsResolveIntegrationTest extends AbstractModuleDependency @Unroll def "The first artifact is used as replacement for metadata if multiple artifacts are declared using #declaration"() { + resolve.expectDefaultConfiguration(useMaven() ? 'runtime' : 'default') + given: repository { 'org:foo:1.0' { @@ -96,6 +92,9 @@ class DependencyArtifactsResolveIntegrationTest extends AbstractModuleDependency } buildFile << """ + repositories.all { + metadataSources { artifact() } + } dependencies { $declaration constraints { @@ -132,6 +131,123 @@ class DependencyArtifactsResolveIntegrationTest extends AbstractModuleDependency notation | artifactName | declaration 'multiple dependency declarations (AT notation)' | 'foo' | "conf('org:foo@distribution-tgz'); conf('org:foo@zip')" 'multiple dependency declarations (artifact notation)' | 'bar' | "conf('org:foo') { artifact { name = 'bar'; type = 'distribution-tgz' } }; conf('org:foo') { artifact { name = 'bar'; type = 'zip' } }" - 'multiple artifact declaration' | 'bar' | "conf('org:foo') { artifact { name = 'bar'; type = 'distribution-tgz' }; artifact { name = 'bar'; type = 'zip' } }" + 'multiple artifact declarations' | 'bar' | "conf('org:foo') { artifact { name = 'bar'; type = 'distribution-tgz' }; artifact { name = 'bar'; type = 'zip' } }" + } + + @Unroll + def "client module for version 1.0 is selected over other dependency with version #otherVersion"() { + resolve.expectDefaultConfiguration('default') + + given: + repository { + 'org:foo:0.9' { + dependsOn 'org:bar:1.0' + } + 'org:foo:1.0' { + dependsOn 'org:bar:1.0' + } + 'org:bar:1.0'() + } + + buildFile << """ + dependencies { + conf 'org:foo:$otherVersion' + conf module('org:foo:1.0') { + // no dependencies + } + } + + """ + + when: + repositoryInteractions { + 'org:foo:1.0' { + expectResolve() + } + } + succeeds 'checkDeps' + + then: + def notSelectedModule = "org:foo:$otherVersion" + resolve.expectGraph { + root(":", ":test:") { + module('org:foo:1.0') + if (notSelectedModule != 'org:foo:1.0') { + edge(notSelectedModule, 'org:foo:1.0') + } + } + } + + where: + otherVersion << ['0.9', '1.0'] + } + + def "client module for a not selected version or range is ignored"() { + given: + repository { + 'org:foo:1.1' { + dependsOn 'org:bar:1.0' + } + 'org:foo:1.0' { + dependsOn 'org:bar:1.0' + } + 'org:bar:1.0'() + } + + buildFile << """ + dependencies { + conf 'org:foo:1.1' + conf module('org:foo:[1.0,1.1]') { + // no dependencies + } + conf module('org:foo:1.0') { + // no dependencies + } + } + """ + + when: + repositoryInteractions { + 'org:foo:1.1' { + expectResolve() + } + 'org:bar:1.0' { + expectResolve() + } + } + succeeds 'checkDeps' + + then: + resolve.expectGraph { + root(":", ":test:") { + module('org:foo:1.1') { + module('org:bar:1.0') + } + edge('org:foo:[1.0,1.1]', 'org:foo:1.1') + edge('org:foo:1.0', 'org:foo:1.1') + } + } + } + + def "clashing client modules fail the build"() { + given: + buildFile << """ + configurations { + conf + } + dependencies { + conf module('org:foo:1.0') { + } + conf module('org:foo:1.0') { + dependency("org:baz:1.0") + } + } + """ + + when: + fails 'checkDeps' + + then: + failure.assertHasCause("org:foo:1.0 has more than one client module definitions.") } } From f4a6f6318b0f4843d57e3a17831cec9e477a1880 Mon Sep 17 00:00:00 2001 From: Jendrik Johannes Date: Thu, 10 Oct 2019 11:02:27 +0200 Subject: [PATCH 16/19] Add missing hashCode() method --- .../internal/artifacts/dependencies/DefaultClientModule.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/dependencies/DefaultClientModule.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/dependencies/DefaultClientModule.java index 543dd0f73916..28b816195c4f 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/dependencies/DefaultClientModule.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/dependencies/DefaultClientModule.java @@ -72,6 +72,11 @@ public boolean equals(Object o) { return contentEquals((Dependency) o); } + @Override + public int hashCode() { + return super.hashCode(); + } + @Override public boolean contentEquals(Dependency dependency) { if (this == dependency) { From 91c47cbc49db0b23ddb3cebd6609b99b1ff2890d Mon Sep 17 00:00:00 2001 From: Jendrik Johannes Date: Thu, 10 Oct 2019 12:02:37 +0200 Subject: [PATCH 17/19] Fix code comment --- .../ivyservice/resolveengine/graph/builder/SelectorState.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/SelectorState.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/SelectorState.java index 7874438d0c28..f45d7c324201 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/SelectorState.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/SelectorState.java @@ -78,7 +78,7 @@ class SelectorState implements DependencyGraphSelector, ResolvableSelectorState private boolean softForced; private boolean fromLock; - // The following state needs to be tracked to consistently construct `` independent of the order dependencies are vistited + // The following state needs to be tracked to consistently construct `ComponentOverrideMetadata` independent of the order dependencies are visited private IvyArtifactName firstDependencyArtifact; private ClientModule clientModule; private boolean changing; From 9ce0df0a3161904895c065d7ea739532ce7cf10d Mon Sep 17 00:00:00 2001 From: Jendrik Johannes Date: Thu, 10 Oct 2019 13:32:34 +0200 Subject: [PATCH 18/19] Add equality check in case update() is called twice for the same input --- .../ivyservice/resolveengine/graph/builder/SelectorState.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/SelectorState.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/SelectorState.java index f45d7c324201..958fdbf21635 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/SelectorState.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/SelectorState.java @@ -372,7 +372,7 @@ private void trackDetailsForOverrideMetadata(DependencyState dependencyState) { } } ClientModule nextClientModule = DefaultComponentOverrideMetadata.extractClientModule(dependencyState.getDependency()); - if (nextClientModule != null) { + if (nextClientModule != null && !nextClientModule.equals(clientModule)) { if (clientModule == null) { clientModule = nextClientModule; } else { From 75f57bf692dabf853cb34b4aaef7897ba819d11e Mon Sep 17 00:00:00 2001 From: Jendrik Johannes Date: Thu, 10 Oct 2019 13:35:26 +0200 Subject: [PATCH 19/19] Relax flaky test expectation --- .../ComponentOverrideMetadataResolveIntegrationTest.groovy | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/override/ComponentOverrideMetadataResolveIntegrationTest.groovy b/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/override/ComponentOverrideMetadataResolveIntegrationTest.groovy index bed693fff80d..03fbc41aaf61 100644 --- a/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/override/ComponentOverrideMetadataResolveIntegrationTest.groovy +++ b/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/override/ComponentOverrideMetadataResolveIntegrationTest.groovy @@ -59,9 +59,8 @@ class ComponentOverrideMetadataResolveIntegrationTest extends AbstractModuleDepe when: repositoryInteractions { 'org:foo:1.0' { - expectHeadArtifact(type: 'distribution-tgz') // test for the artifact when we would usually download metadata - expectHeadArtifact(type: 'distribution-tgz') // test for the artifact before actually retrieving it - expectGetArtifact(type: 'distribution-tgz') + // expectHeadArtifact(type: 'distribution-tgz') <- head request can happen once or twice depending on timing + maybeHeadOrGetArtifact(type: 'distribution-tgz') } } succeeds 'checkDeps'