Fix inconsistent resolution ordering

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

@@ -0,0 +1,136 @@
+/*
+ * Copyright 2020 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
+
+import org.gradle.integtests.fixtures.AbstractIntegrationSpec
+import org.gradle.integtests.fixtures.ToBeFixedForInstantExecution
+import spock.lang.Issue
+import spock.lang.Unroll
+
+class ConcurrentDerivationStrategyIntegTest extends AbstractIntegrationSpec {
+
+    @ToBeFixedForInstantExecution
+    @Issue("https://github.com/gradle/gradle/issues/13555")
+    @Unroll("consistent resolution using rules=#displayName")
+    // If this test becomes flaky it means we broke the code which prevents mutation of in-memory cached module metadata
+    def "selected variants are consistent using concurrent resolution of graphs from cache having different derivation strategies"() {
+        executer.requireOwnGradleUserHomeDir()
+        settingsFile << """
+            include 'app'
+            include 'lib'
+        """
+
+        buildFile << """
+            subprojects {
+                ${mavenCentralRepository()}
+                dependencies {
+                    components {
+                        $rules
+                    }
+                }
+            }
+
+            class NonCachedRule implements ComponentMetadataRule {
+                @Override
+                void execute(ComponentMetadataContext context) {
+                    println("Applying rule on \$context.details.id")
+                }
+            }
+
+            @CacheableRule
+            class CachedRule implements ComponentMetadataRule {
+                @Override
+                void execute(ComponentMetadataContext context) {
+                    println("Applying rule on \$context.details.id")
+                }
+            }
+        """
+
+        file('app/build.gradle') << """
+            configurations {
+               foo
+            }
+
+            dependencies {
+               foo 'org.apache.commons:commons-lang3:3.3.1'
+               foo 'org.springframework.boot:spring-boot-starter-web:2.2.2.RELEASE'
+            }
+
+            tasks.register("resolve") {
+               doLast {
+                  configurations.foo.incoming.resolutionResult.allComponents {
+                      assert it instanceof ResolvedComponentResult
+                      if (id instanceof ModuleComponentIdentifier) {
+                          variants.each {
+                              println "\$id -> \${it.displayName}"
+                              if (it.displayName != 'default') {
+                                  throw new AssertionError("Unexpected resolved variant \$it")
+                              }
+                          }
+                      }
+                  }
+               }
+            }
+        """
+        file("lib/build.gradle") << """
+            plugins {
+               id 'java-library'
+            }
+
+            dependencies {
+               api 'org.apache.commons:commons-lang3:3.3.1'
+               implementation 'org.springframework.boot:spring-boot-starter-web:2.2.2.RELEASE'
+            }
+
+            tasks.register("resolve") {
+               doLast {
+                  configurations.compileClasspath.incoming.resolutionResult.allComponents {
+                      assert it instanceof ResolvedComponentResult
+                      if (id instanceof ModuleComponentIdentifier) {
+                          variants.each {
+                              println "\$id -> \${it.displayName}"
+                              if (it.displayName != 'compile') {
+                                  throw new AssertionError("Unexpected resolved variant \$it")
+                              }
+                          }
+                      }
+                  }
+               }
+            }
+        """
+
+        when:
+        executer.withArgument('--parallel')
+        run 'resolve'
+
+        then:
+        noExceptionThrown()
+
+        when: "second build from cache"
+        executer.withArgument('--parallel')
+        run 'resolve'
+
+        then:
+        noExceptionThrown()
+
+        where:
+        displayName       | rules
+        "no rules"        | ""
+        "non-cached rule" | "all(NonCachedRule)"
+        "cached rule"     | "all(CachedRule)"
+    }
+}

subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/dsl/DefaultComponentMetadataProcessor.java

@@ -43,6 +43,7 @@
 import org.gradle.internal.action.InstantiatingAction;
 import org.gradle.internal.component.external.model.ModuleComponentResolveMetadata;
 import org.gradle.internal.component.external.model.MutableModuleComponentResolveMetadata;
+import org.gradle.internal.component.external.model.VariantDerivationStrategy;
 import org.gradle.internal.component.external.model.ivy.DefaultIvyModuleResolveMetadata;
 import org.gradle.internal.component.external.model.ivy.RealisedIvyModuleResolveMetadata;
 import org.gradle.internal.component.external.model.maven.DefaultMavenModuleResolveMetadata;
@@ -69,7 +70,8 @@ public class DefaultComponentMetadataProcessor implements ComponentMetadataProce
     private final static boolean FORCE_REALIZE = Boolean.getBoolean("org.gradle.integtest.force.realize.metadata");
 
     private static final Transformer<ModuleComponentResolveMetadata, WrappingComponentMetadataContext> DETAILS_TO_RESULT = componentMetadataContext -> {
-        ModuleComponentResolveMetadata metadata = componentMetadataContext.getMutableMetadata().asImmutable();
+        ModuleComponentResolveMetadata metadata = componentMetadataContext
+            .getImmutableMetadataWithDerivationStrategy(componentMetadataContext.getVariantDerivationStrategy());
         return realizeMetadata(metadata);
     };
 
@@ -150,8 +152,9 @@ public DefaultComponentMetadataProcessor(ComponentMetadataRuleContainer metadata
     }
 
     @Override
-    public ModuleComponentResolveMetadata processMetadata(ModuleComponentResolveMetadata metadata) {
-        metadata.getVariantMetadataRules().setVariantDerivationStrategy(metadataRuleContainer.getVariantDerivationStrategy());
+    public ModuleComponentResolveMetadata processMetadata(ModuleComponentResolveMetadata origin) {
+        VariantDerivationStrategy curStrategy = metadataRuleContainer.getVariantDerivationStrategy();
+        ModuleComponentResolveMetadata metadata = origin.withDerivationStrategy(curStrategy);
         ModuleComponentResolveMetadata updatedMetadata;
         if (metadataRuleContainer.isEmpty()) {
             updatedMetadata = maybeForceRealisation(metadata);

subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/dsl/WrappingComponentMetadataContext.java

@@ -25,6 +25,7 @@
 import org.gradle.api.internal.artifacts.repositories.resolver.DirectDependencyMetadataImpl;
 import org.gradle.internal.component.external.model.ModuleComponentResolveMetadata;
 import org.gradle.internal.component.external.model.MutableModuleComponentResolveMetadata;
+import org.gradle.internal.component.external.model.VariantDerivationStrategy;
 import org.gradle.internal.reflect.Instantiator;
 import org.gradle.internal.typeconversion.NotationParser;
 
@@ -66,19 +67,24 @@ public ComponentMetadataDetails getDetails() {
         createMutableMetadataIfNeeded();
         if (details == null) {
             details = instantiator.newInstance(ComponentMetadataDetailsAdapter.class, mutableMetadata, instantiator, dependencyMetadataNotationParser, dependencyConstraintMetadataNotationParser, componentIdentifierParser, platformSupport);
-
         }
         return details;
     }
 
-    MutableModuleComponentResolveMetadata getMutableMetadata() {
-        createMutableMetadataIfNeeded();
-        return mutableMetadata;
+    VariantDerivationStrategy getVariantDerivationStrategy() {
+        return metadata.getVariantDerivationStrategy();
     }
 
-    private void createMutableMetadataIfNeeded() {
+    ModuleComponentResolveMetadata getImmutableMetadataWithDerivationStrategy(VariantDerivationStrategy variantDerivationStrategy) {
+        // We need to create a copy or the rules will be added to the wrong container
+        return createMutableMetadataIfNeeded().asImmutable()
+            .withDerivationStrategy(variantDerivationStrategy);
+    }
+
+    private MutableModuleComponentResolveMetadata createMutableMetadataIfNeeded() {
         if (mutableMetadata == null) {
             mutableMetadata = metadata.asMutable();
         }
+        return mutableMetadata;
     }
 }

subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/LenientPlatformResolveMetadata.java

@@ -31,7 +31,9 @@
 import org.gradle.internal.component.external.model.ModuleComponentResolveMetadata;
 import org.gradle.internal.component.external.model.ModuleDependencyMetadata;
 import org.gradle.internal.component.external.model.MutableModuleComponentResolveMetadata;
+import org.gradle.internal.component.external.model.NoOpDerivationStrategy;
 import org.gradle.internal.component.external.model.RealisedConfigurationMetadata;
+import org.gradle.internal.component.external.model.VariantDerivationStrategy;
 import org.gradle.internal.component.external.model.VariantMetadataRules;
 import org.gradle.internal.component.external.model.VirtualComponentIdentifier;
 import org.gradle.internal.component.model.ConfigurationMetadata;
@@ -152,6 +154,16 @@ public ModuleComponentResolveMetadata withSources(ModuleSources sources) {
         return this;
     }
 
+    @Override
+    public ModuleComponentResolveMetadata withDerivationStrategy(VariantDerivationStrategy derivationStrategy) {
+        return this;
+    }
+
+    @Override
+    public VariantDerivationStrategy getVariantDerivationStrategy() {
+        return NoOpDerivationStrategy.getInstance();
+    }
+
     @Override
     public ModuleComponentArtifactMetadata artifact(String type, @Nullable String extension, @Nullable String classifier) {
         throw new UnsupportedOperationException();

subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/AbstractLazyModuleComponentResolveMetadata.java

@@ -61,8 +61,8 @@ protected AbstractLazyModuleComponentResolveMetadata(AbstractMutableModuleCompon
     /**
      * Creates a copy of the given metadata
      */
-    protected AbstractLazyModuleComponentResolveMetadata(AbstractLazyModuleComponentResolveMetadata metadata, ModuleSources sources) {
-        super(metadata, sources);
+    protected AbstractLazyModuleComponentResolveMetadata(AbstractLazyModuleComponentResolveMetadata metadata, ModuleSources sources, VariantDerivationStrategy variantDerivationStrategy) {
+        super(metadata, sources, variantDerivationStrategy);
         this.configurationDefinitions = metadata.configurationDefinitions;
         variantMetadataRules = metadata.variantMetadataRules;
     }
@@ -71,10 +71,12 @@ protected AbstractLazyModuleComponentResolveMetadata(AbstractLazyModuleComponent
      * Clear any cached state, for the case where the inputs are invalidated.
      * This only happens when constructing a copy
      */
-    protected void copyCachedState(AbstractLazyModuleComponentResolveMetadata metadata) {
+    protected void copyCachedState(AbstractLazyModuleComponentResolveMetadata metadata, boolean copyGraphVariants) {
         // Copy built-on-demand state
         metadata.copyCachedConfigurations(this.configurations);
-        this.graphVariants = metadata.graphVariants;
+        if (copyGraphVariants) {
+            this.graphVariants = metadata.graphVariants;
+        }
     }
 
     private synchronized void copyCachedConfigurations(Map<String, ConfigurationMetadata> target) {

subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/AbstractModuleComponentResolveMetadata.java

@@ -47,6 +47,7 @@ abstract class AbstractModuleComponentResolveMetadata implements ModuleComponent
     private final ImmutableAttributes attributes;
     private final ImmutableList<? extends VirtualComponentIdentifier> platformOwners;
     private final AttributesSchemaInternal schema;
+    private final VariantDerivationStrategy variantDerivationStrategy;
 
     public AbstractModuleComponentResolveMetadata(AbstractMutableModuleComponentResolveMetadata metadata) {
         this.componentIdentifier = metadata.getId();
@@ -60,6 +61,7 @@ public AbstractModuleComponentResolveMetadata(AbstractMutableModuleComponentReso
         attributes = extractAttributes(metadata);
         variants = metadata.getVariants();
         platformOwners = metadata.getPlatformOwners() == null ? ImmutableList.of() : ImmutableList.copyOf(metadata.getPlatformOwners());
+        variantDerivationStrategy = metadata.getVariantDerivationStrategy();
     }
 
     public AbstractModuleComponentResolveMetadata(AbstractModuleComponentResolveMetadata metadata, ImmutableList<? extends ComponentVariant> variants) {
@@ -74,6 +76,7 @@ public AbstractModuleComponentResolveMetadata(AbstractModuleComponentResolveMeta
         attributes = metadata.getAttributes();
         this.variants = variants;
         this.platformOwners = metadata.getPlatformOwners();
+        this.variantDerivationStrategy = metadata.getVariantDerivationStrategy();
     }
 
     public AbstractModuleComponentResolveMetadata(AbstractModuleComponentResolveMetadata metadata) {
@@ -88,9 +91,10 @@ public AbstractModuleComponentResolveMetadata(AbstractModuleComponentResolveMeta
         attributes = metadata.attributes;
         variants = metadata.variants;
         platformOwners = metadata.platformOwners;
+        variantDerivationStrategy = metadata.getVariantDerivationStrategy();
     }
 
-    public AbstractModuleComponentResolveMetadata(AbstractModuleComponentResolveMetadata metadata, ModuleSources sources) {
+    public AbstractModuleComponentResolveMetadata(AbstractModuleComponentResolveMetadata metadata, ModuleSources sources, VariantDerivationStrategy derivationStrategy) {
         this.componentIdentifier = metadata.componentIdentifier;
         this.moduleVersionIdentifier = metadata.moduleVersionIdentifier;
         changing = metadata.changing;
@@ -102,6 +106,7 @@ public AbstractModuleComponentResolveMetadata(AbstractModuleComponentResolveMeta
         variants = metadata.variants;
         platformOwners = metadata.platformOwners;
         moduleSources = ImmutableModuleSources.of(sources);
+        variantDerivationStrategy = derivationStrategy;
     }
 
     private static ImmutableAttributes extractAttributes(AbstractMutableModuleComponentResolveMetadata metadata) {
@@ -188,6 +193,11 @@ public ImmutableList<? extends VirtualComponentIdentifier> getPlatformOwners() {
         return platformOwners;
     }
 
+    @Override
+    public VariantDerivationStrategy getVariantDerivationStrategy() {
+        return variantDerivationStrategy;
+    }
+
     @Override
     public boolean equals(Object o) {
         if (this == o) {

subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/AbstractMutableModuleComponentResolveMetadata.java

@@ -67,19 +67,24 @@ public abstract class AbstractMutableModuleComponentResolveMetadata implements M
     private final AttributesSchemaInternal schema;
 
     private final VariantMetadataRules variantMetadataRules;
+    private final VariantDerivationStrategy variantDerivationStrategy;
 
     private List<MutableComponentVariant> newVariants;
     private ImmutableList<? extends ComponentVariant> variants;
     private Set<VirtualComponentIdentifier> owners;
 
-    protected AbstractMutableModuleComponentResolveMetadata(ImmutableAttributesFactory attributesFactory, ModuleVersionIdentifier moduleVersionId, ModuleComponentIdentifier componentIdentifier, AttributesSchemaInternal schema) {
+    protected AbstractMutableModuleComponentResolveMetadata(ImmutableAttributesFactory attributesFactory,
+                                                            ModuleVersionIdentifier moduleVersionId,
+                                                            ModuleComponentIdentifier componentIdentifier,
+                                                            AttributesSchemaInternal schema) {
         this.attributesFactory = attributesFactory;
         this.componentId = componentIdentifier;
         this.moduleVersionId = moduleVersionId;
         this.componentLevelAttributes = defaultAttributes(attributesFactory);
         this.schema = schema;
         this.variantMetadataRules = new VariantMetadataRules(attributesFactory, moduleVersionId);
         this.moduleSources = new MutableModuleSources();
+        this.variantDerivationStrategy = NoOpDerivationStrategy.getInstance();
     }
 
     protected AbstractMutableModuleComponentResolveMetadata(ModuleComponentResolveMetadata metadata) {
@@ -93,8 +98,8 @@ protected AbstractMutableModuleComponentResolveMetadata(ModuleComponentResolveMe
         this.attributesFactory = metadata.getAttributesFactory();
         this.schema = metadata.getAttributesSchema();
         this.componentLevelAttributes = attributesFactory.mutable(metadata.getAttributes());
+        this.variantDerivationStrategy = metadata.getVariantDerivationStrategy();
         this.variantMetadataRules = new VariantMetadataRules(attributesFactory, moduleVersionId);
-        this.variantMetadataRules.setVariantDerivationStrategy(metadata.getVariantMetadataRules().getVariantDerivationStrategy());
     }
 
     private static AttributeContainerInternal defaultAttributes(ImmutableAttributesFactory attributesFactory) {
@@ -117,6 +122,10 @@ public void setId(ModuleComponentIdentifier componentId) {
         this.moduleVersionId = DefaultModuleVersionIdentifier.newId(componentId);
     }
 
+    public VariantDerivationStrategy getVariantDerivationStrategy() {
+        return variantDerivationStrategy;
+    }
+
     @Override
     public String getStatus() {
         return componentLevelAttributes.getAttribute(ProjectInternal.STATUS_ATTRIBUTE);

subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/AbstractRealisedModuleComponentResolveMetadata.java

@@ -55,8 +55,8 @@ public AbstractRealisedModuleComponentResolveMetadata(AbstractRealisedModuleComp
         this.configurations = metadata.configurations;
     }
 
-    public AbstractRealisedModuleComponentResolveMetadata(AbstractRealisedModuleComponentResolveMetadata metadata, ModuleSources sources) {
-        super(metadata, sources);
+    public AbstractRealisedModuleComponentResolveMetadata(AbstractRealisedModuleComponentResolveMetadata metadata, ModuleSources sources, VariantDerivationStrategy derivationStrategy) {
+        super(metadata, sources, derivationStrategy);
         this.configurations = metadata.configurations;
     }
 

subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/ModuleComponentResolveMetadata.java

@@ -48,6 +48,13 @@ public interface ModuleComponentResolveMetadata extends ComponentResolveMetadata
     @Override
     ModuleComponentResolveMetadata withSources(ModuleSources sources);
 
+
+    /**
+     * Creates a copy of this meta-data with the given derivation strategy.
+     */
+    ModuleComponentResolveMetadata withDerivationStrategy(VariantDerivationStrategy derivationStrategy);
+
+
     /**
      * Creates an artifact for this module. Does not mutate this metadata.
      */
@@ -62,4 +69,6 @@ public interface ModuleComponentResolveMetadata extends ComponentResolveMetadata
     ImmutableAttributesFactory getAttributesFactory();
 
     VariantMetadataRules getVariantMetadataRules();
+
+    VariantDerivationStrategy getVariantDerivationStrategy();
 }