From 8fcba91a7317c702ff56d13a74a15802348503ce Mon Sep 17 00:00:00 2001 From: Justin Van Dort Date: Thu, 2 Mar 2023 09:35:49 -0500 Subject: [PATCH] Clear root metadata when extending configurations The hierarchy is saved as an instance variable in configuration metadata. The mutation type for configuration extension was DEPENDENCIES, which only caused the cached metadata to be reevaluated. This is not sufficient. We add a new mutation type for HIERARCHY which completely discards the metadata, allowing it to be recalculated and the new hierarchy value to be passed to the configuration metadata --- ...endingConfigurationsIntegrationTest.groovy | 31 +++++++++++++++++++ .../configurations/DefaultConfiguration.java | 4 +-- .../configurations/MutationValidator.java | 7 ++++- .../DefaultRootComponentMetadataBuilder.java | 5 +++ ...ultRootComponentMetadataBuilderTest.groovy | 22 +++++++++++++ 5 files changed, 66 insertions(+), 3 deletions(-) diff --git a/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/api/ExtendingConfigurationsIntegrationTest.groovy b/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/api/ExtendingConfigurationsIntegrationTest.groovy index 9734cae1345b..9db89f8983e9 100644 --- a/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/api/ExtendingConfigurationsIntegrationTest.groovy +++ b/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/api/ExtendingConfigurationsIntegrationTest.groovy @@ -104,4 +104,35 @@ task checkResolveParentThenChild { succeeds "checkResolveChild" succeeds "checkResolveParentThenChild" } + + @Issue("https://github.com/gradle/gradle/issues/24109") + def "can resolve configuration after extending a resolved configuration"() { + given: + mavenRepo.module("org", "foo").publish() + + buildFile << """ + repositories { + maven { url "${mavenRepo.uri}" } + } + configurations { + superConfiguration + subConfiguration + } + dependencies { + superConfiguration 'org:foo:1.0' + } + + task resolve { + println configurations.superConfiguration.files.collect { it.name } + configurations.subConfiguration.extendsFrom(configurations.superConfiguration) + println configurations.subConfiguration.files.collect { it.name } + } + """ + + when: + succeeds("resolve") + + then: + output.contains("[foo-1.0.jar]\n[foo-1.0.jar]") + } } diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/configurations/DefaultConfiguration.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/configurations/DefaultConfiguration.java index bf4a24224d56..c69b63ce6614 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/configurations/DefaultConfiguration.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/configurations/DefaultConfiguration.java @@ -381,7 +381,7 @@ public Set getExtendsFrom() { @Override public Configuration setExtendsFrom(Iterable extendsFrom) { - validateMutation(MutationType.DEPENDENCIES); + validateMutation(MutationType.HIERARCHY); for (Configuration configuration : this.extendsFrom) { if (inheritedArtifacts != null) { inheritedArtifacts.removeCollection(configuration.getAllArtifacts()); @@ -403,7 +403,7 @@ public Configuration setExtendsFrom(Iterable extendsFrom) { @Override public Configuration extendsFrom(Configuration... extendsFrom) { - validateMutation(MutationType.DEPENDENCIES); + validateMutation(MutationType.HIERARCHY); for (Configuration configuration : extendsFrom) { if (configuration.getHierarchy().contains(this)) { throw new InvalidUserDataException(String.format( diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/configurations/MutationValidator.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/configurations/MutationValidator.java index f4f466b0f742..948c5782207d 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/configurations/MutationValidator.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/configurations/MutationValidator.java @@ -46,7 +46,12 @@ enum MutationType { /** * The mutation of the role of the configuration (can be queries, resolved, ...) */ - ROLE("role"); + ROLE("role"), + + /** + * The mutation of the hierarchy of the configuration, i.e. which configurations this configuration extends from. + */ + HIERARCHY("hierarchy"); private final String displayName; diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/moduleconverter/DefaultRootComponentMetadataBuilder.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/moduleconverter/DefaultRootComponentMetadataBuilder.java index 0bf55564f8c9..8caa6806c5cf 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/moduleconverter/DefaultRootComponentMetadataBuilder.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/moduleconverter/DefaultRootComponentMetadataBuilder.java @@ -156,6 +156,11 @@ public void validateMutation(MutationType type) { cachedValue.reevaluate(); } } + } else if (type == MutationType.HIERARCHY) { + // The hierarchy is provided to the configuration metadata on construction. Since it is not + // computed lazily, there is no lazy value to invalidate. Thus, we need to recompute the + // entire component in order to reconstruct new configuration metadatas with new hierarchy values. + cachedValue = null; } } diff --git a/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/moduleconverter/DefaultRootComponentMetadataBuilderTest.groovy b/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/moduleconverter/DefaultRootComponentMetadataBuilderTest.groovy index a1622fbb8888..b27b34a37278 100644 --- a/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/moduleconverter/DefaultRootComponentMetadataBuilderTest.groovy +++ b/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/moduleconverter/DefaultRootComponentMetadataBuilderTest.groovy @@ -137,6 +137,28 @@ class DefaultRootComponentMetadataBuilderTest extends Specification { ] } + def "discards component metadata when hierarchy changes"() { + componentIdentifierFactory.createComponentIdentifier(_) >> { + new DefaultModuleComponentIdentifier(mid, '1.0') + } + def root = builder.toRootComponentMetaData() + + def conf = root.getConfiguration("conf") + assert conf.needsReevaluate() + conf.realizeDependencies() + assert !conf.needsReevaluate() + + when: + builder.validator.validateMutation(MutationValidator.MutationType.HIERARCHY) + def otherRoot = builder.toRootComponentMetaData() + def otherConf = otherRoot.getConfiguration("conf") + + then: + root != otherRoot + conf != otherConf + otherConf.needsReevaluate() + } + def "does not reevaluate component metadata when #mutationType change"() { componentIdentifierFactory.createComponentIdentifier(_) >> { new DefaultModuleComponentIdentifier(mid, '1.0')