Handle more javacopts
Previously turbine whitelisted -source and -target flags and ignored all other
configured javacopts, including custom annotation processor options and javac
debug flags.
The change switches to allowing all javacopts, and blackisting options that are
known to be incompatible.
Also ensure that invalid flags result in an error.
--
MOS_MIGRATED_REVID=118718745
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 acc09d0..a3f8d72 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
@@ -48,7 +48,6 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.Enumeration;
-import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.regex.Pattern;
@@ -125,12 +124,15 @@
ImmutableList.Builder<String> argbuilder = ImmutableList.builder();
- addLanguageLevel(argbuilder, turbineOptions.javacOpts());
+ filterJavacopts(argbuilder, turbineOptions.javacOpts());
// Disable compilation of implicit source files.
// This is insurance: the sourcepath is empty, so we don't expect implicit sources.
argbuilder.add("-implicit:none");
+ // Disable debug info
+ argbuilder.add("-g:none");
+
ImmutableList<Path> processorpath;
if (!turbineOptions.processors().isEmpty()) {
argbuilder.add("-processor");
@@ -321,27 +323,27 @@
return result.build();
}
- /** Extract the language level from the javacopts. */
@VisibleForTesting
- static void addLanguageLevel(
+ static void filterJavacopts(
ImmutableList.Builder<String> javacArgs, Iterable<String> defaultJavacopts) {
- Iterator<String> it = defaultJavacopts.iterator();
- while (it.hasNext()) {
- String curr = it.next();
- switch (curr) {
- case "-source":
- case "-target":
- if (it.hasNext()) {
- javacArgs.add(curr);
- javacArgs.add(it.next());
- }
- break;
- default:
- break;
+ for (String opt : defaultJavacopts) {
+ 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("-extra_checks") || 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/java/com/google/devtools/build/java/turbine/javac/JavacTurbineCompileResult.java b/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/JavacTurbineCompileResult.java
index 0d03431..a28f860 100644
--- a/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/JavacTurbineCompileResult.java
+++ b/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/JavacTurbineCompileResult.java
@@ -25,25 +25,29 @@
/** The output from a {@link JavacTurbineCompiler} compilation. */
class JavacTurbineCompileResult {
+ enum Status {
+ OK, ERROR
+ }
+
private final ImmutableMap<String, OutputFileObject> files;
- private final boolean success;
+ private final Status status;
private final StringWriter sb;
private final Context context;
JavacTurbineCompileResult(
ImmutableMap<String, OutputFileObject> files,
- boolean success,
+ Status status,
StringWriter sb,
Context context) {
this.files = files;
- this.success = success;
+ this.status = status;
this.sb = sb;
this.context = context;
}
/** True iff the compilation succeeded. */
boolean success() {
- return success;
+ return status == Status.OK;
}
/** The stderr from the compilation. */
diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/JavacTurbineCompiler.java b/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/JavacTurbineCompiler.java
index af697a8..0a59323 100644
--- a/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/JavacTurbineCompiler.java
+++ b/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/JavacTurbineCompiler.java
@@ -16,6 +16,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.buildjar.javac.plugins.dependency.StrictJavaDepsPlugin;
+import com.google.devtools.build.java.turbine.javac.JavacTurbineCompileResult.Status;
import com.google.devtools.build.java.turbine.javac.ZipOutputFileManager.OutputFileObject;
import com.sun.tools.javac.file.CacheFSInfo;
@@ -43,7 +44,7 @@
static JavacTurbineCompileResult compile(JavacTurbineCompileRequest request) throws IOException {
Map<String, OutputFileObject> files = new LinkedHashMap<>();
- boolean success;
+ Status status;
StringWriter sw = new StringWriter();
Context context = new Context();
@@ -70,6 +71,11 @@
fm.setContext(context);
fm.handleOptions(args.getDeferredFileManagerOptions());
+ if (!args.validate()) {
+ return new JavacTurbineCompileResult(
+ ImmutableMap.<String, OutputFileObject>of(), Status.ERROR, sw, context);
+ }
+
JavaCompiler comp = JavaCompiler.instance(context);
if (request.strictJavaDepsPlugin() != null) {
request.strictJavaDepsPlugin().init(context, Log.instance(context), comp);
@@ -77,14 +83,14 @@
try {
comp.compile(args.getFileObjects(), args.getClassNames(), null);
- success = comp.errorCount() == 0;
+ status = comp.errorCount() == 0 ? Status.OK : Status.ERROR;
} catch (Throwable t) {
t.printStackTrace(pw);
- success = false;
+ status = Status.ERROR;
}
}
- return new JavacTurbineCompileResult(ImmutableMap.copyOf(files), success, sw, context);
+ return new JavacTurbineCompileResult(ImmutableMap.copyOf(files), status, sw, context);
}
static void setupContext(Context context, @Nullable StrictJavaDepsPlugin sjd) {
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 1b4945b..015a122 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
@@ -343,14 +343,26 @@
};
@Test
- public void extractLanguageLevel() {
+ public void filterJavacopts() {
ImmutableList.Builder<String> output = ImmutableList.builder();
- JavacTurbine.addLanguageLevel(
+ JavacTurbine.filterJavacopts(
output,
Arrays.asList(
- "-g", "-source", "6", "-target", "6", "-Xlint:all", "-source", "7", "-target", "7"));
+ "-g",
+ "-source",
+ "6",
+ "-target",
+ "6",
+ "-Xlint:all",
+ "-source",
+ "7",
+ "-target",
+ "7",
+ "-extra_checks:off",
+ "-Xep:GuardedBy:ERROR"));
assertThat(output.build())
- .containsExactly("-source", "6", "-target", "6", "-source", "7", "-target", "7")
+ .containsExactly(
+ "-g", "-source", "6", "-target", "6", "-Xlint:all", "-source", "7", "-target", "7")
.inOrder();
}
@@ -1018,4 +1030,16 @@
};
assertThat(text).isEqualTo(Joiner.on('\n').join(expected));
}
+
+ @Test
+ public void invalidJavacopts() throws Exception {
+ addSourceLines("Hello.java", "class Hello {}");
+ optionsBuilder.addAllJavacOpts(Arrays.asList("-NOT_AN_OPTION"));
+ StringWriter errOutput = new StringWriter();
+ try (JavacTurbine turbine =
+ new JavacTurbine(new PrintWriter(errOutput, true), optionsBuilder.build())) {
+ assertThat(turbine.compile()).isEqualTo(Result.ERROR);
+ }
+ assertThat(errOutput.toString()).contains("invalid flag: -NOT_AN_OPTION");
+ }
}
