Configurably block errors from universe eval in query environments

Universe evaluation only occurs in the SkyQueryEnvironment
implementation. This setting is a no-op for other QueryEnvironment
implementations.

This support is needed to correctly test an upcoming bugfix.

--
PiperOrigin-RevId: 149786616
MOS_MIGRATED_REVID=149786616

4 files changed, 73 insertions, 23 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/query2/QueryEnvironmentFactory.java b/src/main/java/com/google/devtools/build/lib/query2/QueryEnvironmentFactory.java
index d43f71beb2..ac692d047b 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/QueryEnvironmentFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/QueryEnvironmentFactory.java
@@ -47,7 +47,8 @@ public class QueryEnvironmentFactory {
       ExtendedEventHandler eventHandler,
       Set<Setting> settings,
       Iterable<QueryFunction> functions,
-      @Nullable PathPackageLocator packagePath) {
+      @Nullable PathPackageLocator packagePath,
+      boolean blockUniverseEvaluationErrors) {
     Preconditions.checkNotNull(universeScope);
     if (canUseSkyQuery(orderedResults, universeScope, packagePath, strictScope, labelFilter)) {
       return new SkyQueryEnvironment(
@@ -59,7 +60,8 @@ public class QueryEnvironmentFactory {
           targetPatternEvaluator.getOffset(),
           graphFactory,
           universeScope,
-          packagePath);
+          packagePath,
+          blockUniverseEvaluationErrors);
     } else {
       return new BlazeQueryEnvironment(transitivePackageLoader, targetProvider,
           targetPatternEvaluator, keepGoing, strictScope, loadingPhaseThreads, labelFilter,
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 de5175d86a..e720bee64f 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
@@ -44,7 +44,9 @@ import com.google.devtools.build.lib.collect.CompactHashSet;
 import com.google.devtools.build.lib.concurrent.BlockingStack;
 import com.google.devtools.build.lib.concurrent.MultisetSemaphore;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
+import com.google.devtools.build.lib.events.DelegatingEventHandler;
 import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.events.EventKind;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
 import com.google.devtools.build.lib.graph.Digraph;
 import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
@@ -146,6 +148,9 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target>
   protected final int loadingPhaseThreads;
   protected final WalkableGraphFactory graphFactory;
   protected final ImmutableList<String> universeScope;
+  protected boolean blockUniverseEvaluationErrors;
+  protected ExtendedEventHandler universeEvalEventHandler;
+
   protected final String parserPrefix;
   protected final PathPackageLocator pkgPath;
   private final int queryEvaluationParallelismLevel;
@@ -169,7 +174,8 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target>
       String parserPrefix,
       WalkableGraphFactory graphFactory,
       List<String> universeScope,
-      PathPackageLocator pkgPath) {
+      PathPackageLocator pkgPath,
+      boolean blockUniverseEvaluationErrors) {
     this(
         keepGoing,
         loadingPhaseThreads,
@@ -182,7 +188,8 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target>
         parserPrefix,
         graphFactory,
         universeScope,
-        pkgPath);
+        pkgPath,
+        blockUniverseEvaluationErrors);
   }
 
   protected SkyQueryEnvironment(
@@ -195,7 +202,8 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target>
       String parserPrefix,
       WalkableGraphFactory graphFactory,
       List<String> universeScope,
-      PathPackageLocator pkgPath) {
+      PathPackageLocator pkgPath,
+      boolean blockUniverseEvaluationErrors) {
     super(
         keepGoing,
         /*strictScope=*/ true,
@@ -212,7 +220,12 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target>
         !universeScope.isEmpty(), "No queries can be performed with an empty universe");
     this.queryEvaluationParallelismLevel = queryEvaluationParallelismLevel;
     this.universeKey = graphFactory.getUniverseKey(universeScope, parserPrefix);
-    universeTargetPatternKeys =
+    this.blockUniverseEvaluationErrors = blockUniverseEvaluationErrors;
+    this.universeEvalEventHandler =
+        this.blockUniverseEvaluationErrors
+            ? new ErrorBlockingForwardingEventHandler(this.eventHandler)
+            : this.eventHandler;
+    this.universeTargetPatternKeys =
         PrepareDepsOfPatternsFunction.getTargetPatternKeys(
             PrepareDepsOfPatternsFunction.getSkyKeys(universeKey, eventHandler));
   }
@@ -224,7 +237,8 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target>
       // to date.
       EvaluationResult<SkyValue> result;
       try (AutoProfiler p = AutoProfiler.logged("evaluation and walkable graph", LOG)) {
-        result = graphFactory.prepareAndGet(universeKey, loadingPhaseThreads, eventHandler);
+        result =
+            graphFactory.prepareAndGet(universeKey, loadingPhaseThreads, universeEvalEventHandler);
       }
       checkEvaluationResult(result);
 
@@ -1227,7 +1241,7 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target>
     // TODO(nharmata): Now that we know the wrapped callback is ThreadSafe, there's no correctness
     // concern that requires the prohibition of concurrent uses of the callback; the only concern is
     // memory. We should have a threshold for when to invoke the callback with a batch, and also a
-    // separate, larger, bound on the number of targets being processed at the same time. 
+    // separate, larger, bound on the number of targets being processed at the same time.
     private final ThreadSafeOutputFormatterCallback<Target> callback;
     private final Uniquifier<Target> uniquifier =
         new UniquifierImpl<>(TargetKeyExtractor.INSTANCE, DEFAULT_THREAD_COUNT);
@@ -1451,4 +1465,32 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target>
       }
     }
   }
+
+  /**
+   * Query evaluation behavior is specified with respect to errors it emits. (Or at least it should
+   * be. Tools rely on it.) Notably, errors that occur during evaluation of a query's universe must
+   * not be emitted during query command evaluation. Consider the case of a simple single target
+   * query when {@code //...} is the universe: errors in far flung parts of the workspace should not
+   * be emitted when that query command is evaluated.
+   *
+   * <p>Non-error message events are not specified. For instance, it's useful (and expected by some
+   * unit tests that should know better) for query commands to emit {@link EventKind#PROGRESS}
+   * events during package loading.
+   *
+   * <p>Therefore, this class is used to forward only non-{@link EventKind#ERROR} events during
+   * universe loading to the {@link SkyQueryEnvironment}'s {@link ExtendedEventHandler}.
+   */
+  protected static class ErrorBlockingForwardingEventHandler extends DelegatingEventHandler {
+
+    public ErrorBlockingForwardingEventHandler(ExtendedEventHandler delegate) {
+      super(delegate);
+    }
+
+    @Override
+    public void handle(Event e) {
+      if (!e.getKind().equals(EventKind.ERROR)) {
+        super.handle(e);
+      }
+    }
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java b/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java
index e01aa4fcbd..a53c57ccdd 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java
@@ -304,20 +304,25 @@ public class GenQuery implements RuleConfiguredTargetFactory {
       // All the packages are already loaded at this point, so there is no need
       // to start up many threads. 4 are started up to make good use of multiple
       // cores.
-      BlazeQueryEnvironment queryEnvironment = (BlazeQueryEnvironment) QUERY_ENVIRONMENT_FACTORY
-          .create(
-              /*transitivePackageLoader=*/null, /*graph=*/null, packageProvider,
-              evaluator,
-              /*keepGoing=*/false,
-              ruleContext.attributes().get("strict", Type.BOOLEAN),
-              /*orderedResults=*/!QueryOutputUtils.shouldStreamResults(queryOptions, formatter),
-              /*universeScope=*/ImmutableList.<String>of(),
-              /*loadingPhaseThreads=*/4,
-              labelFilter,
-              getEventHandler(ruleContext),
-              settings,
-              ImmutableList.<QueryFunction>of(),
-              /*packagePath=*/null);
+      BlazeQueryEnvironment queryEnvironment =
+          (BlazeQueryEnvironment)
+              QUERY_ENVIRONMENT_FACTORY.create(
+                  /*transitivePackageLoader=*/ null,
+                  /*graph=*/ null,
+                  packageProvider,
+                  evaluator,
+                  /*keepGoing=*/ false,
+                  ruleContext.attributes().get("strict", Type.BOOLEAN),
+                  /*orderedResults=*/ !QueryOutputUtils.shouldStreamResults(
+                      queryOptions, formatter),
+                  /*universeScope=*/ ImmutableList.<String>of(),
+                  /*loadingPhaseThreads=*/ 4,
+                  labelFilter,
+                  getEventHandler(ruleContext),
+                  settings,
+                  ImmutableList.<QueryFunction>of(),
+                  /*packagePath=*/ null,
+                  /*blockUniverseEvaluationErrors=*/ false);
       queryResult = (DigraphQueryEvalResult<Target>) queryEnvironment.evaluateQuery(query, targets);
     } catch (SkyframeRestartQueryException e) {
       // Do not emit errors for skyframe restarts. They make output of the ConfiguredTargetFunction
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryCommand.java
index 118f529220..7594bef0fa 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryCommand.java
@@ -275,6 +275,7 @@ public final class QueryCommand implements BlazeCommand {
             env.getReporter(),
             settings,
             env.getRuntime().getQueryFunctions(),
-            env.getPackageManager().getPackagePath());
+            env.getPackageManager().getPackagePath(),
+            /*blockUniverseEvaluationErrors=*/ false);
   }
 }