Add attribute validation to IncompatibleTargetChecker.

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

Fixes bazelbuild#18021.

Closes bazelbuild#18027.

PiperOrigin-RevId: 523976899
Change-Id: I2602aa3d4febc4c486d610e19c3a61632b0519b5

src/main/java/com/google/devtools/build/lib/analysis/constraints/IncompatibleTargetChecker.java

@@ -46,6 +46,7 @@
 import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.packages.BuildType;
 import com.google.devtools.build.lib.packages.ConfiguredAttributeMapper;
+import com.google.devtools.build.lib.packages.ConfiguredAttributeMapper.ValidationException;
 import com.google.devtools.build.lib.packages.Package;
 import com.google.devtools.build.lib.packages.PackageSpecification;
 import com.google.devtools.build.lib.packages.PackageSpecification.PackageGroupContents;
@@ -55,8 +56,13 @@
 import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
 import com.google.devtools.build.lib.skyframe.RuleConfiguredTargetValue;
 import com.google.devtools.build.lib.util.OrderedSetMultimap;
+<<<<<<< HEAD
 import com.google.devtools.build.skyframe.SkyFunction.Environment;
 import com.google.devtools.build.skyframe.SkyframeLookupResult;
+=======
+import com.google.devtools.build.skyframe.SkyValue;
+import com.google.devtools.build.skyframe.state.StateMachine;
+>>>>>>> 73d937d71a (Add attribute validation to IncompatibleTargetChecker.)
 import java.util.List;
 import java.util.Optional;
 import javax.annotation.Nullable;
@@ -111,8 +117,29 @@ public static Optional<RuleConfiguredTargetValue> createDirectlyIncompatibleTarg
     Target target = targetAndConfiguration.getTarget();
     Rule rule = target.getAssociatedRule();
 
+<<<<<<< HEAD
     if (rule == null || rule.getRuleClass().equals("toolchain") || platformInfo == null) {
       return Optional.empty();
+=======
+    private final Target target;
+    @Nullable // Non-null when the target has an associated rule.
+    private final BuildConfigurationValue configuration;
+    private final ConfigConditions configConditions;
+    // Non-null when the target has an associated rule and does not opt out of toolchain resolution.
+    @Nullable private final PlatformInfo platformInfo;
+    @Nullable private final NestedSetBuilder<Package> transitivePackages;
+
+    private final ResultSink sink;
+
+    private final ImmutableList.Builder<ConstraintValueInfo> invalidConstraintValuesBuilder =
+        new ImmutableList.Builder<>();
+
+    /** Sink for the output of this state machine. */
+    public interface ResultSink {
+      void accept(Optional<RuleConfiguredTargetValue> incompatibleTarget);
+
+      void acceptValidationException(ValidationException e);
+>>>>>>> 73d937d71a (Add attribute validation to IncompatibleTargetChecker.)
     }
 
     // Retrieve the label list for the target_compatible_with attribute.
@@ -123,6 +150,7 @@ public static Optional<RuleConfiguredTargetValue> createDirectlyIncompatibleTarg
       return Optional.empty();
     }
 
+<<<<<<< HEAD
     // Resolve the constraint labels.
     List<Label> labels = attrs.get("target_compatible_with", BuildType.LABEL_LIST);
     ImmutableList<ConfiguredTargetKey> constraintKeys =
@@ -138,6 +166,38 @@ public static Optional<RuleConfiguredTargetValue> createDirectlyIncompatibleTarg
     SkyframeLookupResult constraintValues = env.getValuesAndExceptions(constraintKeys);
     if (env.valuesMissing()) {
       return null;
+=======
+    @Override
+    public StateMachine step(Tasks tasks, ExtendedEventHandler listener) {
+      Rule rule = target.getAssociatedRule();
+      if (rule == null || !rule.useToolchainResolution() || platformInfo == null) {
+        sink.accept(Optional.empty());
+        return DONE;
+      }
+
+      // Retrieves the label list for the target_compatible_with attribute.
+      ConfiguredAttributeMapper attrs =
+          ConfiguredAttributeMapper.of(rule, configConditions.asProviders(), configuration);
+      if (!attrs.has("target_compatible_with", BuildType.LABEL_LIST)) {
+        sink.accept(Optional.empty());
+        return DONE;
+      }
+
+      // Resolves the constraint labels, checking for invalid configured attributes.
+      List<Label> targetCompatibleWith;
+      try {
+        targetCompatibleWith = attrs.getAndValidate("target_compatible_with", BuildType.LABEL_LIST);
+      } catch (ValidationException e) {
+        sink.acceptValidationException(e);
+        return DONE;
+      }
+      for (Label label : targetCompatibleWith) {
+        tasks.lookUp(
+            ConfiguredTargetKey.builder().setLabel(label).setConfiguration(configuration).build(),
+            this);
+      }
+      return this::processResult;
+>>>>>>> 73d937d71a (Add attribute validation to IncompatibleTargetChecker.)
     }
 
     // Find the constraints that don't satisfy the target platform.

src/main/java/com/google/devtools/build/lib/analysis/constraints/TopLevelConstraintSemantics.java

@@ -299,7 +299,7 @@ private static String reportOnIncompatibility(ConfiguredTarget target) {
     String message = "\nDependency chain:";
     IncompatiblePlatformProvider provider = null;
 
-    // TODO(austinschuh): While the first eror is helpful, reporting all the errors at once would
+    // TODO(austinschuh): While the first error is helpful, reporting all the errors at once would
     // save the user bazel round trips.
     while (target != null) {
       message +=

src/main/java/com/google/devtools/build/lib/packages/ConfiguredAttributeMapper.java

@@ -127,7 +127,7 @@ private ValidationException(String message) {
    * Variation of {@link #get} that throws an informative exception if the attribute can't be
    * resolved due to intrinsic contradictions in the configuration.
    */
-  private <T> T getAndValidate(String attributeName, Type<T> type) throws ValidationException {
+  public <T> T getAndValidate(String attributeName, Type<T> type) throws ValidationException {
     SelectorList<T> selectorList = getSelectorList(attributeName, type);
     if (selectorList == null) {
       // This is a normal attribute.
@@ -295,8 +295,8 @@ public <T> T get(String attributeName, Type<T> type) {
       return getAndValidate(attributeName, type);
     } catch (ValidationException e) {
       // Callers that reach this branch should explicitly validate the attribute through an
-      // appropriate call and handle the exception directly. This method assumes
-      // pre-validated attributes.
+      // appropriate call (either {@link #validateAttributes} or {@link #getAndValidate}) and handle
+      // the exception directly. This method assumes pre-validated attributes.
       throw new IllegalStateException(
           "lookup failed on attribute " + attributeName + ": " + e.getMessage());
     }

src/main/java/com/google/devtools/build/lib/skyframe/BUILD

@@ -307,6 +307,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/io:inconsistent_filesystem_exception",
         "//src/main/java/com/google/devtools/build/lib/io:process_package_directory_exception",
         "//src/main/java/com/google/devtools/build/lib/packages",
+        "//src/main/java/com/google/devtools/build/lib/packages:configured_attribute_mapper",
         "//src/main/java/com/google/devtools/build/lib/packages:exec_group",
         "//src/main/java/com/google/devtools/build/lib/packages:globber",
         "//src/main/java/com/google/devtools/build/lib/packages:globber_utils",