Permalink
Browse files

Invoke javac using the API, instead of main()

This unifies the test and production javac invocations. Location arguments
(e.g. classpath, sources) are now set programatically from Paths, instead
of going through string args.

The classloader masking plugin is now just a custom filemanager, since javac
uses the same context for the entire compilation and we don't need a plugin
to carry it across annotation processing rounds.

--
PiperOrigin-RevId: 144110025
MOS_MIGRATED_REVID=144110025
  • Loading branch information...
1 parent 4d3dbbc commit 3c5e55ff8e058b624ce26e803ff00434c70d4b91 @cushon cushon committed with mhlopko Jan 10, 2017
Showing with 348 additions and 306 deletions.
  1. +5 −2 src/java_tools/buildjar/java/com/google/devtools/build/buildjar/AbstractJavaBuilder.java
  2. +1 −1 src/java_tools/buildjar/java/com/google/devtools/build/buildjar/AbstractLibraryBuilder.java
  3. +1 −1 src/java_tools/buildjar/java/com/google/devtools/build/buildjar/BUILD
  4. +0 −2 src/java_tools/buildjar/java/com/google/devtools/build/buildjar/BazelJavaBuilder.java
  5. +0 −54 src/java_tools/buildjar/java/com/google/devtools/build/buildjar/CommonJavaLibraryProcessor.java
  6. +106 −1 src/java_tools/buildjar/java/com/google/devtools/build/buildjar/JavaLibraryBuildRequest.java
  7. +23 −1 src/java_tools/buildjar/java/com/google/devtools/build/buildjar/OptionsParser.java
  8. +8 −4 ...va_tools/buildjar/java/com/google/devtools/build/buildjar/ReducedClasspathJavaLibraryBuilder.java
  9. +2 −75 src/java_tools/buildjar/java/com/google/devtools/build/buildjar/SimpleJavaLibraryBuilder.java
  10. +94 −0 src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/BlazeJavacArguments.java
  11. +104 −89 src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/BlazeJavacMain.java
  12. +3 −1 src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/JavacRunner.java
  13. +0 −10 src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/BUILD
  14. +0 −65 ...r/java/com/google/devtools/build/buildjar/javac/plugins/classloader/ClassLoaderMaskingPlugin.java
  15. +1 −0 ...s/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/dependency/DependencyModule.java
@@ -17,6 +17,7 @@
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.BlazeJavacArguments;
import com.google.devtools.build.buildjar.javac.BlazeJavacMain;
import com.google.devtools.build.buildjar.javac.JavacRunner;
import com.google.devtools.build.buildjar.javac.plugins.BlazeJavaCompilerPlugin;
@@ -60,8 +61,10 @@ public Result compileJavaLibrary(final JavaLibraryBuildRequest build, final Prin
new JavacRunner() {
@Override
public Result invokeJavac(
- ImmutableList<BlazeJavaCompilerPlugin> plugins, String[] args, PrintWriter output) {
- return new BlazeJavacMain(output, plugins).compile(args);
+ ImmutableList<BlazeJavaCompilerPlugin> plugins,
+ BlazeJavacArguments arguments,
+ PrintWriter output) {
+ return new BlazeJavacMain(output, plugins).compile(arguments);
}
};
Result result = compileSources(build, javacRunner, err);
@@ -34,7 +34,7 @@
*
* <p>Implements common functionality like source files preparation and output jar creation.
*/
-public abstract class AbstractLibraryBuilder extends CommonJavaLibraryProcessor {
+public abstract class AbstractLibraryBuilder {
/**
* Prepares a compilation run. This involves cleaning up temporary directories and writing the
@@ -37,6 +37,7 @@ java_library(
":invalid_command_line_exception",
":javac_options",
"//src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins",
+ "//third_party:auto_value",
"//third_party:guava",
"//third_party:jsr305",
"//third_party/java/jdk/langtools:javac",
@@ -63,7 +64,6 @@ java_library(
":javac_options",
"//src/java_tools/buildjar/java/com/google/devtools/build/buildjar/jarhelper",
"//src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins",
- "//src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins:classloader",
"//src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins:dependency",
"//src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins:errorprone",
"//src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins:processing",
@@ -19,7 +19,6 @@
import com.google.devtools.build.buildjar.instrumentation.JacocoInstrumentationProcessor;
import com.google.devtools.build.buildjar.javac.JavacOptions;
import com.google.devtools.build.buildjar.javac.plugins.BlazeJavaCompilerPlugin;
-import com.google.devtools.build.buildjar.javac.plugins.classloader.ClassLoaderMaskingPlugin;
import com.google.devtools.build.buildjar.javac.plugins.dependency.DependencyModule;
import com.google.devtools.build.buildjar.javac.plugins.errorprone.ErrorPronePlugin;
import com.google.devtools.build.lib.worker.WorkerProtocol.WorkRequest;
@@ -133,7 +132,6 @@ public static JavaLibraryBuildRequest parse(List<String> args)
throws IOException, InvalidCommandLineException {
OptionsParser optionsParser = new OptionsParser(args);
ImmutableList.Builder<BlazeJavaCompilerPlugin> plugins = ImmutableList.builder();
- plugins.add(new ClassLoaderMaskingPlugin());
// Support for -extra_checks:off was removed from ErrorPronePlugin, but Bazel still needs it,
// so we'll emulate support for this here by handling the flag ourselves and not loading the
@@ -1,54 +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;
-
-import java.io.File;
-import java.util.ArrayList;
-import java.util.List;
-
-/**
- * Superclass for all JavaBuilder processor classes involved in compiling and processing java code.
- */
-public abstract class CommonJavaLibraryProcessor {
-
- /**
- * 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.
- *
- * @param build The build request for the initial set of arguments is needed
- * @return The list of initial arguments
- */
- protected List<String> createInitialJavacArgs(JavaLibraryBuildRequest build, String classPath) {
- List<String> args = new ArrayList<>();
- if (!classPath.isEmpty()) {
- args.add("-cp");
- args.add(classPath);
- }
- args.add("-d");
- args.add(build.getClassDir());
-
- // Add an empty source path to prevent javac from sucking in source files
- // from .jar files on the classpath.
- args.add("-sourcepath");
- args.add(File.pathSeparator);
-
- if (build.getExtdir() != null) {
- args.add("-extdirs");
- args.add(build.getExtdir());
- }
-
- return args;
- }
-}
@@ -14,11 +14,20 @@
package com.google.devtools.build.buildjar;
+import com.google.common.base.Joiner;
+import com.google.common.base.Splitter;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
+import com.google.devtools.build.buildjar.javac.BlazeJavacArguments;
import com.google.devtools.build.buildjar.javac.plugins.BlazeJavaCompilerPlugin;
import com.google.devtools.build.buildjar.javac.plugins.dependency.DependencyModule;
import com.google.devtools.build.buildjar.javac.plugins.processing.AnnotationProcessingModule;
+import java.io.File;
+import java.io.IOError;
import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.List;
import java.util.Map.Entry;
@@ -45,7 +54,7 @@
private final ImmutableList<String> rootResourceFiles;
private final String classPath;
-
+ private final String bootClassPath;
private final String extdir;
private final String processorPath;
@@ -142,6 +151,7 @@ public JavaLibraryBuildRequest(
this.resourceJars = ImmutableList.copyOf(optionsParser.getResourceJars());
this.rootResourceFiles = ImmutableList.copyOf(optionsParser.getRootResourceFiles());
this.classPath = optionsParser.getClassPath();
+ this.bootClassPath = optionsParser.getBootClassPath();
this.extdir = optionsParser.getExtdir();
this.processorPath = optionsParser.getProcessorPath();
this.processorNames = optionsParser.getProcessorNames();
@@ -218,6 +228,10 @@ public String getClassPath() {
return classPath;
}
+ public String getBootClassPath() {
+ return bootClassPath;
+ }
+
public String getExtdir() {
return extdir;
}
@@ -263,4 +277,95 @@ public AnnotationProcessingModule getProcessingModule() {
public ImmutableList<BlazeJavaCompilerPlugin> getPlugins() {
return plugins;
}
+
+ public BlazeJavacArguments toBlazeJavacArguments(final String classPath) {
+ return BlazeJavacArguments.builder()
+ .classPath(toPaths(classPath))
+ .classOutput(Paths.get(getClassDir()))
+ .bootClassPath(
+ ImmutableList.copyOf(Iterables.concat(toPaths(getBootClassPath()), getExtJars())))
+ .javacOptions(makeJavacArguments())
+ .sourceFiles(toPaths(getSourceFiles()))
+ .processors(null)
+ .sourceOutput(getSourceGenDir() != null ? Paths.get(getSourceGenDir()) : null)
+ .processorPath(toPaths(getProcessorPath()))
+ .build();
+ }
+
+ // TODO(cushon): make Bazel pass the individual files instead of inferring a directory and
+ // listing it here
+ List<Path> getExtJars() {
+ if (getExtdir() == null) {
+ return ImmutableList.of();
+ }
+ ImmutableList.Builder<Path> jars = ImmutableList.builder();
+ for (String file : Splitter.on(File.pathSeparatorChar).split(getExtdir())) {
+ try {
+ Path path = Paths.get(file);
+ if (Files.isDirectory(path)) {
+ Files.list(path).forEach(jars::add);
+ } else {
+ jars.add(path);
+ }
+ } catch (IOException e) {
+ throw new IOError(e);
+ }
+ }
+ return jars.build();
+ }
+
+ static ImmutableList<Path> toPaths(List<String> files) {
+ if (files == null) {
+ return ImmutableList.of();
+ }
+ ImmutableList.Builder<Path> result = ImmutableList.builder();
+ for (String e : files) {
+ result.add(Paths.get(e));
+ }
+ return result.build();
+ }
+
+ static ImmutableList<Path> toPaths(String classPath) {
+ if (classPath == null) {
+ return ImmutableList.of();
+ }
+ ImmutableList.Builder<Path> result = ImmutableList.builder();
+ for (String e : Splitter.on(File.pathSeparatorChar).split(classPath)) {
+ result.add(Paths.get(e));
+ }
+ return result.build();
+ }
+
+ /** Constructs a command line that can be used for a javac invocation. */
+ ImmutableList<String> makeJavacArguments() {
+ ImmutableList.Builder<String> javacArguments = ImmutableList.builder();
+ javacArguments.addAll(getJavacOpts());
+
+ if (!getProcessors().isEmpty() && !getSourceFiles().isEmpty()) {
+ // ImmutableSet.copyOf maintains order
+ ImmutableSet<String> deduplicatedProcessorNames = ImmutableSet.copyOf(getProcessors());
+ javacArguments.add("-processor");
+ javacArguments.add(Joiner.on(',').join(deduplicatedProcessorNames));
+ } else {
+ // This is necessary because some jars contain discoverable annotation processors that
+ // previously didn't run, and they break builds if the "-proc:none" option is not passed to
+ // javac.
+ javacArguments.add("-proc:none");
+ }
+
+ for (String option : getJavacOpts()) {
+ if (option.startsWith("-J")) { // ignore the VM options.
+ continue;
+ }
+ if (option.equals("-processor") || option.equals("-processorpath")) {
+ throw new IllegalStateException(
+ "Using "
+ + option
+ + " in javacopts is no longer supported."
+ + " Use a java_plugin() rule instead.");
+ }
+ }
+
+ return javacArguments.build();
+ }
}
@@ -27,6 +27,7 @@
import java.util.Deque;
import java.util.HashMap;
import java.util.HashSet;
+import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
@@ -64,7 +65,7 @@
private final List<String> rootResourceFiles = new ArrayList<>();
private String classPath = "";
-
+ private String bootClassPath;
private String extdir;
private String processorPath = "";
@@ -107,6 +108,7 @@ private void processCommandlineArgs(Deque<String> argQueue) throws InvalidComman
// otherwise we have to do something like adding a "--"
// terminator to the passed arguments.
collectFlagArguments(javacOpts, argQueue, "--");
+ bootClassPathFromJavacOpts();
break;
case "--direct_dependency":
{
@@ -167,6 +169,9 @@ private void processCommandlineArgs(Deque<String> argQueue) throws InvalidComman
case "--classpath":
classPath = getArgument(argQueue, arg);
break;
+ case "--bootclasspath":
+ bootClassPath = getArgument(argQueue, arg);
+ break;
case "--processorpath":
processorPath = getArgument(argQueue, arg);
break;
@@ -314,6 +319,19 @@ private void addExternalPostProcessor(Deque<String> args, String arg)
postProcessors.put(processorName, arguments);
}
+ // TODO(cushon): update Blaze to set --bootclasspath directly
+ private void bootClassPathFromJavacOpts() {
+ Iterator<String> it = javacOpts.iterator();
+ while (it.hasNext()) {
+ String curr = it.next();
+ if (curr.equals("-bootclasspath") && it.hasNext()) {
+ it.remove();
+ bootClassPath = it.next();
+ it.remove();
+ }
+ }
+ }
+
public List<String> getJavacOpts() {
return javacOpts;
}
@@ -386,6 +404,10 @@ public String getClassPath() {
return classPath;
}
+ public String getBootClassPath() {
+ return bootClassPath;
+ }
+
public String getExtdir() {
return extdir;
}
@@ -56,12 +56,15 @@ Result compileSources(JavaLibraryBuildRequest build, JavacRunner javacRunner, Pr
// class output directory to prevent that from happening.
compressedClasspath = build.getClassDir();
}
- String[] javacArguments = makeJavacArguments(build, compressedClasspath);
// Compile!
StringWriter javacOutput = new StringWriter();
PrintWriter javacOutputWriter = new PrintWriter(javacOutput);
- Result result = javacRunner.invokeJavac(build.getPlugins(), javacArguments, javacOutputWriter);
+ Result result =
+ javacRunner.invokeJavac(
+ build.getPlugins(),
+ build.toBlazeJavacArguments(compressedClasspath),
+ javacOutputWriter);
javacOutputWriter.close();
// If javac errored out because of missing entries on the classpath, give it another try.
@@ -75,8 +78,9 @@ Result compileSources(JavaLibraryBuildRequest build, JavacRunner javacRunner, Pr
prepareSourceCompilation(build);
// Fall back to the regular compile, but add extra checks to catch transitive uses
- javacArguments = makeJavacArguments(build);
- result = javacRunner.invokeJavac(build.getPlugins(), javacArguments, err);
+ result =
+ javacRunner.invokeJavac(
+ build.getPlugins(), build.toBlazeJavacArguments(build.getClassPath()), err);
} else {
err.print(javacOutput.getBuffer());
}
Oops, something went wrong.

0 comments on commit 3c5e55f

Please sign in to comment.