Remove runWithLargeStack and remove some single-use abstractions
--
MOS_MIGRATED_REVID=119188524
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 5e04ffb..c55891a 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,9 +15,11 @@
package com.google.devtools.build.buildjar;
import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableList;
import com.google.common.io.Files;
+import com.google.devtools.build.buildjar.javac.BlazeJavacMain;
import com.google.devtools.build.buildjar.javac.JavacRunner;
-import com.google.devtools.build.buildjar.javac.JavacRunnerImpl;
+import com.google.devtools.build.buildjar.javac.plugins.BlazeJavaCompilerPlugin;
import com.sun.tools.javac.main.Main.Result;
@@ -45,79 +47,32 @@
protected boolean debug = false;
/**
- * Flush the buffers of this JavaBuilder
- */
- @SuppressWarnings("unused") // IOException
- public synchronized void flush(OutputStream err) throws IOException {
- }
-
- /**
- * Shut this JavaBuilder down
- */
- @SuppressWarnings("unused") // IOException
- public synchronized void shutdown(OutputStream err) throws IOException {
- }
-
- /**
- * Prepares a compilation run and sets everything up so that the source files
- * in the build request can be compiled. Invokes compileSources to do the
- * actual compilation.
- *
- * @param build A JavaLibraryBuildRequest request object describing what to
- * compile
- * @param err PrintWriter for logging any diagnostic output
- */
- public void compileJavaLibrary(final JavaLibraryBuildRequest build, final OutputStream err)
- throws Exception {
- prepareSourceCompilation(build);
-
- final Exception[] exception = {null};
- final JavacRunner javacRunner = new JavacRunnerImpl(build.getPlugins());
- runWithLargeStack(
- new Runnable() {
- @Override
- public void run() {
- try {
- internalCompileJavaLibrary(build, javacRunner, err);
- } catch (Exception e) {
- exception[0] = e;
- }
- }
- },
- 4L * 1024 * 1024); // 4MB stack
-
- if (exception[0] != null) {
- throw exception[0];
- }
- }
-
- /**
- * Compiles the java files of the java library specified in the build request.<p>
- * The compilation consists of two parts:<p>
- * First, javac is invoked directly to compile the java files in the build request.<p>
- * Second, additional processing is done to the .class files that came out of the compile.<p>
+ * Prepares a compilation run and sets everything up so that the source files in the build request
+ * 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
*/
- private void internalCompileJavaLibrary(JavaLibraryBuildRequest build, JavacRunner javacRunner,
- OutputStream err) throws IOException, JavacException {
- // result may not be null, in case somebody changes the set of source files
- // to the empty set
- Result result = Result.OK;
- if (!build.getSourceFiles().isEmpty()) {
- PrintWriter javacErrorOutputWriter = new PrintWriter(err);
- try {
- result = compileSources(build, javacRunner, javacErrorOutputWriter);
- } finally {
- javacErrorOutputWriter.flush();
- }
+ public Result compileJavaLibrary(final JavaLibraryBuildRequest build, final OutputStream err)
+ throws Exception {
+ prepareSourceCompilation(build);
+ if (build.getSourceFiles().isEmpty()) {
+ return Result.OK;
}
-
- if (!result.isOK()) {
- throw new JavacException(result);
+ JavacRunner javacRunner =
+ new JavacRunner() {
+ @Override
+ public Result invokeJavac(
+ ImmutableList<BlazeJavaCompilerPlugin> plugins, String[] args, PrintWriter output) {
+ return new BlazeJavacMain(output, plugins).compile(args);
+ }
+ };
+ Result result;
+ try (PrintWriter javacErrorOutputWriter = new PrintWriter(err)) {
+ result = compileSources(build, javacRunner, javacErrorOutputWriter);
}
runClassPostProcessing(build);
+ return result;
}
/**
@@ -152,59 +107,23 @@
/**
* Perform the build.
*/
- public void run(JavaLibraryBuildRequest build, PrintStream err) throws Exception {
- boolean successful = false;
+ public Result run(JavaLibraryBuildRequest build, PrintStream err) throws Exception {
+ Result result = Result.ERROR;
try {
- compileJavaLibrary(build, err);
- buildJar(build);
- if (!build.getProcessors().isEmpty()) {
- if (build.getGeneratedSourcesOutputJar() != null) {
- buildGensrcJar(build, err);
+ result = compileJavaLibrary(build, err);
+ if (result.isOK()) {
+ buildJar(build);
+ if (!build.getProcessors().isEmpty()) {
+ if (build.getGeneratedSourcesOutputJar() != null) {
+ buildGensrcJar(build, err);
+ }
}
}
- successful = true;
} finally {
- build.getDependencyModule().emitDependencyInformation(build.getClassPath(), successful);
+ build.getDependencyModule().emitDependencyInformation(build.getClassPath(), result.isOK());
build.getProcessingModule().emitManifestProto();
- shutdown(err);
}
- }
-
- // Utility functions
-
- /**
- * Runs "run" in another thread (whose lifetime is contained within the
- * activation of this function call) using a stack size of 'stackSize' bytes.
- * Unchecked exceptions thrown by the Runnable will be re-thrown in the main
- * thread.
- */
- private static void runWithLargeStack(final Runnable run, long stackSize) {
- final Throwable[] unchecked = { null };
- Thread t = new Thread(null, run, "runWithLargeStack", stackSize);
- t.setUncaughtExceptionHandler(new Thread.UncaughtExceptionHandler() {
- @Override
- public void uncaughtException(Thread t, Throwable e) {
- unchecked[0] = e;
- }
- });
- t.start();
- boolean wasInterrupted = false;
- for (;;) {
- try {
- t.join(0);
- break;
- } catch (InterruptedException e) {
- wasInterrupted = true;
- }
- }
- if (wasInterrupted) {
- Thread.currentThread().interrupt();
- }
- if (unchecked[0] instanceof Error) {
- throw (Error) unchecked[0];
- } else if (unchecked[0] instanceof RuntimeException) {
- throw (RuntimeException) unchecked[0];
- }
+ return result;
}
/**
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 d7ee1aa..398f0d6 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
@@ -96,15 +96,14 @@
AbstractJavaBuilder builder = build.getDependencyModule().reduceClasspath()
? new ReducedClasspathJavaLibraryBuilder()
: new SimpleJavaLibraryBuilder();
- builder.run(build, System.err);
- } catch (JavacException | InvalidCommandLineException e) {
+ return builder.run(build, System.err).exitCode;
+ } catch (InvalidCommandLineException e) {
System.err.println(CMDNAME + " threw exception: " + e.getMessage());
return 1;
} catch (Exception e) {
e.printStackTrace();
return 1;
}
- return 0;
}
/**
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
deleted file mode 100644
index 5ad1127..0000000
--- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/JavacException.java
+++ /dev/null
@@ -1,29 +0,0 @@
-// Copyright 2015 The Bazel Authors. 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());
- }
-}
diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/ReducedClasspathJavaLibraryBuilder.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/ReducedClasspathJavaLibraryBuilder.java
index 9f12705..8c2b28f 100644
--- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/ReducedClasspathJavaLibraryBuilder.java
+++ b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/ReducedClasspathJavaLibraryBuilder.java
@@ -63,7 +63,7 @@
// Compile!
StringWriter javacOutput = new StringWriter();
PrintWriter javacOutputWriter = new PrintWriter(javacOutput);
- Result result = javacRunner.invokeJavac(javacArguments, javacOutputWriter);
+ Result result = javacRunner.invokeJavac(build.getPlugins(), javacArguments, javacOutputWriter);
javacOutputWriter.close();
// If javac errored out because of missing entries on the classpath, give it another try.
@@ -78,7 +78,7 @@
// Fall back to the regular compile, but add extra checks to catch transitive uses
javacArguments = makeJavacArguments(build);
- result = javacRunner.invokeJavac(javacArguments, err);
+ result = javacRunner.invokeJavac(build.getPlugins(), javacArguments, err);
} else {
err.print(javacOutput.getBuffer());
}
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 b6049bf..6417b47 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
@@ -37,7 +37,7 @@
Result compileSources(JavaLibraryBuildRequest build, JavacRunner javacRunner, PrintWriter err)
throws IOException {
String[] javacArguments = makeJavacArguments(build, build.getClassPath());
- return javacRunner.invokeJavac(javacArguments, err);
+ return javacRunner.invokeJavac(build.getPlugins(), javacArguments, err);
}
@Override
diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/JavacRunner.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/JavacRunner.java
index e4045cb..799ff9f 100644
--- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/JavacRunner.java
+++ b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/JavacRunner.java
@@ -14,6 +14,9 @@
package com.google.devtools.build.buildjar.javac;
+import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.buildjar.javac.plugins.BlazeJavaCompilerPlugin;
+
import com.sun.tools.javac.main.Main.Result;
import java.io.PrintWriter;
@@ -21,35 +24,9 @@
/**
* The JavacRunner is a type that can be used to invoke
* javac and provides a convenient hook for modifications.
- * It is split in two parts: An interface "JavacRunner" and
- * an implementation of that interface, "JavacRunnerImpl".
- *
- * The type is split in two parts to allow us to load
- * the implementation multiple times in different classloaders.
- * This is neccessary, as a single javac can not run multiple
- * times in parallel. By using different classloaders to load
- * different copies of javac in different JavacRunnerImpls,
- * we can run them in parallel.
- *
- * However, since each JavacRunnerImpl will then be loaded
- * in a different classloader, we then would not be able to
- * refer to it by simply declaring a type as "JavacRunnerImpl",
- * as this refers to the JavacRunnerImpl type loaded with the
- * default classloader. Therefore, we'd have to address each
- * of the different JavacRunnerImpls as "Object" and invoke
- * its method via reflection.
- *
- * We can circumvent this problem by declaring an interface
- * that JavacRunnerImpl implements (i.e. JavacRunner).
- * If we always load this super-interface in the default
- * classloader, and make each JavacRunnerImpl (loaded in its
- * own classloader) implement it, we can refer to the
- * JavacRunnerImpls as "JavacRunner"s in the main program.
- * That way, we can avoid using reflection and "Object"
- * to deal with the different JavacRunnerImpls.
*/
public interface JavacRunner {
- Result invokeJavac(String[] args, PrintWriter output);
-
+ Result invokeJavac(
+ ImmutableList<BlazeJavaCompilerPlugin> plugins, String[] args, PrintWriter output);
}
diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/JavacRunnerImpl.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/JavacRunnerImpl.java
deleted file mode 100644
index f3eb689..0000000
--- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/JavacRunnerImpl.java
+++ /dev/null
@@ -1,47 +0,0 @@
-// Copyright 2014 The Bazel Authors. 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.javac;
-
-import com.google.devtools.build.buildjar.javac.plugins.BlazeJavaCompilerPlugin;
-
-import com.sun.tools.javac.main.Main.Result;
-
-import java.io.PrintWriter;
-import java.util.List;
-
-/**
- * This class wraps a single invocation of Javac. We
- * invoke javac statically but wrap it with a synchronization.
- * This is because the same javac cannot be invoked multiple
- * times in parallel.
- */
-public class JavacRunnerImpl implements JavacRunner {
-
- private final List<BlazeJavaCompilerPlugin> plugins;
-
- /**
- * Passes extra information to BlazeJavacMain in case strict Java
- * dependencies are enforced.
- */
- public JavacRunnerImpl(List<BlazeJavaCompilerPlugin> plugins) {
- this.plugins = plugins;
- }
-
- @Override
- public synchronized Result invokeJavac(String[] args, PrintWriter output) {
- return new BlazeJavacMain(output, plugins).compile(args);
- }
-
-}
