Set up only one link action for JavaBinary launcher with fission

This is a fixed version of https://github.com/bazelbuild/bazel/commit/4ba134f008719a52c1f74dc070121017d0b08f44, along with new tests for the issue provoking the rollback.

Original description:
Change getLauncher to return both a stripped and unstripped launcher binary
artifact under fission, instead of invoking getLauncher twice. This was
setting up two identical link actions that required later work to filter
out the redundant action in filterSharedActionsAndThrowActionConflict.
This becomes extremely inefficient under ThinLTO, where each launcher link
is actually 1 LTO indexing action, N LTO Backend actions, and 1 native link
action.

PiperOrigin-RevId: 197391873

2 files changed, 28 insertions, 18 deletions

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 16427d1df3..69ff3d9834 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
@@ -50,6 +50,7 @@ import com.google.devtools.build.lib.rules.java.ProguardHelper.ProguardOutput;
 import com.google.devtools.build.lib.rules.java.proto.GeneratedExtensionRegistryProvider;
 import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.util.OS;
+import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -139,23 +140,22 @@ public class JavaBinary implements RuleConfiguredTargetFactory {
     // TODO(b/64384912): Remove in favor of CcToolchainProvider
     boolean stripAsDefault =
         ccToolchain.useFission() && cppConfiguration.getCompilationMode() == CompilationMode.OPT;
-    Artifact launcher = semantics.getLauncher(ruleContext, common, deployArchiveBuilder,
-        runfilesBuilder, jvmFlags, attributesBuilder, stripAsDefault);
-
     DeployArchiveBuilder unstrippedDeployArchiveBuilder = null;
-    Artifact unstrippedLauncher = null;
     if (stripAsDefault) {
       unstrippedDeployArchiveBuilder = new DeployArchiveBuilder(semantics, ruleContext);
-      unstrippedLauncher =
-          semantics.getLauncher(
-              ruleContext,
-              common,
-              unstrippedDeployArchiveBuilder,
-              runfilesBuilder,
-              jvmFlags,
-              attributesBuilder,
-              /* shouldStrip= */ false);
     }
+    Pair<Artifact, Artifact> launcherAndUnstrippedLauncher =
+        semantics.getLauncher(
+            ruleContext,
+            common,
+            deployArchiveBuilder,
+            unstrippedDeployArchiveBuilder,
+            runfilesBuilder,
+            jvmFlags,
+            attributesBuilder,
+            stripAsDefault);
+    Artifact launcher = launcherAndUnstrippedLauncher.first;
+    Artifact unstrippedLauncher = launcherAndUnstrippedLauncher.second;
 
     JavaCompilationArtifacts.Builder javaArtifactsBuilder = new JavaCompilationArtifacts.Builder();
     Artifact instrumentationMetadata =
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaSemantics.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaSemantics.java
index 4325442caf..90972d2315 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaSemantics.java
@@ -44,6 +44,7 @@ import com.google.devtools.build.lib.rules.java.JavaConfiguration.OneVersionEnfo
 import com.google.devtools.build.lib.rules.java.proto.GeneratedExtensionRegistryProvider;
 import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.util.FileType;
+import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.io.File;
 import java.util.List;
@@ -436,16 +437,25 @@ public interface JavaSemantics {
    * @param ruleContext The rule context
    * @param common The common helper class.
    * @param deployArchiveBuilder the builder to construct the deploy archive action (mutable).
+   * @param unstrippedDeployArchiveBuilder the builder to construct the unstripped deploy archive
+   *     action (mutable).
    * @param runfilesBuilder the builder to construct the list of runfiles (mutable).
    * @param jvmFlags the list of flags to pass to the JVM when running the Java binary (mutable).
    * @param attributesBuilder the builder to construct the list of attributes of this target
-   *        (mutable).
-   * @return the launcher as an artifact
+   *     (mutable).
+   * @return the launcher and unstripped launcher as an artifact pair. If shouldStrip is false, then
+   *     they will be the same.
    * @throws InterruptedException
    */
-  Artifact getLauncher(final RuleContext ruleContext, final JavaCommon common,
-      DeployArchiveBuilder deployArchiveBuilder, Runfiles.Builder runfilesBuilder,
-      List<String> jvmFlags, JavaTargetAttributes.Builder attributesBuilder, boolean shouldStrip)
+  Pair<Artifact, Artifact> getLauncher(
+      final RuleContext ruleContext,
+      final JavaCommon common,
+      DeployArchiveBuilder deployArchiveBuilder,
+      DeployArchiveBuilder unstrippedDeployArchiveBuilder,
+      Runfiles.Builder runfilesBuilder,
+      List<String> jvmFlags,
+      JavaTargetAttributes.Builder attributesBuilder,
+      boolean shouldStrip)
       throws InterruptedException;
 
   /** Add extra dependencies for runfiles of a Java binary. */