Defer expanding sources to path fragments until execution for source jar actions.

PiperOrigin-RevId: 169234341

4 files changed, 27 insertions, 22 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java
index 693e15ae8f..7ce177ae8d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java
@@ -18,7 +18,6 @@ import static com.google.devtools.build.lib.rules.java.DeployArchiveBuilder.Comp
 import static java.util.stream.Collectors.toCollection;
 
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Streams;
 import com.google.devtools.build.lib.actions.Artifact;
@@ -63,7 +62,6 @@ import com.google.devtools.build.lib.rules.java.SingleJarActionBuilder;
 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.vfs.PathFragment;
 import java.util.ArrayList;
 import java.util.LinkedHashSet;
 import java.util.List;
@@ -292,7 +290,8 @@ public abstract class AndroidLocalTestBase implements RuleConfiguredTargetFactor
     // DeployArchiveAction ? Needs a few changes there as we can't pass inputs
     SingleJarActionBuilder.createSourceJarAction(
         ruleContext,
-        ImmutableMap.<PathFragment, Artifact>of(),
+        javaSemantics,
+        ImmutableList.of(),
         transitiveSourceJars,
         ruleContext.getImplicitOutputArtifact(JavaSemantics.JAVA_BINARY_DEPLOY_SOURCE_JAR));
 
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 7db9e72a52..10f42eee77 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
@@ -277,8 +277,11 @@ public class JavaBinary implements RuleConfiguredTargetFactory {
 
     // TODO(bazel-team): if (getOptions().sourceJars) then make this a dummy prerequisite for the
     // DeployArchiveAction ? Needs a few changes there as we can't pass inputs
-    SingleJarActionBuilder.createSourceJarAction(ruleContext,
-        ImmutableMap.<PathFragment, Artifact>of(), transitiveSourceJars,
+    SingleJarActionBuilder.createSourceJarAction(
+        ruleContext,
+        semantics,
+        ImmutableList.of(),
+        transitiveSourceJars,
         ruleContext.getImplicitOutputArtifact(JavaSemantics.JAVA_BINARY_DEPLOY_SOURCE_JAR));
 
     RuleConfiguredTargetBuilder builder = new RuleConfiguredTargetBuilder(ruleContext);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java
index 5c70ac72e5..b1629f0063 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java
@@ -45,10 +45,8 @@ import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.LinkedHashMap;
 import java.util.LinkedList;
 import java.util.List;
-import java.util.Map;
 import javax.annotation.Nullable;
 
 /**
@@ -593,12 +591,8 @@ public final class JavaCompilationHelper {
     if (gensrcJar != null) {
       resourceJars.add(gensrcJar);
     }
-    Map<PathFragment, Artifact> resources = new LinkedHashMap<>();
-    for (Artifact sourceFile : attributes.getSourceFiles()) {
-      resources.put(semantics.getDefaultJavaResourcePath(sourceFile.getRootRelativePath()), sourceFile);
-    }
     SingleJarActionBuilder.createSourceJarAction(
-        ruleContext, resources, resourceJars.build(), outputJar);
+        ruleContext, semantics, attributes.getSourceFiles(), resourceJars.build(), outputJar);
   }
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/SingleJarActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/java/SingleJarActionBuilder.java
index af74963150..0d623c3b8c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/SingleJarActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/SingleJarActionBuilder.java
@@ -13,6 +13,7 @@
 // limitations under the License.
 package com.google.devtools.build.lib.rules.java;
 
+import com.google.common.collect.ImmutableCollection;
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.ExecutionRequirements;
@@ -20,13 +21,12 @@ import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.actions.CommandLine;
 import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
+import com.google.devtools.build.lib.analysis.actions.CustomCommandLine.VectorArg;
 import com.google.devtools.build.lib.analysis.actions.ParamFileInfo;
 import com.google.devtools.build.lib.analysis.actions.SpawnAction;
 import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
-import com.google.devtools.build.lib.vfs.PathFragment;
-import java.util.Map;
 
 /**
  * Helper class to create singlejar actions - singlejar can merge multiple zip files without
@@ -51,7 +51,8 @@ public final class SingleJarActionBuilder {
    */
   public static void createSourceJarAction(
       RuleContext ruleContext,
-      Map<PathFragment, Artifact> resources,
+      JavaSemantics semantics,
+      ImmutableCollection<Artifact> resources,
       NestedSet<Artifact> resourceJars,
       Artifact outputJar) {
     Artifact singleJar = getSingleJar(ruleContext);
@@ -72,10 +73,10 @@ public final class SingleJarActionBuilder {
     }
     builder
         .addOutput(outputJar)
-        .addInputs(resources.values())
+        .addInputs(resources)
         .addTransitiveInputs(resourceJars)
         .addCommandLine(
-            sourceJarCommandLine(outputJar, resources, resourceJars),
+            sourceJarCommandLine(outputJar, semantics, resources, resourceJars),
             ParamFileInfo.builder(ParameterFileType.SHELL_QUOTED).setUseAlways(true).build())
         .setProgressMessage("Building source jar %s", outputJar.prettyPrint())
         .setMnemonic("JavaSourceJar");
@@ -91,18 +92,26 @@ public final class SingleJarActionBuilder {
     return ruleContext.getPrerequisiteArtifact("$singlejar", Mode.HOST);
   }
 
-  private static CommandLine sourceJarCommandLine(Artifact outputJar,
-      Map<PathFragment, Artifact> resources, NestedSet<Artifact> resourceJars) {
+  private static CommandLine sourceJarCommandLine(
+      Artifact outputJar,
+      JavaSemantics semantics,
+      ImmutableCollection<Artifact> resources,
+      NestedSet<Artifact> resourceJars) {
     CustomCommandLine.Builder args = CustomCommandLine.builder();
     args.addExecPath("--output", outputJar);
     args.addAll(SOURCE_JAR_COMMAND_LINE_ARGS);
     args.addExecPaths("--sources", resourceJars);
     if (!resources.isEmpty()) {
       args.add("--resources");
-      for (Map.Entry<PathFragment, Artifact> resource : resources.entrySet()) {
-        args.addFormatted("%s:%s", resource.getValue().getExecPath(), resource.getKey());
-      }
+      args.addAll(VectorArg.of(resources).mapped(resource -> getResourceArg(semantics, resource)));
     }
     return args.build();
   }
+
+  private static String getResourceArg(JavaSemantics semantics, Artifact resource) {
+    return String.format(
+        "%s:%s",
+        resource.getExecPathString(),
+        semantics.getDefaultJavaResourcePath(resource.getRootRelativePath()));
+  }
 }