Skip to content

Commit 43bcf80

Browse files
alexjskiWyverald
authored andcommitted
Disable implicitly collecting baseline coverage for toolchain targets.
Toolchain targets can be evaluated with a [custom execution platform](https://github.com/bazelbuild/bazel/blob/b0a01af6fd0c5f3dce634cb827c75e818835e402/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java#L168) without affecting the configuration. In particular, we can have 2 `ToolchainDependencyConfiguredTargetKey` with the same label and configuration, but different `executionPlatformLabel`. When coverage is enabled, [every target](https://github.com/bazelbuild/bazel/blob/b0a01af6fd0c5f3dce634cb827c75e818835e402/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java#L160-L166) will have a [`baseline_coverage.dat` file generated with `BaselineCoverageAction`](https://github.com/bazelbuild/bazel/blob/33f7648fbaa875f73416be47f0b3c10ed93f30d6/src/main/java/com/google/devtools/build/lib/analysis/test/BaselineCoverageAction.java#L108-L114). This action will use the execution platform from the rule, meaning that in case of the same 2 toolchains, differing by execution platform only, we will create a coverage action with the same output (path based on configuration), but [different key](https://github.com/bazelbuild/bazel/blob/d657619c9c162c6e8c8f56b66e8bef4285d81944/src/main/java/com/google/devtools/build/lib/actions/ActionKeyCacher.java#L65-L70). This fails in analysis since that constitutes a conflicting shared action. Disable coverage for toolchain targets to prevent actions from being created for those. Fixes bazelbuild#14521. PiperOrigin-RevId: 421317916
1 parent 9c1c622 commit 43bcf80

File tree

3 files changed

+63
-2
lines changed

3 files changed

+63
-2
lines changed

src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java

+3-1
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import com.google.devtools.build.lib.analysis.constraints.SupportedEnvironments;
3333
import com.google.devtools.build.lib.analysis.constraints.SupportedEnvironmentsProvider;
3434
import com.google.devtools.build.lib.analysis.constraints.SupportedEnvironmentsProvider.RemovedEnvironmentCulprit;
35+
import com.google.devtools.build.lib.analysis.platform.ToolchainInfo;
3536
import com.google.devtools.build.lib.analysis.test.AnalysisTestActionBuilder;
3637
import com.google.devtools.build.lib.analysis.test.AnalysisTestResultInfo;
3738
import com.google.devtools.build.lib.analysis.test.ExecutionInfo;
@@ -161,7 +162,8 @@ public ConfiguredTarget build() throws ActionConflictException, InterruptedExcep
161162
// rule doesn't configure InstrumentedFilesInfo. This needs to be done for non-test rules
162163
// as well, but should be done before initializeTestProvider, which uses that.
163164
if (ruleContext.getConfiguration().isCodeCoverageEnabled()
164-
&& !providersBuilder.contains(InstrumentedFilesInfo.STARLARK_CONSTRUCTOR.getKey())) {
165+
&& !providersBuilder.contains(InstrumentedFilesInfo.STARLARK_CONSTRUCTOR.getKey())
166+
&& !providersBuilder.contains(ToolchainInfo.PROVIDER.getKey())) {
165167
addNativeDeclaredProvider(InstrumentedFilesCollector.forwardAll(ruleContext));
166168
}
167169
// Create test action and artifacts if target was successfully initialized

src/test/java/com/google/devtools/build/lib/skyframe/BUILD

+1
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ java_test(
151151
"//src/main/java/com/google/devtools/build/lib/analysis:target_and_configuration",
152152
"//src/main/java/com/google/devtools/build/lib/analysis:test/test_configuration",
153153
"//src/main/java/com/google/devtools/build/lib/analysis:toolchain_collection",
154+
"//src/main/java/com/google/devtools/build/lib/analysis:toolchain_context",
154155
"//src/main/java/com/google/devtools/build/lib/analysis:top_level_artifact_context",
155156
"//src/main/java/com/google/devtools/build/lib/analysis:transitive_info_provider",
156157
"//src/main/java/com/google/devtools/build/lib/analysis:view_creation_failed_exception",

src/test/java/com/google/devtools/build/lib/skyframe/ToolchainsForTargetsTest.java

+59-1
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,23 @@
1313
// limitations under the License.
1414
package com.google.devtools.build.lib.skyframe;
1515

16+
import static com.google.common.collect.ImmutableList.toImmutableList;
17+
import static com.google.common.truth.Truth.assertThat;
1618
import static com.google.devtools.build.lib.analysis.testing.ToolchainCollectionSubject.assertThat;
19+
import static com.google.devtools.build.lib.analysis.testing.ToolchainContextSubject.assertThat;
1720

1821
import com.google.auto.value.AutoValue;
22+
import com.google.common.collect.ImmutableList;
1923
import com.google.common.collect.ImmutableMap;
2024
import com.google.common.collect.Iterables;
25+
import com.google.devtools.build.lib.actions.Action;
2126
import com.google.devtools.build.lib.analysis.BlazeDirectories;
2227
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
2328
import com.google.devtools.build.lib.analysis.TargetAndConfiguration;
2429
import com.google.devtools.build.lib.analysis.ToolchainCollection;
30+
import com.google.devtools.build.lib.analysis.ToolchainContext;
31+
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
32+
import com.google.devtools.build.lib.analysis.test.BaselineCoverageAction;
2533
import com.google.devtools.build.lib.analysis.util.AnalysisMock;
2634
import com.google.devtools.build.lib.analysis.util.AnalysisTestCase;
2735
import com.google.devtools.build.lib.cmdline.Label;
@@ -57,7 +65,7 @@
5765
* doesn't support direct access to environments.
5866
*/
5967
@RunWith(JUnit4.class)
60-
public class ToolchainsForTargetsTest extends AnalysisTestCase {
68+
public final class ToolchainsForTargetsTest extends AnalysisTestCase {
6169
/** Returns a {@link SkyKey} for a given <Target, BuildConfiguration> pair. */
6270
private static Key key(
6371
TargetAndConfiguration targetAndConfiguration, ConfiguredTargetKey configuredTargetKey) {
@@ -475,4 +483,54 @@ public void keepParentToolchainContext() throws Exception {
475483
.defaultToolchainContext()
476484
.hasExecutionPlatform("//platforms:local_platform_b");
477485
}
486+
487+
/** Regression test for b/214105142, https://github.com/bazelbuild/bazel/issues/14521 */
488+
@Test
489+
public void toolchainWithDifferentExecutionPlatforms_doesNotGenerateConflictingCoverageAction()
490+
throws Exception {
491+
scratch.file(
492+
"platforms/BUILD",
493+
"constraint_setting(name = 'local_setting')",
494+
"constraint_value(name = 'local_value_a', constraint_setting = ':local_setting')",
495+
"constraint_value(name = 'local_value_b', constraint_setting = ':local_setting')",
496+
"platform(name = 'local_platform_a', constraint_values = [':local_value_a'])",
497+
"platform(name = 'local_platform_b', constraint_values = [':local_value_b'])");
498+
scratch.file(
499+
"a/BUILD",
500+
"load('//toolchain:rule.bzl', 'my_rule')",
501+
"my_rule(name='a', exec_compatible_with=['//platforms:local_value_a'])",
502+
"my_rule(name='b', exec_compatible_with=['//platforms:local_value_b'])");
503+
useConfiguration(
504+
"--collect_code_coverage",
505+
"--extra_execution_platforms=//platforms:local_platform_a,//platforms:local_platform_b");
506+
507+
update("//a:a", "//a:b");
508+
509+
// Sanity check that a coverage action was generated for the rule itself.
510+
assertHasBaselineCoverageAction("//a:a", "Writing file a/a/baseline_coverage.dat");
511+
assertHasBaselineCoverageAction("//a:b", "Writing file a/b/baseline_coverage.dat");
512+
assertThat(getActions("//toolchains:toolchain_1_impl")).isEmpty();
513+
ToolchainContext toolchainAContext =
514+
getToolchainCollection("//a:a").getDefaultToolchainContext();
515+
assertThat(toolchainAContext).hasExecutionPlatform("//platforms:local_platform_a");
516+
assertThat(toolchainAContext).hasToolchainType("//toolchain:test_toolchain");
517+
assertThat(toolchainAContext).hasResolvedToolchain("//toolchains:toolchain_1_impl");
518+
ToolchainContext toolchainBContext =
519+
getToolchainCollection("//a:b").getDefaultToolchainContext();
520+
assertThat(toolchainBContext).hasExecutionPlatform("//platforms:local_platform_b");
521+
assertThat(toolchainBContext).hasToolchainType("//toolchain:test_toolchain");
522+
assertThat(toolchainBContext).hasResolvedToolchain("//toolchains:toolchain_1_impl");
523+
}
524+
525+
private void assertHasBaselineCoverageAction(String label, String progressMessage)
526+
throws InterruptedException {
527+
Action coverageAction = Iterables.getOnlyElement(getActions(label));
528+
assertThat(coverageAction).isInstanceOf(BaselineCoverageAction.class);
529+
assertThat(coverageAction.getProgressMessage()).isEqualTo(progressMessage);
530+
}
531+
532+
private ImmutableList<Action> getActions(String label) throws InterruptedException {
533+
return ((RuleConfiguredTarget) getConfiguredTarget(label))
534+
.getActions().stream().map(Action.class::cast).collect(toImmutableList());
535+
}
478536
}

0 commit comments

Comments
 (0)