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 fe9df0d761ce82..5edffe202482bd 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 @@ -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 plugins, String[] args, PrintWriter output) { - return new BlazeJavacMain(output, plugins).compile(args); + ImmutableList plugins, + BlazeJavacArguments arguments, + PrintWriter output) { + return new BlazeJavacMain(output, plugins).compile(arguments); } }; Result result = compileSources(build, javacRunner, err); diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/AbstractLibraryBuilder.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/AbstractLibraryBuilder.java index 86a5b2db22980a..efac18a705382e 100644 --- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/AbstractLibraryBuilder.java +++ b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/AbstractLibraryBuilder.java @@ -34,7 +34,7 @@ * *

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 diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/BUILD b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/BUILD index e603285c188162..5e948d2ee90030 100644 --- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/BUILD +++ b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/BUILD @@ -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", 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 f3765928234dd7..b1bd54dedf6dd3 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 @@ -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 args) throws IOException, InvalidCommandLineException { OptionsParser optionsParser = new OptionsParser(args); ImmutableList.Builder 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 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 deleted file mode 100644 index 5d4dba62465f9f..00000000000000 --- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/CommonJavaLibraryProcessor.java +++ /dev/null @@ -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 createInitialJavacArgs(JavaLibraryBuildRequest build, String classPath) { - List 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; - } -} diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/JavaLibraryBuildRequest.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/JavaLibraryBuildRequest.java index e77617bdd421dc..8b4e947140346f 100644 --- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/JavaLibraryBuildRequest.java +++ b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/JavaLibraryBuildRequest.java @@ -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 @@ public final class JavaLibraryBuildRequest { private final ImmutableList 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 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 getExtJars() { + if (getExtdir() == null) { + return ImmutableList.of(); + } + ImmutableList.Builder 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 toPaths(List files) { + if (files == null) { + return ImmutableList.of(); + } + ImmutableList.Builder result = ImmutableList.builder(); + for (String e : files) { + result.add(Paths.get(e)); + } + return result.build(); + } + + static ImmutableList toPaths(String classPath) { + if (classPath == null) { + return ImmutableList.of(); + } + ImmutableList.Builder 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 makeJavacArguments() { + ImmutableList.Builder javacArguments = ImmutableList.builder(); + javacArguments.addAll(getJavacOpts()); + + if (!getProcessors().isEmpty() && !getSourceFiles().isEmpty()) { + // ImmutableSet.copyOf maintains order + ImmutableSet 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(); + } } diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/OptionsParser.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/OptionsParser.java index fecbf3c6b00fd4..7815284fd6d7fb 100644 --- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/OptionsParser.java +++ b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/OptionsParser.java @@ -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 @@ public final class OptionsParser { private final List rootResourceFiles = new ArrayList<>(); private String classPath = ""; - + private String bootClassPath; private String extdir; private String processorPath = ""; @@ -107,6 +108,7 @@ private void processCommandlineArgs(Deque 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 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 args, String arg) postProcessors.put(processorName, arguments); } + // TODO(cushon): update Blaze to set --bootclasspath directly + private void bootClassPathFromJavacOpts() { + Iterator 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 getJavacOpts() { return javacOpts; } @@ -386,6 +404,10 @@ public String getClassPath() { return classPath; } + public String getBootClassPath() { + return bootClassPath; + } + public String getExtdir() { return extdir; } 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 7c1186cf03b6d6..8e29cf16448d9f 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 @@ -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()); } 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 13939ad1877c8d..4080cbfbb6af01 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 @@ -14,16 +14,12 @@ package com.google.devtools.build.buildjar; -import com.google.common.base.Joiner; -import com.google.common.collect.ImmutableSet; import com.google.devtools.build.buildjar.jarhelper.JarCreator; import com.google.devtools.build.buildjar.javac.JavacRunner; import com.sun.tools.javac.main.Main.Result; import java.io.File; import java.io.IOException; import java.io.PrintWriter; -import java.util.ArrayList; -import java.util.List; /** An implementation of the JavaBuilder that uses in-process javac to compile java files. */ public class SimpleJavaLibraryBuilder extends AbstractJavaBuilder { @@ -31,8 +27,8 @@ public class SimpleJavaLibraryBuilder extends AbstractJavaBuilder { @Override Result compileSources(JavaLibraryBuildRequest build, JavacRunner javacRunner, PrintWriter err) throws IOException { - String[] javacArguments = makeJavacArguments(build, build.getClassPath()); - return javacRunner.invokeJavac(build.getPlugins(), javacArguments, err); + return javacRunner.invokeJavac( + build.getPlugins(), build.toBlazeJavacArguments(build.getClassPath()), err); } @Override @@ -53,75 +49,6 @@ protected void prepareSourceCompilation(JavaLibraryBuildRequest build) throws IO } } - /** - * For the build configuration 'build', construct a command line that can be used for a javac - * invocation. - */ - protected String[] makeJavacArguments(JavaLibraryBuildRequest build) { - return makeJavacArguments(build, build.getClassPath()); - } - - /** - * For the build configuration 'build', construct a command line that can be used for a javac - * invocation. - */ - protected String[] makeJavacArguments(JavaLibraryBuildRequest build, String classPath) { - List javacArguments = createInitialJavacArgs(build, classPath); - - javacArguments.addAll(getAnnotationProcessingOptions(build)); - - for (String option : build.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."); - } - javacArguments.add(option); - } - - javacArguments.addAll(build.getSourceFiles()); - return javacArguments.toArray(new String[0]); - } - - /** - * Given a JavaLibraryBuildRequest, computes the javac options for the annotation processing - * requested. - */ - private List getAnnotationProcessingOptions(JavaLibraryBuildRequest build) { - List args = new ArrayList<>(); - - // Javac treats "-processorpath ''" as setting the processor path to an empty list, - // whereas omitting the option is treated as not having a processor path (which causes - // processor path searches to fallback to the class path). - args.add("-processorpath"); - args.add(build.getProcessorPath().isEmpty() ? "" : build.getProcessorPath()); - - if (!build.getProcessors().isEmpty() && !build.getSourceFiles().isEmpty()) { - // ImmutableSet.copyOf maintains order - ImmutableSet deduplicatedProcessorNames = ImmutableSet.copyOf(build.getProcessors()); - args.add("-processor"); - args.add(Joiner.on(',').join(deduplicatedProcessorNames)); - - // Set javac output directory for generated sources. - if (build.getSourceGenDir() != null) { - args.add("-s"); - args.add(build.getSourceGenDir()); - } - } 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. - args.add("-proc:none"); - } - - return args; - } - @Override public void buildGensrcJar(JavaLibraryBuildRequest build) throws IOException { JarCreator jar = new JarCreator(build.getGeneratedSourcesOutputJar()); diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/BlazeJavacArguments.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/BlazeJavacArguments.java new file mode 100644 index 00000000000000..7554ddc2e1843a --- /dev/null +++ b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/BlazeJavacArguments.java @@ -0,0 +1,94 @@ +// Copyright 2017 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.auto.value.AutoValue; +import com.google.common.collect.ImmutableList; +import java.nio.file.Path; +import javax.annotation.Nullable; +import javax.annotation.processing.Processor; + +/** + * Arguments to a single compilation performed by {@link BlazeJavacMain}. + * + *

This includes a subset of arguments to {@link javax.tools.JavaCompiler#getTask} and {@link + * javax.tools.StandardFileManager#setLocation} for a single compilation, with sensible defaults and + * a builder. + */ +@AutoValue +public abstract class BlazeJavacArguments { + /** The sources to compile. */ + public abstract ImmutableList sourceFiles(); + + /** Javac options, not including location settings. */ + public abstract ImmutableList javacOptions(); + + /** The compilation classpath. */ + public abstract ImmutableList classPath(); + + /** The compilation bootclasspath. */ + public abstract ImmutableList bootClassPath(); + + /** The classpath to load processors from. */ + public abstract ImmutableList processorPath(); + + /** + * Annotation processor classes. In production builds, processors are specified by string class + * name in {@link javacOptions}; this is used for tests that instantate processors directly. + */ + @Nullable + public abstract ImmutableList processors(); + + /** The class output directory (-d). */ + public abstract Path classOutput(); + + /** The generated source output directory (-s). */ + @Nullable + public abstract Path sourceOutput(); + + public static Builder builder() { + return new AutoValue_BlazeJavacArguments.Builder() + .classPath(ImmutableList.of()) + .classOutput(null) + .bootClassPath(ImmutableList.of()) + .javacOptions(ImmutableList.of()) + .sourceFiles(ImmutableList.of()) + .processors(null) + .sourceOutput(null) + .processorPath(ImmutableList.of()); + } + + /** {@link BlazeJavacArguments}Builder. */ + @AutoValue.Builder + public interface Builder { + Builder classPath(ImmutableList classPath); + + Builder classOutput(Path classOutput); + + Builder bootClassPath(ImmutableList bootClassPath); + + Builder javacOptions(ImmutableList javacOptions); + + Builder sourceFiles(ImmutableList sourceFiles); + + Builder processors(ImmutableList processors); + + Builder sourceOutput(Path sourceOutput); + + Builder processorPath(ImmutableList processorPath); + + BlazeJavacArguments build(); + } +} diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/BlazeJavacMain.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/BlazeJavacMain.java index de6f0816cb3fca..77c84952902740 100644 --- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/BlazeJavacMain.java +++ b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/BlazeJavacMain.java @@ -14,32 +14,38 @@ package com.google.devtools.build.buildjar.javac; +import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Verify.verifyNotNull; +import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Function; -import com.google.common.base.Preconditions; import com.google.common.base.Verify; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.Iterables; import com.google.devtools.build.buildjar.InvalidCommandLineException; import com.google.devtools.build.buildjar.javac.plugins.BlazeJavaCompilerPlugin; import com.google.devtools.build.buildjar.javac.plugins.BlazeJavaCompilerPlugin.PluginException; import com.sun.source.util.TaskEvent; import com.sun.source.util.TaskListener; +import com.sun.tools.javac.api.ClientCodeWrapper.Trusted; import com.sun.tools.javac.api.JavacTaskImpl; import com.sun.tools.javac.api.JavacTool; import com.sun.tools.javac.api.MultiTaskListener; -import com.sun.tools.javac.main.Main; +import com.sun.tools.javac.file.JavacFileManager; import com.sun.tools.javac.main.Main.Result; import com.sun.tools.javac.util.Context; -import com.sun.tools.javac.util.Options; import com.sun.tools.javac.util.PropagatedException; +import java.io.IOError; +import java.io.IOException; import java.io.PrintWriter; -import java.util.Arrays; +import java.net.URL; +import java.net.URLClassLoader; +import java.nio.file.Path; import java.util.List; -import javax.annotation.processing.Processor; import javax.tools.DiagnosticListener; -import javax.tools.JavaFileManager; import javax.tools.JavaFileObject; +import javax.tools.StandardLocation; /** * Main class for our custom patched javac. @@ -61,11 +67,9 @@ public class BlazeJavacMain { private List plugins; private final PrintWriter errOutput; - private final String compilerName; private BlazeJavaCompiler compiler = null; public BlazeJavacMain(PrintWriter errOutput, List plugins) { - this.compilerName = "blaze javac"; this.errOutput = errOutput; this.plugins = plugins; } @@ -76,8 +80,13 @@ public BlazeJavacMain(PrintWriter errOutput, List plugi * * @param context JavaCompiler's associated Context */ + @VisibleForTesting void setupBlazeJavaCompiler(Context context) { - preRegister(context, plugins); + for (BlazeJavaCompilerPlugin plugin : plugins) { + plugin.initializeContext(context); + } + BlazeJavaCompiler.preRegister(context, plugins, compilerListener); + enableEndPositions(context); } private final Function compilerListener = @@ -85,65 +94,59 @@ void setupBlazeJavaCompiler(Context context) { @Override public Void apply(BlazeJavaCompiler compiler) { Verify.verify(BlazeJavacMain.this.compiler == null); - BlazeJavacMain.this.compiler = Preconditions.checkNotNull(compiler); + BlazeJavacMain.this.compiler = checkNotNull(compiler); return null; } }; - public void preRegister(Context context, List plugins) { - this.plugins = plugins; - for (BlazeJavaCompilerPlugin plugin : plugins) { - plugin.initializeContext(context); - } - BlazeJavaCompiler.preRegister(context, plugins, compilerListener); + public Result compile(BlazeJavacArguments arguments) { + return compile(null, arguments); } - public Result compile(String[] argv) { - // set up a fresh Context with our custom bindings for JavaCompiler - Context context = new Context(); - - Options options = Options.instance(context); - - // enable Java 8-style type inference features - // - // This is currently duplicated in JAVABUILDER. That's deliberate for now, because - // (1) JavaBuilder's integration test coverage for default options isn't very good, and - // (2) the migration from JAVABUILDER to java_toolchain configs is in progress so blaze - // integration tests for defaults options are also not trustworthy. - // - // TODO(bazel-team): removed duplication with JAVABUILDER - options.put("usePolyAttribution", "true"); - options.put("useStrictMethodClashCheck", "true"); - options.put("useStructuralMostSpecificResolution", "true"); - options.put("useGraphInference", "true"); + public Result compile( + DiagnosticListener diagnosticListener, BlazeJavacArguments arguments) { - String[] processedArgs; + JavacFileManager fileManager = new ClassloaderMaskingFileManager(); + List javacArguments = arguments.javacOptions(); try { - processedArgs = processPluginArgs(argv); + javacArguments = processPluginArgs(javacArguments); } catch (InvalidCommandLineException e) { errOutput.println(e.getMessage()); return Result.CMDERR; } + Context context = new Context(); setupBlazeJavaCompiler(context); - return compile(processedArgs, context); - } - @VisibleForTesting - public Result compile(String[] argv, Context context) { - enableEndPositions(context); Result result = Result.ABNORMAL; try { - result = new Main(compilerName, errOutput).compile(argv, context); + JavacTool tool = JavacTool.create(); + JavacTaskImpl task = + (JavacTaskImpl) + tool.getTask( + errOutput, + fileManager, + diagnosticListener, + javacArguments, + ImmutableList.of() /*classes*/, + fileManager.getJavaFileObjectsFromPaths(arguments.sourceFiles()), + context); + if (arguments.processors() != null) { + task.setProcessors(arguments.processors()); + } + fileManager.setContext(context); + setLocations(fileManager, arguments); + result = task.doCall(); } catch (PropagatedException e) { if (e.getCause() instanceof PluginException) { PluginException pluginException = (PluginException) e.getCause(); errOutput.println(pluginException.getMessage()); - return pluginException.getResult(); + result = pluginException.getResult(); + } else { + e.printStackTrace(errOutput); + result = Result.ABNORMAL; } - e.printStackTrace(errOutput); - result = Result.ABNORMAL; } finally { if (result.isOK()) { verifyNotNull(compiler); @@ -161,47 +164,6 @@ public Result compile(String[] argv, Context context) { return result; } - // javac9 removes the ability to pass lists of {@link JavaFileObject}s or {@link Processors}s to - // it's 'Main' class (i.e. the entry point for command-line javac). Having BlazeJavacMain - // continue to call into javac's Main has the nice property that it keeps JavaBuilder's - // behaviour closer to stock javac, but it makes it harder to write integration tests. This class - // provides a compile method that accepts file objects and processors, but it isn't - // guaranteed to behave exactly the same way as JavaBuilder does when used from the command-line. - // TODO(b/26750160): either stop using Main and commit to using the the API for everything, or - // re-write integration tests for JavaBuilder to use the real compile() method. - @VisibleForTesting - @Deprecated - public Result compile( - String[] argv, - Context context, - JavaFileManager fileManager, - DiagnosticListener diagnosticListener, - List javaFileObjects, - Iterable processors) { - - JavacTool tool = JavacTool.create(); - JavacTaskImpl task = - (JavacTaskImpl) - tool.getTask( - errOutput, - fileManager, - diagnosticListener, - Arrays.asList(argv), - null, - javaFileObjects, - context); - if (processors != null) { - task.setProcessors(processors); - } - - try { - return task.doCall(); - } catch (PluginException e) { - errOutput.println(e.getMessage()); - return e.getResult(); - } - } - private static final TaskListener EMPTY_LISTENER = new TaskListener() { @Override @@ -221,16 +183,69 @@ private static void enableEndPositions(Context context) { /** Processes Plugin-specific arguments and removes them from the args array. */ @VisibleForTesting - String[] processPluginArgs(String[] args) throws InvalidCommandLineException { - List processedArgs = Arrays.asList(args); + List processPluginArgs(List args) throws InvalidCommandLineException { + List processedArgs = args; for (BlazeJavaCompilerPlugin plugin : plugins) { processedArgs = plugin.processArgs(processedArgs); } - return processedArgs.toArray(new String[processedArgs.size()]); + return processedArgs; } @VisibleForTesting BlazeJavaCompiler getCompiler() { return verifyNotNull(compiler); } + + private void setLocations(JavacFileManager fileManager, BlazeJavacArguments arguments) { + try { + fileManager.setLocationFromPaths(StandardLocation.CLASS_PATH, arguments.classPath()); + fileManager.setLocationFromPaths( + StandardLocation.CLASS_OUTPUT, ImmutableList.of(arguments.classOutput())); + fileManager.setLocationFromPaths(StandardLocation.SOURCE_PATH, ImmutableList.of()); + Iterable bootClassPath = arguments.bootClassPath(); + if (!Iterables.isEmpty(bootClassPath)) { + fileManager.setLocationFromPaths(StandardLocation.PLATFORM_CLASS_PATH, bootClassPath); + } + fileManager.setLocationFromPaths( + StandardLocation.ANNOTATION_PROCESSOR_PATH, arguments.processorPath()); + if (arguments.sourceOutput() != null) { + fileManager.setLocationFromPaths( + StandardLocation.SOURCE_OUTPUT, ImmutableList.of(arguments.sourceOutput())); + } + } catch (IOException e) { + throw new IOError(e); + } + } + + /** + * When Bazel invokes JavaBuilder, it puts javac.jar on the bootstrap class path and + * JavaBuilder_deploy.jar on the user class path. We need Error Prone to be available on the + * annotation processor path, but we want to mask out any other classes to minimize class version + * skew. + */ + @Trusted + private static class ClassloaderMaskingFileManager extends JavacFileManager { + + public ClassloaderMaskingFileManager() { + super(new Context(), false, UTF_8); + } + + @Override + protected ClassLoader getClassLoader(URL[] urls) { + return new URLClassLoader( + urls, + new ClassLoader(JavacFileManager.class.getClassLoader()) { + @Override + protected Class findClass(String name) throws ClassNotFoundException { + if (name.startsWith("com.google.errorprone.")) { + return Class.forName(name); + } else if (name.startsWith("org.checkerframework.dataflow.")) { + return Class.forName(name); + } else { + throw new ClassNotFoundException(name); + } + } + }); + } + } } 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 992ddf689dcc34..11f72b2f2d6735 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 @@ -26,5 +26,7 @@ public interface JavacRunner { Result invokeJavac( - ImmutableList plugins, String[] args, PrintWriter output); + ImmutableList plugins, + BlazeJavacArguments arguments, + PrintWriter err); } diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/BUILD b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/BUILD index f3ae58e9d63fb5..c7c76e4f01a1cc 100644 --- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/BUILD +++ b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/BUILD @@ -55,16 +55,6 @@ java_library( ], ) -java_library( - name = "classloader", - srcs = glob(["classloader/*.java"]), - deps = [ - "//src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins", - "//third_party:guava", - "//third_party/java/jdk/langtools:javac", - ], -) - # ## Bootstrapping using Skylark rules # diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/classloader/ClassLoaderMaskingPlugin.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/classloader/ClassLoaderMaskingPlugin.java deleted file mode 100644 index d77ea8bc6d6ad2..00000000000000 --- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/classloader/ClassLoaderMaskingPlugin.java +++ /dev/null @@ -1,65 +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.javac.plugins.classloader; - -import com.google.devtools.build.buildjar.javac.plugins.BlazeJavaCompilerPlugin; -import com.sun.tools.javac.file.JavacFileManager; -import com.sun.tools.javac.util.Context; -import java.net.URL; -import java.net.URLClassLoader; -import javax.tools.JavaFileManager; - -/** A plugin that customizes the Java compiler for the Error Prone plugin. */ -public final class ClassLoaderMaskingPlugin extends BlazeJavaCompilerPlugin { - - @Override - public void initializeContext(Context context) { - context.put( - JavaFileManager.class, - new Context.Factory() { - @Override - public JavaFileManager make(Context c) { - return new JavacFileManager(c, true, null) { - @Override - protected ClassLoader getClassLoader(URL[] urls) { - return new URLClassLoader(urls, makeMaskedClassLoader()); - } - }; - } - }); - super.initializeContext(context); - } - - /** - * When Bazel invokes JavaBuilder, it puts javac.jar on the bootstrap class path and - * JavaBuilder_deploy.jar on the user class path. We need Error Prone to be available on the - * annotation processor path, but we want to mask out any other classes to minimize class version - * skew. - */ - private ClassLoader makeMaskedClassLoader() { - return new ClassLoader(JavacFileManager.class.getClassLoader()) { - @Override - protected Class findClass(String name) throws ClassNotFoundException { - if (name.startsWith("com.google.errorprone.")) { - return Class.forName(name); - } else if (name.startsWith("org.checkerframework.dataflow.")) { - return Class.forName(name); - } else { - throw new ClassNotFoundException(name); - } - } - }; - } -} diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/dependency/DependencyModule.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/dependency/DependencyModule.java index c35d0d756c0fdf..79a8c1fb70a3b8 100644 --- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/dependency/DependencyModule.java +++ b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/dependency/DependencyModule.java @@ -286,6 +286,7 @@ public List computeStrictClasspath(Iterable originalClasspath) } @VisibleForTesting + // TODO(cushon): use Paths instead of strings, or inject a FileSystem void setStrictClasspath(Set strictClasspath) { this.requiredClasspath = strictClasspath; }