In SkyQueryEnvironment, check TransitiveTraversalValues for errors

In SkyQueryEnvironment, the buildTransitiveClosure method ignored the
error messages in successfully evaluated TransitiveTraversalValues,
under the belief that any such error messages are also obtainable by
calling getMissingAndExceptions. This happens to be true right now,
but may not be true in the future.

If, in particular, the TransitiveTraversal skyfunction inlines its
call to the TargetMarker skyfunction, those error messages won't be
available through getMissingAndExceptions.

--
MOS_MIGRATED_REVID=105980430

1 files changed, 35 insertions, 6 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java
index 302410144e..8c409fd943 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java
@@ -23,6 +23,7 @@ import com.google.common.collect.Collections2;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableSet.Builder;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Multimap;
@@ -193,11 +194,11 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target> {
   }
 
   private Map<Target, Collection<Target>> getRawFwdDeps(Iterable<Target> targets) {
-    return makeTargetsMap(graph.getDirectDeps(makeKeys(targets)));
+    return makeTargetsMap(graph.getDirectDeps(makeTransitiveTraversalKeys(targets)));
   }
 
   private Map<Target, Collection<Target>> getRawReverseDeps(Iterable<Target> targets) {
-    return makeTargetsMap(graph.getReverseDeps(makeKeys(targets)));
+    return makeTargetsMap(graph.getReverseDeps(makeTransitiveTraversalKeys(targets)));
   }
 
   private Set<Label> getAllowedDeps(Rule rule) {
@@ -394,12 +395,40 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target> {
       throws QueryException {
     // Everything has already been loaded, so here we just check for errors so that we can
     // pre-emptively throw/report if needed.
-    for (Map.Entry<SkyKey, Exception> entry :
-      graph.getMissingAndExceptions(makeKeys(targets)).entrySet()) {
+    Iterable<SkyKey> transitiveTraversalKeys = makeTransitiveTraversalKeys(targets);
+    ImmutableList.Builder<String> errorMessagesBuilder = ImmutableList.builder();
+
+    // First, look for errors in the successfully evaluated TransitiveTraversalValues. They may
+    // have encountered errors that they were able to recover from.
+    Set<Entry<SkyKey, SkyValue>> successfulEntries =
+        graph.getSuccessfulValues(transitiveTraversalKeys).entrySet();
+    Builder<SkyKey> successfulKeysBuilder = ImmutableSet.builder();
+    for (Entry<SkyKey, SkyValue> successfulEntry : successfulEntries) {
+      successfulKeysBuilder.add(successfulEntry.getKey());
+      TransitiveTraversalValue value = (TransitiveTraversalValue) successfulEntry.getValue();
+      String firstErrorMessage = value.getFirstErrorMessage();
+      if (firstErrorMessage != null) {
+        errorMessagesBuilder.add(firstErrorMessage);
+      }
+    }
+    ImmutableSet<SkyKey> successfulKeys = successfulKeysBuilder.build();
+
+    // Next, look for errors from the unsuccessfully evaluated TransitiveTraversal skyfunctions.
+    Iterable<SkyKey> unsuccessfulKeys =
+        Iterables.filter(transitiveTraversalKeys, Predicates.not(Predicates.in(successfulKeys)));
+    Set<Entry<SkyKey, Exception>> errorEntries =
+        graph.getMissingAndExceptions(unsuccessfulKeys).entrySet();
+    for (Map.Entry<SkyKey, Exception> entry : errorEntries) {
       if (entry.getValue() == null) {
         throw new QueryException(entry.getKey().argument() + " does not exist in graph");
       }
-      reportBuildFileError(caller, entry.getValue().getMessage());
+      errorMessagesBuilder.add(entry.getValue().getMessage());
+    }
+
+    // Lastly, report all found errors.
+    ImmutableList<String> errorMessages = errorMessagesBuilder.build();
+    for (String errorMessage : errorMessages) {
+      reportBuildFileError(caller, errorMessage);
     }
   }
 
@@ -547,7 +576,7 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target> {
         }
       };
 
-  private static Iterable<SkyKey> makeKeys(Iterable<Target> targets) {
+  private static Iterable<SkyKey> makeTransitiveTraversalKeys(Iterable<Target> targets) {
     return Iterables.transform(targets, TARGET_TO_SKY_KEY);
   }