Let JavaBuilder rethrow the real exception in case of a failed compilation instead of just putting the message into an IOException.

This allows us to more fine-grainly consider printing just the error message (which is the right thing to do in case of compilation errors due to Java syntax errors, wrong command-line arguments, ...) or a stack trace (which is interesting in case the JavaBuilder really hit an IOException or something worse).

--
MOS_MIGRATED_REVID=97413024

4 files changed, 47 insertions, 34 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 035d3b3bbf..8c41d528ec 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
@@ -15,7 +15,6 @@
 package com.google.devtools.build.buildjar;
 
 import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Throwables;
 import com.google.common.io.Files;
 import com.google.devtools.build.buildjar.javac.JavacRunner;
 import com.google.devtools.build.buildjar.javac.JavacRunnerImpl;
@@ -74,26 +73,26 @@ public abstract class AbstractJavaBuilder extends AbstractLibraryBuilder {
    * @param err PrintWriter for logging any diagnostic output
    */
   public void compileJavaLibrary(final JavaLibraryBuildRequest build, final OutputStream err)
-      throws IOException {
+      throws Exception {
     prepareSourceCompilation(build);
 
-    final String[] message = { null };
+    final Exception[] exception = {null};
     final JavacRunner javacRunner = new JavacRunnerImpl(build.getPlugins());
-    runWithLargeStack(new Runnable() {
-        @Override
-        public void run() {
-          try {
-            internalCompileJavaLibrary(build, javacRunner, err);
-          } catch (JavacException e) {
-            message[0] = e.getMessage();
-          } catch (Exception e) {
-            message[0] = Throwables.getStackTraceAsString(e);
+    runWithLargeStack(
+        new Runnable() {
+          @Override
+          public void run() {
+            try {
+              internalCompileJavaLibrary(build, javacRunner, err);
+            } catch (Exception e) {
+              exception[0] = e;
+            }
           }
-        }
-      }, 4L * 1024 * 1024);  // 4MB stack
+        },
+        4L * 1024 * 1024); // 4MB stack
 
-    if (message[0] != null) {
-      throw new IOException("Error compiling java source: " + message[0]);
+    if (exception[0] != null) {
+      throw exception[0];
     }
   }
 
@@ -158,8 +157,7 @@ public abstract class AbstractJavaBuilder extends AbstractLibraryBuilder {
   /**
    * Perform the build.
    */
-  public void run(JavaLibraryBuildRequest build, PrintStream err)
-      throws IOException {
+  public void run(JavaLibraryBuildRequest build, PrintStream err) throws Exception {
     boolean successful = false;
     try {
       compileJavaLibrary(build, err);
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 7a52e103ff..6d8b2530ff 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
@@ -102,10 +102,10 @@ public abstract class BazelJavaBuilder {
           ? new ReducedClasspathJavaLibraryBuilder()
           : new SimpleJavaLibraryBuilder();
       builder.run(build, System.err);
-    } catch (InvalidCommandLineException e) {
+    } catch (JavacException | InvalidCommandLineException e) {
       System.err.println(CMDNAME + " threw exception: " + e.getMessage());
       return 1;
-    } catch (IOException e) {
+    } catch (Exception e) {
       e.printStackTrace();
       return 1;
     }
diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/CommonJavaLibraryProcessor.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/CommonJavaLibraryProcessor.java
index 38d16dfd10..424a961e53 100644
--- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/CommonJavaLibraryProcessor.java
+++ b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/CommonJavaLibraryProcessor.java
@@ -14,8 +14,6 @@
 
 package com.google.devtools.build.buildjar;
 
-import com.sun.tools.javac.main.Main.Result;
-
 import java.util.ArrayList;
 import java.util.List;
 
@@ -26,18 +24,6 @@ import java.util.List;
 public abstract class CommonJavaLibraryProcessor {
 
   /**
-   * Exception used to represent failed javac invocation.
-   */
-  static final class JavacException extends Exception {
-    public JavacException(Result result) {
-      super("java compilation returned status " + result);
-      if (result.isOK()) {
-        throw new IllegalArgumentException();
-      }
-    }
-  }
-
-  /**
    * Creates the initial set of arguments to javac from the Build
    * configuration supplied. This set of arguments should be extended
    * by the code invoking it.
diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/JavacException.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/JavacException.java
new file mode 100644
index 0000000000..387f6d8483
--- /dev/null
+++ b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/JavacException.java
@@ -0,0 +1,29 @@
+// Copyright 2015 Google Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.buildjar;
+
+import com.google.common.base.Preconditions;
+
+import com.sun.tools.javac.main.Main.Result;
+
+/**
+ * Exception used to represent failed javac invocation.
+ */
+public final class JavacException extends Exception {
+  public JavacException(Result result) {
+    super("java compilation returned status " + result);
+    Preconditions.checkArgument(!result.isOK());
+  }
+}