Skip to content

Commit

Permalink
Disable tree pruning for blacklisted annotation processors
Browse files Browse the repository at this point in the history
Some annotation processors use non-standard APIs to examine the AST, and are
broken by tree pruning. This change adds configuration to Turbine to allow a
blacklist of processors to be provided, and disables tree pruning if any
blacklisted processors are found.

--
MOS_MIGRATED_REVID=119079114
  • Loading branch information
cushon authored and lberki committed Apr 7, 2016
1 parent 709984d commit eb89ccc
Show file tree
Hide file tree
Showing 9 changed files with 236 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ public class TurbineOptions {
private final ImmutableList<String> sources;
private final ImmutableList<String> processorPath;
private final ImmutableSet<String> processors;
private final ImmutableSet<String> blacklistedProcessors;
private final String tempDir;
private final ImmutableList<String> sourceJars;
private final Optional<String> outputDeps;
Expand All @@ -50,6 +51,7 @@ private TurbineOptions(
ImmutableList<String> sources,
ImmutableList<String> processorPath,
ImmutableSet<String> processors,
ImmutableSet<String> blacklistedProcessors,
String tempDir,
ImmutableList<String> sourceJars,
@Nullable String outputDeps,
Expand All @@ -66,6 +68,8 @@ private TurbineOptions(
this.sources = checkNotNull(sources, "sources must not be null");
this.processorPath = checkNotNull(processorPath, "processorPath must not be null");
this.processors = checkNotNull(processors, "processors must not be null");
this.blacklistedProcessors =
checkNotNull(blacklistedProcessors, "blacklistedProcessors must not be null");
this.tempDir = checkNotNull(tempDir, "tempDir must not be null");
this.sourceJars = checkNotNull(sourceJars, "sourceJars must not be null");
this.outputDeps = Optional.fromNullable(outputDeps);
Expand Down Expand Up @@ -115,6 +119,14 @@ public ImmutableSet<String> processors() {
return processors;
}

/**
* Annotation processors that require tree pruning to be disabled, for example because they use
* internal compiler APIs to inspect information that would be removed during pruning.
*/
public ImmutableSet<String> blacklistedProcessors() {
return blacklistedProcessors;
}

/** Source jars for compilation. */
public ImmutableList<String> sourceJars() {
return sourceJars;
Expand Down Expand Up @@ -176,6 +188,7 @@ public static class Builder {
private final ImmutableList.Builder<String> sources = ImmutableList.builder();
private final ImmutableList.Builder<String> processorPath = ImmutableList.builder();
private final ImmutableSet.Builder<String> processors = ImmutableSet.builder();
private final ImmutableSet.Builder<String> blacklistedProcessors = ImmutableSet.builder();
private String tempDir;
private final ImmutableList.Builder<String> sourceJars = ImmutableList.builder();
private final ImmutableList.Builder<String> bootClassPath = ImmutableList.builder();
Expand All @@ -197,6 +210,7 @@ public TurbineOptions build() {
sources.build(),
processorPath.build(),
processors.build(),
blacklistedProcessors.build(),
tempDir,
sourceJars.build(),
outputDeps,
Expand Down Expand Up @@ -244,6 +258,11 @@ public Builder addProcessors(Iterable<String> processors) {
return this;
}

public Builder addBlacklistedProcessors(Iterable<String> processors) {
this.blacklistedProcessors.addAll(processors);
return this;
}

public Builder setTempDir(String tempDir) {
this.tempDir = tempDir;
return this;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,24 @@
public class TurbineOptionsParser {

/**
* Parses a list of command-line options into a {@link TurbineOptions}, expanding any
* {@code @params} files.
* Parses command line options into {@link TurbineOptions}, expanding any {@code @params}
* files.
*/
public static TurbineOptions parse(Iterable<String> args) throws IOException {
TurbineOptions.Builder builder = TurbineOptions.builder();
parse(builder, args);
return builder.build();
}

/**
* Parses command line options into a {@link TurbineOptions.Builder}, expanding any
* {@code @params} files.
*/
public static void parse(TurbineOptions.Builder builder, Iterable<String> args)
throws IOException {
Deque<String> argumentDeque = new ArrayDeque<>();
expandParamsFiles(argumentDeque, args);
TurbineOptions.Builder builder = TurbineOptions.builder();
parse(builder, argumentDeque);
return builder.build();
}

private static final Splitter ARG_SPLITTER =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ java_library(
name = "javac_turbine_java_compiler",
srcs = ["JavacTurbineJavaCompiler.java"],
deps = [
":javac_turbine_compile_request",
":tree_pruner",
"//src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins:dependency",
"//third_party:jsr305",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.google.devtools.build.buildjar.javac.plugins.dependency.StrictJavaDepsPlugin;
import com.google.devtools.build.java.turbine.TurbineOptions;
import com.google.devtools.build.java.turbine.TurbineOptionsParser;
import com.google.devtools.build.java.turbine.javac.JavacTurbineCompileRequest.Prune;
import com.google.devtools.build.java.turbine.javac.ZipOutputFileManager.OutputFileObject;

import com.sun.tools.javac.util.Context;
Expand Down Expand Up @@ -78,7 +79,7 @@ public static Result compile(TurbineOptions turbineOptions) throws IOException {
}

/** A header compilation result. */
enum Result {
public enum Result {
/** The compilation succeeded with the reduced classpath optimization. */
OK_WITH_REDUCED_CLASSPATH(true),

Expand All @@ -94,11 +95,11 @@ private Result(boolean ok) {
this.ok = ok;
}

boolean ok() {
public boolean ok() {
return ok;
}

int exitCode() {
public int exitCode() {
return ok ? 0 : 1;
}
}
Expand Down Expand Up @@ -156,6 +157,11 @@ Result compile() throws IOException {
.setBootClassPath(asPaths(turbineOptions.bootClassPath()))
.setProcessorClassPath(processorpath);

if (!Collections.disjoint(
turbineOptions.processors(), turbineOptions.blacklistedProcessors())) {
requestBuilder.setPrune(Prune.NO);
}

StrictJavaDeps strictDepsMode = StrictJavaDeps.valueOf(turbineOptions.strictDepsMode());

DependencyModule dependencyModule = buildDependencyModule(turbineOptions, strictDepsMode);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,11 @@

package com.google.devtools.build.java.turbine.javac;

import static com.google.common.base.Preconditions.checkNotNull;

import com.google.common.collect.ImmutableList;
import com.google.devtools.build.buildjar.javac.plugins.dependency.StrictJavaDepsPlugin;
import com.google.devtools.build.java.turbine.javac.JavacTurbineCompileRequest.Prune;

import java.nio.file.Path;

Expand All @@ -24,23 +27,31 @@
/** The input to a {@link JavacTurbineCompiler} compilation. */
class JavacTurbineCompileRequest {

enum Prune {
YES,
NO
}

private final ImmutableList<Path> classPath;
private final ImmutableList<Path> bootClassPath;
private final ImmutableList<Path> processorClassPath;
private final ImmutableList<String> javacOptions;
@Nullable private final StrictJavaDepsPlugin strictJavaDepsPlugin;
private final Prune prune;

JavacTurbineCompileRequest(
ImmutableList<Path> classPath,
ImmutableList<Path> bootClassPath,
ImmutableList<Path> processorClassPath,
ImmutableList<String> javacOptions,
@Nullable StrictJavaDepsPlugin strictJavaDepsPlugin) {
this.classPath = classPath;
this.bootClassPath = bootClassPath;
this.processorClassPath = processorClassPath;
this.javacOptions = javacOptions;
@Nullable StrictJavaDepsPlugin strictJavaDepsPlugin,
Prune prune) {
this.classPath = checkNotNull(classPath);
this.bootClassPath = checkNotNull(bootClassPath);
this.processorClassPath = checkNotNull(processorClassPath);
this.javacOptions = checkNotNull(javacOptions);
this.strictJavaDepsPlugin = strictJavaDepsPlugin;
this.prune = checkNotNull(prune);
}

/** The class path; correspond's to javac -classpath. */
Expand Down Expand Up @@ -71,6 +82,11 @@ StrictJavaDepsPlugin strictJavaDepsPlugin() {
return strictJavaDepsPlugin;
}

/** Whether to perform a relaxed header-only compilation. */
Prune prune() {
return prune;
}

static JavacTurbineCompileRequest.Builder builder() {
return new Builder();
}
Expand All @@ -81,12 +97,13 @@ static class Builder {
private ImmutableList<Path> processorClassPath;
private ImmutableList<String> javacOptions;
@Nullable private StrictJavaDepsPlugin strictDepsPlugin;
private Prune prune = Prune.YES;

private Builder() {}

JavacTurbineCompileRequest build() {
return new JavacTurbineCompileRequest(
classPath, bootClassPath, processorClassPath, javacOptions, strictDepsPlugin);
classPath, bootClassPath, processorClassPath, javacOptions, strictDepsPlugin, prune);
}

Builder setClassPath(ImmutableList<Path> classPath) {
Expand All @@ -113,5 +130,10 @@ Builder setStrictDepsPlugin(@Nullable StrictJavaDepsPlugin strictDepsPlugin) {
this.strictDepsPlugin = strictDepsPlugin;
return this;
}

Builder setPrune(Prune prune) {
this.prune = prune;
return this;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.JavacTurbineCompileRequest.Prune;
import com.google.devtools.build.java.turbine.javac.JavacTurbineCompileResult.Status;
import com.google.devtools.build.java.turbine.javac.ZipOutputFileManager.OutputFileObject;

Expand Down Expand Up @@ -50,7 +51,7 @@ static JavacTurbineCompileResult compile(JavacTurbineCompileRequest request) thr

try (PrintWriter pw = new PrintWriter(sw)) {
ZipOutputFileManager.preRegister(context, files);
setupContext(context, request.strictJavaDepsPlugin());
setupContext(context, request.strictJavaDepsPlugin(), request.prune());
CacheFSInfo.preRegister(context);

context.put(Log.outKey, pw);
Expand Down Expand Up @@ -93,7 +94,7 @@ static JavacTurbineCompileResult compile(JavacTurbineCompileRequest request) thr
return new JavacTurbineCompileResult(ImmutableMap.copyOf(files), status, sw, context);
}

static void setupContext(Context context, @Nullable StrictJavaDepsPlugin sjd) {
JavacTurbineJavaCompiler.preRegister(context, sjd);
static void setupContext(Context context, @Nullable StrictJavaDepsPlugin sjd, Prune prune) {
JavacTurbineJavaCompiler.preRegister(context, sjd, prune);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package com.google.devtools.build.java.turbine.javac;

import com.google.devtools.build.buildjar.javac.plugins.dependency.StrictJavaDepsPlugin;
import com.google.devtools.build.java.turbine.javac.JavacTurbineCompileRequest.Prune;

import com.sun.tools.javac.comp.AttrContext;
import com.sun.tools.javac.comp.CompileStates.CompileState;
Expand All @@ -38,15 +39,21 @@
class JavacTurbineJavaCompiler extends JavaCompiler implements AutoCloseable {

@Nullable private final StrictJavaDepsPlugin strictJavaDeps;
private final boolean prune;

public JavacTurbineJavaCompiler(Context context, @Nullable StrictJavaDepsPlugin strictJavaDeps) {
public JavacTurbineJavaCompiler(
Context context, @Nullable StrictJavaDepsPlugin strictJavaDeps, Prune prune) {
super(context);
this.strictJavaDeps = strictJavaDeps;
this.prune = prune == Prune.YES;
}

@Override
protected JCCompilationUnit parse(JavaFileObject javaFileObject, CharSequence charSequence) {
JCCompilationUnit result = super.parse(javaFileObject, charSequence);
if (!prune) {
return result;
}
TreePruner.prune(result);
return result;
}
Expand All @@ -65,6 +72,10 @@ public Env<AttrContext> attribute(Env<AttrContext> env) {

@Override
protected void flow(Env<AttrContext> env, Queue<Env<AttrContext>> results) {
if (!prune) {
super.flow(env, results);
return;
}
// skip FLOW (as if -relax was enabled, except -relax is broken for JDK >= 8)
if (!compileStates.isDone(env, CompileState.FLOW)) {
compileStates.put(env, CompileState.FLOW);
Expand All @@ -83,13 +94,14 @@ public void close() {
* Override the default {@link JavaCompiler} implementation with {@link JavacTurbineJavaCompiler}
* for the given compilation context.
*/
public static void preRegister(Context context, @Nullable final StrictJavaDepsPlugin sjd) {
public static void preRegister(
Context context, @Nullable final StrictJavaDepsPlugin sjd, final Prune prune) {
context.put(
compilerKey,
new Context.Factory<JavaCompiler>() {
@Override
public JavaCompiler make(Context c) {
return new JavacTurbineJavaCompiler(c, sjd);
return new JavacTurbineJavaCompiler(c, sjd, prune);
}
});
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
java_test(
name = "JavacTurbineTest",
srcs = ["JavacTurbineTest.java"],
data = [
"//third_party/java/jdk/langtools:javac_jar",
],
jvm_flags = [
# Simulates how Bazel invokes turbine
"-Xbootclasspath/p:$(location //third_party/java/jdk/langtools:javac_jar)",
],
tags = ["jdk8"],
deps = [
"//src/java_tools/buildjar/java/com/google/devtools/build/java/turbine:turbine_options",
Expand Down
Loading

0 comments on commit eb89ccc

Please sign in to comment.