Bazel crash when target_compatible_with doesn't have a default entry.

[katre]

Description of the bug:

When the target_compatible_with attribute is a select, and the select has no match for the current configuration, bazel crashes because the IncompatibleTargetChecker does not handle the ConfiguredAttributeMapper.ValidationException.

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

See katre@63939e2 for a change to target_compatible_with_test.sh that shows the error.

$ bazel test //src/test/shell/integration:target_compatible_with_test --test_filter=test_missing_default
INFO: Analyzed target //src/test/shell/integration:target_compatible_with_test (1 packages loaded, 847 targets configured).
INFO: Found 1 test target...
FAIL: //src/test/shell/integration:target_compatible_with_test (see /usr/local/google/home/jcater/.cache/bazel/_bazel_jcater/a85a0dacd9e20e18c47928db0f1530e0/execroot/io_bazel/bazel-out/k8-fastbuild/testlogs/src/test/shell/integration/target_compatible_with_test/test.log)
INFO: From Testing //src/test/shell/integration:target_compatible_with_test:
==================== Test output for //src/test/shell/integration:target_compatible_with_test:
INFO[target_compatible_with_test 2023-04-10 20:14:09 (+0000)] bazel binary is at /usr/local/google/home/jcater/.cache/bazel/_bazel_jcater/a85a0dacd9e20e18c47928db0f1530e0/sandbox/linux-sandbox/1268/execroot/io_bazel/bazel-out/k8-fastbuild/bin/src/test/shell/integration/target_compatible_with_test.runfiles/io_bazel/src/test/shell/bin
INFO[target_compatible_with_test 2023-04-10 20:14:09 (+0000)] setting up client in /usr/local/google/home/jcater/.cache/bazel/_bazel_jcater/a85a0dacd9e20e18c47928db0f1530e0/sandbox/linux-sandbox/1268/execroot/io_bazel/_tmp/4a28868e660c8b58f7795306ee6f829b/workspace

target_compatible_with tests

Mon Apr 10 20:14:09 UTC 2023
** test_missing_default ********************************************************
-- Test log: -----------------------------------------------------------
$TEST_TMPDIR defined: output root default is '/usr/local/google/home/jcater/.cache/bazel/_bazel_jcater/a85a0dacd9e20e18c47928db0f1530e0/sandbox/linux-sandbox/1268/execroot/io_bazel/_tmp/4a28868e660c8b58f7795306ee6f829b' and max_idle_secs default is '15'.
WARNING: The following rc files are no longer being read, please transfer their contents or import their path into one of the standard rc files:
/etc/bazel.bazelrc
Extracting Bazel installation...
Starting local Bazel server and connecting to it...
Loading: 
Loading: 
Loading: 0 packages loaded
Analyzing: target //target_skipping:pass_on_foo1_or_foo2_but_not_on_foo3 (1 packages loaded, 0 targets configured)
FATAL: bazel crashed due to an internal error. Printing stack trace:
java.lang.RuntimeException: Unrecoverable error while evaluating node 'ConfiguredTargetKey{label=//target_skipping:pass_on_foo1_or_foo2_but_not_on_foo3, config=BuildConfigurationKey[ba87565edeaaaab5b6f8aa7ebde8e64f145d546f518b1ec653f8810f21062bd6]}' (requested by nodes )
	at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:591)
	at com.google.devtools.build.lib.concurrent.AbstractQueueVisitor$WrappedRunnable.run(AbstractQueueVisitor.java:375)
	at java.base/java.util.concurrent.ForkJoinTask$RunnableExecuteAction.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.IllegalStateException: lookup failed on attribute target_compatible_with: configurable attribute "target_compatible_with" in //target_skipping:pass_on_foo1_or_foo2_but_not_on_foo3 doesn't match this configuration. Would a default condition help?

Conditions checked:
 //target_skipping:foo1

To see a condition's definition, run: bazel query --output=build <condition label>.

This instance of //target_skipping:pass_on_foo1_or_foo2_but_not_on_foo3 has configuration identifier ba87565. To inspect its configuration, run: bazel config ba87565.

For more help, see https://bazel.build/docs/configurable-attributes#faq-select-choose-condition.


	at com.google.devtools.build.lib.packages.ConfiguredAttributeMapper.get(ConfiguredAttributeMapper.java:298)
	at com.google.devtools.build.lib.analysis.constraints.IncompatibleTargetChecker$IncompatibleTargetProducer.step(IncompatibleTargetChecker.java:152)
	at com.google.devtools.build.skyframe.state.TaskTreeNode.run(TaskTreeNode.java:95)
	at com.google.devtools.build.skyframe.state.Driver.drive(Driver.java:87)
	at com.google.devtools.build.lib.skyframe.PrerequisiteProducer.checkForIncompatibleTarget(PrerequisiteProducer.java:494)
	at com.google.devtools.build.lib.skyframe.PrerequisiteProducer.evaluate(PrerequisiteProducer.java:342)
	at com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.compute(ConfiguredTargetFunction.java:200)
	at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:501)
	... 7 more
------------------------------------------------------------------------
test_missing_default FAILED: Expected regexp ERROR: Target //target_skipping:pass_on_foo1_or_foo2_but_not_on_foo3 is incompatible and cannot be built, but was explicitly requested not found.
/usr/local/google/home/jcater/.cache/bazel/_bazel_jcater/a85a0dacd9e20e18c47928db0f1530e0/sandbox/linux-sandbox/1268/execroot/io_bazel/bazel-out/k8-fastbuild/bin/src/test/shell/integration/target_compatible_with_test.runfiles/io_bazel/src/test/shell/integration/target_compatible_with_test:917: in call to test_missing_default

Tear down:

$TEST_TMPDIR defined: output root default is '/usr/local/google/home/jcater/.cache/bazel/_bazel_jcater/a85a0dacd9e20e18c47928db0f1530e0/sandbox/linux-sandbox/1268/execroot/io_bazel/_tmp/4a28868e660c8b58f7795306ee6f829b' and max_idle_secs default is '15'.
WARNING: The following rc files are no longer being read, please transfer their contents or import their path into one of the standard rc files:
/etc/bazel.bazelrc
INFO[target_compatible_with_test 2023-04-10 20:14:16 (+0000)] Cleaning up workspace

FAILED: test_missing_default

** 0 / 1 tests passed. *********************************************************
** There were errors. **********************************************************
$TEST_TMPDIR defined: output root default is '/usr/local/google/home/jcater/.cache/bazel/_bazel_jcater/a85a0dacd9e20e18c47928db0f1530e0/sandbox/linux-sandbox/1268/execroot/io_bazel/_tmp/4a28868e660c8b58f7795306ee6f829b' and max_idle_secs default is '15'.
WARNING: The following rc files are no longer being read, please transfer their contents or import their path into one of the standard rc files:
/etc/bazel.bazelrc
================================================================================
Target //src/test/shell/integration:target_compatible_with_test up-to-date:
  bazel-bin/src/test/shell/integration/target_compatible_with_test
INFO: Elapsed time: 12.387s, Critical Path: 10.68s
INFO: 2 processes: 1 internal, 1 linux-sandbox.
INFO: Build completed, 1 test FAILED, 2 total actions
//src/test/shell/integration:target_compatible_with_test                 FAILED in 10.3s
    ERROR   .test_missing_default (9.6s)
Test cases: finished with 0 passing and 1 failing out of 1 test cases

Executed 1 out of 1 test: 1 fails locally.

Which operating system are you running Bazel on?

Linux

What is the output of bazel info release?

release 6.1.1

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

No response

[katre]

The question is: is this an error (and users should add the default condition), or do we treat a missing default as "incompatible y default"?

Paging @gregestren and @philsc for their input

[fmeum]

Sideline opinion: I would find it weird if non-matching selects are an error in certain cases but not in others. I don't expect users to be aware of the fact that it's really the rule (or even the attribute) that determines the exact semantics of select.

+1 for failing with a descriptive error message.

[philsc]

I agree with @fmeum . I don't think having select() behave differently just for target_compatible_with is a good user experience.

Users should specify a default condition.

Sorry about the crash. I'm assuming this is a side effect of my refactor in #14096. If I get my other stuff done, I can take a look at fixing it later this week.

[gregestren]

Agreed. Thanks @philsc .

[katre]

Thanks!

The problem stems from the call to ConfiguredAttributeMapper.get, which silently converts the ValidationException (from getAndValidate) into an IllegalArgumentException.

I am most of the way through tunneling the error back out, I'll see if I can clean that up and send a review. The biggest problem isn't in IncompatibleTargetChecker, it's funneling the exception back out to PrerequisiteProducer and then handling it appropriately so that the full error is displayed.

[brentleyjones]

Does this apply to 6.x as well, and if so can we cherry-pick it?

[katre]

Vesion matrix:

• 5.4.0: Shows error
• 6.0.0: Crash
• 6.1.1: Crash

We can try to cherrypick it, I'm not sure it'll apply cleanly, but we should give it a go.

@bazel-io flag

[ShreeM01]

@bazel-io fork 6.2.0

[keertk]

Hi @katre I tried cherry picking this but ran into some issues. It seems like another commit likely needs to be included too?
Can you please send a PR against the release-6.2.0 branch for this if we want to include it? Thanks!

[katre]

There are too many changes due to splitting out PrerequisiteProducer from ConfiguredTargetFunction. I could conceivably make a similar change directly to the version of IncompatibleTargetChecker in the release-6.2.0 branch, but that's a pretty bit change for a point release. Otherwise we'll have to leave this in and make sure users know that the fix is to correctly add a default condition for selects on target_compatible_with.

[katre]

See #18135