From 3a0ce7780a5707450d7fb013c006e90bf6861e27 Mon Sep 17 00:00:00 2001
From: John Cater <jcater@google.com>
Date: Wed, 9 Aug 2017 21:09:53 +0200
Subject: Ensure that invalid target errors are properly reported in
 ConfiguredTargetFunction.

Fixes #3430.

Change-Id: Iac1ab3fb4958dc6fb23e92a43a32b81461dcf0f3
PiperOrigin-RevId: 164754851
---
 .../lib/skyframe/ConfiguredTargetFunction.java     | 40 ++++++++++++++++++----
 1 file changed, 33 insertions(+), 7 deletions(-)

(limited to 'src/main/java/com/google/devtools/build/lib/skyframe')

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
index b208c08b8a..87de6cf5bf 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
@@ -20,6 +20,7 @@ import com.google.common.base.Verify;
 import com.google.common.base.VerifyException;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.LinkedHashMultimap;
 import com.google.common.collect.LinkedListMultimap;
@@ -177,7 +178,7 @@ public final class ConfiguredTargetFunction implements SkyFunction {
       target = pkg.getTarget(lc.getLabel().getName());
     } catch (NoSuchTargetException e) {
       throw new ConfiguredTargetFunctionException(
-          new ConfiguredValueCreationException(e.getMessage()));
+          new ConfiguredValueCreationException(e.getMessage(), lc.getLabel()));
     }
     if (pkg.containsErrors()) {
       transitiveLoadingRootCauses.add(lc.getLabel());
@@ -205,6 +206,8 @@ public final class ConfiguredTargetFunction implements SkyFunction {
 
     SkyframeDependencyResolver resolver = view.createDependencyResolver(env);
 
+    ToolchainContext toolchainContext = null;
+
     // TODO(janakr): this acquire() call may tie up this thread indefinitely, reducing the
     // parallelism of Skyframe. This is a strict improvement over the prior state of the code, in
     // which we ran with #processors threads, but ideally we would call #tryAcquire here, and if we
@@ -231,7 +234,6 @@ public final class ConfiguredTargetFunction implements SkyFunction {
       }
 
       // Determine what toolchains are needed by this target.
-      ToolchainContext toolchainContext = null;
       if (target instanceof Rule) {
         Rule rule = ((Rule) target);
         ImmutableList<Label> requiredToolchains = rule.getRuleClassObject().getRequiredToolchains();
@@ -277,18 +279,42 @@ public final class ConfiguredTargetFunction implements SkyFunction {
       return ans;
     } catch (DependencyEvaluationException e) {
       if (e.getCause() instanceof ConfiguredValueCreationException) {
-        throw new ConfiguredTargetFunctionException(
-            (ConfiguredValueCreationException) e.getCause());
+        ConfiguredValueCreationException cvce = (ConfiguredValueCreationException) e.getCause();
+
+        // Check if this is caused by an unresolved toolchain, and report it as such.
+        if (toolchainContext != null) {
+          ImmutableSet.Builder<Label> causes = new ImmutableSet.Builder<Label>();
+          if (cvce.getAnalysisRootCause() != null) {
+            causes.add(cvce.getAnalysisRootCause());
+          }
+          if (!cvce.getRootCauses().isEmpty()) {
+            causes.addAll(cvce.getRootCauses());
+          }
+          Set<Label> toolchainDependencyErrors =
+              toolchainContext.filterToolchainLabels(causes.build());
+          if (!toolchainDependencyErrors.isEmpty()) {
+            env.getListener()
+                .handle(
+                    Event.error(
+                        String.format(
+                            "While resolving toolchains for target %s: %s",
+                            target.getLabel(), e.getCause().getMessage())));
+          }
+        }
+
+        throw new ConfiguredTargetFunctionException(cvce);
       } else if (e.getCause() instanceof InconsistentAspectOrderException) {
         InconsistentAspectOrderException cause = (InconsistentAspectOrderException) e.getCause();
         throw new ConfiguredTargetFunctionException(
             new ConfiguredValueCreationException(cause.getMessage(), target.getLabel()));
-      } else {
-        // Cast to InvalidConfigurationException as a consistency check. If you add any
-        // DependencyEvaluationException constructors, you may need to change this code, too.
+      } else if (e.getCause() instanceof InvalidConfigurationException) {
         InvalidConfigurationException cause = (InvalidConfigurationException) e.getCause();
         throw new ConfiguredTargetFunctionException(
             new ConfiguredValueCreationException(cause.getMessage(), target.getLabel()));
+      } else {
+        // Unknown exception type.
+        throw new ConfiguredTargetFunctionException(
+            new ConfiguredValueCreationException(e.getMessage(), target.getLabel()));
       }
     } catch (AspectCreationException e) {
       // getAnalysisRootCause may be null if the analysis of the aspect itself failed.
-- 
cgit v1.2.3