Fix discovery of components without metadata in repositories

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<Object, Capability> capabilityNotationParser;
     private DefaultExcludeRuleContainer excludeRuleContainer = new DefaultExcludeRuleContainer();
-    private Set<DependencyArtifact> artifacts = new HashSet<DependencyArtifact>();
+    private Set<DependencyArtifact> artifacts = new LinkedHashSet<>();
     private ImmutableActionSet<ModuleDependency> onMutate = ImmutableActionSet.empty();
     private AttributeContainerInternal attributes;
     private ModuleDependencyCapabilitiesInternal moduleDependencyCapabilities;
@@ -142,7 +142,7 @@ public DependencyArtifact artifact(Action<? super DependencyArtifact> configureA
 
     protected void copyTo(AbstractModuleDependency target) {
         super.copyTo(target);
-        target.setArtifacts(new HashSet<DependencyArtifact>(getArtifacts()));
+        target.setArtifacts(new LinkedHashSet<>(getArtifacts()));
         target.setExcludeRuleContainer(new DefaultExcludeRuleContainer(getExcludeRules()));
         target.setTransitive(isTransitive());
         if (attributes != null) {

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)
     }

subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/override/ComponentOverrideMetadataResolveIntegrationTest.groovy

@@ -0,0 +1,253 @@
+/*
+ * 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.override
+
+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 ComponentOverrideMetadataResolveIntegrationTest extends AbstractModuleDependencyResolveTest {
+
+    def "can combine artifact notation and constraints"() {
+        resolve.expectDefaultConfiguration(useMaven() ? 'runtime' : 'default')
+
+        given:
+        repository {
+            'org:foo:1.0' {
+                withModule {
+                    undeclaredArtifact(type: 'distribution-tgz')
+                }
+            }
+        }
+
+        buildFile << """
+            repositories.all {
+                metadataSources { artifact() }
+            }
+            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"() {
+        resolve.expectDefaultConfiguration(useMaven() ? 'runtime' : 'default')
+
+        given:
+        repository {
+            'org:foo:1.0' {
+                withModule {
+                    undeclaredArtifact(name: artifactName, type: 'distribution-tgz')
+                    undeclaredArtifact(name: artifactName, type: 'zip')
+                }
+            }
+        }
+
+        buildFile << """
+            repositories.all {
+                metadataSources { artifact() }
+            }
+            dependencies {
+                $declaration
+                constraints {
+                   conf('org:foo:1.0')
+                }
+            }
+        """
+
+        when:
+        repositoryInteractions {
+            'org:foo:1.0' {
+                // 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')
+            }
+        }
+        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 | 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 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.")
+    }
+}

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();

subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/dependencies/DefaultClientModule.java

@@ -64,6 +64,19 @@ public ClientModule copy() {
         return copiedClientModule;
     }
 
+    @Override
+    public boolean equals(Object o) {
+        if (!(o instanceof Dependency)) {
+            return false;
+        }
+        return contentEquals((Dependency) o);
+    }
+
+    @Override
+    public int hashCode() {
+        return super.hashCode();
+    }
+
     @Override
     public boolean contentEquals(Dependency dependency) {
         if (this == dependency) {

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.isChanging(), firstArtifact, DefaultComponentOverrideMetadata.extractClientModule(dependency)), result);
             attemptCollector.execute(result);
         }
 

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.
      *