Fixed bug where blaze run with the --color=no flag was still printing out in color when the build fails.

--
MOS_MIGRATED_REVID=98736813

6 files changed, 58 insertions, 5 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/events/Reporter.java b/src/main/java/com/google/devtools/build/lib/events/Reporter.java
index e0c39250ba..a3cf6cce86 100644
--- a/src/main/java/com/google/devtools/build/lib/events/Reporter.java
+++ b/src/main/java/com/google/devtools/build/lib/events/Reporter.java
@@ -25,7 +25,7 @@ import java.util.List;
  * is not intended as a logging mechanism for developer-only messages; use a
  * Logger for that.
  *
- * The reporter instance is consumed by the build system, and passes events to
+ * <p>The reporter instance is consumed by the build system, and passes events to
  * {@link EventHandler} instances. These handlers are registered via {@link
  * #addHandler(EventHandler)}.
  *
@@ -42,6 +42,9 @@ public final class Reporter implements EventHandler, ExceptionListener {
    */
   private final OutErr outErrToReporter = outErrForReporter(this);
   private volatile OutputFilter outputFilter = OutputFilter.OUTPUT_EVERYTHING;
+  private EventHandler ansiAllowingHandler;
+  private EventHandler ansiStrippingHandler;
+  private boolean ansiAllowingHandlerRegistered;
 
   public Reporter() {}
 
@@ -143,4 +146,34 @@ public final class Reporter implements EventHandler, ExceptionListener {
   public void setOutputFilter(OutputFilter outputFilter) {
     this.outputFilter = outputFilter;
   }
+
+  /**
+   * Registers an ANSI-control-code-allowing EventHandler with an ANSI-stripping EventHandler
+   * that is already registered with the reporter.  The ANSI-stripping handler can then be replaced
+   * with the ANSI-allowing handler by calling {@code #switchToAnsiAllowingHandler} which
+   * calls {@code removeHandler} for the ANSI-stripping handler and then {@code addHandler} for the
+   * ANSI-allowing handler.
+   */
+  public synchronized void registerAnsiAllowingHandler(
+      EventHandler ansiStrippingHandler,
+      EventHandler ansiAllowingHandler) {
+    this.ansiAllowingHandler = ansiAllowingHandler;
+    this.ansiStrippingHandler = ansiStrippingHandler;
+    ansiAllowingHandlerRegistered = true;
+  }
+
+  /**
+   * Restores the ANSI-allowing EventHandler registered using
+   * {@code #registerAnsiAllowingHandler(...)}.
+   */
+  public synchronized void switchToAnsiAllowingHandler() {
+    if (ansiAllowingHandlerRegistered) {
+      removeHandler(ansiStrippingHandler);
+      addHandler(ansiAllowingHandler);
+      ansiStrippingHandler = null;
+      ansiAllowingHandler = null;
+      ansiAllowingHandlerRegistered = false;
+    }
+  }
+
 }
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java
index 4d7e9aefe8..e73f8c4a79 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java
@@ -362,13 +362,14 @@ public class BlazeCommandDispatcher {
     // Setup log filtering
     BlazeCommandEventHandler.Options eventHandlerOptions =
         optionsParser.getOptions(BlazeCommandEventHandler.Options.class);
+    OutErr colorfulOutErr = outErr;
     if (!eventHandlerOptions.useColor()) {
+      outErr = ansiStripOut(ansiStripErr(outErr));
       if (!commandAnnotation.binaryStdOut()) {
-        outErr = ansiStripOut(outErr);
+        colorfulOutErr = ansiStripOut(colorfulOutErr);
       }
-
       if (!commandAnnotation.binaryStdErr()) {
-        outErr = ansiStripErr(outErr);
+        colorfulOutErr = ansiStripErr(colorfulOutErr);
       }
     }
 
@@ -384,6 +385,16 @@ public class BlazeCommandDispatcher {
     EventHandler handler = createEventHandler(outErr, eventHandlerOptions);
     Reporter reporter = runtime.getReporter();
     reporter.addHandler(handler);
+
+    // We register an ANSI-allowing handler associated with {@code handler} so that ANSI control
+    // codes can be re-introduced later even if blaze is invoked with --color=no. This is useful
+    // for commands such as 'blaze run' where the output of the final executable shouldn't be
+    // modified.
+    if (!eventHandlerOptions.useColor()) {
+      EventHandler ansiAllowingHandler = createEventHandler(colorfulOutErr, eventHandlerOptions);
+      reporter.registerAnsiAllowingHandler(handler, ansiAllowingHandler);
+    }
+
     try {
       // While a Blaze command is active, direct all errors to the client's
       // event handler (and out/err streams).
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/Command.java b/src/main/java/com/google/devtools/build/lib/runtime/Command.java
index 318e3e419b..c030a1d558 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/Command.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/Command.java
@@ -75,12 +75,16 @@ public @interface Command {
   /**
    * Returns true if this command wants to write binary data to stdout.
    * Enabling this flag will disable ANSI escape stripping for this command.
+   * This should be used in conjunction with {@code Reporter#switchToAnsiAllowingHandler}.
+   * See {@link RunCommand} for example usage.
    */
   boolean binaryStdOut() default false;
 
   /**
    * Returns true if this command wants to write binary data to stderr.
    * Enabling this flag will disable ANSI escape stripping for this command.
+   * This should be used in conjunction with {@code Reporter#switchToAnsiAllowingHandler}.
+   * See {@link RunCommand} for example usage.
    */
   boolean binaryStdErr() default false;
 
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/InfoCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/InfoCommand.java
index 2448d90aca..d1f18cefcd 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/InfoCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/InfoCommand.java
@@ -181,8 +181,8 @@ public class InfoCommand implements BlazeCommand {
 
   @Override
   public ExitCode exec(final BlazeRuntime runtime, final OptionsProvider optionsProvider) {
+    runtime.getReporter().switchToAnsiAllowingHandler();
     Options infoOptions = optionsProvider.getOptions(Options.class);
-
     OutErr outErr = runtime.getReporter().getOutErr();
     // Creating a BuildConfiguration is expensive and often unnecessary. Delay the creation until
     // it is needed.
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 2792a15b66..c654dd5fac 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
@@ -132,6 +132,7 @@ public final class QueryCommand implements BlazeCommand {
       return ExitCode.ANALYSIS_FAILURE;
     }
 
+    runtime.getReporter().switchToAnsiAllowingHandler();
     // 3. Output results:
     PrintStream output = new PrintStream(runtime.getReporter().getOutErr().getOutputStream());
     try {
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java
index 715ef0e385..16ed00abe9 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java
@@ -305,6 +305,10 @@ public class RunCommand implements BlazeCommand  {
         .addArgs(cmdLine).setEnv(runtime.getClientEnv()).setWorkingDir(workingDir).build();
 
     try {
+      // Restore a raw EventHandler if it is registered. This allows for blaze run to produce the
+      // actual output of the command being run even if --color=no is specified.
+      runtime.getReporter().switchToAnsiAllowingHandler();
+
       // The command API is a little strange in that the following statement
       // will return normally only if the program exits with exit code 0.
       // If it ends with any other code, we have to catch BadExitStatusException.