Use EventHandler even before we have a Reporter instance

...instead of relying on all the methods to call printErrLn with exactly the
right format string.

PiperOrigin-RevId: 158951236

2 files changed, 60 insertions, 49 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/events/PrintingEventHandler.java b/src/main/java/com/google/devtools/build/lib/events/PrintingEventHandler.java
index cad3439710..fc16475694 100644
--- a/src/main/java/com/google/devtools/build/lib/events/PrintingEventHandler.java
+++ b/src/main/java/com/google/devtools/build/lib/events/PrintingEventHandler.java
@@ -14,9 +14,8 @@
 package com.google.devtools.build.lib.events;
 
 import com.google.devtools.build.lib.util.io.OutErr;
-
 import java.io.IOException;
-import java.io.PrintWriter;
+import java.nio.charset.StandardCharsets;
 import java.util.Set;
 
 /**
@@ -26,13 +25,6 @@ import java.util.Set;
  * ERROR: /home/jrluser/src/workspace/x/BUILD:23:1: syntax error.
  * </pre>
  * This syntax is parseable by Emacs's compile.el.
- *
- * <p>
- * By default, the output will go to SYSTEM_OUT_ERR,
- * but this can be changed by calling the setOut() method.
- *
- * <p>
- * This class is used only for tests.
  */
 public class PrintingEventHandler extends AbstractEventHandler
     implements EventHandler {
@@ -55,15 +47,22 @@ public class PrintingEventHandler extends AbstractEventHandler
   public static final PrintingEventHandler ERRORS_TO_STDERR =
       new PrintingEventHandler(EventKind.ERRORS_AND_OUTPUT);
 
-  private OutErr outErr = OutErr.SYSTEM_OUT_ERR;
+  private OutErr outErr;
 
   /**
-   * Setup a printing event handler that will handle events matching the mask.
-   * Events will be printed to the original System.out and System.err
-   * unless/until redirected by a call to setOutErr().
+   * Setup a printing event handler that prints events matching the mask.
    */
-  public PrintingEventHandler(Set<EventKind> mask) {
+  public PrintingEventHandler(OutErr outErr, Set<EventKind> mask) {
     super(mask);
+    this.outErr = outErr;
+  }
+
+  /**
+   * Setup a printing event handler that prints events matching the mask. Events are printed to the
+   * System.out and System.err unless/until redirected by a call to setOutErr().
+   */
+  public PrintingEventHandler(Set<EventKind> mask) {
+    this(OutErr.SYSTEM_OUT_ERR, mask);
   }
 
   /**
@@ -96,15 +95,14 @@ public class PrintingEventHandler extends AbstractEventHandler
           outErr.getErrorStream().flush();
           break;
         default:
-          PrintWriter err = new PrintWriter(outErr.getErrorStream());
-          err.print(event.getKind());
-          err.print(": ");
+          StringBuilder builder = new StringBuilder();
+          builder.append(event.getKind()).append(": ");
           if (event.getLocation() != null) {
-            err.print(event.getLocation().print());
-            err.print(": ");
+            builder.append(event.getLocation().print()).append(": ");
           }
-          err.println(event.getMessage());
-          err.flush();
+          builder.append(event.getMessage()).append("\n");
+          outErr.getErrorStream().write(builder.toString().getBytes(StandardCharsets.UTF_8));
+          outErr.getErrorStream().flush();
       }
     } catch (IOException e) {
       /*
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 7751ca60e0..e15d202866 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
@@ -31,6 +31,8 @@ import com.google.common.io.Flushables;
 import com.google.common.util.concurrent.UncheckedExecutionException;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
+import com.google.devtools.build.lib.events.EventKind;
+import com.google.devtools.build.lib.events.PrintingEventHandler;
 import com.google.devtools.build.lib.events.Reporter;
 import com.google.devtools.build.lib.runtime.commands.ProjectFileSupport;
 import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy;
@@ -157,22 +159,25 @@ public class BlazeCommandDispatcher {
    * was called from the output directory and fail if it was.
    */
   private ExitCode checkCwdInWorkspace(CommandEnvironment env, Command commandAnnotation,
-      String commandName, OutErr outErr) {
+      String commandName, EventHandler eventHandler) {
     if (!commandAnnotation.mustRunInWorkspace()) {
       return ExitCode.SUCCESS;
     }
 
     if (!env.inWorkspace()) {
-      outErr.printErrLn(
-          "The '" + commandName + "' command is only supported from within a workspace.");
+      eventHandler.handle(
+          Event.error(
+              "The '" + commandName + "' command is only supported from within a workspace."));
       return ExitCode.COMMAND_LINE_ERROR;
     }
 
     Path workspace = env.getWorkspace();
     // TODO(kchodorow): Remove this once spaces are supported.
     if (workspace.getPathString().contains(" ")) {
-      outErr.printErrLn(runtime.getProductName() + " does not currently work properly from paths "
-          + "containing spaces (" + workspace + ").");
+      eventHandler.handle(
+          Event.error(
+              runtime.getProductName() + " does not currently work properly from paths "
+                  + "containing spaces (" + workspace + ")."));
       return ExitCode.LOCAL_ENVIRONMENTAL_ERROR;
     }
 
@@ -181,21 +186,21 @@ public class BlazeCommandDispatcher {
 
     if (doNotBuild.exists()) {
       if (!commandAnnotation.canRunInOutputDirectory()) {
-        outErr.printErrLn(getNotInRealWorkspaceError(doNotBuild));
+        eventHandler.handle(Event.error(getNotInRealWorkspaceError(doNotBuild)));
         return ExitCode.COMMAND_LINE_ERROR;
       } else {
-        outErr.printErrLn(
-            "WARNING: "
-                + runtime.getProductName()
-                + " is run from output directory. This is unsound.");
+        eventHandler.handle(
+            Event.warn(
+                runtime.getProductName() + " is run from output directory. This is unsound."));
       }
     }
     return ExitCode.SUCCESS;
   }
 
   private ExitCode parseArgsAndConfigs(CommandEnvironment env, OptionsParser optionsParser,
-      Command commandAnnotation, List<String> args, List<String> rcfileNotes, OutErr outErr)
-      throws OptionsParsingException {
+      Command commandAnnotation, List<String> args, List<String> rcfileNotes,
+      EventHandler eventHandler)
+          throws OptionsParsingException {
 
     Function<String, String> commandOptionSourceFunction =
         new Function<String, String>() {
@@ -218,7 +223,7 @@ public class BlazeCommandDispatcher {
     // and --rc_source. A no-op if none are provided.
     CommonCommandOptions rcFileOptions = optionsParser.getOptions(CommonCommandOptions.class);
     List<Pair<String, ListMultimap<String, String>>> optionsMap =
-        getOptionsMap(outErr, rcFileOptions.rcSource, rcFileOptions.optionsOverrides,
+        getOptionsMap(eventHandler, rcFileOptions.rcSource, rcFileOptions.optionsOverrides,
             runtime.getCommandMap().keySet());
 
     parseOptionsForCommand(rcfileNotes, commandAnnotation, optionsParser, optionsMap, null, null);
@@ -240,11 +245,15 @@ public class BlazeCommandDispatcher {
     }
     if (!unknownConfigs.isEmpty()) {
       if (commonOptions.allowUndefinedConfigs) {
-        outErr.printErrLn("WARNING: Config values are not defined in any .rc file: "
-            + Joiner.on(", ").join(unknownConfigs));
+        eventHandler.handle(
+            Event.warn(
+                "Config values are not defined in any .rc file: "
+                    + Joiner.on(", ").join(unknownConfigs)));
       } else {
-        outErr.printErrLn("ERROR: Config values are not defined in any .rc file: "
-            + Joiner.on(", ").join(unknownConfigs));
+        eventHandler.handle(
+            Event.error(
+                "Config values are not defined in any .rc file: "
+                    + Joiner.on(", ").join(unknownConfigs)));
         return ExitCode.COMMAND_LINE_ERROR;
       }
     }
@@ -395,7 +404,8 @@ public class BlazeCommandDispatcher {
       LoggingUtil.logToRemote(Level.WARNING, "Unable to delete or open command.log", ioException);
     }
 
-    ExitCode result = checkCwdInWorkspace(env, commandAnnotation, commandName, outErr);
+    EventHandler eventHandler = new PrintingEventHandler(outErr, EventKind.ALL_EVENTS);
+    ExitCode result = checkCwdInWorkspace(env, commandAnnotation, commandName, eventHandler);
     if (!result.equals(ExitCode.SUCCESS)) {
       return result.getNumericExitCode();
     }
@@ -407,12 +417,14 @@ public class BlazeCommandDispatcher {
     try {
       optionsParser = createOptionsParser(command);
     } catch (OptionsParser.ConstructionException e) {
-      outErr.printErrLn("Internal error while constructing options parser: " + e.getMessage());
+      eventHandler.handle(
+          Event.error("Internal error while constructing options parser: " + e.getMessage()));
       return ExitCode.BLAZE_INTERNAL_ERROR.getNumericExitCode();
     }
     try {
       ExitCode parseResult =
-          parseArgsAndConfigs(env, optionsParser, commandAnnotation, args, rcfileNotes, outErr);
+          parseArgsAndConfigs(
+              env, optionsParser, commandAnnotation, args, rcfileNotes, eventHandler);
       if (!parseResult.equals(ExitCode.SUCCESS)) {
         return parseResult.getNumericExitCode();
       }
@@ -428,9 +440,9 @@ public class BlazeCommandDispatcher {
       optionsPolicyEnforcer.enforce(optionsParser, commandName);
     } catch (OptionsParsingException e) {
       for (String note : rcfileNotes) {
-        outErr.printErrLn("INFO: " + note);
+        eventHandler.handle(Event.info(note));
       }
-      outErr.printErrLn(e.getMessage());
+      eventHandler.handle(Event.error(e.getMessage()));
       return ExitCode.COMMAND_LINE_ERROR.getNumericExitCode();
     }
 
@@ -758,7 +770,7 @@ public class BlazeCommandDispatcher {
    */
   @VisibleForTesting
   static List<Pair<String, ListMultimap<String, String>>> getOptionsMap(
-      OutErr outErr,
+      EventHandler eventHandler,
       List<String> rcFiles,
       List<CommonCommandOptions.OptionOverride> overrides,
       Set<String> validCommands) {
@@ -768,8 +780,8 @@ public class BlazeCommandDispatcher {
     ListMultimap<String, String> lastMap = null;
     for (CommonCommandOptions.OptionOverride override : overrides) {
       if (override.blazeRc < 0 || override.blazeRc >= rcFiles.size()) {
-        outErr.printErrLn("WARNING: inconsistency in generated command line "
-            + "args. Ignoring bogus argument\n");
+        eventHandler.handle(
+            Event.warn("inconsistency in generated command line args. Ignoring bogus argument\n"));
         continue;
       }
       String rcFile = rcFiles.get(override.blazeRc);
@@ -780,9 +792,10 @@ public class BlazeCommandDispatcher {
         command = command.substring(0, index);
       }
       if (!validCommands.contains(command) && !command.equals("common")) {
-        outErr.printErrLn("WARNING: while reading option defaults file '"
-            + rcFile + "':\n"
-            + "  invalid command name '" + override.command + "'.");
+        eventHandler.handle(
+            Event.warn(
+                "while reading option defaults file '" + rcFile + "':\n"
+                    + "  invalid command name '" + override.command + "'."));
         continue;
       }