From 706f5acd02363e48076dc97e37613fd968932d03 Mon Sep 17 00:00:00 2001 From: Philipp Schrader Date: Mon, 22 Mar 2021 07:06:39 -0700 Subject: [PATCH] Fix bazel crash when passing config_setting to target_compatible_with 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 --- .../lib/analysis/ConfiguredTargetFactory.java | 12 ++++---- .../target_compatible_with_test.sh | 30 +++++++++++++++++++ 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java index bdd03ea0c0ad4c..e891605c4b8834 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java @@ -322,12 +322,6 @@ private ConfiguredTarget createRule( prerequisiteMap.values())) .build(); - ConfiguredTarget incompatibleTarget = - RuleContextConstraintSemantics.incompatibleConfiguredTarget(ruleContext, prerequisiteMap); - if (incompatibleTarget != null) { - return incompatibleTarget; - } - List> analysisFailures = depAnalysisFailures(ruleContext); if (!analysisFailures.isEmpty()) { return erroredConfiguredTargetWithFailures(ruleContext, analysisFailures); @@ -336,6 +330,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 : diff --git a/src/test/shell/integration/target_compatible_with_test.sh b/src/test/shell/integration/target_compatible_with_test.sh index d247dc1c5662d3..1c9be212c0835d 100755 --- a/src/test/shell/integration/target_compatible_with_test.sh +++ b/src/test/shell/integration/target_compatible_with_test.sh @@ -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 < "${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() {