Error out gracefully instead of crashing blaze when --output=transiti…

…ons is used without the --transitions flag PiperOrigin-RevId: 191755762
mmikitka · Apr 5, 2018 · 08dda86 · 08dda86
1 parent e1ed9e9
commit 08dda86
Show file tree

Hide file tree

Showing 5 changed files with 33 additions and 15 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/CqueryBuildTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/CqueryBuildTool.java
@@ -131,7 +131,7 @@ private void doConfiguredTargetQuery(
         configuredTargetQueryEnvironment.getDefaultOutputFormatters(
             configuredTargetQueryEnvironment.getAccessor(),
             cqueryOptions,
-            env.getReporter().getOutErr().getOutputStream(),
+            env.getReporter(),
             env.getSkyframeExecutor(),
             hostConfiguration);
     CqueryThreadsafeCallback callback =

diff --git a/src/main/java/com/google/devtools/build/lib/query2/ConfiguredTargetQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/ConfiguredTargetQueryEnvironment.java
@@ -31,6 +31,7 @@
 import com.google.devtools.build.lib.concurrent.MultisetSemaphore;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
+import com.google.devtools.build.lib.events.Reporter;
 import com.google.devtools.build.lib.packages.DependencyFilter;
 import com.google.devtools.build.lib.packages.NoSuchTargetException;
 import com.google.devtools.build.lib.packages.Rule;
@@ -207,14 +208,17 @@ public static Label getCorrectLabel(ConfiguredTarget target) {
   public ImmutableList<CqueryThreadsafeCallback> getDefaultOutputFormatters(
       TargetAccessor<ConfiguredTarget> accessor,
       CqueryOptions options,
-      OutputStream out,
+      Reporter reporter,
       SkyframeExecutor skyframeExecutor,
       BuildConfiguration hostConfiguration) {
+    OutputStream out = reporter.getOutErr().getOutputStream();
     return new ImmutableList.Builder<CqueryThreadsafeCallback>()
-        .add(new LabelAndConfigurationOutputFormatterCallback(options, out, skyframeExecutor))
+        .add(
+            new LabelAndConfigurationOutputFormatterCallback(
+                reporter, options, out, skyframeExecutor))
         .add(
             new TransitionsOutputFormatterCallback(
-                options, out, skyframeExecutor, accessor, hostConfiguration))
+                reporter, options, out, skyframeExecutor, accessor, hostConfiguration))
         .build();
   }
 

diff --git a/src/main/java/com/google/devtools/build/lib/query2/CqueryThreadsafeCallback.java b/src/main/java/com/google/devtools/build/lib/query2/CqueryThreadsafeCallback.java
@@ -18,6 +18,7 @@
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.Streams;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
+import com.google.devtools.build.lib.events.Reporter;
 import com.google.devtools.build.lib.query2.engine.ThreadSafeOutputFormatterCallback;
 import com.google.devtools.build.lib.query2.output.CqueryOptions;
 import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
@@ -34,14 +35,19 @@
 public abstract class CqueryThreadsafeCallback
     extends ThreadSafeOutputFormatterCallback<ConfiguredTarget> {
 
+  protected final Reporter reporter;
   protected final CqueryOptions options;
   protected PrintStream printStream = null;
   protected final SkyframeExecutor skyframeExecutor;
 
   private final List<String> result = new ArrayList<>();
 
   CqueryThreadsafeCallback(
-      CqueryOptions options, OutputStream out, SkyframeExecutor skyframeExecutor) {
+      Reporter reporter,
+      CqueryOptions options,
+      OutputStream out,
+      SkyframeExecutor skyframeExecutor) {
+    this.reporter = reporter;
     this.options = options;
     if (out != null) {
       this.printStream = new PrintStream(out);

diff --git a/...va/com/google/devtools/build/lib/query2/LabelAndConfigurationOutputFormatterCallback.java b/...va/com/google/devtools/build/lib/query2/LabelAndConfigurationOutputFormatterCallback.java
@@ -16,6 +16,7 @@
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.events.NullEventHandler;
+import com.google.devtools.build.lib.events.Reporter;
 import com.google.devtools.build.lib.query2.output.CqueryOptions;
 import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
 import java.io.OutputStream;
@@ -24,8 +25,11 @@
 public class LabelAndConfigurationOutputFormatterCallback extends CqueryThreadsafeCallback {
 
   LabelAndConfigurationOutputFormatterCallback(
-      CqueryOptions options, OutputStream out, SkyframeExecutor skyframeExecutor) {
-    super(options, out, skyframeExecutor);
+      Reporter reporter,
+      CqueryOptions options,
+      OutputStream out,
+      SkyframeExecutor skyframeExecutor) {
+    super(reporter, options, out, skyframeExecutor);
   }
 
   @Override

diff --git a/src/main/java/com/google/devtools/build/lib/query2/TransitionsOutputFormatterCallback.java b/src/main/java/com/google/devtools/build/lib/query2/TransitionsOutputFormatterCallback.java
@@ -41,7 +41,7 @@
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
-import com.google.devtools.build.lib.events.NullEventHandler;
+import com.google.devtools.build.lib.events.Reporter;
 import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.BuildType;
 import com.google.devtools.build.lib.packages.ConfiguredAttributeMapper;
@@ -83,12 +83,13 @@ public String getName() {
    * @param hostConfiguration host configuration for this query.
    */
   TransitionsOutputFormatterCallback(
+      Reporter reporter,
       CqueryOptions options,
       OutputStream out,
       SkyframeExecutor skyframeExecutor,
       TargetAccessor<ConfiguredTarget> accessor,
       BuildConfiguration hostConfiguration) {
-    super(options, out, skyframeExecutor);
+    super(reporter, options, out, skyframeExecutor);
     this.accessor = (ConfiguredTargetAccessor) accessor;
     this.hostConfiguration = hostConfiguration;
     this.partialResultMap = Maps.newHashMap();
@@ -98,17 +99,20 @@ public String getName() {
   public void processOutput(Iterable<ConfiguredTarget> partialResult)
       throws IOException, InterruptedException {
     CqueryOptions.Transitions verbosity = options.transitions;
-    Preconditions.checkArgument(
-        !verbosity.equals(CqueryOptions.Transitions.NONE),
-        "Instead of using --output=transitions, set the --transition flag explicitly to 'lite' or"
-            + "'full'");
+    if (verbosity.equals(CqueryOptions.Transitions.NONE)) {
+      reporter.handle(
+          Event.error(
+              "Instead of using --output=transitions, set the --transition flag"
+                  + " explicitly to 'lite' or 'full'"));
+      return;
+    }
     partialResult.forEach(
         ct -> partialResultMap.put(ct.getLabel(), accessor.getTargetFromConfiguredTarget(ct)));
     for (ConfiguredTarget configuredTarget : partialResult) {
       Target target = partialResultMap.get(configuredTarget.getLabel());
       BuildConfiguration config =
           skyframeExecutor.getConfiguration(
-              NullEventHandler.INSTANCE, configuredTarget.getConfigurationKey());
+              reporter, configuredTarget.getConfigurationKey());
       addResult(
           getRuleClassTransition(configuredTarget, target)
               + configuredTarget.getLabel()
@@ -128,7 +132,7 @@ public void processOutput(Iterable<ConfiguredTarget> partialResult)
         // Also, we don't actually use fromOptions in our implementation of DependencyResolver but
         // passing to avoid passing a null and since we have the information anyway.
         deps =
-            new FormatterDependencyResolver(configuredTarget, NullEventHandler.INSTANCE)
+            new FormatterDependencyResolver(configuredTarget, reporter)
                 .dependentNodeMap(
                     new TargetAndConfiguration(target, config),
                     hostConfiguration,