Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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
- Loading branch information
Showing
19 changed files
with
270 additions
and
55 deletions.
There are no files selected for viewing
136 changes: 136 additions & 0 deletions
136
...tegTest/groovy/org/gradle/integtests/resolve/ConcurrentDerivationStrategyIntegTest.groovy
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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)" | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.