Skip to content

Commit

Permalink
Deletes non-native singlejar support in native java rules
Browse files Browse the repository at this point in the history
Removal is now possible since the following issues are resolved:
#2241
#7365

PiperOrigin-RevId: 408004890
  • Loading branch information
hvadehra authored and Copybara-Service committed Nov 6, 2021
1 parent 21dfe4c commit fad4933
Show file tree
Hide file tree
Showing 8 changed files with 31 additions and 137 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -603,7 +603,6 @@ public CustomCommandLine buildSingleJarCommandLine(
boolean includeBuildData,
Compression compression,
Artifact launcher,
boolean usingNativeSinglejar,
// Explicitly ignoring params since Bazel doesn't yet support one version
OneVersionEnforcementLevel oneVersionEnforcementLevel,
Artifact oneVersionAllowlistArtifact,
Expand All @@ -617,8 +616,7 @@ public CustomCommandLine buildSingleJarCommandLine(
classpath,
includeBuildData,
compression,
launcher,
usingNativeSinglejar)
launcher)
.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
import com.google.devtools.build.lib.rules.java.JavaInfo;
import com.google.devtools.build.lib.rules.java.JavaRuleOutputJarsProvider;
import com.google.devtools.build.lib.rules.java.JavaRuleOutputJarsProvider.JavaOutput;
import com.google.devtools.build.lib.rules.java.JavaRuntimeInfo;
import com.google.devtools.build.lib.rules.java.JavaSemantics;
import com.google.devtools.build.lib.rules.java.JavaSourceJarsProvider;
import com.google.devtools.build.lib.rules.java.JavaToolchainProvider;
Expand Down Expand Up @@ -401,7 +400,10 @@ private static SpawnAction createAarEmbeddedJarsExtractorActions(

private static SpawnAction createAarJarsMergingActions(
RuleContext ruleContext, Artifact jarsTreeArtifact, Artifact mergedJar, Artifact paramFile) {
return singleJarSpawnActionBuilder(ruleContext)
SpawnAction.Builder builder = new SpawnAction.Builder().useDefaultShellEnvironment();
Artifact singleJar = JavaToolchainProvider.from(ruleContext).getSingleJar();
return builder
.setExecutable(singleJar)
.setMnemonic("AarJarsMerger")
.setProgressMessage("Merging AAR embedded jars")
.addInput(jarsTreeArtifact)
Expand Down Expand Up @@ -477,20 +479,4 @@ private static SpecialArtifact createAarTreeArtifact(RuleContext ruleContext, St
return ruleContext.getTreeArtifact(rootRelativePath, ruleContext.getBinOrGenfilesDirectory());
}

// Adds the appropriate SpawnAction options depending on if SingleJar is a jar or not.
private static SpawnAction.Builder singleJarSpawnActionBuilder(RuleContext ruleContext) {
SpawnAction.Builder builder = new SpawnAction.Builder().useDefaultShellEnvironment();
Artifact singleJar = JavaToolchainProvider.from(ruleContext).getSingleJar();
if (singleJar.getFilename().endsWith(".jar")) {
builder
.setJarExecutable(
JavaCommon.getHostJavaExecutable(ruleContext),
singleJar,
JavaToolchainProvider.from(ruleContext).getJvmOptions())
.addTransitiveInputs(JavaRuntimeInfo.forHost(ruleContext).javaBaseInputs());
} else {
builder.setExecutable(singleJar);
}
return builder;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@
import com.google.devtools.build.lib.rules.java.JavaCommon;
import com.google.devtools.build.lib.rules.java.JavaConfiguration;
import com.google.devtools.build.lib.rules.java.JavaConfiguration.OneVersionEnforcementLevel;
import com.google.devtools.build.lib.rules.java.JavaRuntimeInfo;
import com.google.devtools.build.lib.rules.java.JavaSemantics;
import com.google.devtools.build.lib.rules.java.JavaTargetAttributes;
import com.google.devtools.build.lib.rules.java.JavaToolchainProvider;
Expand Down Expand Up @@ -1805,21 +1804,11 @@ private static SpawnAction.Builder createSpawnActionBuilder(RuleContext ruleCont
ruleContext.getRule(), ruleContext.isAllowTagsPropagation()));
}

// Adds the appropriate SpawnAction options depending on if SingleJar is a jar or not.
private static SpawnAction.Builder singleJarSpawnActionBuilder(RuleContext ruleContext) {
Artifact singleJar = JavaToolchainProvider.from(ruleContext).getSingleJar();
SpawnAction.Builder builder =
createSpawnActionBuilder(ruleContext).useDefaultShellEnvironment();
if (singleJar.getFilename().endsWith(".jar")) {
builder
.setJarExecutable(
JavaCommon.getHostJavaExecutable(ruleContext),
singleJar,
JavaToolchainProvider.from(ruleContext).getJvmOptions())
.addTransitiveInputs(JavaRuntimeInfo.forHost(ruleContext).javaBaseInputs());
} else {
builder.setExecutable(singleJar);
}
builder.setExecutable(singleJar);
return builder;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@
import com.google.devtools.build.lib.packages.TargetUtils;
import com.google.devtools.build.lib.packages.Type;
import com.google.devtools.build.lib.rules.android.AndroidConfiguration.ApkSigningMethod;
import com.google.devtools.build.lib.rules.java.JavaCommon;
import com.google.devtools.build.lib.rules.java.JavaRuntimeInfo;
import com.google.devtools.build.lib.rules.java.JavaToolchainProvider;
import java.util.List;

Expand Down Expand Up @@ -391,20 +389,10 @@ private void signApk(
actionBuilder.addCommandLine(commandLine.build()).build(ruleContext));
}

// Adds the appropriate SpawnAction options depending on if SingleJar is a jar or not.
private static void setSingleJarAsExecutable(
RuleContext ruleContext, SpawnAction.Builder builder) {
Artifact singleJar = JavaToolchainProvider.from(ruleContext).getSingleJar();
if (singleJar.getFilename().endsWith(".jar")) {
builder
.setJarExecutable(
JavaCommon.getHostJavaExecutable(ruleContext),
singleJar,
JavaToolchainProvider.from(ruleContext).getJvmOptions())
.addTransitiveInputs(JavaRuntimeInfo.forHost(ruleContext).javaBaseInputs());
} else {
builder.setExecutable(singleJar);
}
builder.setExecutable(singleJar);
}

private Artifact getApkArtifact(RuleContext ruleContext, String baseName) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.CommandLine;
import com.google.devtools.build.lib.actions.ExecutionRequirements;
import com.google.devtools.build.lib.actions.ParamFileInfo;
import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType;
import com.google.devtools.build.lib.actions.ResourceSet;
Expand All @@ -31,7 +30,6 @@
import com.google.devtools.build.lib.analysis.actions.SpawnAction;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.collect.nestedset.Order;
import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
import com.google.devtools.build.lib.packages.TargetUtils;
import com.google.devtools.build.lib.rules.cpp.CppHelper;
Expand All @@ -49,8 +47,6 @@ public class DeployArchiveBuilder {
*/
private static final int SINGLEJAR_MEMORY_MB = 1600;

private static final String SINGLEJAR_MAX_MEMORY = "-Xmx" + SINGLEJAR_MEMORY_MB + "m";

private static final ResourceSet DEPLOY_ACTION_RESOURCE_SET =
ResourceSet.createWithRamCpu(/*memoryMb = */ SINGLEJAR_MEMORY_MB, /*cpuUsage = */ 1);

Expand Down Expand Up @@ -187,8 +183,7 @@ public static CustomCommandLine.Builder defaultSingleJarCommandLineWithoutOneVer
NestedSet<Artifact> runtimeClasspath,
boolean includeBuildData,
Compression compress,
Artifact launcher,
boolean usingNativeSinglejar) {
Artifact launcher) {
return defaultSingleJarCommandLine(
outputJar,
javaMainClass,
Expand All @@ -199,7 +194,6 @@ public static CustomCommandLine.Builder defaultSingleJarCommandLineWithoutOneVer
includeBuildData,
compress,
launcher,
usingNativeSinglejar,
OneVersionEnforcementLevel.OFF,
null);
}
Expand All @@ -214,7 +208,6 @@ public static CustomCommandLine.Builder defaultSingleJarCommandLine(
boolean includeBuildData,
Compression compress,
Artifact launcher,
boolean usingNativeSinglejar,
OneVersionEnforcementLevel oneVersionEnforcementLevel,
@Nullable Artifact oneVersionAllowlistArtifact) {

Expand Down Expand Up @@ -247,14 +240,10 @@ public static CustomCommandLine.Builder defaultSingleJarCommandLine(

args.addExecPaths("--classpath_resources", classpathResources);
if (runtimeClasspath != null) {
if (usingNativeSinglejar) {
args.addAll(
"--sources", OneVersionCheckActionBuilder.jarAndTargetVectorArg(runtimeClasspath));
} else {
args.addExecPaths("--sources", runtimeClasspath);
}
args.addAll(
"--sources", OneVersionCheckActionBuilder.jarAndTargetVectorArg(runtimeClasspath));
}
if (oneVersionEnforcementLevel != OneVersionEnforcementLevel.OFF && usingNativeSinglejar) {
if (oneVersionEnforcementLevel != OneVersionEnforcementLevel.OFF) {
args.add("--enforce_one_version");
// RuleErrors should have been added in Builder.build() before this command
// line is invoked.
Expand Down Expand Up @@ -346,11 +335,8 @@ public void build() throws InterruptedException {
if (sharedArchive != null) {
inputs.add(sharedArchive);
}
// If singlejar's name ends with .jar, it is Java application, otherwise it is native.
// TODO(asmundak): once https://github.com/bazelbuild/bazel/issues/2241 is fixed (that is,
// the native singlejar is used on windows) remove support for the Java implementation

Artifact singlejar = JavaToolchainProvider.from(ruleContext).getSingleJar();
boolean usingNativeSinglejar = !singlejar.getFilename().endsWith(".jar");

String toolchainIdentifier = null;
try {
Expand All @@ -373,52 +359,30 @@ public void build() throws InterruptedException {
includeBuildData,
compression,
launcher,
usingNativeSinglejar,
oneVersionEnforcementLevel,
oneVersionAllowlistArtifact,
sharedArchive);
if (checkDesugarDeps) {
commandLine = CommandLine.concat(commandLine, ImmutableList.of("--check_desugar_deps"));
}

NestedSet<String> jvmArgs = NestedSetBuilder.create(Order.STABLE_ORDER, SINGLEJAR_MAX_MEMORY);

ImmutableMap.Builder<String, String> executionInfo = ImmutableMap.builder();
executionInfo.putAll(
TargetUtils.getExecutionInfo(ruleContext.getRule(), ruleContext.isAllowTagsPropagation()));

if (!usingNativeSinglejar) {
executionInfo.putAll(ExecutionRequirements.WORKER_MODE_ENABLED);
ruleContext.registerAction(
new SpawnAction.Builder()
.useDefaultShellEnvironment()
.addTransitiveInputs(inputs.build())
.addTransitiveInputs(JavaRuntimeInfo.forHost(ruleContext).javaBaseInputs())
.addOutput(outputJar)
.setResources(DEPLOY_ACTION_RESOURCE_SET)
.setJarExecutable(JavaCommon.getHostJavaExecutable(ruleContext), singlejar, jvmArgs)
.addCommandLine(
commandLine,
ParamFileInfo.builder(ParameterFileType.SHELL_QUOTED).setUseAlways(true).build())
.setProgressMessage("Building deploy jar %s", outputJar.prettyPrint())
.setMnemonic("JavaDeployJar")
.setExecutionInfo(executionInfo.build())
.build(ruleContext));
} else {
ruleContext.registerAction(
new SpawnAction.Builder()
.useDefaultShellEnvironment()
.addTransitiveInputs(inputs.build())
.addOutput(outputJar)
.setResources(DEPLOY_ACTION_RESOURCE_SET)
.setExecutable(singlejar)
.addCommandLine(
commandLine,
ParamFileInfo.builder(ParameterFileType.SHELL_QUOTED).setUseAlways(true).build())
.setProgressMessage("Building deploy jar %s", outputJar.prettyPrint())
.setMnemonic("JavaDeployJar")
.setExecutionInfo(executionInfo.build())
.build(ruleContext));
}
ruleContext.registerAction(
new SpawnAction.Builder()
.useDefaultShellEnvironment()
.addTransitiveInputs(inputs.build())
.addOutput(outputJar)
.setResources(DEPLOY_ACTION_RESOURCE_SET)
.setExecutable(singlejar)
.addCommandLine(
commandLine,
ParamFileInfo.builder(ParameterFileType.SHELL_QUOTED).setUseAlways(true).build())
.setProgressMessage("Building deploy jar %{output}")
.setMnemonic("JavaDeployJar")
.setExecutionInfo(executionInfo.build())
.build(ruleContext));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,6 @@ CustomCommandLine buildSingleJarCommandLine(
boolean includeBuildData,
Compression compression,
Artifact launcher,
boolean usingNativeSinglejar,
OneVersionEnforcementLevel oneVersionEnforcementLevel,
Artifact oneVersionAllowlistArtifact,
Artifact sharedArchive);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,18 +87,7 @@ public void build(JavaSemantics semantics, RuleContext ruleContext) {
checkNotNull(javaToolchain, "javaToolchain must not be null");
checkNotNull(javaToolchain.getJavaRuntime(), "javabase must not be null");

Artifact singleJar = javaToolchain.getSingleJar();
SpawnAction.Builder builder = new SpawnAction.Builder();
if (singleJar.getFilename().endsWith(".jar")) {
builder
.setJarExecutable(
javaToolchain.getJavaRuntime().javaBinaryExecPathFragment(),
singleJar,
javaToolchain.getJvmOptions())
.addTransitiveInputs(javaToolchain.getJavaRuntime().javaBaseInputs());
} else {
builder.setExecutable(singleJar);
}
CustomCommandLine.Builder command =
CustomCommandLine.builder()
.add("--normalize")
Expand All @@ -123,6 +112,7 @@ public void build(JavaSemantics semantics, RuleContext ruleContext) {
}
ruleContext.registerAction(
builder
.setExecutable(javaToolchain.getSingleJar())
.useDefaultShellEnvironment()
.addOutput(outputJar)
.addInputs(messages)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.CommandLine;
import com.google.devtools.build.lib.actions.CommandLineItem;
import com.google.devtools.build.lib.actions.ExecutionRequirements;
import com.google.devtools.build.lib.actions.ParamFileInfo;
import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType;
import com.google.devtools.build.lib.analysis.RuleContext;
Expand All @@ -48,27 +47,6 @@ public final class SingleJarActionBuilder {
ImmutableList.of(
"--compression", "--normalize", "--exclude_build_data", "--warn_duplicate_resources");

/** Constructs the base spawn for a singlejar action. */
private static SpawnAction.Builder singleJarActionBuilder(JavaToolchainProvider provider) {
Artifact singleJar = provider.getSingleJar();
SpawnAction.Builder builder = new SpawnAction.Builder();
// If singlejar's name ends with .jar, it is Java application, otherwise it is native.
// TODO(asmundak): once https://github.com/bazelbuild/bazel/issues/2241 is fixed (that is,
// the native singlejar is used on windows) remove support for the Java implementation
if (singleJar.getFilename().endsWith(".jar")) {
builder
.addTransitiveInputs(provider.getJavaRuntime().javaBaseInputs())
.setJarExecutable(
provider.getJavaRuntime().javaBinaryExecPathFragment(),
singleJar,
provider.getJvmOptions())
.setExecutionInfo(ExecutionRequirements.WORKER_MODE_ENABLED);
} else {
builder.setExecutable(singleJar);
}
return builder;
}

/**
* Creates an Action that packages files into a Jar file.
*
Expand Down Expand Up @@ -116,7 +94,8 @@ public static void createSourceJarAction(
requireNonNull(semantics);
}
SpawnAction.Builder builder =
singleJarActionBuilder(toolchainProvider)
new SpawnAction.Builder()
.setExecutable(toolchainProvider.getSingleJar())
.addOutput(outputJar)
.addTransitiveInputs(resources)
.addTransitiveInputs(resourceJars)
Expand All @@ -141,7 +120,8 @@ public static void createSingleJarAction(
requireNonNull(jars);
requireNonNull(output);
SpawnAction.Builder builder =
singleJarActionBuilder(JavaToolchainProvider.from(ruleContext))
new SpawnAction.Builder()
.setExecutable(JavaToolchainProvider.from(ruleContext).getSingleJar())
.addOutput(output)
.addTransitiveInputs(jars)
.addCommandLine(
Expand Down

0 comments on commit fad4933

Please sign in to comment.