Stop filtering out targets not in the graph in SkyQueryEnvironment. Instead, just warn if a target turns out to not be present. This means that queries may return unexpected results. For instance, if "query" means "bazel query --order_output=no ", then here are the results of two queries:

query --universe_scope=//foo/... //foo:output_file
//foo:output_file

query --universe_scope=//foo/... deps(//foo:output_file)
WARNING: Targets not in graph [//foo:output_file generated_file]
//foo:output_file

--
MOS_MIGRATED_REVID=112163475

2 files changed, 25 insertions, 17 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 28431bec76..927b4ec09f 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,7 +23,6 @@ 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;
 import com.google.common.collect.Sets;
 import com.google.devtools.build.lib.cmdline.Label;
@@ -32,6 +31,7 @@ import com.google.devtools.build.lib.cmdline.PackageIdentifier;
 import com.google.devtools.build.lib.cmdline.TargetParsingException;
 import com.google.devtools.build.lib.cmdline.TargetPattern;
 import com.google.devtools.build.lib.collect.CompactHashSet;
+import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.graph.Digraph;
 import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
@@ -231,10 +231,21 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target> {
         });
   }
 
+  /** Targets may not be in the graph because they are not in the universe or depend on cycles. */
+  private void warnIfMissingTargets(
+      Iterable<Target> targets, Set<Target> result) {
+    if (Iterables.size(targets) != result.size()) {
+      Set<Target> missingTargets = Sets.difference(ImmutableSet.copyOf(targets), result);
+      eventHandler.handle(Event.warn("Targets were missing from graph: " + missingTargets));
+    }
+  }
+
   @Override
   public Collection<Target> getFwdDeps(Iterable<Target> targets) {
     Set<Target> result = new HashSet<>();
-    for (Map.Entry<Target, Collection<Target>> entry : getRawFwdDeps(targets).entrySet()) {
+    Map<Target, Collection<Target>> rawFwdDeps = getRawFwdDeps(targets);
+    warnIfMissingTargets(targets, rawFwdDeps.keySet());
+    for (Map.Entry<Target, Collection<Target>> entry : rawFwdDeps.entrySet()) {
       result.addAll(filterFwdDeps(entry.getKey(), entry.getValue()));
     }
     return result;
@@ -244,6 +255,7 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target> {
   public Collection<Target> getReverseDeps(Iterable<Target> targets) {
     Set<Target> result = CompactHashSet.create();
     Map<Target, Collection<Target>> rawReverseDeps = getRawReverseDeps(targets);
+    warnIfMissingTargets(targets, rawReverseDeps.keySet());
 
     CompactHashSet<Target> visited = CompactHashSet.create();
 
@@ -354,7 +366,7 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target> {
           targets.add(target);
         }
       }
-      batchStreamedCallback.process(filterTargetsNotInGraph(targets));
+      batchStreamedCallback.process(targets);
     }
 
     private void processLastPending() throws QueryException, InterruptedException {
@@ -547,9 +559,11 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target> {
         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");
+        // Targets may be in the graph because they are not in the universe or depend on cycles.
+        eventHandler.handle(Event.warn(entry.getKey().argument() + " does not exist in graph"));
+      } else {
+        errorMessagesBuilder.add(entry.getValue().getMessage());
       }
-      errorMessagesBuilder.add(entry.getValue().getMessage());
     }
 
     // Lastly, report all found errors.
@@ -559,16 +573,6 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target> {
     }
   }
 
-  private Set<Target> filterTargetsNotInGraph(Set<Target> targets) {
-    Map<Target, SkyKey> map = Maps.toMap(targets, TARGET_TO_SKY_KEY);
-    Set<SkyKey> present = graph.getSuccessfulValues(map.values()).keySet();
-    if (present.size() == targets.size()) {
-      // Optimize for case of all targets being in graph.
-      return targets;
-    }
-    return Maps.filterValues(map, Predicates.in(present)).keySet();
-  }
-
   @Override
   protected void preloadOrThrow(QueryExpression caller, Collection<String> patterns)
       throws QueryException, TargetParsingException {
diff --git a/src/main/java/com/google/devtools/build/skyframe/DelegatingWalkableGraph.java b/src/main/java/com/google/devtools/build/skyframe/DelegatingWalkableGraph.java
index fb1b00e588..c99bc32285 100644
--- a/src/main/java/com/google/devtools/build/skyframe/DelegatingWalkableGraph.java
+++ b/src/main/java/com/google/devtools/build/skyframe/DelegatingWalkableGraph.java
@@ -15,7 +15,6 @@ package com.google.devtools.build.skyframe;
 
 import com.google.common.base.Function;
 import com.google.common.base.Predicates;
-import com.google.common.collect.Iterables;
 import com.google.common.collect.Maps;
 import com.google.devtools.build.lib.util.Preconditions;
 
@@ -52,9 +51,14 @@ public class DelegatingWalkableGraph implements WalkableGraph {
     return entry;
   }
 
+  /**
+   * Returns a map giving the {@link NodeEntry} corresponding to the given {@code keys}. If there is
+   * no node in the graph corresponding to a {@link SkyKey} in {@code keys}, it is silently ignored
+   * and will not be present in the returned map. This tolerance allows callers to avoid
+   * pre-filtering their keys by checking for existence, which can be expensive.
+   */
   private static Map<SkyKey, NodeEntry> getEntries(Iterable<SkyKey> keys, QueryableGraph graph) {
     Map<SkyKey, NodeEntry> result = graph.getBatch(keys);
-    Preconditions.checkState(result.size() == Iterables.size(keys), "%s %s", keys, result);
     for (Map.Entry<SkyKey, NodeEntry> entry : result.entrySet()) {
       Preconditions.checkState(entry.getValue().isDone(), entry);
     }