Skip to content

Commit

Permalink
Disable implicitly collecting baseline coverage for toolchain targets.
Browse files Browse the repository at this point in the history
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 #14521.

PiperOrigin-RevId: 421317916
  • Loading branch information
alexjski authored and Wyverald committed Jan 12, 2022
1 parent 9c1c622 commit 43bcf80
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions src/test/java/com/google/devtools/build/lib/skyframe/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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());
}
}

0 comments on commit 43bcf80

Please sign in to comment.