Refactor Incompatible Target Skipping

This patch reworks how Incompatible Target Skipping is implemented.

The biggest change is that incompatibility is now checked earlier. This
allows us to avoid evaluating dependencies (such as toolchains) in
situations where a target is "directly" incompatible. "Direct incompatibility"
refers to incompatibility due to a target's `target_compatible_with` value.

Moving the incompatibility checking earlier had the undesired effect of
visibility restrictions being ignored on incompatible targets. This is
tracked in #16044. As per
#14096 (comment),
we will fix it in a separate patch.

Fixes #12897.

Closes #14096.

PiperOrigin-RevId: 466407887
Change-Id: I3014390ddb95c7ff6bfaaf497a32b60c8a6e8fc9

site/en/docs/platforms.md

@@ -245,3 +245,8 @@ def format(target):
 
 $ bazel cquery //... --output=starlark --starlark:file=example.cquery
 ```
+
+### Known Issues
+
+Incompatible targets [ignore visibility
+restrictions](https://github.com/bazelbuild/bazel/issues/16044).

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

@@ -384,7 +384,6 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity",
         "//src/main/java/com/google/devtools/build/lib/actions:package_roots",
         "//src/main/java/com/google/devtools/build/lib/analysis/platform",
-        "//src/main/java/com/google/devtools/build/lib/analysis/platform:utils",
         "//src/main/java/com/google/devtools/build/lib/analysis/starlark/annotations",
         "//src/main/java/com/google/devtools/build/lib/analysis/stringtemplate",
         "//src/main/java/com/google/devtools/build/lib/bugreport",
@@ -2225,6 +2224,39 @@ java_library(
     ],
 )
 
+java_library(
+    name = "constraints/incompatible_target_checker",
+    srcs = ["constraints/IncompatibleTargetChecker.java"],
+    deps = [
+        ":analysis_cluster",
+        ":file_provider",
+        ":incompatible_platform_provider",
+        ":test/test_configuration",
+        ":transitive_info_provider_map_builder",
+        "//src/main/java/com/google/devtools/build/lib/actions:artifacts",
+        "//src/main/java/com/google/devtools/build/lib/analysis:config/build_configuration",
+        "//src/main/java/com/google/devtools/build/lib/analysis:config/config_conditions",
+        "//src/main/java/com/google/devtools/build/lib/analysis:configured_target",
+        "//src/main/java/com/google/devtools/build/lib/analysis:configured_target_value",
+        "//src/main/java/com/google/devtools/build/lib/analysis:dependency",
+        "//src/main/java/com/google/devtools/build/lib/analysis:dependency_kind",
+        "//src/main/java/com/google/devtools/build/lib/analysis:target_and_configuration",
+        "//src/main/java/com/google/devtools/build/lib/analysis/platform",
+        "//src/main/java/com/google/devtools/build/lib/analysis/platform:utils",
+        "//src/main/java/com/google/devtools/build/lib/cmdline",
+        "//src/main/java/com/google/devtools/build/lib/collect/nestedset",
+        "//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/skyframe:configured_target_and_data",
+        "//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_key",
+        "//src/main/java/com/google/devtools/build/lib/skyframe:rule_configured_target_value",
+        "//src/main/java/com/google/devtools/build/lib/util",
+        "//src/main/java/com/google/devtools/build/skyframe",
+        "//third_party:guava",
+        "//third_party:jsr305",
+    ],
+)
+
 # TODO(b/144899336): This should be analysis/starlark/BUILD
 java_library(
     name = "starlark/args",

src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java

@@ -403,8 +403,8 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env)
           .add(
               attr("distribs", DISTRIBUTIONS)
                   .nonconfigurable("Used in core loading phase logic with no access to configs"))
-          // Any rule that has provides its own meaning for the "target_compatible_with" attribute
-          // has to be excluded in `RuleContextConstraintSemantics.incompatibleConfiguredTarget()`.
+          // Any rule that provides its own meaning for the "target_compatible_with" attribute
+          // has to be excluded in `IncompatibleTargetChecker`.
           .add(
               attr(RuleClass.TARGET_COMPATIBLE_WITH_ATTR, LABEL_LIST)
                   .mandatoryProviders(ConstraintValueInfo.PROVIDER.id())

src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java

@@ -38,7 +38,6 @@
 import com.google.devtools.build.lib.analysis.configuredtargets.OutputFileConfiguredTarget;
 import com.google.devtools.build.lib.analysis.configuredtargets.PackageGroupConfiguredTarget;
 import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
-import com.google.devtools.build.lib.analysis.constraints.RuleContextConstraintSemantics;
 import com.google.devtools.build.lib.analysis.starlark.StarlarkRuleConfiguredTargetUtil;
 import com.google.devtools.build.lib.analysis.test.AnalysisFailure;
 import com.google.devtools.build.lib.analysis.test.AnalysisFailureInfo;
@@ -325,12 +324,6 @@ private ConfiguredTarget createRule(
       return erroredConfiguredTarget(ruleContext);
     }
 
-    ConfiguredTarget incompatibleTarget =
-        RuleContextConstraintSemantics.incompatibleConfiguredTarget(ruleContext, prerequisiteMap);
-    if (incompatibleTarget != null) {
-      return incompatibleTarget;
-    }
-
     try {
       Class<?> missingFragmentClass = null;
       for (Class<? extends Fragment> fragmentClass :

src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java

@@ -53,7 +53,6 @@
 import com.google.devtools.build.lib.analysis.config.FragmentCollection;
 import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransition;
 import com.google.devtools.build.lib.analysis.config.transitions.NoTransition;
-import com.google.devtools.build.lib.analysis.constraints.RuleContextConstraintSemantics;
 import com.google.devtools.build.lib.analysis.platform.ConstraintValueInfo;
 import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
 import com.google.devtools.build.lib.analysis.starlark.StarlarkRuleContext;
@@ -2180,14 +2179,6 @@ private static boolean checkRuleDependencyMandatoryProviders(
 
     private void validateDirectPrerequisite(
         Attribute attribute, ConfiguredTargetAndData prerequisite) {
-      if (RuleContextConstraintSemantics.checkForIncompatibility(prerequisite.getConfiguredTarget())
-          .isIncompatible()) {
-        // If the prerequisite is incompatible (e.g. has an incompatible provider), we pretend that
-        // there is no further validation needed. Otherwise, it would be difficult to make the
-        // incompatible target satisfy things like required providers and file extensions.
-        return;
-      }
-
       validateDirectPrerequisiteType(prerequisite, attribute);
       validateDirectPrerequisiteFileTypes(prerequisite, attribute);
       if (attribute.performPrereqValidatorCheck()) {

src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java

@@ -448,22 +448,6 @@ public static RunfilesSupport withExecutable(
         computeActionEnvironment(ruleContext));
   }
 
-  /**
-   * Creates and returns a {@link RunfilesSupport} object for the given rule and executable. This
-   * version discards all arguments. Only use this for <a
-   * href="https://bazel.build/docs/platforms#skipping-incompatible-targets">Incompatible Target
-   * Skipping</a>.
-   */
-  public static RunfilesSupport withExecutableButNoArgs(
-      RuleContext ruleContext, Runfiles runfiles, Artifact executable) {
-    return RunfilesSupport.create(
-        ruleContext,
-        executable,
-        runfiles,
-        CommandLine.EMPTY,
-        computeActionEnvironment(ruleContext));
-  }
-
   private static CommandLine computeArgs(RuleContext ruleContext, CommandLine additionalArgs) {
     if (!ruleContext.getRule().isAttrDefined("args", Type.STRING_LIST)) {
       // Some non-_binary rules create RunfilesSupport instances; it is fine to not have an args

src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/RuleConfiguredTarget.java

@@ -23,6 +23,7 @@
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.FileProvider;
 import com.google.devtools.build.lib.analysis.FilesToRunProvider;
+import com.google.devtools.build.lib.analysis.IncompatiblePlatformProvider;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.RunfilesProvider;
 import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
@@ -155,6 +156,28 @@ public RuleConfiguredTarget(
     }
   }
 
+  /** Use this constructor for creating incompatible ConfiguredTarget instances. */
+  public RuleConfiguredTarget(
+      Label label,
+      BuildConfigurationKey configurationKey,
+      NestedSet<PackageGroupContents> visibility,
+      TransitiveInfoProviderMap providers,
+      ImmutableMap<Label, ConfigMatchingProvider> configConditions,
+      String ruleClassString) {
+    this(
+        label,
+        configurationKey,
+        visibility,
+        providers,
+        configConditions,
+        ImmutableSet.<ConfiguredTargetKey>of(),
+        ruleClassString,
+        ImmutableList.<ActionAnalysisMetadata>of(),
+        ImmutableMap.<Label, Artifact>of());
+
+    Preconditions.checkState(providers.get(IncompatiblePlatformProvider.PROVIDER) != null, label);
+  }
+
   /** The configuration conditions that trigger this rule's configurable attributes. */
   @Override
   public ImmutableMap<Label, ConfigMatchingProvider> getConfigConditions() {