Skip to content

Commit

Permalink
Add javabuilder_data and turbine_data attributes to java_toolchain
Browse files Browse the repository at this point in the history 
corresponding to `javabuilder_jvm_opts` and `turbine_jvm_opts`, to allow
tool-specific inputs. Currently the only way to specify additional inputs is
with the shared `tools` attribute, which means the Class Data Sharing archives
for JavaBuilder and Turbine are both passed when invoking either tool.

Also refactor the handling of the tool-specific inputs into
'JavaToolchainTool', which encapsulates the deploy jars, jvm flags, and data
inputs for each tool.

Also, stop hard-coding `-X:verifyNone` in a couple of places. It's explicitly
configured in our `java_toolchain`, and hard-coding it in Bazel was never
a good idea (see e.g. #11381).

PiperOrigin-RevId: 346151606
  • Loading branch information
@cushon @copybara-github
cushon authored and copybara-github committed Dec 7, 2020
1 parent 68323d8 commit a1b19df
Show file tree
Hide file tree
Showing 8 changed files with 171 additions and 115 deletions.
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/rules/java/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ java_library(
"JavaStrictCompilationArgsProvider.java",
"JavaTargetAttributes.java",
"JavaToolchainProvider.java",
"JavaToolchainTool.java",
"JavaUtil.java",
"MessageBundleInfo.java",
"NativeLibraryNestedSetBuilder.java",
Expand Down
3 changes: 0 additions & 3 deletions src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ public final class JavaCompilationHelper {
private final JavaTargetAttributes.Builder attributes;
private JavaTargetAttributes builtAttributes;
private final ImmutableList<String> customJavacOpts;
private final ImmutableList<String> customJavacJvmOpts;
private final List<Artifact> translations = new ArrayList<>();
private boolean translationsFrozen;
private final JavaSemantics semantics;
Expand All @@ -82,7 +81,6 @@ private JavaCompilationHelper(
this.javaToolchain = Preconditions.checkNotNull(javaToolchainProvider);
this.attributes = attributes;
this.customJavacOpts = javacOpts;
this.customJavacJvmOpts = javaToolchain.getJavabuilderJvmOptions();
this.semantics = semantics;
this.additionalInputsForDatabinding = additionalInputsForDatabinding;
this.strictJavaDeps =
Expand Down Expand Up @@ -301,7 +299,6 @@ public void createCompileAction(JavaCompileOutputs<Artifact> outputs)
builder.setSourceFiles(sourceFiles);
builder.setSourceJars(sourceJars);
builder.setJavacOpts(customJavacOpts);
builder.setJavacJvmOpts(customJavacJvmOpts);
builder.setJavacExecutionInfo(getExecutionInfo());
builder.setCompressJar(true);
builder.setBuiltinProcessorNames(javaToolchain.getHeaderCompilerBuiltinProcessors());
Expand Down
37 changes: 7 additions & 30 deletions src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileActionBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,11 @@
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
import com.google.devtools.build.lib.actions.ActionEnvironment;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.CommandLine;
import com.google.devtools.build.lib.actions.EmptyRunfilesSupplier;
import com.google.devtools.build.lib.actions.ExecutionRequirements;
import com.google.devtools.build.lib.actions.RunfilesSupplier;
import com.google.devtools.build.lib.actions.extra.ExtraActionInfo;
import com.google.devtools.build.lib.actions.extra.JavaCompileInfo;
import com.google.devtools.build.lib.analysis.FilesToRunProvider;
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
import com.google.devtools.build.lib.analysis.config.CoreOptionConverters.StrictDepsMode;
Expand Down Expand Up @@ -143,13 +142,12 @@ public void extend(ExtraActionInfo.Builder builder, ImmutableList<String> argume
private NestedSet<Artifact> compileTimeDependencyArtifacts =
NestedSetBuilder.emptySet(Order.STABLE_ORDER);
private ImmutableList<String> javacOpts = ImmutableList.of();
private ImmutableList<String> javacJvmOpts = ImmutableList.of();
private ImmutableMap<String, String> executionInfo = ImmutableMap.of();
private boolean compressJar;
private NestedSet<Artifact> classpathEntries = NestedSetBuilder.emptySet(Order.NAIVE_LINK_ORDER);
private BootClassPathInfo bootClassPath = BootClassPathInfo.empty();
private ImmutableList<Artifact> sourcePathEntries = ImmutableList.of();
private FilesToRunProvider javaBuilder;
private JavaToolchainTool javaBuilder;
private NestedSet<Artifact> toolsJars = NestedSetBuilder.emptySet(Order.NAIVE_LINK_ORDER);
private JavaPluginInfo plugins = JavaPluginInfo.empty();
private ImmutableSet<String> builtinProcessorNames = ImmutableSet.of();
Expand Down Expand Up @@ -190,26 +188,10 @@ public JavaCompileAction build() {
compileTimeDependencyArtifacts = NestedSetBuilder.emptySet(Order.STABLE_ORDER);
}

CustomCommandLine.Builder executableLine = CustomCommandLine.builder();
NestedSetBuilder<Artifact> toolsBuilder = NestedSetBuilder.compileOrder();

RunfilesSupplier runfilesSupplier = EmptyRunfilesSupplier.INSTANCE;

// The actual params-file-based command line executed for a compile action.
Artifact javaBuilderJar = checkNotNull(javaBuilder.getExecutable());
if (!javaBuilderJar.getExtension().equals("jar")) {
// JavaBuilder is a non-deploy.jar executable.
executableLine.addPath(javaBuilder.getExecutable().getExecPath());
runfilesSupplier = javaBuilder.getRunfilesSupplier();
toolsBuilder.addTransitive(javaBuilder.getFilesToRun());
} else {
toolsBuilder.add(javaBuilderJar);
executableLine
.addPath(toolchain.getJavaRuntime().javaBinaryExecPathFragment())
.addAll(javacJvmOpts)
.add("-jar")
.addPath(javaBuilderJar.getExecPath());
}
CommandLine executableLine = javaBuilder.buildCommandLine(toolchain, toolsBuilder);

toolsBuilder.addTransitive(toolsJars);

ActionEnvironment actionEnvironment =
Expand Down Expand Up @@ -268,7 +250,7 @@ public JavaCompileAction build() {
/* owner= */ ruleContext.getActionOwner(),
/* env= */ actionEnvironment,
/* tools= */ tools,
/* runfilesSupplier= */ runfilesSupplier,
/* runfilesSupplier= */ EmptyRunfilesSupplier.INSTANCE,
/* progressMessage= */ new ProgressMessage(
/* prefix= */ "Building",
/* output= */ outputs.output(),
Expand All @@ -281,7 +263,7 @@ public JavaCompileAction build() {
/* outputs= */ allOutputs(),
/* executionInfo= */ executionInfo,
/* extraActionInfoSupplier= */ extraActionInfoSupplier,
/* executableLine= */ executableLine.build(),
/* executableLine= */ executableLine,
/* flagLine= */ buildParamFileContents(internedJcopts),
/* configuration= */ ruleContext.getConfiguration(),
/* dependencyArtifacts= */ compileTimeDependencyArtifacts,
Expand Down Expand Up @@ -403,11 +385,6 @@ public JavaCompileActionBuilder setJavacOpts(ImmutableList<String> copts) {
return this;
}

public JavaCompileActionBuilder setJavacJvmOpts(ImmutableList<String> opts) {
this.javacJvmOpts = opts;
return this;
}

public JavaCompileActionBuilder setJavacExecutionInfo(
ImmutableMap<String, String> executionInfo) {
this.executionInfo = executionInfo;
Expand Down Expand Up @@ -461,7 +438,7 @@ public JavaCompileActionBuilder setToolsJars(NestedSet<Artifact> toolsJars) {
return this;
}

public JavaCompileActionBuilder setJavaBuilder(FilesToRunProvider javaBuilder) {
public JavaCompileActionBuilder setJavaBuilder(JavaToolchainTool javaBuilder) {
this.javaBuilder = javaBuilder;
return this;
}
Expand Down
32 changes: 6 additions & 26 deletions src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileActionBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,12 @@
import com.google.devtools.build.lib.actions.ActionEnvironment;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.CommandLine;
import com.google.devtools.build.lib.actions.CommandLines;
import com.google.devtools.build.lib.actions.CompositeRunfilesSupplier;
import com.google.devtools.build.lib.actions.EmptyRunfilesSupplier;
import com.google.devtools.build.lib.actions.ExecutionRequirements;
import com.google.devtools.build.lib.actions.ParamFileInfo;
import com.google.devtools.build.lib.actions.RunfilesSupplier;
import com.google.devtools.build.lib.actions.SpawnResult;
import com.google.devtools.build.lib.analysis.FilesToRunProvider;
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
import com.google.devtools.build.lib.analysis.actions.SpawnAction;
Expand Down Expand Up @@ -318,32 +317,13 @@ public void build(JavaToolchainProvider javaToolchain) throws InterruptedExcepti
.addAll(sourceFiles)
.addTransitive(toolsJars);

ImmutableList<RunfilesSupplier> runfilesSuppliers = ImmutableList.of();
FilesToRunProvider headerCompiler =
JavaToolchainTool headerCompiler =
useHeaderCompilerDirect
? javaToolchain.getHeaderCompilerDirect()
: javaToolchain.getHeaderCompiler();
// The header compiler is either a jar file that needs to be executed using
// `java -jar <path>`, or an executable that can be run directly.
CustomCommandLine executableLine;
if (!headerCompiler.getExecutable().getExtension().equals("jar")) {
runfilesSuppliers = ImmutableList.of(headerCompiler.getRunfilesSupplier());
mandatoryInputs.addTransitive(headerCompiler.getFilesToRun());
executableLine =
CustomCommandLine.builder().addExecPath(headerCompiler.getExecutable()).build();
} else {
mandatoryInputs
.addTransitive(javaToolchain.getJavaRuntime().javaBaseInputsMiddleman())
.add(headerCompiler.getExecutable());
executableLine =
CustomCommandLine.builder()
.addPath(javaToolchain.getJavaRuntime().javaBinaryExecPathFragment())
.add("-Xverify:none")
.addAll(javaToolchain.getTurbineJvmOptions())
.add("-jar")
.addExecPath(headerCompiler.getExecutable())
.build();
}
CommandLine executableLine = headerCompiler.buildCommandLine(javaToolchain, mandatoryInputs);

CustomCommandLine.Builder commandLine =
CustomCommandLine.builder()
Expand Down Expand Up @@ -418,7 +398,7 @@ public void build(JavaToolchainProvider javaToolchain) throws InterruptedExcepti
.getConfiguration()
.modifiedExecutionInfo(executionInfo, "Turbine"),
/* progressMessage= */ progressMessage,
/* runfilesSupplier= */ CompositeRunfilesSupplier.fromSuppliers(runfilesSuppliers),
/* runfilesSupplier= */ EmptyRunfilesSupplier.INSTANCE,
/* mnemonic= */ "Turbine",
/* executeUnconditionally= */ false,
/* extraActionInfoSupplier= */ null,
Expand Down Expand Up @@ -455,7 +435,7 @@ public void build(JavaToolchainProvider javaToolchain) throws InterruptedExcepti
/* owner= */ ruleContext.getActionOwner(),
/* env= */ actionEnvironment,
/* tools= */ toolsJars,
/* runfilesSupplier= */ CompositeRunfilesSupplier.fromSuppliers(runfilesSuppliers),
/* runfilesSupplier= */ EmptyRunfilesSupplier.INSTANCE,
/* progressMessage= */ progressMessage,
/* mandatoryInputs= */ mandatoryInputs.build(),
/* transitiveInputs= */ classpathEntries,
Expand Down
35 changes: 12 additions & 23 deletions src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchain.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,6 @@ public ConfiguredTarget create(RuleContext ruleContext)
ruleContext.attributes().get("javac_supports_workers", Type.BOOLEAN);
boolean javacSupportsMultiplexWorkers =
ruleContext.attributes().get("javac_supports_multiplex_workers", Type.BOOLEAN);
FilesToRunProvider javabuilder = ruleContext.getExecutablePrerequisite("javabuilder");
FilesToRunProvider headerCompiler = ruleContext.getExecutablePrerequisite("header_compiler");
FilesToRunProvider headerCompilerDirect =
ruleContext.getExecutablePrerequisite("header_compiler_direct");
ImmutableSet<String> headerCompilerBuiltinProcessors =
ImmutableSet.copyOf(
ruleContext.attributes().get("header_compiler_builtin_processors", Type.STRING_LIST));
Expand All @@ -85,17 +81,18 @@ public ConfiguredTarget create(RuleContext ruleContext)

NestedSet<Artifact> tools = PrerequisiteArtifacts.nestedSet(ruleContext, "tools");

ImmutableList<String> jvmOpts = getJvmOpts(ruleContext, "jvm_opts");
ImmutableList<String> javabuilderJvmOpts =
ImmutableList.<String>builder()
.addAll(jvmOpts)
.addAll(getJvmOpts(ruleContext, "javabuilder_jvm_opts"))
.build();
ImmutableList<String> turbineJvmOpts =
ImmutableList.<String>builder()
.addAll(jvmOpts)
.addAll(getJvmOpts(ruleContext, "turbine_jvm_opts"))
.build();
ImmutableList<String> jvmOpts =
ruleContext.getExpander().withExecLocations(ImmutableMap.of()).list("jvm_opts");

JavaToolchainTool javabuilder =
JavaToolchainTool.fromRuleContext(
ruleContext, "javabuilder", "javabuilder_data", "javabuilder_jvm_opts");
JavaToolchainTool headerCompiler =
JavaToolchainTool.fromRuleContext(
ruleContext, "header_compiler", "turbine_data", "turbine_jvm_opts");
JavaToolchainTool headerCompilerDirect =
JavaToolchainTool.fromFilesToRunProvider(
ruleContext.getExecutablePrerequisite("header_compiler_direct"));

ImmutableList<JavaPackageConfigurationProvider> packageConfiguration =
ImmutableList.copyOf(
Expand All @@ -113,8 +110,6 @@ public ConfiguredTarget create(RuleContext ruleContext)
ruleContext.getLabel(),
javacopts,
jvmOpts,
javabuilderJvmOpts,
turbineJvmOpts,
javacSupportsWorkers,
javacSupportsMultiplexWorkers,
bootclasspath,
Expand Down Expand Up @@ -177,12 +172,6 @@ private static ImmutableListMultimap<String, String> getCompatibleJavacOptions(
return result.build();
}

private static ImmutableList<String> getJvmOpts(
RuleContext ruleContext,
String attribute) {
return ruleContext.getExpander().withExecLocations(ImmutableMap.of()).list(attribute);
}

private static BootClassPathInfo getBootClassPathInfo(RuleContext ruleContext) {
List<BootClassPathInfo> bootClassPathInfos =
ruleContext.getPrerequisites("bootclasspath", BootClassPathInfo.PROVIDER);
Expand Down
47 changes: 14 additions & 33 deletions src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchainProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,13 @@ public static JavaToolchainProvider create(
Label label,
ImmutableList<String> javacOptions,
ImmutableList<String> jvmOptions,
ImmutableList<String> javabuilderJvmOptions,
ImmutableList<String> turbineJvmOptions,
boolean javacSupportsWorkers,
boolean javacSupportsMultiplexWorkers,
BootClassPathInfo bootclasspath,
NestedSet<Artifact> tools,
FilesToRunProvider javaBuilder,
@Nullable FilesToRunProvider headerCompiler,
@Nullable FilesToRunProvider headerCompilerDirect,
JavaToolchainTool javaBuilder,
@Nullable JavaToolchainTool headerCompiler,
@Nullable JavaToolchainTool headerCompilerDirect,
ImmutableSet<String> headerCompilerBuiltinProcessors,
ImmutableSet<String> reducedClasspathIncompatibleProcessors,
boolean forciblyDisableHeaderCompilation,
Expand Down Expand Up @@ -117,8 +115,6 @@ public static JavaToolchainProvider create(
compatibleJavacOptions,
javacOptions,
jvmOptions,
javabuilderJvmOptions,
turbineJvmOptions,
javacSupportsWorkers,
javacSupportsMultiplexWorkers,
packageConfiguration,
Expand All @@ -131,9 +127,9 @@ public static JavaToolchainProvider create(
private final Label label;
private final BootClassPathInfo bootclasspath;
private final NestedSet<Artifact> tools;
private final FilesToRunProvider javaBuilder;
@Nullable private final FilesToRunProvider headerCompiler;
@Nullable private final FilesToRunProvider headerCompilerDirect;
private final JavaToolchainTool javaBuilder;
@Nullable private final JavaToolchainTool headerCompiler;
@Nullable private final JavaToolchainTool headerCompilerDirect;
private final ImmutableSet<String> headerCompilerBuiltinProcessors;
private final ImmutableSet<String> reducedClasspathIncompatibleProcessors;
private final boolean forciblyDisableHeaderCompilation;
Expand All @@ -147,8 +143,6 @@ public static JavaToolchainProvider create(
private final ImmutableListMultimap<String, String> compatibleJavacOptions;
private final ImmutableList<String> javacOptions;
private final ImmutableList<String> jvmOptions;
private final ImmutableList<String> javabuilderJvmOptions;
private final ImmutableList<String> turbineJvmOptions;
private final boolean javacSupportsWorkers;
private final boolean javacSupportsMultiplexWorkers;
private final ImmutableList<JavaPackageConfigurationProvider> packageConfiguration;
Expand All @@ -162,9 +156,9 @@ public static JavaToolchainProvider create(
Label label,
BootClassPathInfo bootclasspath,
NestedSet<Artifact> tools,
FilesToRunProvider javaBuilder,
@Nullable FilesToRunProvider headerCompiler,
@Nullable FilesToRunProvider headerCompilerDirect,
JavaToolchainTool javaBuilder,
@Nullable JavaToolchainTool headerCompiler,
@Nullable JavaToolchainTool headerCompilerDirect,
ImmutableSet<String> headerCompilerBuiltinProcessors,
ImmutableSet<String> reducedClasspathIncompatibleProcessors,
boolean forciblyDisableHeaderCompilation,
Expand All @@ -178,8 +172,6 @@ public static JavaToolchainProvider create(
ImmutableListMultimap<String, String> compatibleJavacOptions,
ImmutableList<String> javacOptions,
ImmutableList<String> jvmOptions,
ImmutableList<String> javabuilderJvmOptions,
ImmutableList<String> turbineJvmOptions,
boolean javacSupportsWorkers,
boolean javacSupportsMultiplexWorkers,
ImmutableList<JavaPackageConfigurationProvider> packageConfiguration,
Expand Down Expand Up @@ -208,8 +200,6 @@ public static JavaToolchainProvider create(
this.compatibleJavacOptions = compatibleJavacOptions;
this.javacOptions = javacOptions;
this.jvmOptions = jvmOptions;
this.javabuilderJvmOptions = javabuilderJvmOptions;
this.turbineJvmOptions = turbineJvmOptions;
this.javacSupportsWorkers = javacSupportsWorkers;
this.javacSupportsMultiplexWorkers = javacSupportsMultiplexWorkers;
this.packageConfiguration = packageConfiguration;
Expand All @@ -234,14 +224,14 @@ public NestedSet<Artifact> getTools() {
return tools;
}

/** Returns the {@link FilesToRunProvider} of JavaBuilder */
public FilesToRunProvider getJavaBuilder() {
/** Returns the {@link JavaToolchainTool} for JavaBuilder */
public JavaToolchainTool getJavaBuilder() {
return javaBuilder;
}

/** @return the {@link FilesToRunProvider} of the Header Compiler deploy jar */
/** Returns the {@link JavaToolchainTool} for the header compiler */
@Nullable
public FilesToRunProvider getHeaderCompiler() {
public JavaToolchainTool getHeaderCompiler() {
return headerCompiler;
}

Expand All @@ -250,7 +240,7 @@ public FilesToRunProvider getHeaderCompiler() {
* non-annotation processing actions.
*/
@Nullable
public FilesToRunProvider getHeaderCompilerDirect() {
public JavaToolchainTool getHeaderCompilerDirect() {
return headerCompilerDirect;
}

Expand Down Expand Up @@ -350,15 +340,6 @@ public ImmutableList<String> getJvmOptions() {
return jvmOptions;
}

/** Returns the list of JVM options for running JavaBuilder. */
public ImmutableList<String> getJavabuilderJvmOptions() {
return javabuilderJvmOptions;
}

public ImmutableList<String> getTurbineJvmOptions() {
return turbineJvmOptions;
}

/** @return whether JavaBuilders supports running as a persistent worker or not */
public boolean getJavacSupportsWorkers() {
return javacSupportsWorkers;
Expand Down
Loading

0 comments on commit a1b19df

Please sign in to comment.