From a44f5596db5d7777399be924dcccbd9d9592602b 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 | 136 ++++++++++++++++++ .../DefaultComponentMetadataProcessor.java | 9 +- .../dsl/WrappingComponentMetadataContext.java | 16 ++- .../LenientPlatformResolveMetadata.java | 12 ++ ...actLazyModuleComponentResolveMetadata.java | 10 +- ...bstractModuleComponentResolveMetadata.java | 12 +- ...MutableModuleComponentResolveMetadata.java | 13 +- ...ealisedModuleComponentResolveMetadata.java | 4 +- .../model/ModuleComponentResolveMetadata.java | 9 ++ .../external/model/VariantMetadataRules.java | 19 +-- .../ivy/DefaultIvyModuleResolveMetadata.java | 21 ++- .../ivy/RealisedIvyModuleResolveMetadata.java | 16 ++- .../DefaultMavenModuleResolveMetadata.java | 19 ++- .../RealisedMavenModuleResolveMetadata.java | 15 +- .../ComponentMetadataRuleExecutor.java | 2 +- ...faultMavenModuleResolveMetadataTest.groovy | 3 +- ...pendencyConstraintMetadataRulesTest.groovy | 3 +- .../VariantFilesMetadataRulesTest.groovy | 4 +- .../ComponentMetadataRuleExecutorTest.groovy | 2 + 19 files changed, 270 insertions(+), 55 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..2c7963c001d5 --- /dev/null +++ b/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)" + } +} 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..99273942af48 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,8 @@ 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 + .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); 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..8ee3641225ef 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; @@ -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; } } 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..e0b7939ca3c4 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 @@ -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(); 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..89ac5c6d81ec 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 = metadata.getVariantDerivationStrategy(); } 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,11 @@ public ImmutableList getPlatformOwners() { return platformOwners; } + @Override + public VariantDerivationStrategy 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/AbstractMutableModuleComponentResolveMetadata.java b/subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/AbstractMutableModuleComponentResolveMetadata.java index 6b2e16d02a61..027dc65ca185 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/AbstractMutableModuleComponentResolveMetadata.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/AbstractMutableModuleComponentResolveMetadata.java @@ -67,12 +67,16 @@ public abstract class AbstractMutableModuleComponentResolveMetadata implements M private final AttributesSchemaInternal schema; private final VariantMetadataRules variantMetadataRules; + private final VariantDerivationStrategy variantDerivationStrategy; private List newVariants; private ImmutableList variants; private Set 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; @@ -80,6 +84,7 @@ protected AbstractMutableModuleComponentResolveMetadata(ImmutableAttributesFacto 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); 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/VariantMetadataRules.java b/subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/VariantMetadataRules.java index e81359abf18e..703bcc8bc4f3 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/VariantMetadataRules.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/internal/component/external/model/VariantMetadataRules.java @@ -47,27 +47,19 @@ public class VariantMetadataRules { private final ImmutableAttributesFactory attributesFactory; + private final ModuleVersionIdentifier moduleVersionId; + private final List additionalVariants = Lists.newArrayList(); + private DependencyMetadataRules dependencyMetadataRules; private VariantAttributesRules variantAttributesRules; private CapabilitiesRules capabilitiesRules; private VariantFilesRules variantFilesRules; - private VariantDerivationStrategy variantDerivationStrategy = NoOpDerivationStrategy.getInstance(); - private final ModuleVersionIdentifier moduleVersionId; - private final List additionalVariants = Lists.newArrayList(); public VariantMetadataRules(ImmutableAttributesFactory attributesFactory, ModuleVersionIdentifier moduleVersionId) { this.attributesFactory = attributesFactory; this.moduleVersionId = moduleVersionId; } - public VariantDerivationStrategy getVariantDerivationStrategy() { - return variantDerivationStrategy; - } - - public void setVariantDerivationStrategy(VariantDerivationStrategy variantDerivationStrategy) { - this.variantDerivationStrategy = variantDerivationStrategy; - } - public ImmutableAttributes applyVariantAttributeRules(VariantResolveMetadata variant, AttributeContainerInternal source) { if (variantAttributesRules != null) { return variantAttributesRules.execute(variant, source); @@ -194,11 +186,6 @@ private ImmutableRules() { super(null, null); } - @Override - public void setVariantDerivationStrategy(VariantDerivationStrategy variantDerivationStrategy) { - throw new UnsupportedOperationException("You are probably trying to set the derivation strategy to something that wasn't supposed to be mutable"); - } - @Override public void addDependencyAction(Instantiator instantiator, NotationParser dependencyNotationParser, NotationParser dependencyConstraintNotationParser, VariantAction action) { throw new UnsupportedOperationException("You are probably trying to add a dependency rule to something that wasn't supposed to be mutable"); 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/component/external/model/DefaultMavenModuleResolveMetadataTest.groovy b/subprojects/dependency-management/src/test/groovy/org/gradle/internal/component/external/model/DefaultMavenModuleResolveMetadataTest.groovy index 295d603dd39b..e8b7de5896a1 100644 --- a/subprojects/dependency-management/src/test/groovy/org/gradle/internal/component/external/model/DefaultMavenModuleResolveMetadataTest.groovy +++ b/subprojects/dependency-management/src/test/groovy/org/gradle/internal/component/external/model/DefaultMavenModuleResolveMetadataTest.groovy @@ -127,10 +127,9 @@ class DefaultMavenModuleResolveMetadataTest extends AbstractLazyModuleComponentR def componentTypeAttribute = Attribute.of(Category.CATEGORY_ATTRIBUTE.getName(), String.class) def metadata = mavenMetadataFactory.create(id, []) metadata.packaging = packaging - metadata.variantMetadataRules.variantDerivationStrategy = JavaEcosystemVariantDerivationStrategy.instance when: - def immutableMetadata = metadata.asImmutable() + def immutableMetadata = metadata.asImmutable().withDerivationStrategy(JavaEcosystemVariantDerivationStrategy.getInstance()) def variantsForGraphTraversal = immutableMetadata.getVariantsForGraphTraversal().orNull() def compileConf = immutableMetadata.getConfiguration("compile") def runtimeConf = immutableMetadata.getConfiguration("runtime") diff --git a/subprojects/dependency-management/src/test/groovy/org/gradle/internal/component/external/model/DependencyConstraintMetadataRulesTest.groovy b/subprojects/dependency-management/src/test/groovy/org/gradle/internal/component/external/model/DependencyConstraintMetadataRulesTest.groovy index 1ee71ba3e047..8d4cad10d338 100644 --- a/subprojects/dependency-management/src/test/groovy/org/gradle/internal/component/external/model/DependencyConstraintMetadataRulesTest.groovy +++ b/subprojects/dependency-management/src/test/groovy/org/gradle/internal/component/external/model/DependencyConstraintMetadataRulesTest.groovy @@ -46,10 +46,9 @@ class DependencyConstraintMetadataRulesTest extends AbstractDependencyMetadataRu def mavenMetadata = mavenMetadataFactory.create(componentIdentifier, [ new MavenDependencyDescriptor(MavenScope.Compile, MavenDependencyType.DEPENDENCY, newSelector(DefaultModuleIdentifier.newId("org", "notOptional"), "1.0"), null, []), new MavenDependencyDescriptor(MavenScope.Compile, MavenDependencyType.OPTIONAL_DEPENDENCY, newSelector(DefaultModuleIdentifier.newId("org", "optional"), "1.0"), null, []) - ]) + ]).asImmutable().withDerivationStrategy(JavaEcosystemVariantDerivationStrategy.instance).asMutable() when: - mavenMetadata.variantMetadataRules.setVariantDerivationStrategy(JavaEcosystemVariantDerivationStrategy.instance) mavenMetadata.variantMetadataRules.addDependencyAction(instantiator, notationParser, constraintNotationParser, variantAction("default", { assert it.size() == 1 assert it[0].name == "notOptional" diff --git a/subprojects/dependency-management/src/test/groovy/org/gradle/internal/component/external/model/VariantFilesMetadataRulesTest.groovy b/subprojects/dependency-management/src/test/groovy/org/gradle/internal/component/external/model/VariantFilesMetadataRulesTest.groovy index b536c310c73b..0113bfd8040c 100644 --- a/subprojects/dependency-management/src/test/groovy/org/gradle/internal/component/external/model/VariantFilesMetadataRulesTest.groovy +++ b/subprojects/dependency-management/src/test/groovy/org/gradle/internal/component/external/model/VariantFilesMetadataRulesTest.groovy @@ -77,7 +77,9 @@ class VariantFilesMetadataRulesTest extends Specification { new MavenDependencyDescriptor(MavenScope.Compile, MavenDependencyType.DEPENDENCY, newSelector(DefaultModuleIdentifier.newId("org.test", name), "1.0"), null, []) } def metadata = mavenMetadataFactory.create(componentIdentifier, dependencies) - metadata.getVariantMetadataRules().setVariantDerivationStrategy(JavaEcosystemVariantDerivationStrategy.instance) + .asImmutable() + .withDerivationStrategy(JavaEcosystemVariantDerivationStrategy.instance) + .asMutable() metadata } 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) {