Fix users of toolchain resolution to call the new skyfunction.

Part of work on execution transitions, bazelbuild#7935.
katre · Apr 16, 2019 · 235ce9d · 235ce9d
1 parent 791190e
commit 235ce9d
Show file tree

Hide file tree

Showing 8 changed files with 42 additions and 1,048 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ToolchainResolver.java b/src/main/java/com/google/devtools/build/lib/analysis/ToolchainResolver.java
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
@@ -31,7 +31,6 @@
 import com.google.devtools.build.lib.analysis.DependencyResolver.InconsistentAspectOrderException;
 import com.google.devtools.build.lib.analysis.ResolvedToolchainContext;
 import com.google.devtools.build.lib.analysis.TargetAndConfiguration;
-import com.google.devtools.build.lib.analysis.ToolchainResolver;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
@@ -413,9 +412,13 @@ public SkyValue compute(SkyKey skyKey, Environment env)
         try {
           ImmutableSet<Label> requiredToolchains = aspect.getDefinition().getRequiredToolchains();
           unloadedToolchainContext =
-              new ToolchainResolver(env, BuildConfigurationValue.key(configuration))
-                  .setRequiredToolchainTypes(requiredToolchains)
-                  .resolve();
+              (UnloadedToolchainContext)
+                  env.getValueOrThrow(
+                      UnloadedToolchainContext.key(
+                          BuildConfigurationValue.key(configuration),
+                          requiredToolchains,
+                          ImmutableSet.of()),
+                      ToolchainException.class);
         } catch (ToolchainException e) {
           // TODO(katre): better error handling
           throw new AspectCreationException(

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
@@ -37,7 +37,6 @@
 import com.google.devtools.build.lib.analysis.EmptyConfiguredTarget;
 import com.google.devtools.build.lib.analysis.ResolvedToolchainContext;
 import com.google.devtools.build.lib.analysis.TargetAndConfiguration;
-import com.google.devtools.build.lib.analysis.ToolchainResolver;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
@@ -309,10 +308,13 @@ public SkyValue compute(SkyKey key, Environment env) throws ConfiguredTargetFunc
           // Collect local (target, rule) constraints for filtering out execution platforms.
           ImmutableSet<Label> execConstraintLabels = getExecutionPlatformConstraints(rule);
           unloadedToolchainContext =
-              new ToolchainResolver(env, configuredTargetKey.getConfigurationKey())
-                  .setRequiredToolchainTypes(requiredToolchains)
-                  .setExecConstraintLabels(execConstraintLabels)
-                  .resolve();
+              (UnloadedToolchainContext)
+                  env.getValueOrThrow(
+                      UnloadedToolchainContext.key(
+                          configuredTargetKey.getConfigurationKey(),
+                          requiredToolchains,
+                          execConstraintLabels),
+                      ToolchainException.class);
           if (env.valuesMissing()) {
             return null;
           }
@@ -424,10 +426,8 @@ public SkyValue compute(SkyKey key, Environment env) throws ConfiguredTargetFunc
               e.getCauses()));
     } catch (ToolchainException e) {
       // We need to throw a ConfiguredValueCreationException, so either find one or make one.
-      ConfiguredValueCreationException cvce;
-      if (e.getCause() instanceof ConfiguredValueCreationException) {
-        cvce = (ConfiguredValueCreationException) e.getCause();
-      } else {
+      ConfiguredValueCreationException cvce = e.asConfiguredValueCreationException();
+      if (cvce == null) {
         cvce =
             new ConfiguredValueCreationException(e.getMessage(), target.getLabel(), configuration);
       }

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainResolutionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainResolutionFunction.java
@@ -59,7 +59,8 @@ public UnloadedToolchainContext compute(SkyKey skyKey, Environment env)
       PlatformConfiguration platformConfiguration =
           configuration.getFragment(PlatformConfiguration.class);
       if (platformConfiguration == null) {
-        throw new ValueMissingException();
+        // throw new ValueMissingException();
+        throw new ToolchainException("missing platform config fragment");
       }
 
       // Check if debug output should be generated.
@@ -236,7 +237,7 @@ private boolean filterPlatform(
             .handle(
                 Event.info(
                     String.format(
-                        "ToolchainResolver: Removed execution platform %s from"
+                        "ToolchainResolution: Removed execution platform %s from"
                             + " available execution platforms, it is missing constraint %s",
                         platformInfo.label(), constraint.label())));
       }
@@ -315,13 +316,20 @@ private void determineToolchainImplementations(
 
     // Find and return the first execution platform which has all required toolchains.
     Optional<ConfiguredTargetKey> selectedExecutionPlatformKey;
-    if (requiredToolchainTypeLabels.isEmpty()
-        && platformKeys.executionPlatformKeys().contains(platformKeys.hostPlatformKey())) {
-      // Fall back to the legacy behavior: use the host platform if it's available, otherwise the
-      // first execution platform.
-      selectedExecutionPlatformKey = Optional.of(platformKeys.hostPlatformKey());
+    if (requiredToolchainTypeLabels.isEmpty()) {
+      if (platformKeys.executionPlatformKeys().contains(platformKeys.hostPlatformKey())) {
+        // Fall back to the legacy behavior: use the host platform if it's available, otherwise the
+        // first execution platform.
+        selectedExecutionPlatformKey = Optional.of(platformKeys.hostPlatformKey());
+      } else if (!platformKeys.executionPlatformKeys().isEmpty()) {
+        // The host platform is invalid and no toolchains were requested: use the first execution
+        // platform, if there is one.
+        selectedExecutionPlatformKey = Optional.of(platformKeys.executionPlatformKeys().get(0));
+      } else {
+        // There's nothing left to try.
+        selectedExecutionPlatformKey = Optional.empty();
+      }
     } else {
-      // If there are no toolchains, this will return the first execution platform.
       selectedExecutionPlatformKey =
           findExecutionPlatformForToolchains(
               environment,
@@ -407,7 +415,7 @@ private static Optional<ConfiguredTargetKey> findExecutionPlatformForToolchains(
             .handle(
                 Event.info(
                     String.format(
-                        "ToolchainResolver: Selected execution platform %s, %s",
+                        "ToolchainResolution: Selected execution platform %s, %s",
                         executionPlatformKey.getLabel(), selectedToolchains)));
       }
       return Optional.of(executionPlatformKey);