From e0a9081f541baa21d7d9dfad8648692baa274d56 Mon Sep 17 00:00:00 2001 From: Googler Date: Fri, 2 Dec 2022 09:24:10 -0800 Subject: [PATCH] Propagate runfiles for singlejar. Since the singlejar toolchain attribute is currently defined as a single file and carried around as an Artifact instead of a FilesToRunProvider, it's not possible to implement it as a wrapper script that dispatches to an actual implementation somewhere in its runfiles. This would be useful to experiment with cross-platform action sharing. PiperOrigin-RevId: 492487124 Change-Id: Ib0f80314eae09bd865b3f31a4180bf068833cdf4 --- .../devtools/build/lib/rules/android/AarImport.java | 3 ++- .../build/lib/rules/android/AndroidBinary.java | 2 +- .../build/lib/rules/android/ApkActionsBuilder.java | 2 +- .../build/lib/rules/java/DeployArchiveBuilder.java | 3 ++- .../devtools/build/lib/rules/java/JavaBinary.java | 8 +++++--- .../devtools/build/lib/rules/java/JavaToolchain.java | 2 +- .../build/lib/rules/java/JavaToolchainProvider.java | 12 ++++++------ .../build/lib/rules/java/JavaToolchainRule.java | 1 - .../java/JavaToolchainStarlarkApiProviderApi.java | 6 +++--- .../builtins_bzl/common/java/java_helper.bzl | 2 +- 10 files changed, 22 insertions(+), 19 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AarImport.java b/src/main/java/com/google/devtools/build/lib/rules/android/AarImport.java index 9375ef0e17a6cb..f6fa70d930744d 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AarImport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AarImport.java @@ -20,6 +20,7 @@ import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.FileProvider; +import com.google.devtools.build.lib.analysis.FilesToRunProvider; import com.google.devtools.build.lib.analysis.OutputGroupInfo; import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder; import com.google.devtools.build.lib.analysis.RuleConfiguredTargetFactory; @@ -403,7 +404,7 @@ private static SpawnAction createAarEmbeddedJarsExtractorActions( private static SpawnAction createAarJarsMergingActions( RuleContext ruleContext, Artifact jarsTreeArtifact, Artifact mergedJar, Artifact paramFile) { SpawnAction.Builder builder = new SpawnAction.Builder().useDefaultShellEnvironment(); - Artifact singleJar = JavaToolchainProvider.from(ruleContext).getSingleJar(); + FilesToRunProvider singleJar = JavaToolchainProvider.from(ruleContext).getSingleJar(); return builder .setExecutable(singleJar) .setMnemonic("AarJarsMerger") diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java index f98256f55e886a..a6612d83fca5dd 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java @@ -1958,7 +1958,7 @@ private static SpawnAction.Builder createSpawnActionBuilder(RuleContext ruleCont } private static SpawnAction.Builder singleJarSpawnActionBuilder(RuleContext ruleContext) { - Artifact singleJar = JavaToolchainProvider.from(ruleContext).getSingleJar(); + FilesToRunProvider singleJar = JavaToolchainProvider.from(ruleContext).getSingleJar(); SpawnAction.Builder builder = createSpawnActionBuilder(ruleContext).useDefaultShellEnvironment(); builder.setExecutable(singleJar); diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ApkActionsBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/ApkActionsBuilder.java index 44cc3659b8b75a..8378171219a736 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/ApkActionsBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/ApkActionsBuilder.java @@ -418,7 +418,7 @@ private void signApk( private static void setSingleJarAsExecutable( RuleContext ruleContext, SpawnAction.Builder builder) { - Artifact singleJar = JavaToolchainProvider.from(ruleContext).getSingleJar(); + FilesToRunProvider singleJar = JavaToolchainProvider.from(ruleContext).getSingleJar(); builder.setExecutable(singleJar); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/DeployArchiveBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/java/DeployArchiveBuilder.java index d196055ce38b76..de011c2f5f404d 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/DeployArchiveBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/DeployArchiveBuilder.java @@ -26,6 +26,7 @@ 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; +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; @@ -420,7 +421,7 @@ public void build() throws InterruptedException { inputs.add(libModules); } - Artifact singlejar = JavaToolchainProvider.from(ruleContext).getSingleJar(); + FilesToRunProvider singlejar = JavaToolchainProvider.from(ruleContext).getSingleJar(); String toolchainIdentifier = null; try { diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java index b86f71ad163e50..87a04c43e38764 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java @@ -31,6 +31,7 @@ import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException; import com.google.devtools.build.lib.analysis.Allowlist; import com.google.devtools.build.lib.analysis.ConfiguredTarget; +import com.google.devtools.build.lib.analysis.FilesToRunProvider; import com.google.devtools.build.lib.analysis.OutputGroupInfo; import com.google.devtools.build.lib.analysis.PrerequisiteArtifacts; import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder; @@ -511,9 +512,10 @@ public ConfiguredTarget create(RuleContext ruleContext) // Make single jar reachable from the coverage environment because it needs to be executed // by the coverage collection script. - Artifact singleJar = JavaToolchainProvider.from(ruleContext).getSingleJar(); - coverageEnvironment.add(new Pair<>("SINGLE_JAR_TOOL", singleJar.getExecPathString())); - coverageSupportFiles.add(singleJar); + FilesToRunProvider singleJar = JavaToolchainProvider.from(ruleContext).getSingleJar(); + coverageEnvironment.add( + new Pair<>("SINGLE_JAR_TOOL", singleJar.getExecutable().getExecPathString())); + coverageSupportFiles.addTransitive(singleJar.getFilesToRun()); } common.addTransitiveInfoProviders( diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchain.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchain.java index 782cc1abb5acd4..20db593e703e21 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchain.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchain.java @@ -78,7 +78,7 @@ public ConfiguredTarget create(RuleContext ruleContext) .get("reduced_classpath_incompatible_processors", Type.STRING_LIST)); boolean forciblyDisableHeaderCompilation = ruleContext.attributes().get("forcibly_disable_header_compilation", Type.BOOLEAN); - Artifact singleJar = ruleContext.getPrerequisiteArtifact("singlejar"); + FilesToRunProvider singleJar = ruleContext.getExecutablePrerequisite("singlejar"); FilesToRunProvider oneVersion = ruleContext.getExecutablePrerequisite("oneversion"); Artifact oneVersionAllowlist = ruleContext.getPrerequisiteArtifact("oneversion_whitelist"); Artifact genClass = ruleContext.getPrerequisiteArtifact("genclass"); diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchainProvider.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchainProvider.java index 416c97cecb9d80..c2ed988a889437 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchainProvider.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchainProvider.java @@ -112,7 +112,7 @@ public static JavaToolchainProvider create( ImmutableSet headerCompilerBuiltinProcessors, ImmutableSet reducedClasspathIncompatibleProcessors, boolean forciblyDisableHeaderCompilation, - Artifact singleJar, + FilesToRunProvider singleJar, @Nullable FilesToRunProvider oneVersion, @Nullable Artifact oneVersionAllowlist, Artifact genClass, @@ -174,7 +174,7 @@ public static JavaToolchainProvider create( private final ImmutableSet headerCompilerBuiltinProcessors; private final ImmutableSet reducedClasspathIncompatibleProcessors; private final boolean forciblyDisableHeaderCompilation; - private final Artifact singleJar; + private final FilesToRunProvider singleJar; @Nullable private final FilesToRunProvider oneVersion; @Nullable private final Artifact oneVersionAllowlist; private final Artifact genClass; @@ -208,7 +208,7 @@ private JavaToolchainProvider( ImmutableSet headerCompilerBuiltinProcessors, ImmutableSet reducedClasspathIncompatibleProcessors, boolean forciblyDisableHeaderCompilation, - Artifact singleJar, + FilesToRunProvider singleJar, @Nullable FilesToRunProvider oneVersion, @Nullable Artifact oneVersionAllowlist, Artifact genClass, @@ -333,14 +333,14 @@ public boolean getForciblyDisableHeaderCompilation() { return forciblyDisableHeaderCompilation; } - /** Returns the {@link Artifact} of the SingleJar deploy jar */ + /** Returns the {@link FilesToRunProvider} of the SingleJar tool. */ @Override - public Artifact getSingleJar() { + public FilesToRunProvider getSingleJar() { return singleJar; } /** - * Return the {@link Artifact} of the binary that enforces one-version compliance of java + * Return the {@link FilesToRunProvider} of the tool that enforces one-version compliance of Java * binaries. */ @Override diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchainRule.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchainRule.java index ad4de48f61a6bb..0647eb38da2694 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchainRule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchainRule.java @@ -153,7 +153,6 @@ The Java target version (e.g., '6' or '7'). It specifies for which Java runtime .add( attr("singlejar", LABEL_LIST) .mandatory() - .singleArtifact() // This needs to be in the execution configuration. .cfg(ExecutionTransitionFactory.create()) .allowedFileTypes(FileTypeSet.ANY_FILE) diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/java/JavaToolchainStarlarkApiProviderApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/java/JavaToolchainStarlarkApiProviderApi.java index 9c6bc388003c10..fc6e47b5f7a5dc 100644 --- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/java/JavaToolchainStarlarkApiProviderApi.java +++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/java/JavaToolchainStarlarkApiProviderApi.java @@ -49,13 +49,13 @@ public interface JavaToolchainStarlarkApiProviderApi extends StructApi { @StarlarkMethod(name = "target_version", doc = "The java target version.", structField = true) String getTargetVersion(); - @StarlarkMethod(name = "single_jar", doc = "The SingleJar deploy jar.", structField = true) - FileApi getSingleJar(); + @StarlarkMethod(name = "single_jar", doc = "The SingleJar tool.", structField = true) + FilesToRunProviderApi getSingleJar(); @Nullable @StarlarkMethod( name = "one_version_tool", - doc = "The artifact that enforces One-Version compliance of java binaries.", + doc = "The tool that enforces One-Version compliance of java binaries.", structField = true, allowReturnNones = true) FilesToRunProviderApi getOneVersionBinary(); diff --git a/src/main/starlark/builtins_bzl/common/java/java_helper.bzl b/src/main/starlark/builtins_bzl/common/java/java_helper.bzl index ae9095944f2991..4155061c435446 100644 --- a/src/main/starlark/builtins_bzl/common/java/java_helper.bzl +++ b/src/main/starlark/builtins_bzl/common/java/java_helper.bzl @@ -239,7 +239,7 @@ def _get_coverage_config(ctx): manifest = manifest, env = { "JAVA_RUNTIME_CLASSPATH_FOR_COVERAGE": manifest.path, - "SINGLE_JAR_TOOL": singlejar.path, + "SINGLE_JAR_TOOL": singlejar.executable.path, }, support_files = [manifest, singlejar], )