Refactor JavaBuilder output handling

Prefer Writer to OutputStream to reduce the number of char/byte conversions,
and pass in a Writer for the persistent worker entry point instead of
redirecting stdout and stderr.

--
MOS_MIGRATED_REVID=120771072

3 files changed, 28 insertions, 42 deletions

diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/AbstractJavaBuilder.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/AbstractJavaBuilder.java
index c55891a538..03240ac4cd 100644
--- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/AbstractJavaBuilder.java
+++ b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/AbstractJavaBuilder.java
@@ -28,7 +28,6 @@ import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.OutputStream;
-import java.io.PrintStream;
 import java.io.PrintWriter;
 import java.util.List;
 import java.util.zip.ZipEntry;
@@ -51,9 +50,9 @@ public abstract class AbstractJavaBuilder extends AbstractLibraryBuilder {
    * can be compiled. Invokes compileSources to do the actual compilation.
    *
    * @param build A JavaLibraryBuildRequest request object describing what to compile
-   * @param err OutputStream for logging any diagnostic output
+   * @param err PrintWriter for logging any diagnostic output
    */
-  public Result compileJavaLibrary(final JavaLibraryBuildRequest build, final OutputStream err)
+  public Result compileJavaLibrary(final JavaLibraryBuildRequest build, final PrintWriter err)
       throws Exception {
     prepareSourceCompilation(build);
     if (build.getSourceFiles().isEmpty()) {
@@ -67,10 +66,7 @@ public abstract class AbstractJavaBuilder extends AbstractLibraryBuilder {
             return new BlazeJavacMain(output, plugins).compile(args);
           }
         };
-    Result result;
-    try (PrintWriter javacErrorOutputWriter = new PrintWriter(err)) {
-      result = compileSources(build, javacRunner, javacErrorOutputWriter);
-    }
+    Result result = compileSources(build, javacRunner, err);
     runClassPostProcessing(build);
     return result;
   }
@@ -78,8 +74,7 @@ public abstract class AbstractJavaBuilder extends AbstractLibraryBuilder {
   /**
    * Build a jar file containing source files that were generated by an annotation processor.
    */
-  public abstract void buildGensrcJar(JavaLibraryBuildRequest build, OutputStream err)
-      throws IOException;
+  public abstract void buildGensrcJar(JavaLibraryBuildRequest build) throws IOException;
 
   @VisibleForTesting
   protected void runClassPostProcessing(JavaLibraryBuildRequest build)
@@ -107,7 +102,7 @@ public abstract class AbstractJavaBuilder extends AbstractLibraryBuilder {
   /**
    * Perform the build.
    */
-  public Result run(JavaLibraryBuildRequest build, PrintStream err) throws Exception {
+  public Result run(JavaLibraryBuildRequest build, PrintWriter err) throws Exception {
     Result result = Result.ERROR;
     try {
       result = compileJavaLibrary(build, err);
@@ -115,7 +110,7 @@ public abstract class AbstractJavaBuilder extends AbstractLibraryBuilder {
         buildJar(build);
         if (!build.getProcessors().isEmpty()) {
           if (build.getGeneratedSourcesOutputJar() != null) {
-            buildGensrcJar(build, err);
+            buildGensrcJar(build);
           }
         }
       }
diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/BazelJavaBuilder.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/BazelJavaBuilder.java
index 398f0d6bd5..5e3e071a1a 100644
--- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/BazelJavaBuilder.java
+++ b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/BazelJavaBuilder.java
@@ -24,9 +24,11 @@ import com.google.devtools.build.buildjar.javac.plugins.errorprone.ErrorPronePlu
 import com.google.devtools.build.lib.worker.WorkerProtocol.WorkRequest;
 import com.google.devtools.build.lib.worker.WorkerProtocol.WorkResponse;
 
-import java.io.ByteArrayOutputStream;
 import java.io.IOException;
-import java.io.PrintStream;
+import java.io.OutputStreamWriter;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.nio.charset.Charset;
 import java.util.Arrays;
 import java.util.List;
 
@@ -45,14 +47,16 @@ public abstract class BazelJavaBuilder {
       System.exit(runPersistentWorker());
     } else {
       // This is a single invocation of JavaBuilder that exits after it processed the request.
-      System.exit(processRequest(Arrays.asList(args)));
+      int exitCode = 1;
+      try (PrintWriter err =
+          new PrintWriter(new OutputStreamWriter(System.err, Charset.defaultCharset()))) {
+        exitCode = processRequest(Arrays.asList(args), err);
+      }
+      System.exit(exitCode);
     }
   }
 
   private static int runPersistentWorker() {
-    PrintStream originalStdOut = System.out;
-    PrintStream originalStdErr = System.err;
-
     while (true) {
       try {
         WorkRequest request = WorkRequest.parseDelimitedFrom(System.in);
@@ -61,27 +65,16 @@ public abstract class BazelJavaBuilder {
           break;
         }
 
-        ByteArrayOutputStream baos = new ByteArrayOutputStream();
-        PrintStream ps = new PrintStream(baos, true);
-        // Make sure that we exit nonzero in case an exception occurs during processRequest.
-        int exitCode = 1;
-        // TODO(philwo) - change this so that a PrintWriter can be passed in and will be used
-        // instead of redirect stdout / stderr.
-        System.setOut(ps);
-        System.setErr(ps);
-        try {
-          exitCode = processRequest(request.getArgumentsList());
-        } finally {
-          System.setOut(originalStdOut);
-          System.setErr(originalStdErr);
+        try (StringWriter sw = new StringWriter();
+            PrintWriter pw = new PrintWriter(sw)) {
+          int exitCode = processRequest(request.getArgumentsList(), pw);
+          WorkResponse.newBuilder()
+              .setOutput(sw.toString())
+              .setExitCode(exitCode)
+              .build()
+              .writeDelimitedTo(System.out);
+          System.out.flush();
         }
-
-        WorkResponse.newBuilder()
-            .setOutput(baos.toString())
-            .setExitCode(exitCode)
-            .build()
-            .writeDelimitedTo(System.out);
-        System.out.flush();
       } catch (IOException e) {
         e.printStackTrace();
         return 1;
@@ -90,13 +83,13 @@ public abstract class BazelJavaBuilder {
     return 0;
   }
 
-  private static int processRequest(List<String> args) {
+  private static int processRequest(List<String> args, PrintWriter err) {
     try {
       JavaLibraryBuildRequest build = parse(args);
       AbstractJavaBuilder builder = build.getDependencyModule().reduceClasspath()
           ? new ReducedClasspathJavaLibraryBuilder()
           : new SimpleJavaLibraryBuilder();
-      return builder.run(build, System.err).exitCode;
+      return builder.run(build, err).exitCode;
     } catch (InvalidCommandLineException e) {
       System.err.println(CMDNAME + " threw exception: " + e.getMessage());
       return 1;
diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/SimpleJavaLibraryBuilder.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/SimpleJavaLibraryBuilder.java
index 6417b473c8..e5b135365e 100644
--- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/SimpleJavaLibraryBuilder.java
+++ b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/SimpleJavaLibraryBuilder.java
@@ -23,7 +23,6 @@ import com.sun.tools.javac.main.Main.Result;
 
 import java.io.File;
 import java.io.IOException;
-import java.io.OutputStream;
 import java.io.PrintWriter;
 import java.util.ArrayList;
 import java.util.List;
@@ -127,8 +126,7 @@ public class SimpleJavaLibraryBuilder extends AbstractJavaBuilder {
   }
 
   @Override
-  public void buildGensrcJar(JavaLibraryBuildRequest build, OutputStream err)
-      throws IOException {
+  public void buildGensrcJar(JavaLibraryBuildRequest build) throws IOException {
     JarCreator jar = new JarCreator(build.getGeneratedSourcesOutputJar());
     jar.setNormalize(true);
     jar.setCompression(build.compressJar());