diff options
8 files changed, 161 insertions, 170 deletions
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 ee748b3f03..87c15f985d 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 @@ -25,11 +25,17 @@ java_library( java_library( name = "javac", - srcs = glob([ - "javac/*.java", - ]), + srcs = glob( + [ + "javac/*.java", + ], + exclude = [ + "javac/JavacOptions.java", + ], + ), deps = [ ":invalid_command_line_exception", + ":javac_options", "//src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins", "//third_party:guava", "//third_party:jsr305", @@ -51,6 +57,7 @@ java_library( ":JarOwner", ":invalid_command_line_exception", ":javac", + ":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", @@ -75,8 +82,13 @@ java_library( srcs = [ "javac/JavacOptions.java", ], - visibility = ["//java/com/google/devtools/build/buildjar:__pkg__"], + visibility = [ + "//java/com/google/devtools/build/buildjar:__pkg__", + "//src/java_tools/buildjar/java/com/google/devtools/build/java/bazel:__pkg__", + "//src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac:__pkg__", + ], deps = [ + "//third_party:auto_value", "//third_party:guava", ], ) @@ -112,6 +124,7 @@ load("//tools/build_rules:java_rules_skylark.bzl", "bootstrap_java_library", "bo bootstrap_java_library( name = "skylark-deps", jars = [ + "//third_party:auto_value-jars", "//third_party:error_prone-jars", "//third_party:guava-jars", "//third_party:jsr305-jars", diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/JavacOptions.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/JavacOptions.java index e4452a36a0..4f7e749c7e 100644 --- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/JavacOptions.java +++ b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/JavacOptions.java @@ -14,45 +14,85 @@ package com.google.devtools.build.buildjar.javac; +import com.google.auto.value.AutoValue; import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; - import java.util.ArrayList; +import java.util.Arrays; import java.util.LinkedHashSet; import java.util.List; import java.util.Set; /** - * Preprocess javac -Xlint options. We - * also need to make the different versions of javac treat -Xlint options - * uniformly. - * - * <p>Some versions of javac now process the -Xlint options without allowing - * later options to override earlier ones on the command line. For example, - * {@code -Xlint:All -Xlint:None} results in all warnings being enabled. + * Preprocess javac -Xlint options. We also need to make the different versions of javac treat + * -Xlint options uniformly. * - * <p>This class preprocesses the -Xlint options within the javac options to - * achieve a command line that is sensitive to ordering. That is, with this - * preprocessing step, {@code -Xlint:all -Xlint:none} results in no - * warnings being enabled. + * <p>Some versions of javac now process the -Xlint options without allowing later options to + * override earlier ones on the command line. For example, {@code -Xlint:All -Xlint:None} results in + * all warnings being enabled. * - * <p>TODO(bazel-team): Convince the current javac owners that the javac options - * need to behave the way this class behaves. :-) + * <p>This class preprocesses the -Xlint options within the javac options to achieve a command line + * that is sensitive to ordering. That is, with this preprocessing step, {@code -Xlint:all + * -Xlint:none} results in no warnings being enabled. */ public final class JavacOptions { + /** Returns an immutable list containing all the non-Bazel specific Javac flags. */ + public static ImmutableList<String> removeBazelSpecificFlags(String[] javacopts) { + return removeBazelSpecificFlags(Arrays.asList(javacopts)); + } + + /** Returns an immutable list containing all the non-Bazel specific Javac flags. */ + public static ImmutableList<String> removeBazelSpecificFlags(Iterable<String> javacopts) { + return filterJavacopts(javacopts).standardJavacopts(); + } + + /** A collection of javac flags, divided into Bazel-specific and standard options. */ + @AutoValue + public abstract static class FilteredJavacopts { + /** Bazel-specific javac flags, e.g. Error Prone's -Xep: flags. */ + public abstract ImmutableList<String> bazelJavacopts(); + + /** Standard javac flags. */ + public abstract ImmutableList<String> standardJavacopts(); + + /** Creates a {@link FilteredJavacopts}. */ + public static FilteredJavacopts create( + ImmutableList<String> bazelJavacopts, ImmutableList<String> standardJavacopts) { + return new AutoValue_JavacOptions_FilteredJavacopts(bazelJavacopts, standardJavacopts); + } + } + + /** Filters a list of javac flags into Bazel-specific and standard flags. */ + public static FilteredJavacopts filterJavacopts(Iterable<String> javacopts) { + ImmutableList.Builder<String> bazelJavacopts = ImmutableList.builder(); + ImmutableList.Builder<String> standardJavacopts = ImmutableList.builder(); + for (String opt : javacopts) { + if (isBazelSpecificFlag(opt)) { + bazelJavacopts.add(opt); + } else { + standardJavacopts.add(opt); + } + } + return FilteredJavacopts.create(bazelJavacopts.build(), standardJavacopts.build()); + } + + private static boolean isBazelSpecificFlag(String opt) { + return opt.startsWith("-Werror:") || opt.startsWith("-Xep"); + } + private static final XlintOptionNormalizer XLINT_OPTION_NORMALIZER = new XlintOptionNormalizer(); /** * Interface to define an option normalizer. For instance, to group all -Xlint: option into one * place. - * - * <p>All normalizers used by the JavacOptions class will be started by calling the - * {@link #start()} method when starting the parsing of a list of option. For each option, the - * first option normalized whose {@link #processOption(String)} method returns true stops its - * parsing and the option is supposed to be added at the end to the normalized list of option with - * the {@link #normalize(List)} method. Options not handled by a normalizer will be returned as - * such in the normalized option list. + * + * <p>All normalizers used by the JavacOptions class will be started by calling the {@link + * #start()} method when starting the parsing of a list of option. For each option, the first + * option normalized whose {@link #processOption(String)} method returns true stops its parsing + * and the option is supposed to be added at the end to the normalized list of option with the + * {@link #normalize(List)} method. Options not handled by a normalizer will be returned as such + * in the normalized option list. */ public static interface JavacOptionNormalizer { /** Resets the state of the normalizer to start a new option parsing. */ diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/java/bazel/BUILD b/src/java_tools/buildjar/java/com/google/devtools/build/java/bazel/BUILD index 5b2b0b7955..3ede677953 100644 --- a/src/java_tools/buildjar/java/com/google/devtools/build/java/bazel/BUILD +++ b/src/java_tools/buildjar/java/com/google/devtools/build/java/bazel/BUILD @@ -91,7 +91,11 @@ java_library( name = "JavaBuilderConfig", srcs = ["JavaBuilderConfig.java"], data = ["@bazel_tools//tools/jdk:JavaBuilder_deploy.jar"], - deps = [":javabuilder-javacopts-lib"], + deps = [ + ":javabuilder-javacopts-lib", + "//src/java_tools/buildjar/java/com/google/devtools/build/buildjar:javac_options", + "//third_party:guava", + ], ) # Provides programmatic access to a bazel compatible javac. @@ -108,6 +112,7 @@ java_library( ":JavaBuilderConfig", ":JavaLangtools", ":JavacBootclasspath", + "//third_party:guava", ], ) diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/java/bazel/BazelJavaCompiler.java b/src/java_tools/buildjar/java/com/google/devtools/build/java/bazel/BazelJavaCompiler.java index 6a4156ad9f..4ca4676685 100644 --- a/src/java_tools/buildjar/java/com/google/devtools/build/java/bazel/BazelJavaCompiler.java +++ b/src/java_tools/buildjar/java/com/google/devtools/build/java/bazel/BazelJavaCompiler.java @@ -14,6 +14,7 @@ package com.google.devtools.build.java.bazel; +import com.google.common.collect.ImmutableList; import java.io.File; import java.io.IOException; import java.io.InputStream; @@ -23,8 +24,6 @@ import java.net.MalformedURLException; import java.net.URL; import java.net.URLClassLoader; import java.nio.charset.Charset; -import java.util.ArrayList; -import java.util.Arrays; import java.util.List; import java.util.Locale; import java.util.Set; @@ -49,27 +48,18 @@ import javax.tools.StandardLocation; public class BazelJavaCompiler { // The default blessed javac options. - - private static final String DEFAULT_BOOTCLASSPATH = JavacBootclasspath.asString(); - private static final String[] DEFAULT_JAVACOPTS; - static { - List<String> defaultJavacopts = new ArrayList<>(); - for (String javacopt : JavaBuilderConfig.defaultJavacOpts()) { - if (javacopt.startsWith("-Xep")) { - // ignore Error Prone-specific flags accepted by JavaBuilder - continue; - } - defaultJavacopts.add(javacopt); - } + private static final String DEFAULT_BOOTCLASSPATH = JavacBootclasspath.asString(); - // The bootclasspath must be specified both via an invocation option and - // via fileManager.setLocation(PLATFORM_CLASS_PATH), to work around what - // appears to be a bug in jdk[6,8] javac. - defaultJavacopts.addAll(Arrays.asList("-bootclasspath", DEFAULT_BOOTCLASSPATH)); - - DEFAULT_JAVACOPTS = defaultJavacopts.toArray(new String[defaultJavacopts.size()]); - } + private static final ImmutableList<String> DEFAULT_JAVACOPTS = + ImmutableList.<String>builder() + .addAll(JavaBuilderConfig.defaultJavacOpts()) + // The bootclasspath must be specified both via an invocation option and + // via fileManager.setLocation(PLATFORM_CLASS_PATH), to work around what + // appears to be a bug in jdk[6,8] javac. + .add("-bootclasspath") + .add(DEFAULT_BOOTCLASSPATH) + .build(); private static final Class<? extends JavaCompiler> JAVA_COMPILER_CLASS = getJavaCompilerClass(); @@ -109,12 +99,10 @@ public class BazelJavaCompiler { public static File getLangtoolsJar() { return JavaLangtools.file(); } - - /** - * Returns the default javacopts, including the blessed bootclasspath. - */ + + /** Returns the default javacopts, including the blessed bootclasspath. */ public static List<String> getDefaultJavacopts() { - return new ArrayList<>(Arrays.asList(DEFAULT_JAVACOPTS)); + return DEFAULT_JAVACOPTS; } /** @@ -186,70 +174,62 @@ public class BazelJavaCompiler { private static JavaCompiler newInstance(final JavaCompiler delegate) { // We forward most operations to the JavaCompiler implementation in langtools.jar. return new JavaCompiler() { - @Override - public CompilationTask getTask( - Writer out, - JavaFileManager fileManager, - DiagnosticListener<? super JavaFileObject> diagnosticListener, - Iterable<String> options, - Iterable<String> classes, - Iterable<? extends JavaFileObject> compilationUnits) { - // We prepend bazel's default javacopts to user javacopts, - // so that the user can override them. javac supports this - // "last option wins" style of option override. - List<String> fullOptions = getDefaultJavacopts(); - if (options != null) { - for (String option : options) { - fullOptions.add(option); - } - } - return delegate.getTask(out, - fileManager, - diagnosticListener, - fullOptions, - classes, - compilationUnits); + @Override + public CompilationTask getTask( + Writer out, + JavaFileManager fileManager, + DiagnosticListener<? super JavaFileObject> diagnosticListener, + Iterable<String> options, + Iterable<String> classes, + Iterable<? extends JavaFileObject> compilationUnits) { + // We prepend bazel's default javacopts to user javacopts, + // so that the user can override them. javac supports this + // "last option wins" style of option override. + ImmutableList.Builder<String> fullOptions = ImmutableList.builder(); + fullOptions.addAll(getDefaultJavacopts()); + if (options != null) { + fullOptions.addAll(options); } + return delegate.getTask( + out, fileManager, diagnosticListener, fullOptions.build(), classes, compilationUnits); + } - @Override - public StandardJavaFileManager getStandardFileManager( - DiagnosticListener<? super JavaFileObject> diagnosticListener, - Locale locale, - Charset charset) { - StandardJavaFileManager fileManager = delegate.getStandardFileManager( - diagnosticListener, - locale, - charset); - - try { - fileManager.setLocation( - StandardLocation.PLATFORM_CLASS_PATH, // bootclasspath - JavacBootclasspath.asFiles()); - } catch (IOException e) { - // Should never happen, according to javadocs for setLocation - throw new RuntimeException(e); - } - return fileManager; + @Override + public StandardJavaFileManager getStandardFileManager( + DiagnosticListener<? super JavaFileObject> diagnosticListener, + Locale locale, + Charset charset) { + StandardJavaFileManager fileManager = + delegate.getStandardFileManager(diagnosticListener, locale, charset); + + try { + fileManager.setLocation( + StandardLocation.PLATFORM_CLASS_PATH, // bootclasspath + JavacBootclasspath.asFiles()); + } catch (IOException e) { + // Should never happen, according to javadocs for setLocation + throw new RuntimeException(e); } + return fileManager; + } - @Override - public int run(InputStream in, OutputStream out, OutputStream err, - String... arguments) { - // prepend bazel's default javacopts to user arguments - List<String> args = getDefaultJavacopts(); - args.addAll(Arrays.asList(arguments)); - return delegate.run(in, out, err, args.toArray(new String[0])); - } + @Override + public int run(InputStream in, OutputStream out, OutputStream err, String... arguments) { + // prepend bazel's default javacopts to user arguments + List<String> args = + ImmutableList.<String>builder().addAll(getDefaultJavacopts()).add(arguments).build(); + return delegate.run(in, out, err, args.toArray(new String[0])); + } - @Override - public Set<SourceVersion> getSourceVersions() { - return delegate.getSourceVersions(); - } + @Override + public Set<SourceVersion> getSourceVersions() { + return delegate.getSourceVersions(); + } - @Override - public int isSupportedOption(String option) { - return delegate.isSupportedOption(option); - } - }; + @Override + public int isSupportedOption(String option) { + return delegate.isSupportedOption(option); + } + }; } } diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/java/bazel/JavaBuilderConfig.java b/src/java_tools/buildjar/java/com/google/devtools/build/java/bazel/JavaBuilderConfig.java index 511dfa584f..c86291dd0b 100644 --- a/src/java_tools/buildjar/java/com/google/devtools/build/java/bazel/JavaBuilderConfig.java +++ b/src/java_tools/buildjar/java/com/google/devtools/build/java/bazel/JavaBuilderConfig.java @@ -14,18 +14,18 @@ package com.google.devtools.build.java.bazel; -import java.util.ArrayList; -import java.util.Collections; +import com.google.common.collect.ImmutableList; +import com.google.devtools.build.buildjar.javac.JavacOptions; import java.util.List; -/** - * Utility class to provide java-level access to the blessed javabuilder javacopts. - */ +/** Utility class to provide Java-level access to the blessed JavaBuilder javacopts. */ public class JavaBuilderConfig { - + + private static final ImmutableList<String> JAVACOPTS = + ImmutableList.copyOf( + JavacOptions.removeBazelSpecificFlags(JavaBuilderJavacOpts.DEFAULT_JAVACOPTS)); + public static List<String> defaultJavacOpts() { - List<String> result = new ArrayList<>(); - Collections.addAll(result, JavaBuilderJavacOpts.DEFAULT_JAVACOPTS); - return result; + return JAVACOPTS; } } diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/BUILD b/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/BUILD index 9e3e6cbeea..9ab49b9ca7 100644 --- a/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/BUILD +++ b/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/BUILD @@ -14,6 +14,7 @@ java_library( ":javac_turbine_compiler", ":zip_output_file_manager", ":zip_util", + "//src/java_tools/buildjar/java/com/google/devtools/build/buildjar:javac_options", "//src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins:dependency", "//src/java_tools/buildjar/java/com/google/devtools/build/java/turbine:turbine_options", "//src/java_tools/buildjar/java/com/google/devtools/build/java/turbine:turbine_options_parser", diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/JavacTurbine.java b/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/JavacTurbine.java index b294bd2a3c..8959d3613b 100644 --- a/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/JavacTurbine.java +++ b/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/JavacTurbine.java @@ -15,10 +15,10 @@ package com.google.devtools.build.java.turbine.javac; import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.CharMatcher; import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.devtools.build.buildjar.javac.JavacOptions; import com.google.devtools.build.buildjar.javac.plugins.dependency.DependencyModule; import com.google.devtools.build.buildjar.javac.plugins.dependency.DependencyModule.StrictJavaDeps; import com.google.devtools.build.buildjar.javac.plugins.dependency.StrictJavaDepsPlugin; @@ -119,7 +119,7 @@ public class JavacTurbine implements AutoCloseable { ImmutableList.Builder<String> argbuilder = ImmutableList.builder(); - filterJavacopts(argbuilder, turbineOptions.javacOpts()); + argbuilder.addAll(JavacOptions.removeBazelSpecificFlags(turbineOptions.javacOpts())); // Disable compilation of implicit source files. // This is insurance: the sourcepath is empty, so we don't expect implicit sources. @@ -321,31 +321,6 @@ public class JavacTurbine implements AutoCloseable { return result.build(); } - @VisibleForTesting - static void filterJavacopts( - ImmutableList.Builder<String> javacArgs, Iterable<String> defaultJavacopts) { - for (String opt : defaultJavacopts) { - - // TODO(cushon): temporary hack until 4149f08bcc8bd1318d4021cf372ec89240ee3dbb is released - opt = CharMatcher.is('\'').trimFrom(opt); - - if (isErrorProneFlag(opt)) { - // drop Error Prone's fake javacopts - continue; - } - javacArgs.add(opt); - } - } - - /** - * Returns true for flags that are specific to Error Prone. - * - * <p>WARNING: keep in sync with ErrorProneOptions#isSupportedOption - */ - static boolean isErrorProneFlag(String opt) { - return opt.startsWith("-Xep"); - } - /** Extra sources in srcjars to disk. */ private static List<String> extractSourceJars(TurbineOptions turbineOptions, Path tmpdir) throws IOException { diff --git a/src/java_tools/buildjar/javatests/com/google/devtools/build/java/turbine/javac/JavacTurbineTest.java b/src/java_tools/buildjar/javatests/com/google/devtools/build/java/turbine/javac/JavacTurbineTest.java index d657ab0fa8..7562e3f070 100644 --- a/src/java_tools/buildjar/javatests/com/google/devtools/build/java/turbine/javac/JavacTurbineTest.java +++ b/src/java_tools/buildjar/javatests/com/google/devtools/build/java/turbine/javac/JavacTurbineTest.java @@ -347,29 +347,6 @@ public class JavacTurbineTest { }; @Test - public void filterJavacopts() { - ImmutableList.Builder<String> output = ImmutableList.builder(); - JavacTurbine.filterJavacopts( - output, - Arrays.asList( - "-g", - "-source", - "6", - "-target", - "6", - "-Xlint:all", - "-source", - "7", - "-target", - "7", - "-Xep:GuardedBy:ERROR")); - assertThat(output.build()) - .containsExactly( - "-g", "-source", "6", "-target", "6", "-Xlint:all", "-source", "7", "-target", "7") - .inOrder(); - } - - @Test public void jdeps() throws Exception { Path libC = temp.newFile("libc.jar").toPath(); |