Skip to content

Commit

Permalink
Fix bazel crash when passing config_setting to target_compatible_with
Browse files Browse the repository at this point in the history
When you pass a `config_setting` into a target's
`target_compatible_with` attribute, we see the following crash:

    FATAL: bazel crashed due to an internal error. Printing stack trace:
    java.lang.RuntimeException: Unrecoverable error while evaluating node 'ConfiguredTargetKey{label=//:hello, config=BuildConfigurationValue.Key[86e57527e1c8bb10edc853e734cb858f8159d8f3e0a4df9ceb16f80aad784b93]}' (requested by nodes )
            at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:563)
            at com.google.devtools.build.lib.concurrent.AbstractQueueVisitor$WrappedRunnable.run(AbstractQueueVisitor.java:398)
            at java.base/java.util.concurrent.ForkJoinTask$AdaptedRunnableAction.exec(Unknown Source)
            at java.base/java.util.concurrent.ForkJoinTask.doExec(Unknown Source)
            at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(Unknown Source)
            at java.base/java.util.concurrent.ForkJoinPool.scan(Unknown Source)
            at java.base/java.util.concurrent.ForkJoinPool.runWorker(Unknown Source)
            at java.base/java.util.concurrent.ForkJoinWorkerThread.run(Unknown Source)
    Caused by: java.lang.NullPointerException
            at com.google.devtools.build.lib.analysis.platform.ConstraintCollection.hasConstraintValue(ConstraintCollection.java:216)
            at com.google.devtools.build.lib.analysis.RuleContext.targetPlatformHasConstraint(RuleContext.java:1226)
            at com.google.devtools.build.lib.analysis.constraints.RuleContextConstraintSemantics.lambda$incompatibleConfiguredTarget$1(RuleContextConstraintSemantics.java:904)
            at java.base/java.util.stream.ReferencePipeline$2$1.accept(Unknown Source)
            at com.google.common.collect.CollectSpliterators$1.lambda$forEachRemaining$1(CollectSpliterators.java:120)
            at java.base/java.util.AbstractList$RandomAccessSpliterator.forEachRemaining(Unknown Source)
            at com.google.common.collect.CollectSpliterators$1.forEachRemaining(CollectSpliterators.java:120)
            at java.base/java.util.stream.AbstractPipeline.copyInto(Unknown Source)
            at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(Unknown Source)
            at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(Unknown Source)
            at java.base/java.util.stream.AbstractPipeline.evaluate(Unknown Source)
            at java.base/java.util.stream.ReferencePipeline.collect(Unknown Source)
            at com.google.devtools.build.lib.analysis.constraints.RuleContextConstraintSemantics.incompatibleConfiguredTarget(RuleContextConstraintSemantics.java:905)
            at com.google.devtools.build.lib.analysis.ConfiguredTargetFactory.createRule(ConfiguredTargetFactory.java:327)
            at com.google.devtools.build.lib.analysis.ConfiguredTargetFactory.createConfiguredTarget(ConfiguredTargetFactory.java:194)
            at com.google.devtools.build.lib.skyframe.SkyframeBuildView.createConfiguredTarget(SkyframeBuildView.java:938)
            at com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.createConfiguredTarget(ConfiguredTargetFunction.java:1013)
            at com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.compute(ConfiguredTargetFunction.java:371)
            at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:477)
            ... 7 more

This happens because we discard various errors generated in the
`RuleContext` builder when the `target_compatible_with` attribute is
specified. This is not the desired behaviour.

This patch changes the code so that the `RuleContext` errors are
checked before evaluating the `target_compatible_with` attribute. That
way Bazel can tell the user something's wrong without crashing. In the
above example we now see this error message:

    ERROR: /bazel-cache/phil/bazel/_bazel_phil/c2296ba7b90d1074a9fe6e8d3b871222/sandbox/linux-sandbox/129/execroot/io_bazel/_tmp/c749fd38ac9a4ad6bd41e9653bbceab5/workspace/target_skipping/BUILD:101:10: in target_compatible_with attribute of sh_binary rule //target_skipping:problematic_foo3_target: '//target_skipping:foo3_config_setting' does not have mandatory providers: 'ConstraintValueInfo'

That should be sufficient to let the user know that they need to
specify `constraint_value` targets instead.

Fixes #13250

Closes #13254.

PiperOrigin-RevId: 364308810
  • Loading branch information
philsc authored and philwo committed Apr 19, 2021
1 parent 18e1887 commit 52e2024
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -323,12 +323,6 @@ private ConfiguredTarget createRule(
prerequisiteMap.values()))
.build();

ConfiguredTarget incompatibleTarget =
RuleContextConstraintSemantics.incompatibleConfiguredTarget(ruleContext, prerequisiteMap);
if (incompatibleTarget != null) {
return incompatibleTarget;
}

List<NestedSet<AnalysisFailure>> analysisFailures = depAnalysisFailures(ruleContext);
if (!analysisFailures.isEmpty()) {
return erroredConfiguredTargetWithFailures(ruleContext, analysisFailures);
Expand All @@ -337,6 +331,12 @@ private ConfiguredTarget createRule(
return erroredConfiguredTarget(ruleContext);
}

ConfiguredTarget incompatibleTarget =
RuleContextConstraintSemantics.incompatibleConfiguredTarget(ruleContext, prerequisiteMap);
if (incompatibleTarget != null) {
return incompatibleTarget;
}

try {
Class<?> missingFragmentClass = null;
for (Class<?> fragmentClass :
Expand Down
30 changes: 30 additions & 0 deletions src/test/shell/integration/target_compatible_with_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,36 @@ function tear_down() {
bazel shutdown
}
# Validates that we get a good error message when passing a config_setting into
# the target_compatible_with attribute. This is a regression test for
# https://github.com/bazelbuild/bazel/issues/13250.
function test_config_setting_in_target_compatible_with() {
cat >> target_skipping/BUILD <<EOF
config_setting(
name = "foo3_config_setting",
constraint_values = [":foo3"],
)
sh_binary(
name = "problematic_foo3_target",
srcs = ["pass.sh"],
target_compatible_with = [
":foo3_config_setting",
],
)
EOF
cd target_skipping || fail "couldn't cd into workspace"
bazel build \
--show_result=10 \
--host_platform=@//target_skipping:foo3_platform \
--platforms=@//target_skipping:foo3_platform \
... &> "${TEST_log}" && fail "Bazel succeeded unexpectedly."
expect_log "'//target_skipping:foo3_config_setting' does not have mandatory providers: 'ConstraintValueInfo'"
}
# Validates that the console log provides useful information to the user for
# builds.
function test_console_log_for_builds() {
Expand Down

0 comments on commit 52e2024

Please sign in to comment.