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
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 d43f71b..ac692d0 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 @@
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 @@
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 de5175d..e720bee 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.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 @@
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 @@
String parserPrefix,
WalkableGraphFactory graphFactory,
List<String> universeScope,
- PathPackageLocator pkgPath) {
+ PathPackageLocator pkgPath,
+ boolean blockUniverseEvaluationErrors) {
this(
keepGoing,
loadingPhaseThreads,
@@ -182,7 +188,8 @@
parserPrefix,
graphFactory,
universeScope,
- pkgPath);
+ pkgPath,
+ blockUniverseEvaluationErrors);
}
protected SkyQueryEnvironment(
@@ -195,7 +202,8 @@
String parserPrefix,
WalkableGraphFactory graphFactory,
List<String> universeScope,
- PathPackageLocator pkgPath) {
+ PathPackageLocator pkgPath,
+ boolean blockUniverseEvaluationErrors) {
super(
keepGoing,
/*strictScope=*/ true,
@@ -212,7 +220,12 @@
!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 @@
// 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 @@
// 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 @@
}
}
}
+
+ /**
+ * 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 e01aa4f..a53c57c 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 @@
// 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 118f529..7594bef 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 @@
env.getReporter(),
settings,
env.getRuntime().getQueryFunctions(),
- env.getPackageManager().getPackagePath());
+ env.getPackageManager().getPackagePath(),
+ /*blockUniverseEvaluationErrors=*/ false);
}
}
