Disable implicitly collecting baseline coverage for toolchain targets.

alexjski · Wyverald · commit 43bcf80a3dfd · 2022-01-12T18:30:58.000+01:00
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
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
@@ -32,6 +32,7 @@
 import com.google.devtools.build.lib.analysis.constraints.SupportedEnvironments;
 import com.google.devtools.build.lib.analysis.constraints.SupportedEnvironmentsProvider;
 import com.google.devtools.build.lib.analysis.constraints.SupportedEnvironmentsProvider.RemovedEnvironmentCulprit;
+import com.google.devtools.build.lib.analysis.platform.ToolchainInfo;
 import com.google.devtools.build.lib.analysis.test.AnalysisTestActionBuilder;
 import com.google.devtools.build.lib.analysis.test.AnalysisTestResultInfo;
 import com.google.devtools.build.lib.analysis.test.ExecutionInfo;
@@ -161,7 +162,8 @@ public ConfiguredTarget build() throws ActionConflictException, InterruptedExcep
     // rule doesn't configure InstrumentedFilesInfo. This needs to be done for non-test rules
     // as well, but should be done before initializeTestProvider, which uses that.
     if (ruleContext.getConfiguration().isCodeCoverageEnabled()
-        && !providersBuilder.contains(InstrumentedFilesInfo.STARLARK_CONSTRUCTOR.getKey())) {
+        && !providersBuilder.contains(InstrumentedFilesInfo.STARLARK_CONSTRUCTOR.getKey())
+        && !providersBuilder.contains(ToolchainInfo.PROVIDER.getKey())) {
       addNativeDeclaredProvider(InstrumentedFilesCollector.forwardAll(ruleContext));
     }
     // Create test action and artifacts if target was successfully initialized
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/BUILD b/src/test/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -151,6 +151,7 @@ java_test(
         "//src/main/java/com/google/devtools/build/lib/analysis:target_and_configuration",
         "//src/main/java/com/google/devtools/build/lib/analysis:test/test_configuration",
         "//src/main/java/com/google/devtools/build/lib/analysis:toolchain_collection",
+        "//src/main/java/com/google/devtools/build/lib/analysis:toolchain_context",
         "//src/main/java/com/google/devtools/build/lib/analysis:top_level_artifact_context",
         "//src/main/java/com/google/devtools/build/lib/analysis:transitive_info_provider",
         "//src/main/java/com/google/devtools/build/lib/analysis:view_creation_failed_exception",
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ToolchainsForTargetsTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ToolchainsForTargetsTest.java
@@ -13,15 +13,23 @@
 // limitations under the License.
 package com.google.devtools.build.lib.skyframe;
 
+import static com.google.common.collect.ImmutableList.toImmutableList;
+import static com.google.common.truth.Truth.assertThat;
 import static com.google.devtools.build.lib.analysis.testing.ToolchainCollectionSubject.assertThat;
+import static com.google.devtools.build.lib.analysis.testing.ToolchainContextSubject.assertThat;
 
 import com.google.auto.value.AutoValue;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
+import com.google.devtools.build.lib.actions.Action;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.TargetAndConfiguration;
 import com.google.devtools.build.lib.analysis.ToolchainCollection;
+import com.google.devtools.build.lib.analysis.ToolchainContext;
+import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
+import com.google.devtools.build.lib.analysis.test.BaselineCoverageAction;
 import com.google.devtools.build.lib.analysis.util.AnalysisMock;
 import com.google.devtools.build.lib.analysis.util.AnalysisTestCase;
 import com.google.devtools.build.lib.cmdline.Label;
@@ -57,7 +65,7 @@
  * doesn't support direct access to environments.
  */
 @RunWith(JUnit4.class)
-public class ToolchainsForTargetsTest extends AnalysisTestCase {
+public final class ToolchainsForTargetsTest extends AnalysisTestCase {
   /** Returns a {@link SkyKey} for a given <Target, BuildConfiguration> pair. */
   private static Key key(
       TargetAndConfiguration targetAndConfiguration, ConfiguredTargetKey configuredTargetKey) {
@@ -475,4 +483,54 @@ public void keepParentToolchainContext() throws Exception {
         .defaultToolchainContext()
         .hasExecutionPlatform("//platforms:local_platform_b");
   }
+
+  /** Regression test for b/214105142, https://github.com/bazelbuild/bazel/issues/14521 */
+  @Test
+  public void toolchainWithDifferentExecutionPlatforms_doesNotGenerateConflictingCoverageAction()
+      throws Exception {
+    scratch.file(
+        "platforms/BUILD",
+        "constraint_setting(name = 'local_setting')",
+        "constraint_value(name = 'local_value_a', constraint_setting = ':local_setting')",
+        "constraint_value(name = 'local_value_b', constraint_setting = ':local_setting')",
+        "platform(name = 'local_platform_a', constraint_values = [':local_value_a'])",
+        "platform(name = 'local_platform_b', constraint_values = [':local_value_b'])");
+    scratch.file(
+        "a/BUILD",
+        "load('//toolchain:rule.bzl', 'my_rule')",
+        "my_rule(name='a', exec_compatible_with=['//platforms:local_value_a'])",
+        "my_rule(name='b', exec_compatible_with=['//platforms:local_value_b'])");
+    useConfiguration(
+        "--collect_code_coverage",
+        "--extra_execution_platforms=//platforms:local_platform_a,//platforms:local_platform_b");
+
+    update("//a:a", "//a:b");
+
+    // Sanity check that a coverage action was generated for the rule itself.
+    assertHasBaselineCoverageAction("//a:a", "Writing file a/a/baseline_coverage.dat");
+    assertHasBaselineCoverageAction("//a:b", "Writing file a/b/baseline_coverage.dat");
+    assertThat(getActions("//toolchains:toolchain_1_impl")).isEmpty();
+    ToolchainContext toolchainAContext =
+        getToolchainCollection("//a:a").getDefaultToolchainContext();
+    assertThat(toolchainAContext).hasExecutionPlatform("//platforms:local_platform_a");
+    assertThat(toolchainAContext).hasToolchainType("//toolchain:test_toolchain");
+    assertThat(toolchainAContext).hasResolvedToolchain("//toolchains:toolchain_1_impl");
+    ToolchainContext toolchainBContext =
+        getToolchainCollection("//a:b").getDefaultToolchainContext();
+    assertThat(toolchainBContext).hasExecutionPlatform("//platforms:local_platform_b");
+    assertThat(toolchainBContext).hasToolchainType("//toolchain:test_toolchain");
+    assertThat(toolchainBContext).hasResolvedToolchain("//toolchains:toolchain_1_impl");
+  }
+
+  private void assertHasBaselineCoverageAction(String label, String progressMessage)
+      throws InterruptedException {
+    Action coverageAction = Iterables.getOnlyElement(getActions(label));
+    assertThat(coverageAction).isInstanceOf(BaselineCoverageAction.class);
+    assertThat(coverageAction.getProgressMessage()).isEqualTo(progressMessage);
+  }
+
+  private ImmutableList<Action> getActions(String label) throws InterruptedException {
+    return ((RuleConfiguredTarget) getConfiguredTarget(label))
+        .getActions().stream().map(Action.class::cast).collect(toImmutableList());
+  }
 }
