From 333fb18000ae4efa3114b685666765285333a1ce Mon Sep 17 00:00:00 2001 From: Cedric Champeau Date: Wed, 24 Jun 2020 18:04:26 +0200 Subject: [PATCH] Fix inconsistent resolution ordering The fix for #12951 introduced by #12957 worked but made a number of other bugs much more likely to happen: by using the right derivation strategy in each project, we were properly fetching the component metadata we wanted to process from the cache. However, this component metadata was mutable, because of laziness. In particular, derivation of variants require to set the derivation strategy on "deemed immutable" metadata. The problem is that when several configurations are resolved concurrently, we ended up mutating the same component from different threads, with different derivation strategies. The ideal fix would be to have real immutable component metadata from the cache. However, because of laziness (required for performance), we can't do that. The fix in this PR is therefore to create a copy of the module metadata whenever a different derivation strategy needs to be used. It also highlighted another bug, which is that when we use cached component metadata rules, the derivation strategy wasn't part of the cache key, meaning that we would get whatever was first stored in the binary cache. We now properly separate the entries in the cache by amending the cache key with the name of the strategy. This isn't ideal as we could potentially have _stateful_ strategies, but for now there are no such things in the wild. Again in reality we'd like to get rid of the "magic snapshotting" of rules and do something similar to what transforms do, with proper declared input types. Fixes #13555 --- ...ncurrentDerivationStrategyIntegTest.groovy | 134 ++++++++++++++++++ .../DefaultComponentMetadataProcessor.java | 12 +- .../dsl/WrappingComponentMetadataContext.java | 9 ++ .../LenientPlatformResolveMetadata.java | 11 ++ ...actLazyModuleComponentResolveMetadata.java | 10 +- ...bstractModuleComponentResolveMetadata.java | 15 +- ...ealisedModuleComponentResolveMetadata.java | 4 +- .../model/ModuleComponentResolveMetadata.java | 9 ++ .../ivy/DefaultIvyModuleResolveMetadata.java | 21 ++- .../ivy/RealisedIvyModuleResolveMetadata.java | 16 ++- .../DefaultMavenModuleResolveMetadata.java | 19 ++- .../RealisedMavenModuleResolveMetadata.java | 15 +- .../ComponentMetadataRuleExecutor.java | 2 +- .../ComponentMetadataRuleExecutorTest.groovy | 2 + 14 files changed, 251 insertions(+), 28 deletions(-) create mode 100644 subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/ConcurrentDerivationStrategyIntegTest.groovy diff --git a/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/ConcurrentDerivationStrategyIntegTest.groovy b/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/ConcurrentDerivationStrategyIntegTest.groovy new file mode 100644 index 000000000000..41406476f465 --- /dev/null +++ b/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/ConcurrentDerivationStrategyIntegTest.groovy @@ -0,0 +1,134 @@ +/* + * 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 spock.lang.Issue +import spock.lang.Unroll + +class ConcurrentDerivationStrategyIntegTest extends AbstractIntegrationSpec { + + @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)" + } +} diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/dsl/DefaultComponentMetadataProcessor.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/dsl/DefaultComponentMetadataProcessor.java index 16569affce6f..573682f995ce 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/dsl/DefaultComponentMetadataProcessor.java +++ b/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,9 @@ public class DefaultComponentMetadataProcessor implements ComponentMetadataProce private final static boolean FORCE_REALIZE = Boolean.getBoolean("org.gradle.integtest.force.realize.metadata"); private static final Transformer DETAILS_TO_RESULT = componentMetadataContext -> { - ModuleComponentResolveMetadata metadata = componentMetadataContext.getMutableMetadata().asImmutable(); + ModuleComponentResolveMetadata metadata = componentMetadataContext + .getImmutableMetadata() + .withDerivationStrategy(componentMetadataContext.getVariantDerivationStrategy()); return realizeMetadata(metadata); }; @@ -150,8 +153,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); @@ -179,7 +183,7 @@ public ModuleComponentResolveMetadata processMetadata(ModuleComponentResolveMeta if (!updatedMetadata.getStatusScheme().contains(updatedMetadata.getStatus())) { throw new ModuleVersionResolveException(updatedMetadata.getModuleVersionId(), () -> String.format("Unexpected status '%s' specified for %s. Expected one of: %s", updatedMetadata.getStatus(), updatedMetadata.getId().getDisplayName(), updatedMetadata.getStatusScheme())); } - return updatedMetadata; + return updatedMetadata.withDerivationStrategy(curStrategy); } protected ComponentMetadataDetails createDetails(MutableModuleComponentResolveMetadata mutableMetadata) { diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/dsl/WrappingComponentMetadataContext.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/dsl/WrappingComponentMetadataContext.java index 6635360d3627..8faab6fe5e01 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/dsl/WrappingComponentMetadataContext.java +++ b/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; @@ -71,11 +72,19 @@ public ComponentMetadataDetails getDetails() { return details; } + VariantDerivationStrategy getVariantDerivationStrategy() { + return metadata.getVariantDerivationStrategy(); + } + MutableModuleComponentResolveMetadata getMutableMetadata() { createMutableMetadataIfNeeded(); return mutableMetadata; } + ModuleComponentResolveMetadata getImmutableMetadata() { + return metadata; + } + private void createMutableMetadataIfNeeded() { if (mutableMetadata == null) { mutableMetadata = metadata.asMutable(); diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/LenientPlatformResolveMetadata.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/LenientPlatformResolveMetadata.java index eedbd8011eaa..f70d2f4fcbdf 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/LenientPlatformResolveMetadata.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/LenientPlatformResolveMetadata.java @@ -32,6 +32,7 @@ import org.gradle.internal.component.external.model.ModuleDependencyMetadata; import org.gradle.internal.component.external.model.MutableModuleComponentResolveMetadata; 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 +153,16 @@ public ModuleComponentResolveMetadata withSources(ModuleSources sources) { return this; } + @Override + public ModuleComponentResolveMetadata withDerivationStrategy(VariantDerivationStrategy derivationStrategy) { + return this; + } + + @Override + public VariantDerivationStrategy getVariantDerivationStrategy() { + return getVariantMetadataRules().getVariantDerivationStrategy(); + } + @Override public ModuleComponentArtifactMetadata artifact(String type, @Nullable String extension, @Nullable String classifier) { throw new UnsupportedOperationException(); diff --git a/subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/AbstractLazyModuleComponentResolveMetadata.java b/subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/AbstractLazyModuleComponentResolveMetadata.java index f83a3028edc5..21a4eed94891 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/AbstractLazyModuleComponentResolveMetadata.java +++ b/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 target) { diff --git a/subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/AbstractModuleComponentResolveMetadata.java b/subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/AbstractModuleComponentResolveMetadata.java index 1bf945aae02b..e0665679e7e7 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/AbstractModuleComponentResolveMetadata.java +++ b/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 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 = null; } public AbstractModuleComponentResolveMetadata(AbstractModuleComponentResolveMetadata metadata, ImmutableList 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,14 @@ public ImmutableList getPlatformOwners() { return platformOwners; } + @Override + public VariantDerivationStrategy getVariantDerivationStrategy() { + if (variantDerivationStrategy == null) { + return getVariantMetadataRules().getVariantDerivationStrategy(); + } + return variantDerivationStrategy; + } + @Override public boolean equals(Object o) { if (this == o) { diff --git a/subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/AbstractRealisedModuleComponentResolveMetadata.java b/subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/AbstractRealisedModuleComponentResolveMetadata.java index 1565fa324927..c5b9655ba272 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/AbstractRealisedModuleComponentResolveMetadata.java +++ b/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; } diff --git a/subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/ModuleComponentResolveMetadata.java b/subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/ModuleComponentResolveMetadata.java index cb32f6ee6a17..36cda32a4b95 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/ModuleComponentResolveMetadata.java +++ b/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(); } diff --git a/subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/ivy/DefaultIvyModuleResolveMetadata.java b/subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/ivy/DefaultIvyModuleResolveMetadata.java index 15f9ba0f8f68..1bcb1d12320c 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/ivy/DefaultIvyModuleResolveMetadata.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/ivy/DefaultIvyModuleResolveMetadata.java @@ -28,6 +28,8 @@ import org.gradle.internal.component.external.model.DefaultConfigurationMetadata; import org.gradle.internal.component.external.model.DefaultModuleComponentSelector; import org.gradle.internal.component.external.model.ModuleComponentArtifactMetadata; +import org.gradle.internal.component.external.model.ModuleComponentResolveMetadata; +import org.gradle.internal.component.external.model.VariantDerivationStrategy; import org.gradle.internal.component.external.model.VariantMetadataRules; import org.gradle.internal.component.model.Exclude; import org.gradle.internal.component.model.ExcludeMetadata; @@ -63,8 +65,8 @@ public class DefaultIvyModuleResolveMetadata extends AbstractLazyModuleComponent this.extraAttributes = metadata.getExtraAttributes(); } - private DefaultIvyModuleResolveMetadata(DefaultIvyModuleResolveMetadata metadata, ModuleSources sources) { - super(metadata, sources); + private DefaultIvyModuleResolveMetadata(DefaultIvyModuleResolveMetadata metadata, ModuleSources sources, VariantDerivationStrategy variantDerivationStrategy) { + super(metadata, sources, variantDerivationStrategy); this.configurationDefinitions = metadata.configurationDefinitions; this.branch = metadata.branch; this.artifactDefinitions = metadata.artifactDefinitions; @@ -72,11 +74,11 @@ private DefaultIvyModuleResolveMetadata(DefaultIvyModuleResolveMetadata metadata this.excludes = metadata.excludes; this.extraAttributes = metadata.extraAttributes; - copyCachedState(metadata); + copyCachedState(metadata, metadata.getVariantDerivationStrategy() != variantDerivationStrategy); } private DefaultIvyModuleResolveMetadata(DefaultIvyModuleResolveMetadata metadata, List dependencies) { - super(metadata, metadata.getSources()); + super(metadata, metadata.getSources(), metadata.getVariantDerivationStrategy()); this.configurationDefinitions = metadata.configurationDefinitions; this.branch = metadata.branch; this.artifactDefinitions = metadata.artifactDefinitions; @@ -108,7 +110,16 @@ public MutableIvyModuleResolveMetadata asMutable() { @Override public DefaultIvyModuleResolveMetadata withSources(ModuleSources sources) { - return new DefaultIvyModuleResolveMetadata(this, sources); + return new DefaultIvyModuleResolveMetadata(this, sources, getVariantDerivationStrategy()); + } + + + @Override + public ModuleComponentResolveMetadata withDerivationStrategy(VariantDerivationStrategy derivationStrategy) { + if (getVariantDerivationStrategy() == derivationStrategy) { + return this; + } + return new DefaultIvyModuleResolveMetadata(this, getSources(), derivationStrategy); } @Override diff --git a/subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/ivy/RealisedIvyModuleResolveMetadata.java b/subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/ivy/RealisedIvyModuleResolveMetadata.java index 25ebe630e124..8f089abc4181 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/ivy/RealisedIvyModuleResolveMetadata.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/ivy/RealisedIvyModuleResolveMetadata.java @@ -38,8 +38,10 @@ import org.gradle.internal.component.external.model.ImmutableCapabilities; import org.gradle.internal.component.external.model.LazyToRealisedModuleComponentResolveMetadataHelper; import org.gradle.internal.component.external.model.ModuleComponentArtifactMetadata; +import org.gradle.internal.component.external.model.ModuleComponentResolveMetadata; import org.gradle.internal.component.external.model.ModuleDependencyMetadata; 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.model.ConfigurationMetadata; import org.gradle.internal.component.model.DependencyMetadata; @@ -167,8 +169,8 @@ private RealisedIvyModuleResolveMetadata(RealisedIvyModuleResolveMetadata metada this.metadata = metadata.metadata; } - private RealisedIvyModuleResolveMetadata(RealisedIvyModuleResolveMetadata metadata, ModuleSources sources) { - super(metadata, sources); + private RealisedIvyModuleResolveMetadata(RealisedIvyModuleResolveMetadata metadata, ModuleSources sources, VariantDerivationStrategy derivationStrategy) { + super(metadata, sources, derivationStrategy); this.configurationDefinitions = metadata.configurationDefinitions; this.branch = metadata.branch; this.artifactDefinitions = metadata.artifactDefinitions; @@ -234,7 +236,15 @@ public MutableIvyModuleResolveMetadata asMutable() { @Override public RealisedIvyModuleResolveMetadata withSources(ModuleSources sources) { - return new RealisedIvyModuleResolveMetadata(this, sources); + return new RealisedIvyModuleResolveMetadata(this, sources, getVariantDerivationStrategy()); + } + + @Override + public ModuleComponentResolveMetadata withDerivationStrategy(VariantDerivationStrategy derivationStrategy) { + if (getVariantDerivationStrategy() == derivationStrategy) { + return this; + } + return new RealisedIvyModuleResolveMetadata(this, getSources(), derivationStrategy); } @Nullable diff --git a/subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/maven/DefaultMavenModuleResolveMetadata.java b/subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/maven/DefaultMavenModuleResolveMetadata.java index 79e59901c403..4d745e795642 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/maven/DefaultMavenModuleResolveMetadata.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/maven/DefaultMavenModuleResolveMetadata.java @@ -31,6 +31,7 @@ import org.gradle.internal.component.external.model.DefaultConfigurationMetadata; import org.gradle.internal.component.external.model.DefaultModuleComponentArtifactMetadata; import org.gradle.internal.component.external.model.ModuleComponentArtifactMetadata; +import org.gradle.internal.component.external.model.ModuleComponentResolveMetadata; import org.gradle.internal.component.external.model.ModuleDependencyMetadata; import org.gradle.internal.component.external.model.VariantDerivationStrategy; import org.gradle.internal.component.external.model.VariantMetadataRules; @@ -78,8 +79,8 @@ public class DefaultMavenModuleResolveMetadata extends AbstractLazyModuleCompone dependencies = metadata.getDependencies(); } - private DefaultMavenModuleResolveMetadata(DefaultMavenModuleResolveMetadata metadata, ModuleSources sources) { - super(metadata, sources); + private DefaultMavenModuleResolveMetadata(DefaultMavenModuleResolveMetadata metadata, ModuleSources sources, VariantDerivationStrategy derivationStrategy) { + super(metadata, sources, derivationStrategy); this.objectInstantiator = metadata.objectInstantiator; this.mavenImmutableAttributesFactory = metadata.mavenImmutableAttributesFactory; packaging = metadata.packaging; @@ -87,7 +88,7 @@ private DefaultMavenModuleResolveMetadata(DefaultMavenModuleResolveMetadata meta snapshotTimestamp = metadata.snapshotTimestamp; dependencies = metadata.dependencies; - copyCachedState(metadata); + copyCachedState(metadata, metadata.getVariantDerivationStrategy() != derivationStrategy); } @Override @@ -110,7 +111,7 @@ protected Optional> maybeDeriveVa } private ImmutableList getDerivedVariants() { - VariantDerivationStrategy strategy = getVariantMetadataRules().getVariantDerivationStrategy(); + VariantDerivationStrategy strategy = getVariantDerivationStrategy(); if (derivedVariants == null && strategy.derivesVariants()) { filterConstraints = false; derivedVariants = strategy.derive(this); @@ -209,7 +210,15 @@ public MutableMavenModuleResolveMetadata asMutable() { @Override public DefaultMavenModuleResolveMetadata withSources(ModuleSources sources) { - return new DefaultMavenModuleResolveMetadata(this, sources); + return new DefaultMavenModuleResolveMetadata(this, sources, getVariantDerivationStrategy()); + } + + @Override + public ModuleComponentResolveMetadata withDerivationStrategy(VariantDerivationStrategy derivationStrategy) { + if (getVariantDerivationStrategy() == derivationStrategy) { + return this; + } + return new DefaultMavenModuleResolveMetadata(this, getSources(), derivationStrategy); } @Override diff --git a/subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/maven/RealisedMavenModuleResolveMetadata.java b/subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/maven/RealisedMavenModuleResolveMetadata.java index 78da08a1b819..bca9cdbf5d29 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/maven/RealisedMavenModuleResolveMetadata.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/maven/RealisedMavenModuleResolveMetadata.java @@ -41,6 +41,7 @@ import org.gradle.internal.component.external.model.ModuleComponentResolveMetadata; import org.gradle.internal.component.external.model.ModuleDependencyMetadata; 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.model.ConfigurationMetadata; import org.gradle.internal.component.model.DefaultIvyArtifactName; @@ -240,8 +241,8 @@ private static boolean include(MavenDependencyDescriptor dependency, Collection< this.derivedVariants = ImmutableList.copyOf(derivedVariants); } - private RealisedMavenModuleResolveMetadata(RealisedMavenModuleResolveMetadata metadata, ModuleSources sources) { - super(metadata, sources); + private RealisedMavenModuleResolveMetadata(RealisedMavenModuleResolveMetadata metadata, ModuleSources sources, VariantDerivationStrategy derivationStrategy) { + super(metadata, sources, derivationStrategy); this.objectInstantiator = metadata.objectInstantiator; packaging = metadata.packaging; relocated = metadata.relocated; @@ -261,7 +262,15 @@ ImmutableList getDerivedVariants() { @Override public RealisedMavenModuleResolveMetadata withSources(ModuleSources sources) { - return new RealisedMavenModuleResolveMetadata(this, sources); + return new RealisedMavenModuleResolveMetadata(this, sources, getVariantDerivationStrategy()); + } + + @Override + public ModuleComponentResolveMetadata withDerivationStrategy(VariantDerivationStrategy derivationStrategy) { + if (getVariantDerivationStrategy() == derivationStrategy) { + return this; + } + return new RealisedMavenModuleResolveMetadata(this, getSources(), derivationStrategy); } @Override diff --git a/subprojects/dependency-management/src/main/java/org/gradle/internal/resolve/caching/ComponentMetadataRuleExecutor.java b/subprojects/dependency-management/src/main/java/org/gradle/internal/resolve/caching/ComponentMetadataRuleExecutor.java index 7d303187e1f2..434296a772c5 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/internal/resolve/caching/ComponentMetadataRuleExecutor.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/internal/resolve/caching/ComponentMetadataRuleExecutor.java @@ -39,7 +39,7 @@ public static boolean isMetadataRuleExecutorCache(InMemoryCacheController contro private static Transformer getKeyToSnapshotableTransformer() { return moduleMetadata -> moduleMetadata.getSources().withSource(ModuleDescriptorHashModuleSource.class, source -> { - return source.map(metadataFileSource -> metadataFileSource.getDescriptorHash().toString()) + return source.map(metadataFileSource -> metadataFileSource.getDescriptorHash().toString() + moduleMetadata.getVariantDerivationStrategy().getClass().getName()) .orElseThrow(() -> new RuntimeException("Cannot find original content hash")); }); } diff --git a/subprojects/dependency-management/src/test/groovy/org/gradle/internal/resolve/caching/ComponentMetadataRuleExecutorTest.groovy b/subprojects/dependency-management/src/test/groovy/org/gradle/internal/resolve/caching/ComponentMetadataRuleExecutorTest.groovy index 4ce17d0d7d50..a2a887305644 100644 --- a/subprojects/dependency-management/src/test/groovy/org/gradle/internal/resolve/caching/ComponentMetadataRuleExecutorTest.groovy +++ b/subprojects/dependency-management/src/test/groovy/org/gradle/internal/resolve/caching/ComponentMetadataRuleExecutorTest.groovy @@ -38,6 +38,7 @@ import org.gradle.internal.action.DefaultConfigurableRule import org.gradle.internal.action.DefaultConfigurableRules import org.gradle.internal.action.InstantiatingAction import org.gradle.internal.component.external.model.ModuleComponentResolveMetadata +import org.gradle.internal.component.external.model.VariantDerivationStrategy import org.gradle.internal.component.model.MutableModuleSources import org.gradle.internal.hash.HashCode import org.gradle.internal.hash.Hashing @@ -129,6 +130,7 @@ class ComponentMetadataRuleExecutorTest extends Specification { then: 1 * key.getSources() >> moduleSources + 1 * key.getVariantDerivationStrategy() >> Stub(VariantDerivationStrategy) 1 * valueSnapshotter.snapshot(_) >> inputsSnapshot 1 * store.get(keyHash) >> cachedEntry if (expired) {