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. (This is functionally the same change as ebfb529, but applied to the release-6.2.0 branch)
katre · Apr 18, 2023 · 4218f87 · 4218f87
1 parent 062b14d
commit 4218f87
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 10 deletions.
diff --git a/...in/java/com/google/devtools/build/lib/analysis/constraints/IncompatibleTargetChecker.java b/...in/java/com/google/devtools/build/lib/analysis/constraints/IncompatibleTargetChecker.java
@@ -107,7 +107,7 @@ public static Optional<RuleConfiguredTargetValue> createDirectlyIncompatibleTarg
       Environment env,
       @Nullable PlatformInfo platformInfo,
       NestedSetBuilder<Package> transitivePackages)
-      throws InterruptedException {
+      throws ConfiguredAttributeMapper.ValidationException, InterruptedException {
     Target target = targetAndConfiguration.getTarget();
     Rule rule = target.getAssociatedRule();
 
@@ -124,7 +124,7 @@ public static Optional<RuleConfiguredTargetValue> createDirectlyIncompatibleTarg
     }
 
     // Resolve the constraint labels.
-    List<Label> labels = attrs.get("target_compatible_with", BuildType.LABEL_LIST);
+    List<Label> labels = attrs.getAndValidate("target_compatible_with", BuildType.LABEL_LIST);
     ImmutableList<ConfiguredTargetKey> constraintKeys =
         labels.stream()
             .map(

diff --git a/src/main/java/com/google/devtools/build/lib/packages/ConfiguredAttributeMapper.java b/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.

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/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",

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
@@ -71,6 +71,7 @@
 import com.google.devtools.build.lib.events.StoredEventHandler;
 import com.google.devtools.build.lib.packages.Aspect;
 import com.google.devtools.build.lib.packages.BuildType;
+import com.google.devtools.build.lib.packages.ConfiguredAttributeMapper;
 import com.google.devtools.build.lib.packages.ExecGroup;
 import com.google.devtools.build.lib.packages.NoSuchTargetException;
 import com.google.devtools.build.lib.packages.NonconfigurableAttributeMapper;
@@ -108,6 +109,7 @@
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
 import javax.annotation.Nullable;
+import net.starlark.java.syntax.Location;
 
 /**
  * SkyFunction for {@link ConfiguredTargetValue}s.
@@ -335,13 +337,28 @@ public SkyValue compute(SkyKey key, Environment env)
                 getPrioritizedDetailedExitCode(causes)));
       }
 
-      Optional<RuleConfiguredTargetValue> incompatibleTarget =
-          IncompatibleTargetChecker.createDirectlyIncompatibleTarget(
-              targetAndConfiguration,
-              configConditions,
-              env,
-              platformInfo,
-              state.transitivePackages);
+      Optional<RuleConfiguredTargetValue> incompatibleTarget;
+      try {
+        incompatibleTarget =
+            IncompatibleTargetChecker.createDirectlyIncompatibleTarget(
+                targetAndConfiguration,
+                configConditions,
+                env,
+                platformInfo,
+                state.transitivePackages);
+      } catch (ConfiguredAttributeMapper.ValidationException e) {
+        BuildConfigurationValue configuration = targetAndConfiguration.getConfiguration();
+        Label label = targetAndConfiguration.getLabel();
+        Location location = targetAndConfiguration.getTarget().getLocation();
+        env.getListener().post(new AnalysisRootCauseEvent(configuration, label, e.getMessage()));
+        throw new DependencyEvaluationException(
+            new ConfiguredValueCreationException(
+                location, e.getMessage(), label, configuration.getEventId(), null, null),
+            // These errors occur within DependencyResolver, which is attached to the current
+            // target. i.e. no dependent ConfiguredTargetFunction call happens to report its own
+            // error.
+            /* depReportedOwnError= */ false);
+      }
       if (incompatibleTarget == null) {
         return null;
       }