Always run toolchain resolution, even when no toolchain types are requested, in order to properly choose the execution platform from the available execution platforms.

Change-Id: I05dc84403e0db765865e9b91c4222894fa867cd9
PiperOrigin-RevId: 200211635

1 files changed, 121 insertions, 39 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainUtil.java b/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainUtil.java
index 39149f54e4..b5f363158a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainUtil.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainUtil.java
@@ -49,6 +49,7 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.stream.Collectors;
 import javax.annotation.Nullable;
 
 /**
@@ -59,8 +60,19 @@ import javax.annotation.Nullable;
 public class ToolchainUtil {
 
   /**
-   * Returns a new {@link ToolchainContext}, with the correct toolchain labels based on the results
-   * of the {@link ToolchainResolutionFunction}.
+   * Returns a new {@link ToolchainContext}, containing:
+   *
+   * <ul>
+   *   <li>If {@code requiredToolchains} was non-empty, the resolved toolchains and execution
+   *       platform (as labels), based on the results of the {@link ToolchainResolutionFunction}
+   *   <li>If {@code requiredToolchains} was empty:
+   *       <ul>
+   *         <li>The resolved toolchains will be empty.
+   *         <li>The execution platform will be the host platform, if the host platform was in the
+   *             set of available execution platforms.
+   *         <li>Otherwise, the execution platform will be the first available execution platform.
+   *       </ul>
+   * </ul>
    *
    * @param env the Skyframe environment to use to acquire dependencies
    * @param targetDescription a description of the target use, for error and debug message context
@@ -126,42 +138,33 @@ public class ToolchainUtil {
             .build();
 
     // Filter out execution platforms that don't satisfy the extra constraints.
+    boolean debug = configuration.getOptions().get(PlatformOptions.class).toolchainResolutionDebug;
     availableExecutionPlatformKeys =
-        filterPlatforms(availableExecutionPlatformKeys, execConstraintKeys, env);
+        filterPlatforms(availableExecutionPlatformKeys, execConstraintKeys, env, debug);
     if (availableExecutionPlatformKeys == null) {
       return null;
     }
 
-    Optional<ResolvedToolchains> resolvedToolchains =
+    ResolvedToolchains resolvedToolchains =
         resolveToolchainLabels(
             env,
             requiredToolchains,
-            configuration,
             configurationKey,
+            hostPlatformKey,
             availableExecutionPlatformKeys,
-            targetPlatformKey);
+            targetPlatformKey,
+            debug);
     if (resolvedToolchains == null) {
       return null;
     }
 
-    if (resolvedToolchains.isPresent()) {
-      return createContext(
-          env,
-          targetDescription,
-          resolvedToolchains.get().executionPlatformKey(),
-          resolvedToolchains.get().targetPlatformKey(),
-          requiredToolchains,
-          resolvedToolchains.get().toolchains());
-    } else {
-      // No toolchain could be resolved, but no error happened, so fall back to host platform.
-      return createContext(
-          env,
-          targetDescription,
-          hostPlatformKey,
-          targetPlatformKey,
-          requiredToolchains,
-          ImmutableBiMap.of());
-    }
+    return createContext(
+        env,
+        targetDescription,
+        resolvedToolchains.executionPlatformKey(),
+        resolvedToolchains.targetPlatformKey(),
+        requiredToolchains,
+        resolvedToolchains.toolchains());
   }
 
   private static RegisteredExecutionPlatformsValue loadRegisteredExecutionPlatforms(
@@ -253,20 +256,16 @@ public class ToolchainUtil {
   }
 
   @Nullable
-  private static Optional<ResolvedToolchains> resolveToolchainLabels(
+  private static ResolvedToolchains resolveToolchainLabels(
       Environment env,
       Set<Label> requiredToolchains,
-      BuildConfiguration configuration,
       BuildConfigurationValue.Key configurationKey,
+      ConfiguredTargetKey hostPlatformKey,
       ImmutableList<ConfiguredTargetKey> availableExecutionPlatformKeys,
-      ConfiguredTargetKey targetPlatformKey)
+      ConfiguredTargetKey targetPlatformKey,
+      boolean debug)
       throws InterruptedException, ToolchainContextException {
 
-    // If there are no required toolchains, bail out early.
-    if (requiredToolchains.isEmpty()) {
-      return Optional.absent();
-    }
-
     // Find the toolchains for the required toolchain types.
     List<ToolchainResolutionValue.Key> registeredToolchainKeys = new ArrayList<>();
     for (Label toolchainType : requiredToolchains) {
@@ -333,9 +332,37 @@ public class ToolchainUtil {
       return null;
     }
 
-    boolean debug = configuration.getOptions().get(PlatformOptions.class).toolchainResolutionDebug;
-
     // Find and return the first execution platform which has all required toolchains.
+    Optional<ConfiguredTargetKey> selectedExecutionPlatformKey;
+    if (requiredToolchains.isEmpty() && availableExecutionPlatformKeys.contains(hostPlatformKey)) {
+      // Fall back to the legacy behavior: use the host platform if it's available, otherwise the
+      // first execution platform.
+      selectedExecutionPlatformKey = Optional.of(hostPlatformKey);
+    } else {
+      // If there are no toolchains, this will return the first execution platform.
+      selectedExecutionPlatformKey =
+          findExecutionPlatformForToolchains(
+              env, requiredToolchains, availableExecutionPlatformKeys, resolvedToolchains, debug);
+    }
+
+    if (!selectedExecutionPlatformKey.isPresent()) {
+      throw new ToolchainContextException(
+          new NoMatchingPlatformException(
+              requiredToolchains, availableExecutionPlatformKeys, targetPlatformKey));
+    }
+
+    return ResolvedToolchains.create(
+        selectedExecutionPlatformKey.get(),
+        targetPlatformKey,
+        resolvedToolchains.row(selectedExecutionPlatformKey.get()));
+  }
+
+  private static Optional<ConfiguredTargetKey> findExecutionPlatformForToolchains(
+      Environment env,
+      Set<Label> requiredToolchains,
+      ImmutableList<ConfiguredTargetKey> availableExecutionPlatformKeys,
+      Table<ConfiguredTargetKey, Label, Label> resolvedToolchains,
+      boolean debug) {
     for (ConfiguredTargetKey executionPlatformKey : availableExecutionPlatformKeys) {
       // PlatformInfo executionPlatform = platforms.get(executionPlatformKey);
       Map<Label, Label> toolchains = resolvedToolchains.row(executionPlatformKey);
@@ -360,8 +387,7 @@ public class ToolchainUtil {
                                         "type %s -> toolchain %s", e.getKey(), e.getValue()))
                             .collect(joining(", ")))));
       }
-      return Optional.of(
-          ResolvedToolchains.create(executionPlatformKey, targetPlatformKey, toolchains));
+      return Optional.of(executionPlatformKey);
     }
 
     return Optional.absent();
@@ -449,7 +475,8 @@ public class ToolchainUtil {
   private static ImmutableList<ConfiguredTargetKey> filterPlatforms(
       ImmutableList<ConfiguredTargetKey> platformKeys,
       ImmutableList<ConfiguredTargetKey> constraintKeys,
-      Environment env)
+      Environment env,
+      boolean debug)
       throws ToolchainContextException, InterruptedException {
 
     // Short circuit if not needed.
@@ -468,7 +495,7 @@ public class ToolchainUtil {
 
     return platformKeys
         .stream()
-        .filter(key -> filterPlatform(platformInfoMap.get(key), constraints))
+        .filter(key -> filterPlatform(platformInfoMap.get(key), constraints, env, debug))
         .collect(toImmutableList());
   }
 
@@ -519,13 +546,25 @@ public class ToolchainUtil {
   }
 
   private static boolean filterPlatform(
-      PlatformInfo platformInfo, List<ConstraintValueInfo> constraints) {
+      PlatformInfo platformInfo,
+      List<ConstraintValueInfo> constraints,
+      Environment env,
+      boolean debug) {
     for (ConstraintValueInfo filterConstraint : constraints) {
       ConstraintValueInfo platformInfoConstraint =
           platformInfo.getConstraint(filterConstraint.constraint());
       if (platformInfoConstraint == null || !platformInfoConstraint.equals(filterConstraint)) {
         // The value for this setting is not present in the platform, or doesn't match the expected
         // value.
+        if (debug) {
+          env.getListener()
+              .handle(
+                  Event.info(
+                      String.format(
+                          "ToolchainUtil: Removed execution platform %s from"
+                              + " available execution platforms, it is missing constraint %s",
+                          platformInfo.label(), filterConstraint.label())));
+        }
         return false;
       }
     }
@@ -533,6 +572,45 @@ public class ToolchainUtil {
     return true;
   }
 
+  /** Exception used when no execution platform can be found. */
+  static final class NoMatchingPlatformException extends Exception {
+    NoMatchingPlatformException() {
+      super("No available execution platform satisfies all requested toolchain types");
+    }
+
+    public NoMatchingPlatformException(
+        Set<Label> requiredToolchains,
+        ImmutableList<ConfiguredTargetKey> availableExecutionPlatformKeys,
+        ConfiguredTargetKey targetPlatformKey) {
+      super(formatError(requiredToolchains, availableExecutionPlatformKeys, targetPlatformKey));
+    }
+
+    private static String formatError(
+        Set<Label> requiredToolchains,
+        ImmutableList<ConfiguredTargetKey> availableExecutionPlatformKeys,
+        ConfiguredTargetKey targetPlatformKey) {
+      if (requiredToolchains.isEmpty()) {
+        return String.format(
+            "Unable to find an execution platform for target platform %s"
+                + " from available execution platforms [%s]",
+            targetPlatformKey.getLabel(),
+            availableExecutionPlatformKeys
+                .stream()
+                .map(key -> key.getLabel().toString())
+                .collect(Collectors.joining(", ")));
+      }
+      return String.format(
+          "Unable to find an execution platform for toolchains [%s] and target platform %s"
+              + " from available execution platforms [%s]",
+          Joiner.on(", ").join(requiredToolchains),
+          targetPlatformKey.getLabel(),
+          availableExecutionPlatformKeys
+              .stream()
+              .map(key -> key.getLabel().toString())
+              .collect(Collectors.joining(", ")));
+    }
+  }
+
   /**
    * Exception used when an error occurs in {@link #expandTargetPatterns(Environment, List,
    * FilteringPolicy)}.
@@ -609,6 +687,10 @@ public class ToolchainUtil {
 
   /** Exception used to wrap exceptions during toolchain resolution. */
   public static class ToolchainContextException extends Exception {
+    public ToolchainContextException(NoMatchingPlatformException e) {
+      super(e);
+    }
+
     public ToolchainContextException(InvalidPlatformException e) {
       super(e);
     }