diff options
author | brandjon <brandjon@google.com> | 2017-06-16 21:27:56 +0200 |
---|---|---|
committer | Philipp Wollermann <philwo@google.com> | 2017-06-19 18:24:05 +0200 |
commit | 4929ad79865f8c13ef3b33c827040f4a037e4afe (patch) | |
tree | c97035c6f60aba479384bf95ccd5ec348d8781e7 /src | |
parent | 9e26369575f04776c0416fd75a9434a22b4d5e9a (diff) |
Automated g4 rollback of commit 923d7df521f67d031b288180560848bd35e20976.
*** Reason for rollback ***
Breaks dozens of targets in the nightly with Tool Failure errors
*** Original change description ***
Clean up turbine action creation
Support disabling javac fallback for actions without a direct
classpath, and only use the 'JavacTurbine' mnemonic for spawns
that require javac-turbine due to annotation processing to make
it easier to collect metrics on that.
Finally, remove --java_header_compilation_direct_classpath now
that it has been productionized and enabled by default.
PiperOrigin-RevId: 159260596
Diffstat (limited to 'src')
3 files changed, 64 insertions, 55 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaConfiguration.java index c70d0cc481..1c9364dc5c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaConfiguration.java @@ -146,6 +146,7 @@ public final class JavaConfiguration extends Fragment { private final Label javaLauncherLabel; private final boolean useIjars; private final boolean useHeaderCompilation; + private final boolean headerCompilationDirectClasspath; private final boolean headerCompilationDisableJavacFallback; private final boolean generateJavaDeps; private final boolean strictDepsJavaProtos; @@ -178,6 +179,7 @@ public final class JavaConfiguration extends Fragment { this.javaLauncherLabel = javaOptions.javaLauncher; this.useIjars = javaOptions.useIjars; this.useHeaderCompilation = javaOptions.headerCompilation; + this.headerCompilationDirectClasspath = javaOptions.headerCompilationDirectClasspath; this.headerCompilationDisableJavacFallback = javaOptions.headerCompilationDisableJavacFallback; this.generateJavaDeps = generateJavaDeps; this.javaClasspath = javaOptions.javaClasspath; @@ -259,6 +261,11 @@ public final class JavaConfiguration extends Fragment { return useHeaderCompilation; } + /** Returns true if header compilations should use direct dependencies only. */ + public boolean headerCompilationDirectClasspath() { + return headerCompilationDirectClasspath; + } + /** * If --java_header_compilation is set, report diagnostics from turbine instead of falling back to * javac. Diagnostics will be produced more quickly, but may be less helpful. diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java index 371f0e1610..02c072002e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java @@ -14,12 +14,10 @@ package com.google.devtools.build.lib.rules.java; -import static com.google.common.base.Preconditions.checkState; import static com.google.devtools.build.lib.util.Preconditions.checkNotNull; import static java.nio.charset.StandardCharsets.ISO_8859_1; import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -344,42 +342,41 @@ public class JavaHeaderCompileAction extends SpawnAction { directJars = NestedSetBuilder.emptySet(Order.NAIVE_LINK_ORDER); compileTimeDependencyArtifacts = NestedSetBuilder.emptySet(Order.STABLE_ORDER); } - - // The compilation uses API-generating annotation processors and has to fall back to - // javac-turbine. - boolean requiresAnnotationProcessing = !processorNames.isEmpty(); - + boolean useDirectClasspath = useDirectClasspath(); + boolean disableJavacFallback = + ruleContext.getFragment(JavaConfiguration.class).headerCompilationDisableJavacFallback(); + CommandLine directCommandLine = null; + if (useDirectClasspath) { + CustomCommandLine.Builder builder = + baseCommandLine(getBaseArgs(javaToolchain)).addExecPaths("--classpath", directJars); + if (disableJavacFallback) { + builder.add("--nojavac_fallback"); + } + directCommandLine = builder.build(); + } Iterable<Artifact> tools = ImmutableList.of(javacJar, javaToolchain.getHeaderCompiler()); ImmutableList<Artifact> outputs = ImmutableList.of(outputJar, outputDepsProto); - NestedSet<Artifact> baseInputs = + NestedSet<Artifact> directInputs = NestedSetBuilder.<Artifact>stableOrder() .addTransitive(javabaseInputs) .addAll(bootclasspathEntries) .addAll(sourceJars) .addAll(sourceFiles) + .addTransitive(directJars) .addAll(tools) .build(); - boolean noFallback = - ruleContext.getFragment(JavaConfiguration.class).headerCompilationDisableJavacFallback(); - // The action doesn't require annotation processing and either javac-turbine fallback is - // disabled, or the action doesn't distinguish between direct and transitive deps, so - // use a plain SpawnAction to invoke turbine. - if ((noFallback || directJars.isEmpty()) && !requiresAnnotationProcessing) { - NestedSet<Artifact> classpath = !directJars.isEmpty() ? directJars : classpathEntries; - CustomCommandLine.Builder commandLine = - baseCommandLine(getBaseArgs(javaToolchain), classpath); - if (noFallback) { - commandLine.add("--nojavac_fallback"); - } + if (useDirectClasspath && disableJavacFallback) { + // use a regular SpawnAction to invoke turbine with direct deps only, + // and no fallback to javac-turbine return new ActionAnalysisMetadata[] { new SpawnAction( ruleContext.getActionOwner(), tools, - NestedSetBuilder.fromNestedSet(baseInputs).addTransitive(classpath).build(), + directInputs, outputs, LOCAL_RESOURCES, - commandLine.build(), + directCommandLine, false, JavaCompileAction.UTF8_ENVIRONMENT, /*executionInfo=*/ ImmutableSet.<String>of(), @@ -405,16 +402,15 @@ public class JavaHeaderCompileAction extends SpawnAction { getBaseArgs(javaToolchain).addPaths("@%s", paramsFile.getExecPath()).build(); NestedSet<Artifact> transitiveInputs = NestedSetBuilder.<Artifact>stableOrder() - .addTransitive(baseInputs) + .addTransitive(directInputs) .addTransitive(classpathEntries) .addTransitive(processorPath) .addTransitive(compileTimeDependencyArtifacts) .add(paramsFile) .build(); - - if (requiresAnnotationProcessing) { - // turbine doesn't support API-generating annotation processors, so skip the two-tiered - // turbine/javac-turbine action and just use SpawnAction to invoke javac-turbine. + if (!useDirectClasspath) { + // If direct classpaths are disabled (e.g. because the compilation uses API-generating + // annotation processors) skip the custom action implementation and just use SpawnAction. return new ActionAnalysisMetadata[] { parameterFileWriteAction, new SpawnAction( @@ -427,22 +423,10 @@ public class JavaHeaderCompileAction extends SpawnAction { false, JavaCompileAction.UTF8_ENVIRONMENT, /*executionInfo=*/ ImmutableSet.<String>of(), - getProgressMessageWithAnnotationProcessors(), + getProgressMessage(), "JavacTurbine") }; } - - // The action doesn't require annotation processing, javac-turbine fallback is enabled, and - // the target distinguishes between direct and transitive deps. Try a two-tiered spawn - // the invokes turbine with direct deps, and falls back to javac-turbine on failures to - // produce better diagnostics. (At the cost of slower failed actions and a larger - // cache footprint.) - // TODO(cushon): productionize --nojavac_fallback and remove this path - checkState(!directJars.isEmpty()); - NestedSet<Artifact> directInputs = - NestedSetBuilder.fromNestedSet(baseInputs).addTransitive(directJars).build(); - CustomCommandLine directCommandLine = - baseCommandLine(getBaseArgs(javaToolchain), directJars).build(); return new ActionAnalysisMetadata[] { parameterFileWriteAction, new JavaHeaderCompileAction( @@ -457,17 +441,6 @@ public class JavaHeaderCompileAction extends SpawnAction { }; } - private String getProgressMessageWithAnnotationProcessors() { - List<String> shortNames = new ArrayList<>(); - for (String name : processorNames) { - shortNames.add(name.substring(name.lastIndexOf('.') + 1)); - } - return getProgressMessage() - + " and running annotation processors (" - + Joiner.on(", ").join(shortNames) - + ")"; - } - private String getProgressMessage() { return String.format( "Compiling Java headers %s (%d files)", @@ -487,8 +460,7 @@ public class JavaHeaderCompileAction extends SpawnAction { * Adds the command line arguments shared by direct classpath and transitive classpath * invocations. */ - private CustomCommandLine.Builder baseCommandLine( - CustomCommandLine.Builder result, NestedSet<Artifact> classpathEntries) { + private CustomCommandLine.Builder baseCommandLine(CustomCommandLine.Builder result) { result.addExecPath("--output", outputJar); if (outputDepsProto != null) { @@ -522,14 +494,13 @@ public class JavaHeaderCompileAction extends SpawnAction { result.add("@" + targetLabel); } } - result.addExecPaths("--classpath", classpathEntries); return result; } /** Builds a transitive classpath command line. */ private CommandLine transitiveCommandLine() { CustomCommandLine.Builder result = CustomCommandLine.builder(); - baseCommandLine(result, classpathEntries); + baseCommandLine(result); if (!processorNames.isEmpty()) { result.add("--processors", processorNames); } @@ -545,7 +516,28 @@ public class JavaHeaderCompileAction extends SpawnAction { result.addExecPaths("--deps_artifacts", compileTimeDependencyArtifacts); } } + result.addExecPaths("--classpath", classpathEntries); return result.build(); } + + /** Returns true if the header compilation classpath should only include direct deps. */ + boolean useDirectClasspath() { + if (directJars.isEmpty() && !classpathEntries.isEmpty()) { + // the compilation doesn't distinguish direct deps, e.g. because it doesn't support strict + // java deps + return false; + } + if (!processorNames.isEmpty()) { + // the compilation uses API-generating annotation processors and has to fall back to + // javac-turbine, which doesn't support direct classpaths + return false; + } + JavaConfiguration javaConfiguration = ruleContext.getFragment(JavaConfiguration.class); + if (!javaConfiguration.headerCompilationDirectClasspath()) { + // the experiment is disabled + return false; + } + return true; + } } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaOptions.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaOptions.java index f5764e6a18..55f83e5395 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaOptions.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaOptions.java @@ -417,6 +417,15 @@ public class JavaOptions extends FragmentOptions { public boolean strictDepsJavaProtos; @Option( + name = "java_header_compilation_direct_classpath", + defaultValue = "true", + optionUsageRestrictions = OptionUsageRestrictions.UNDOCUMENTED, + help = "Experimental option to limit the header compilation classpath to direct deps.", + oldName = "experimental_java_header_compilation_direct_classpath" + ) + public boolean headerCompilationDirectClasspath; + + @Option( name = "experimental_java_header_compilation_disable_javac_fallback", defaultValue = "false", optionUsageRestrictions = OptionUsageRestrictions.UNDOCUMENTED, @@ -454,6 +463,7 @@ public class JavaOptions extends FragmentOptions { // incremental build performance is important. host.useIjars = useIjars; host.headerCompilation = headerCompilation; + host.headerCompilationDirectClasspath = headerCompilationDirectClasspath; host.headerCompilationDisableJavacFallback = headerCompilationDisableJavacFallback; host.javaDeps = javaDeps; |