From 4a6f40d8ced5b2fc1555a1536b0eb4d8570faacc Mon Sep 17 00:00:00 2001
From: Philipp Schrader <philipp.schrader@gmail.com>
Date: Tue, 10 Mar 2020 21:23:40 -0700
Subject: [PATCH] Add basic incompatible target skipping

This patch aims to implement a basic version of incompatible target
skipping outlined here:
https://docs.google.com/document/d/12n5QNHmFSkuh5yAbdEex64ot4hRgR-moL1zRimU7wHQ/edit?usp=sharing

The implementation in this patch supports only "top level" target
skipping. In other words we only filter out targets if they are
globbed as part of an invocation like "bazel build //...".

The following features are not yet implemented:
- Filtering targets based on transitive dependencies. For example, if
  //a:b depends on //c:d an invocation of "bazel build //a/..." will
  still build //c:d even if it is incompatible with the current
  platform.
- Being able to select() on constraint values. For example, it's not
  possible to select() based on whether or not GCC is being used.
  Discussion of this aspect of the proposal is ongoing:
  https://groups.google.com/d/msg/bazel-dev/xK7diubpljQ/s3KSRwTWAQAJ

The goal is to implement the missing features in follow-up patches.

Austin Schuh (austin.linux@gmail.com) provided me with the majority of
the logic in TopLevelConstraintSemantics.java. I added the remaining
error reporting logic and the tests.

The patch largely re-uses the existing support for skipping targets
based on incompatible CPU constraints. There are a few enhancements to
the error reporting so that it's a little more obvious on the command
line when a target was skipped. To a lesser extent the Build Event
Stream should also contain less misleading information now. I did have
to introduce the notion of to-be-failed targets determined during the
analysis phase. For now these are targets that are incompatible with
the current platform, but were explicitly requested on the command
line. The list of targets ("targetsToFail" in AnalysisResult) is used
to affect the outcome of the build. I.e. if the list is non-empty,
then the build is considered to have failed.
---
 .../build/lib/analysis/AnalysisResult.java    |  12 +
 .../build/lib/analysis/BaseRuleClasses.java   |  10 +
 .../build/lib/analysis/BuildView.java         |  43 ++-
 .../TopLevelConstraintSemantics.java          | 102 +++++-
 .../proto/build_event_stream.proto            |   1 +
 .../lib/buildtool/AnalysisPhaseRunner.java    |  25 +-
 .../lib/buildtool/BuildResultPrinter.java     |  15 +-
 .../build/lib/buildtool/ExecutionTool.java    |  21 +-
 .../build/lib/packages/RuleClass.java         |   4 +
 .../lib/rules/platform/ToolchainRule.java     |   9 -
 .../lib/runtime/AggregatingTestListener.java  |  22 +-
 .../lib/runtime/BuildEventStreamerUtils.java  |   2 +
 .../runtime/TerminalTestResultNotifier.java   |   5 +-
 .../lib/runtime/TestResultAggregator.java     |  18 +-
 .../build/lib/runtime/UiEventHandler.java     |   3 +-
 src/main/protobuf/test_status.proto           |   1 +
 src/test/shell/bazel/BUILD                    |   7 +
 .../bazel/target_compatible_with_test.sh      | 299 ++++++++++++++++++
 18 files changed, 557 insertions(+), 42 deletions(-)
 create mode 100755 src/test/shell/bazel/target_compatible_with_test.sh

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisResult.java b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisResult.java
index c4aedce9ab7525..e7ecc37766aa2c 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisResult.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisResult.java
@@ -33,6 +33,7 @@ public final class AnalysisResult {
   private final ImmutableSet<ConfiguredTarget> targetsToBuild;
   @Nullable private final ImmutableList<ConfiguredTarget> targetsToTest;
   private final ImmutableSet<ConfiguredTarget> targetsToSkip;
+  private final ImmutableSet<ConfiguredTarget> targetsToFail;
   @Nullable private final String error;
   private final ActionGraph actionGraph;
   private final ArtifactsToOwnerLabels topLevelArtifactsToOwnerLabels;
@@ -51,6 +52,7 @@ public final class AnalysisResult {
       ImmutableSet<AspectValue> aspects,
       @Nullable ImmutableList<ConfiguredTarget> targetsToTest,
       ImmutableSet<ConfiguredTarget> targetsToSkip,
+      ImmutableSet<ConfiguredTarget> targetsToFail,
       @Nullable String error,
       ActionGraph actionGraph,
       ArtifactsToOwnerLabels topLevelArtifactsToOwnerLabels,
@@ -66,6 +68,7 @@ public final class AnalysisResult {
     this.aspects = aspects;
     this.targetsToTest = targetsToTest;
     this.targetsToSkip = targetsToSkip;
+    this.targetsToFail = targetsToFail;
     this.error = error;
     this.actionGraph = actionGraph;
     this.topLevelArtifactsToOwnerLabels = topLevelArtifactsToOwnerLabels;
@@ -123,6 +126,14 @@ public ImmutableSet<ConfiguredTarget> getTargetsToSkip() {
     return targetsToSkip;
   }
 
+  /**
+   * Returns the incompatible targets that were explicitly requested on the command line.
+   */
+  // TODO(phil): Document this properly.
+  public ImmutableSet<ConfiguredTarget> getTargetsToFail() {
+    return targetsToFail;
+  }
+
   public ArtifactsToOwnerLabels getTopLevelArtifactsToOwnerLabels() {
     return topLevelArtifactsToOwnerLabels;
   }
@@ -180,6 +191,7 @@ public AnalysisResult withExclusiveTestsAsParallelTests() {
         aspects,
         targetsToTest,
         targetsToSkip,
+        targetsToFail,
         error,
         actionGraph,
         topLevelArtifactsToOwnerLabels,
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java
index b7ffed1e55052a..af8ea819321ab5 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java
@@ -34,6 +34,7 @@
 import com.google.devtools.build.lib.analysis.config.HostTransition;
 import com.google.devtools.build.lib.analysis.config.RunUnder;
 import com.google.devtools.build.lib.analysis.constraints.EnvironmentRule;
+import com.google.devtools.build.lib.analysis.platform.ConstraintValueInfo;
 import com.google.devtools.build.lib.analysis.test.TestConfiguration;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.packages.Attribute;
@@ -329,6 +330,15 @@ public static RuleClass.Builder commonCoreAndSkylarkAttributes(RuleClass.Builder
                 .dontCheckConstraints()
                 .nonconfigurable(
                     "special logic for constraints and select: see ConstraintSemantics"))
+        /* <!-- #BLAZE_RULE(toolchain).ATTRIBUTE(target_compatible_with) -->
+        A list of <code>constraint_value</code>s that must be satisfied by the target platform in
+        order for this toolchain to be selected for a target building for that platform.
+        <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
+        .add(
+            attr("target_compatible_with", LABEL_LIST)
+                .mandatoryProviders(ConstraintValueInfo.PROVIDER.id())
+                .allowedFileTypes()
+                .nonconfigurable("Logic for target compatibility"))
         .add(
             attr(RuleClass.CONFIG_SETTING_DEPS_ATTRIBUTE, LABEL_LIST)
                 .nonconfigurable("stores configurability keys"))
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
index f2ff1980590576..1f025322f6350e 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
@@ -206,6 +206,7 @@ public AnalysisResult update(
       TargetPatternPhaseValue loadingResult,
       BuildOptions targetOptions,
       Set<String> multiCpu,
+      List<String> requestedTargets,
       List<String> aspects,
       AnalysisOptions viewOptions,
       boolean keepGoing,
@@ -388,18 +389,18 @@ public AnalysisResult update(
 
     getArtifactFactory().noteAnalysisStarting();
     SkyframeAnalysisResult skyframeAnalysisResult;
-    try {
-      Supplier<Map<BuildConfigurationValue.Key, BuildConfiguration>> configurationLookupSupplier =
-          () -> {
-            Map<BuildConfigurationValue.Key, BuildConfiguration> result = new HashMap<>();
-            for (TargetAndConfiguration node : topLevelTargetsWithConfigs) {
-              if (node.getConfiguration() != null) {
-                result.put(
-                    BuildConfigurationValue.key(node.getConfiguration()), node.getConfiguration());
-              }
+    Supplier<Map<BuildConfigurationValue.Key, BuildConfiguration>> configurationLookupSupplier =
+        () -> {
+          Map<BuildConfigurationValue.Key, BuildConfiguration> result = new HashMap<>();
+          for (TargetAndConfiguration node : topLevelTargetsWithConfigs) {
+            if (node.getConfiguration() != null) {
+              result.put(
+                  BuildConfigurationValue.key(node.getConfiguration()), node.getConfiguration());
             }
-            return result;
-          };
+          }
+          return result;
+        };
+    try {
       skyframeAnalysisResult =
           skyframeBuildView.configureTargets(
               eventHandler,
@@ -425,10 +426,16 @@ public AnalysisResult update(
 
     Set<ConfiguredTarget> targetsToSkip =
         new TopLevelConstraintSemantics(
+                skyframeBuildView,
                 skyframeExecutor.getPackageManager(),
                 input -> skyframeExecutor.getConfiguration(eventHandler, input),
                 eventHandler)
-            .checkTargetEnvironmentRestrictions(skyframeAnalysisResult.getConfiguredTargets());
+            .checkTargetEnvironmentRestrictions(
+                configurationLookupSupplier,
+                keepGoing,
+                loadingPhaseThreads,
+                eventBus,
+                skyframeAnalysisResult.getConfiguredTargets());
 
     AnalysisResult result =
         createResult(
@@ -440,6 +447,7 @@ public AnalysisResult update(
             viewOptions,
             skyframeAnalysisResult,
             targetsToSkip,
+            requestedTargets,
             topLevelTargetsWithConfigsResult);
     logger.atInfo().log("Finished analysis");
     return result;
@@ -454,6 +462,7 @@ private AnalysisResult createResult(
       AnalysisOptions viewOptions,
       SkyframeAnalysisResult skyframeAnalysisResult,
       Set<ConfiguredTarget> targetsToSkip,
+      List<String> requestedTargets,
       TopLevelTargetsAndConfigsResult topLevelTargetsWithConfigs)
       throws InterruptedException {
     Set<Label> testsToRun = loadingResult.getTestsToRunLabels();
@@ -461,6 +470,15 @@ private AnalysisResult createResult(
         Sets.newLinkedHashSet(skyframeAnalysisResult.getConfiguredTargets());
     ImmutableSet<AspectValue> aspects = ImmutableSet.copyOf(skyframeAnalysisResult.getAspects());
 
+    // Targets that should be skipped and were requested on the command line
+    // will be marked as failed.
+    ImmutableSet.Builder<ConfiguredTarget> failedTargets = ImmutableSet.builder();
+    for (ConfiguredTarget topLevelTarget : targetsToSkip) {
+      if (requestedTargets.contains(topLevelTarget.getLabel().toString())) {
+        failedTargets.add(topLevelTarget);
+      }
+    }
+
     Set<ConfiguredTarget> allTargetsToTest = null;
     if (testsToRun != null) {
       // Determine the subset of configured targets that are meant to be run as tests.
@@ -545,6 +563,7 @@ public ActionAnalysisMetadata getGeneratingAction(Artifact artifact) {
         aspects,
         allTargetsToTest == null ? null : ImmutableList.copyOf(allTargetsToTest),
         ImmutableSet.copyOf(targetsToSkip),
+        ImmutableSet.copyOf(failedTargets.build()),
         error,
         actionGraph,
         artifactsToOwnerLabelsBuilder.build(),
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/constraints/TopLevelConstraintSemantics.java b/src/main/java/com/google/devtools/build/lib/analysis/constraints/TopLevelConstraintSemantics.java
index c9b89bf1ed5483..433e773b6441cc 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/constraints/TopLevelConstraintSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/constraints/TopLevelConstraintSemantics.java
@@ -14,18 +14,26 @@
 
 package com.google.devtools.build.lib.analysis.constraints;
 
+import com.google.common.base.Preconditions;
 import com.google.common.base.Predicates;
+import com.google.common.base.Supplier;
+import com.google.common.base.Suppliers;
 import com.google.common.base.Verify;
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Multimap;
+import com.google.common.eventbus.EventBus;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
+import com.google.devtools.build.lib.analysis.PlatformConfiguration;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.analysis.ViewCreationFailedException;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.configuredtargets.OutputFileConfiguredTarget;
 import com.google.devtools.build.lib.analysis.constraints.SupportedEnvironmentsProvider.RemovedEnvironmentCulprit;
+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.platform.PlatformProviderUtils;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
@@ -35,7 +43,11 @@
 import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.pkgcache.PackageManager;
 import com.google.devtools.build.lib.rules.AliasConfiguredTarget;
-import com.google.devtools.build.lib.skyframe.BuildConfigurationValue.Key;
+import com.google.devtools.build.lib.skyframe.AspectValue.AspectValueKey;
+import com.google.devtools.build.lib.skyframe.BuildConfigurationValue;
+import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
+import com.google.devtools.build.lib.skyframe.SkyframeAnalysisResult;
+import com.google.devtools.build.lib.skyframe.SkyframeBuildView;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
@@ -55,8 +67,9 @@
  */
 public class TopLevelConstraintSemantics {
   private final PackageManager packageManager;
-  private final Function<Key, BuildConfiguration> configurationProvider;
+  private final Function<BuildConfigurationValue.Key, BuildConfiguration> configurationProvider;
   private final ExtendedEventHandler eventHandler;
+  private final SkyframeBuildView skyframeBuildView;
 
   /**
    * Constructor with helper classes for loading targets.
@@ -65,9 +78,11 @@ public class TopLevelConstraintSemantics {
    * @param eventHandler the build's event handler
    */
   public TopLevelConstraintSemantics(
+      SkyframeBuildView skyframeBuildView,
       PackageManager packageManager,
-      Function<Key, BuildConfiguration> configurationProvider,
+      Function<BuildConfigurationValue.Key, BuildConfiguration> configurationProvider,
       ExtendedEventHandler eventHandler) {
+    this.skyframeBuildView = skyframeBuildView;
     this.packageManager = packageManager;
     this.configurationProvider = configurationProvider;
     this.eventHandler = eventHandler;
@@ -105,6 +120,10 @@ private MissingEnvironment(Label environment, RemovedEnvironmentCulprit culprit)
    *     environment declared through {@link CoreOptions#targetEnvironments}
    */
   public Set<ConfiguredTarget> checkTargetEnvironmentRestrictions(
+      Supplier<Map<BuildConfigurationValue.Key, BuildConfiguration>> configurationLookupSupplier,
+      boolean keepGoing,
+      int loadingPhaseThreads,
+      EventBus eventBus,
       ImmutableList<ConfiguredTarget> topLevelTargets)
       throws ViewCreationFailedException, InterruptedException {
     ImmutableSet.Builder<ConfiguredTarget> badTargets = ImmutableSet.builder();
@@ -136,6 +155,83 @@ public Set<ConfiguredTarget> checkTargetEnvironmentRestrictions(
         continue; // This target doesn't participate in constraints.
       }
 
+      Object target_compatible_with_object =
+          target.getAssociatedRule().getAttributeContainer().getAttr("target_compatible_with");
+
+      // Get the target platform key.
+      ConfiguredTargetKey targetPlatformKey =
+          ConfiguredTargetKey.of(
+              Preconditions.checkNotNull(config.getFragment(PlatformConfiguration.class))
+                  .getTargetPlatform(),
+              config);
+
+      ImmutableList.Builder<ConfiguredTargetKey> builder = ImmutableList.builder();
+
+      builder.add(targetPlatformKey);
+      if (target_compatible_with_object instanceof ImmutableList) {
+        ImmutableList<Label> target_compatible_with_list =
+            (ImmutableList<Label>) target_compatible_with_object;
+
+        for (Label label : target_compatible_with_list) {
+          Target constraintTarget = null;
+          try {
+            constraintTarget = packageManager.getTarget(eventHandler, label);
+          } catch (NoSuchPackageException | NoSuchTargetException e) {
+            eventHandler.handle(
+                Event.error(
+                    "Unable to get target from package when checking environment restrictions. "
+                        + e));
+            continue;
+          }
+
+          builder.add(ConfiguredTargetKey.of(label, config));
+        }
+      }
+
+      SkyframeAnalysisResult skyframeAnalysisResult;
+
+      ImmutableList<ConfiguredTargetKey> targets = builder.build();
+      try {
+        skyframeAnalysisResult =
+            skyframeBuildView.configureTargets(
+                eventHandler,
+                targets,
+                ImmutableList.<AspectValueKey>of(),
+                Suppliers.memoize(configurationLookupSupplier),
+                eventBus,
+                keepGoing,
+                loadingPhaseThreads);
+      } finally {
+        skyframeBuildView.clearInvalidatedConfiguredTargets();
+      }
+
+      if (skyframeAnalysisResult.getConfiguredTargets().size() != targets.size()) {
+        throw new ViewCreationFailedException("found wrong number of targets");
+      }
+
+      // Grab the platform info.
+      ConfiguredTarget platformTarget = skyframeAnalysisResult.getConfiguredTargets().get(0);
+      PlatformInfo platformInfo = PlatformProviderUtils.platform(platformTarget);
+      if (platformInfo == null) {
+        throw new ViewCreationFailedException("Couldn't find platform info");
+      }
+
+      boolean targetIsBad = false;
+      for (int i = 1; i < targets.size(); ++i) {
+        ConstraintValueInfo constraintValue =
+            PlatformProviderUtils.constraintValue(
+                skyframeAnalysisResult.getConfiguredTargets().get(i));
+        if (!platformInfo.constraints().containsAll(ImmutableList.<ConstraintValueInfo>of(constraintValue))) {
+          // Violated constraint, skipping this target.
+          targetIsBad = true;
+          break;
+        }
+      }
+      if (targetIsBad) {
+        badTargets.add(topLevelTarget);
+        continue;
+      }
+
       // Check explicitly expected environments.
       exceptionInducingTargets.putAll(topLevelTarget, // This is a no-op on empty collections.
           getMissingEnvironments(topLevelTarget, config.getTargetEnvironments()));
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto b/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto
index 8e901fb2ece3b5..4842681e9bf591 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto
+++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto
@@ -534,6 +534,7 @@ enum TestStatus {
   REMOTE_FAILURE = 6;
   FAILED_TO_BUILD = 7;
   TOOL_HALTED_BEFORE_TESTING = 8;
+  SKIPPED = 9;
 }
 
 // Payload on events reporting about individual test action.
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java b/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java
index 248193e95259a9..f070a943081c50 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java
@@ -140,13 +140,23 @@ public static AnalysisResult execute(
             env.getSkyframeExecutor()
                 .getConfiguration(env.getReporter(), target.getConfigurationKey());
         Label label = target.getLabel();
-        env.getEventBus()
-            .post(
-                new AbortedEvent(
-                    BuildEventId.targetCompleted(label, config.getEventId()),
-                    AbortReason.SKIPPED,
-                    String.format("Target %s build was skipped.", label),
-                    label));
+        if (analysisResult.getTargetsToFail().contains(target)) {
+          env.getEventBus()
+              .post(
+                  new AbortedEvent(
+                      BuildEventId.targetCompleted(label, config.getEventId()),
+                      AbortReason.ANALYSIS_FAILURE,
+                      String.format("Target %s cannot be built for this platform.", label),
+                      label));
+        } else {
+          env.getEventBus()
+              .post(
+                  new AbortedEvent(
+                      BuildEventId.targetCompleted(label, config.getEventId()),
+                      AbortReason.SKIPPED,
+                      String.format("Target %s build was skipped.", label),
+                      label));
+        }
       }
     } else {
       env.getReporter().handle(Event.progress("Loading complete."));
@@ -214,6 +224,7 @@ private static AnalysisResult runAnalysisPhase(
             loadingResult,
             targetOptions,
             multiCpu,
+            request.getTargets(),
             request.getAspects(),
             request.getViewOptions(),
             request.getKeepGoing(),
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildResultPrinter.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildResultPrinter.java
index da05c9dc4fa5a4..a44a7911db45e4 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildResultPrinter.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildResultPrinter.java
@@ -58,6 +58,7 @@ public void showBuildResult(
       BuildResult result,
       Collection<ConfiguredTarget> configuredTargets,
       Collection<ConfiguredTarget> configuredTargetsToSkip,
+      Collection<ConfiguredTarget> configuredTargetsToFail,
       Collection<AspectValue> aspects) {
     // NOTE: be careful what you print!  We don't want to create a consistency
     // problem where the summary message and the exit code disagree.  The logic
@@ -83,16 +84,22 @@ public void showBuildResult(
       if (targetsToPrint.size() > request.getBuildOptions().maxResultTargets) {
         return;
       }
-      // Filter the targets we care about into two buckets:
+      // Filter the targets we care about into three buckets:
       Collection<ConfiguredTarget> succeeded = new ArrayList<>();
       Collection<ConfiguredTarget> failed = new ArrayList<>();
+      Collection<ConfiguredTarget> skipped = new ArrayList<>();
       Collection<ConfiguredTarget> successfulTargets = result.getSuccessfulTargets();
       for (ConfiguredTarget target : targetsToPrint) {
-        (successfulTargets.contains(target) ? succeeded : failed).add(target);
+        if (configuredTargetsToSkip.contains(target) && !configuredTargetsToFail.contains(target)) {
+          skipped.add(target);
+        } else {
+          (successfulTargets.contains(target) ? succeeded : failed).add(target);
+        }
       }
 
-      // TODO(bazel-team): convert these to a new "SKIPPED" status when ready: b/62191890.
-      failed.addAll(configuredTargetsToSkip);
+      for (ConfiguredTarget target : skipped) {
+        outErr.printErr("Target " + target.getLabel() + " was skipped\n");
+      }
 
       TopLevelArtifactContext context = request.getTopLevelArtifactContext();
       for (ConfiguredTarget target : succeeded) {
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
index e4ad3c1db56ab3..697f7d4724812e 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
@@ -340,6 +340,9 @@ void executeBuild(
           env.getBlazeWorkspace().getLastExecutionTimeRange(),
           topLevelArtifactContext);
       buildCompleted = true;
+      if (analysisResult.getTargetsToFail().size() > 0) {
+        throw new BuildFailedException("Some incompatible targets were explicitly requested");
+      }
     } catch (BuildFailedException | TestExecException e) {
       buildCompleted = true;
       throw e;
@@ -382,13 +385,15 @@ void executeBuild(
         buildResult.setSuccessfulTargets(
             determineSuccessfulTargets(configuredTargets, builtTargets));
         buildResult.setSuccessfulAspects(determineSuccessfulAspects(aspects, builtAspects));
-        buildResult.setSkippedTargets(analysisResult.getTargetsToSkip());
+        buildResult.setSkippedTargets(
+            determineSkippedTargets(analysisResult.getTargetsToSkip(), analysisResult.getTargetsToFail()));
         BuildResultPrinter buildResultPrinter = new BuildResultPrinter(env);
         buildResultPrinter.showBuildResult(
             request,
             buildResult,
             configuredTargets,
             analysisResult.getTargetsToSkip(),
+            analysisResult.getTargetsToFail(),
             analysisResult.getAspects());
       }
 
@@ -407,9 +412,12 @@ void executeBuild(
           // Ignored
         }
       }
+
       // Finalize output service last, so that if we do throw an exception, we know all the other
       // code has already run.
       if (env.getOutputService() != null) {
+        // TODO(phil): Make this logic deal with skipped targets also.
+        // Especially when incompatible targets are explicitly requested.
         boolean isBuildSuccessful =
             buildResult.getSuccessfulTargets().size() == configuredTargets.size();
         env.getOutputService().finalizeBuild(isBuildSuccessful);
@@ -665,6 +673,17 @@ private Collection<AspectValue> determineSuccessfulAspects(
     return successfulAspects;
   }
 
+  private Collection<ConfiguredTarget> determineSkippedTargets(
+      Collection<ConfiguredTarget> targetsToSkip, Collection<ConfiguredTarget> targetsToFail) {
+    ImmutableSet.Builder<ConfiguredTarget> skippedTargets = ImmutableSet.builder();
+    for (ConfiguredTarget target : targetsToSkip) {
+      if (!targetsToFail.contains(target)) {
+        skippedTargets.add(target);
+      }
+    }
+    return ImmutableSet.copyOf(skippedTargets.build());
+  }
+
   /** Get action cache if present or reload it from the on-disk cache. */
   private ActionCache getActionCache() throws LocalEnvironmentException {
     try {
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
index 0d9156c07fcc9f..915f4accf3c7b4 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
@@ -239,6 +239,8 @@ final class RuleErrorException extends Exception {
    */
   public static final String RESTRICTED_ENVIRONMENT_ATTR = "restricted_to";
 
+  public static final String TARGET_RESTRICTED_TO_ATTR = "target_restricted_to";
+
   /**
    * For Bazel's constraint system: the attribute that declares the set of environments a rule
    * supports, appending them to the defaults for their respective groups.
@@ -1308,6 +1310,8 @@ public <TYPE> Builder exemptFromConstraintChecking(String reason) {
       this.supportsConstraintChecking = false;
       attributes.remove(RuleClass.COMPATIBLE_ENVIRONMENT_ATTR);
       attributes.remove(RuleClass.RESTRICTED_ENVIRONMENT_ATTR);
+      // TODO(austin): Should this remove target_compatible_with too?
+      attributes.remove(RuleClass.TARGET_RESTRICTED_TO_ATTR);
       return this;
     }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/platform/ToolchainRule.java b/src/main/java/com/google/devtools/build/lib/rules/platform/ToolchainRule.java
index a7c6e7f9a0b81d..e5758d1ba58062 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/platform/ToolchainRule.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/platform/ToolchainRule.java
@@ -69,15 +69,6 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env)
                 .mandatoryProviders(ConstraintValueInfo.PROVIDER.id())
                 .allowedFileTypes()
                 .nonconfigurable("part of toolchain configuration"))
-        /* <!-- #BLAZE_RULE(toolchain).ATTRIBUTE(target_compatible_with) -->
-        A list of <code>constraint_value</code>s that must be satisfied by the target platform in
-        order for this toolchain to be selected for a target building for that platform.
-        <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
-        .add(
-            attr(TARGET_COMPATIBLE_WITH_ATTR, BuildType.LABEL_LIST)
-                .mandatoryProviders(ConstraintValueInfo.PROVIDER.id())
-                .allowedFileTypes()
-                .nonconfigurable("part of toolchain configuration"))
         /* <!-- #BLAZE_RULE(toolchain).ATTRIBUTE(toolchain) -->
         The target representing the actual tool or tool suite that is made available when this
         toolchain is selected.
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/AggregatingTestListener.java b/src/main/java/com/google/devtools/build/lib/runtime/AggregatingTestListener.java
index c5a06fea413838..3af353a2be5ba1 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/AggregatingTestListener.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/AggregatingTestListener.java
@@ -137,21 +137,37 @@ private void targetFailure(ConfiguredTargetKey configuredTargetKey) {
     }
   }
 
+  private void targetSkipped(ConfiguredTargetKey configuredTargetKey) {
+    TestResultAggregator aggregator = aggregators.get(configuredTargetKey);
+    if (aggregator != null) {
+      aggregator.targetSkipped();
+    }
+  }
+
   @VisibleForTesting
   void buildComplete(
-      Collection<ConfiguredTarget> actualTargets, Collection<ConfiguredTarget> successfulTargets) {
+      Collection<ConfiguredTarget> actualTargets, Collection<ConfiguredTarget> skippedTargets, Collection<ConfiguredTarget> successfulTargets) {
     if (actualTargets == null || successfulTargets == null) {
       return;
     }
 
+    ImmutableSet<ConfiguredTarget> nonSuccessfulTargets = Sets.difference(
+        ImmutableSet.copyOf(actualTargets), ImmutableSet.copyOf(successfulTargets)).immutableCopy();
     for (ConfiguredTarget target :
         Sets.difference(
-            ImmutableSet.copyOf(actualTargets), ImmutableSet.copyOf(successfulTargets))) {
+            nonSuccessfulTargets, ImmutableSet.copyOf(skippedTargets))) {
       if (isAlias(target)) {
         continue;
       }
       targetFailure(asKey(target));
     }
+
+    for (ConfiguredTarget target : skippedTargets) {
+      if (isAlias(target)) {
+        continue;
+      }
+      targetSkipped(asKey(target));
+    }
   }
 
   @Subscribe
@@ -161,7 +177,7 @@ public void buildCompleteEvent(BuildCompleteEvent event) {
       blazeHalted = true;
     }
     skipTargetsOnFailure = result.getStopOnFirstFailure();
-    buildComplete(result.getActualTargets(), result.getSuccessfulTargets());
+    buildComplete(result.getActualTargets(), result.getSkippedTargets(), result.getSuccessfulTargets());
   }
 
   @Subscribe
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BuildEventStreamerUtils.java b/src/main/java/com/google/devtools/build/lib/runtime/BuildEventStreamerUtils.java
index fc0f161e5a145c..8b15084bb843e1 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BuildEventStreamerUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BuildEventStreamerUtils.java
@@ -45,6 +45,8 @@ public static TestStatus bepStatus(BlazeTestStatus status) {
         return BuildEventStreamProtos.TestStatus.REMOTE_FAILURE;
       case BLAZE_HALTED_BEFORE_TESTING:
         return BuildEventStreamProtos.TestStatus.TOOL_HALTED_BEFORE_TESTING;
+      case SKIPPED:
+        return BuildEventStreamProtos.TestStatus.SKIPPED;
       default:
         // Not used as the above is a complete case distinction; however, by the open
         // nature of protobuf enums, we need the clause to convice java, that we always
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/TerminalTestResultNotifier.java b/src/main/java/com/google/devtools/build/lib/runtime/TerminalTestResultNotifier.java
index 56d195371ea803..63161cf1995be4 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/TerminalTestResultNotifier.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/TerminalTestResultNotifier.java
@@ -56,6 +56,7 @@ private static class TestResultStats {
     int failedRemotelyCount;
     int failedLocallyCount;
     int noStatusCount;
+    int skippedCount;
     int numberOfExecutedTargets;
     boolean wasUnreportedWrongSize;
 
@@ -213,6 +214,8 @@ public void notify(Set<TestSummary> summaries, int numberOfExecutedTargets) {
         stats.passCount++;
       } else if (summary.getStatus() == BlazeTestStatus.NO_STATUS) {
         stats.noStatusCount++;
+      } else if (summary.getStatus() == BlazeTestStatus.SKIPPED) {
+        stats.skippedCount++;
       } else if (summary.getStatus() == BlazeTestStatus.FAILED_TO_BUILD) {
         stats.failedToBuildCount++;
       } else if (summary.ranRemotely()) {
@@ -311,7 +314,7 @@ private void printStats(TestResultStats stats) {
       addFailureToErrorList(results, "to build", stats.failedToBuildCount);
       addFailureToErrorList(results, "locally", stats.failedLocallyCount);
       addFailureToErrorList(results, "remotely", stats.failedRemotelyCount);
-      addToErrorList(results, "was", "were", "skipped", stats.noStatusCount);
+      addToErrorList(results, "was", "were", "skipped", stats.noStatusCount + stats.skippedCount);
       printer.print(String.format("\nExecuted %d out of %d %s: %s.\n",
               stats.numberOfExecutedTargets,
               stats.numberOfTargets,
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/TestResultAggregator.java b/src/main/java/com/google/devtools/build/lib/runtime/TestResultAggregator.java
index 55f9b778abc89f..356352194ae9bb 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/TestResultAggregator.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/TestResultAggregator.java
@@ -147,6 +147,21 @@ synchronized void targetFailure(boolean blazeHalted, boolean skipTargetsOnFailur
     policy.eventBus.post(summary.build());
   }
 
+  synchronized void targetSkipped() {
+    if (remainingRuns.isEmpty()) {
+      // Blaze does not guarantee that BuildResult.getSuccessfulTargets() and posted TestResult
+      // events are in sync. Thus, it is possible that a test event was posted, but the target is
+      // not present in the set of successful targets.
+      return;
+    }
+
+    summary.setStatus(BlazeTestStatus.SKIPPED);
+
+    // These are never going to run; removing them marks the target complete.
+    remainingRuns.clear();
+    policy.eventBus.post(summary.build());
+  }
+
   /** Returns the known aggregate results for the given target at the current moment. */
   synchronized TestSummary.Builder getCurrentSummaryForTesting() {
     return summary;
@@ -339,7 +354,8 @@ private void markUnbuilt(boolean blazeHalted, boolean skipTargetsOnFailure) {
             : (policy.testCheckUpToDate || skipTargetsOnFailure
                 ? BlazeTestStatus.NO_STATUS
                 : BlazeTestStatus.FAILED_TO_BUILD);
-
+    // TODO(phil): Explicitly requested incompatible targets are given
+    // NO_STATUS here. Should this be FAILED_TO_BUILD instead?
     summary.setStatus(runStatus);
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/UiEventHandler.java b/src/main/java/com/google/devtools/build/lib/runtime/UiEventHandler.java
index db315d7229e29e..7a78d1eb2a08e4 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/UiEventHandler.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/UiEventHandler.java
@@ -855,7 +855,8 @@ private boolean testSummaryProvidesNewInformation(TestSummary summary) {
             BlazeTestStatus.PASSED,
             BlazeTestStatus.FAILED_TO_BUILD,
             BlazeTestStatus.BLAZE_HALTED_BEFORE_TESTING,
-            BlazeTestStatus.NO_STATUS);
+            BlazeTestStatus.NO_STATUS,
+            BlazeTestStatus.SKIPPED);
 
     if (statusToIgnore.contains(summary.getStatus())) {
       return false;
diff --git a/src/main/protobuf/test_status.proto b/src/main/protobuf/test_status.proto
index 72cb268607f0c0..95f9a2775aea63 100644
--- a/src/main/protobuf/test_status.proto
+++ b/src/main/protobuf/test_status.proto
@@ -41,6 +41,7 @@ enum BlazeTestStatus {
   REMOTE_FAILURE = 6;
   FAILED_TO_BUILD = 7;
   BLAZE_HALTED_BEFORE_TESTING = 8;
+  SKIPPED = 9;
 };
 
 // TestCase contains detailed data about all tests (cases/suites/decorators)
diff --git a/src/test/shell/bazel/BUILD b/src/test/shell/bazel/BUILD
index 12448cd1b7d387..548a43db76d9dc 100644
--- a/src/test/shell/bazel/BUILD
+++ b/src/test/shell/bazel/BUILD
@@ -1185,6 +1185,13 @@ sh_test(
     tags = ["no_windows"],
 )
 
+sh_test(
+    name = "target_compatible_with_test",
+    srcs = ["target_compatible_with_test.sh"],
+    data = [":test-deps"],
+    deps = ["@bazel_tools//tools/bash/runfiles"],
+)
+
 sh_test(
     name = "resource_compiler_toolchain_test",
     srcs = ["resource_compiler_toolchain_test.sh"],
diff --git a/src/test/shell/bazel/target_compatible_with_test.sh b/src/test/shell/bazel/target_compatible_with_test.sh
new file mode 100755
index 00000000000000..794ffb816b02dc
--- /dev/null
+++ b/src/test/shell/bazel/target_compatible_with_test.sh
@@ -0,0 +1,299 @@
+#!/bin/bash
+
+# --- begin runfiles.bash initialization ---
+set -euo pipefail
+if [[ ! -d "${RUNFILES_DIR:-/dev/null}" && ! -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then
+  if [[ -f "$0.runfiles_manifest" ]]; then
+    export RUNFILES_MANIFEST_FILE="$0.runfiles_manifest"
+  elif [[ -f "$0.runfiles/MANIFEST" ]]; then
+    export RUNFILES_MANIFEST_FILE="$0.runfiles/MANIFEST"
+  elif [[ -f "$0.runfiles/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then
+    export RUNFILES_DIR="$0.runfiles"
+  fi
+fi
+if [[ -f "${RUNFILES_DIR:-/dev/null}/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then
+  source "${RUNFILES_DIR}/bazel_tools/tools/bash/runfiles/runfiles.bash"
+elif [[ -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then
+  source "$(grep -m1 "^bazel_tools/tools/bash/runfiles/runfiles.bash " \
+            "$RUNFILES_MANIFEST_FILE" | cut -d ' ' -f 2-)"
+else
+  echo >&2 "ERROR: cannot find @bazel_tools//tools/bash/runfiles:runfiles.bash"
+  exit 1
+fi
+# --- end runfiles.bash initialization ---
+
+source "$(rlocation "io_bazel/src/test/shell/integration_test_setup.sh")" \
+  || { echo "integration_test_setup.sh not found!" >&2; exit 1; }
+
+# `uname` returns the current platform, e.g "MSYS_NT-10.0" or "Linux".
+# `tr` converts all upper case letters to lower case.
+# `case` matches the result if the `uname | tr` expression to string prefixes
+# that use the same wildcards as names do in Bash, i.e. "msys*" matches strings
+# starting with "msys", and "*" matches everything (it's the default case).
+case "$(uname -s | tr [:upper:] [:lower:])" in
+msys*)
+  # As of 2019-01-15, Bazel on Windows only supports MSYS Bash.
+  declare -r is_windows=true
+  ;;
+*)
+  declare -r is_windows=false
+  ;;
+esac
+
+function set_up() {
+  mkdir -p target_skipping || fail "couldn't create directory"
+  cat <<EOT > target_skipping/pass.sh || fail "couldn't create pass.sh"
+#!/bin/bash
+exit 0
+EOT
+  chmod +x target_skipping/pass.sh
+
+  cat <<EOT > target_skipping/fail.sh || fail "couldn't create fail.sh"
+#!/bin/bash
+exit 1
+EOT
+  chmod +x target_skipping/fail.sh
+
+  cat <<EOT > target_skipping/BUILD || fail "couldn't create BUILD file"
+constraint_setting(name = "foo_version")
+
+constraint_value(
+    name = "foo1",
+    constraint_setting = ":foo_version",
+)
+
+constraint_value(
+    name = "foo2",
+    constraint_setting = ":foo_version",
+)
+
+constraint_value(
+    name = "foo3",
+    constraint_setting = ":foo_version",
+)
+
+constraint_setting(name = "bar_version")
+
+constraint_value(
+    name = "bar1",
+    constraint_setting = "bar_version",
+)
+
+constraint_value(
+    name = "bar2",
+    constraint_setting = "bar_version",
+)
+
+# TODO(phil): Use more generic constraints for os and cpu.
+platform(
+    name = "foo1_bar1_platform",
+    constraint_values = [
+        "@platforms//os:linux",
+        "@platforms//cpu:x86_64",
+        ":foo1",
+        ":bar1",
+    ],
+)
+
+platform(
+    name = "foo2_bar1_platform",
+    constraint_values = [
+        "@platforms//os:linux",
+        "@platforms//cpu:x86_64",
+        ":foo2",
+        ":bar1",
+    ],
+)
+
+platform(
+    name = "foo1_bar2_platform",
+    constraint_values = [
+        "@platforms//os:linux",
+        "@platforms//cpu:x86_64",
+        ":foo1",
+        ":bar2",
+    ],
+)
+
+platform(
+    name = "foo3_platform",
+    constraint_values = [
+        "@platforms//os:linux",
+        "@platforms//cpu:x86_64",
+        ":foo3",
+    ],
+)
+
+sh_test(
+    name = "pass_on_foo1",
+    srcs = [":pass.sh"],
+    target_compatible_with = [":foo1"],
+)
+
+sh_test(
+    name = "fail_on_foo2",
+    srcs = [":fail.sh"],
+    target_compatible_with = [":foo2"],
+)
+
+sh_test(
+    name = "pass_on_foo1_bar2",
+    srcs = [":pass.sh"],
+    target_compatible_with = [
+        ":foo1",
+        ":bar2",
+    ],
+)
+
+sh_binary(
+    name = "some_foo3_target",
+    srcs = [":pass.sh"],
+    target_compatible_with = [
+        ":foo3",
+    ],
+)
+EOT
+
+  cat <<EOT > target_skipping/WORKSPACE || fail "couldn't create WORKSPACE"
+EOT
+}
+
+function test_console_log_for_builds() {
+  cd target_skipping || fail "couldn't cd into workspace"
+
+  if ! bazel build --show_result=10 --host_platform=@//:foo3_platform \
+    --platforms=@//:foo3_platform ... &> "${TEST_log}"; then
+    cat "${TEST_log}"
+    fail "Bazel failed the test."
+  fi
+
+  expect_log 'Target //:some_foo3_target up-to-date'
+  expect_log 'Target //:fail_on_foo2 was skipped'
+  expect_log 'Target //:pass_on_foo1 was skipped'
+  expect_log 'Target //:pass_on_foo1_bar2 was skipped'
+  expect_not_log 'Target //:fail_on_foo2 failed to build'
+  expect_not_log 'Target //:pass_on_foo1 failed to build'
+  expect_not_log 'Target //:pass_on_foo1_bar2 failed to build'
+  expect_not_log 'Target //:fail_on_foo2 up-to-date'
+  expect_not_log 'Target //:pass_on_foo1 up-to-date'
+  expect_not_log 'Target //:pass_on_foo1_bar2 up-to-date'
+}
+
+function test_console_log_for_tests() {
+  cd target_skipping || fail "couldn't cd into workspace"
+
+  # Get repeatable results from this test.
+  bazel clean --expunge
+
+  if ! bazel test --host_platform=@//:foo1_bar1_platform \
+    --platforms=@//:foo1_bar1_platform ... &> "${TEST_log}"; then
+    cat "${TEST_log}"
+    fail "Bazel failed unexpectedly."
+  fi
+  expect_log 'Executed 1 out of 3 tests: 1 test passes and 2 were skipped'
+  expect_log '^//:pass_on_foo1  *  PASSED in'
+  expect_log '^//:fail_on_foo2  *  SKIPPED$'
+  expect_log '^//:pass_on_foo1_bar2  *  SKIPPED$'
+
+  if bazel test --host_platform=@//:foo2_bar1_platform \
+    --platforms=@//:foo2_bar1_platform ... &> "${TEST_log}"; then
+    cat "${TEST_log}"
+    fail "Bazel passed unexpectedly."
+  fi
+  expect_log 'Executed 1 out of 3 tests: 1 fails locally and 2 were skipped.'
+  expect_log '^//:pass_on_foo1  *  SKIPPED$'
+  expect_log '^//:fail_on_foo2  *  FAILED in'
+  expect_log '^//:pass_on_foo1_bar2  *  SKIPPED$'
+
+  # Use :all for this one to validate similar behaviour.
+  if ! bazel test --host_platform=@//:foo1_bar2_platform \
+    --platforms=@//:foo1_bar2_platform :all &> "${TEST_log}"; then
+    cat "${TEST_log}"
+    fail "Bazel failed unexpectedly."
+  fi
+  expect_log 'Executed 2 out of 3 tests: 2 tests pass and 1 was skipped'
+  expect_log '^//:pass_on_foo1  *  PASSED in'
+  expect_log '^//:fail_on_foo2  *  SKIPPED$'
+  expect_log '^//:pass_on_foo1_bar2  *  PASSED in'
+}
+
+function test_failure_on_incompatible_top_level_target() {
+  cd target_skipping || fail "couldn't cd into workspace"
+
+  if bazel test --show_result=10 --host_platform=@//:foo1_bar1_platform \
+    --platforms=@//:foo1_bar1_platform //:pass_on_foo1_bar2 \
+    --build_event_text_file="${TEST_log}".build.json \
+    |& tee "${TEST_log}"; then
+    fail "Bazel passed unexpectedly."
+  fi
+
+  expect_log '^Target //:pass_on_foo1_bar2 failed to build'
+  expect_log '^ERROR: Some incompatible targets were explicitly requested$'
+  expect_log '^FAILED: Build did NOT complete successfully'
+  expect_log '^//:pass_on_foo1_bar2  *  NO STATUS$'
+
+  # Now look at the build event log.
+  mv "${TEST_log}".build.json "${TEST_log}"
+
+  expect_log "^  reason: ANALYSIS_FAILURE$"
+  expect_log "Target //:pass_on_foo1_bar2 cannot be built for this platform."
+
+  # Run an additional (passing) test and make sure we still fail the build.
+  if bazel test --show_result=10 --host_platform=@//:foo1_bar1_platform \
+    --platforms=@//:foo1_bar1_platform //:pass_on_foo1_bar2 //:pass_on_foo1 \
+    |& tee "${TEST_log}"; then
+    fail "Bazel passed unexpectedly."
+  fi
+
+  expect_log '^//:pass_on_foo1  *  PASSED in'
+  expect_log '^ERROR: Some incompatible targets were explicitly requested$'
+  expect_log '^FAILED: Build did NOT complete successfully'
+}
+
+function test_build_event_protocol() {
+  cd target_skipping || fail "couldn't cd into workspace"
+
+  if ! bazel test --show_result=10 --host_platform=@//:foo1_bar1_platform \
+    --build_event_text_file=$TEST_log \
+    --platforms=@//:foo1_bar1_platform ...; then
+    cat "${TEST_log}"
+    fail "Bazel failed unexpectedly."
+  fi
+  expect_not_log 'Target //:pass_on_foo1 build was skipped\.'
+  expect_log_n 'reason: SKIPPED\>' 3
+  expect_log 'Target //:fail_on_foo2 build was skipped\.'
+  expect_log 'Target //:pass_on_foo1_bar2 build was skipped\.'
+}
+
+function test_non_top_level_skipping() {
+  # TODO(phil): We still build dependencies even if they're incompatible. This
+  # needs to be fixed in a future patch.
+  cat >> target_skipping/BUILD <<EOT
+genrule(
+    name = "genrule_foo1",
+    target_compatible_with = [":foo1"],
+    outs = ["foo1.sh"],
+    cmd = "echo 'Should not be executed' &>2; exit 1",
+)
+
+sh_binary(
+    name = "sh_foo2",
+    srcs = ["foo1.sh"],
+    target_compatible_with = [":foo2"],
+)
+EOT
+
+  cd target_skipping || fail "couldn't cd into workspace"
+
+  if bazel build --show_result=10 --host_platform=@//:foo2_bar1_platform \
+    --platforms=@//:foo2_bar1_platform //:sh_foo2 &> "${TEST_log}"; then
+    cat "${TEST_log}"
+    fail "Bazel passed unexpectedly."
+  fi
+  expect_log 'FAILED: Build did NOT complete successfully'
+  # TODO(phil): We should not see the this message at all. The build should be
+  # skipped altogether.
+  expect_log 'Should not be executed'
+}
+
+run_suite "target_compatible_with tests"