Add attribute validation to IncompatibleTargetChecker.

[katre]

This causes errors with configurable target_compatible_with to be reported as errors, rather than causing a Bazel crash.

Fixes #18021.

[katre]

@philsc: Can you review the changes to incompatible target checker? Specifically I'm not sure about the error: we might want to try and give a better error message here (something like "target_compatible_with should always have a default"?)

[gregestren]

Noted #18027 (comment): deferring to @philsc .

[philsc]

@philsc: Can you review the changes to incompatible target checker? Specifically I'm not sure about the error: we might want to try and give a better error message here (something like "target_compatible_with should always have a default"?)

From your PR here, it looks to me like the error is:

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?

That sounds like a good error message to me, but I understand where you're coming from. On the one hand, I like that it's the same error message as all the other select() errors. It's the same kind of error, after all. On the other hand, I agree that it probably doesn't make much sense to have a select() without a default. Telling the user as much sounds reasonable.

I don't have a strong enough opinion on this, apologies. I think the current error message is fine, but if you want to add a more specific one, then that sounds good to me too.

[katre]

So, for this target:

genrule(
    name = "foo",
    srcs = ["foo.in"],
    outs = ["foo.out"],
    cmd = "cp $< $@",
    target_compatible_with = select({
        "@platforms//cpu:armv7": [],
    }),
)

the error is

ERROR: /usr/local/google/home/jcater/repos/demo/BUILD:3:8: configurable attribute "target_compatible_with" in //:foo doesn't match this configuration. Would a default condition help?

Conditions checked:
 @platforms//cpu:armv7

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

This instance of //:foo has configuration identifier 3c37c8e. To inspect its configuration, run: bazel config 3c37c8e.

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

WARNING: errors encountered while analyzing target '//:foo': it will not be built
INFO: Analyzed target //:foo (1 packages loaded, 1 target configured).
INFO: Found 0 targets...
ERROR: command succeeded, but not all targets were analyzed
INFO: Elapsed time: 0.144s, Critical Path: 0.01s
INFO: 1 process: 1 internal.
ERROR: Build did NOT complete successfully

[philsc]

That looks good to me 👍

[katre]

I'm merging this myself.