Fix bazel crash when passing config_setting to target_compatible_with

[philsc]

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

[philsc]

@gregestren , @katre , FYI

[katre]

Looks good, I'll merge this today.

[aaron-michaux]

The documentation says:

target_settings  |  List of labels ; optional
A list of config_settings that must be satisfied by the target configuration in order for this toolchain to be selected during toolchain resolution.

But it should say config_values?????

[katre]

No, the target_settings attribute requires values of config_setting, not constraint_values. They are used differently and have different semantics.