Use nested sets for configured target runfiles instead of flattened lists.

RELNOTES: None
PiperOrigin-RevId: 157124371

13 files changed, 49 insertions, 31 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/CommandHelper.java b/src/main/java/com/google/devtools/build/lib/analysis/CommandHelper.java
index ae8736cdc9..09c5d104ad 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/CommandHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/CommandHelper.java
@@ -115,7 +115,7 @@ public final class CommandHelper {
         continue;
       }
 
-      Collection<Artifact> files = tool.getFilesToRun();
+      Iterable<Artifact> files = tool.getFilesToRun();
       resolvedToolsBuilder.addAll(files);
       Artifact executableArtifact = tool.getExecutable();
       // If the label has an executable artifact add that to the multimaps.
@@ -125,7 +125,7 @@ public final class CommandHelper {
         toolsRunfilesBuilder.add(tool.getRunfilesSupplier());
       } else {
         // Map all depArtifacts to the respective label using the multimaps.
-        mapGet(tempLabelMap, label).addAll(files);
+        Iterables.addAll(mapGet(tempLabelMap, label), files);
       }
     }
 
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/FilesToRunProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/FilesToRunProvider.java
index 3a755ff500..bd3353eaf8 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/FilesToRunProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/FilesToRunProvider.java
@@ -14,10 +14,12 @@
 
 package com.google.devtools.build.lib.analysis;
 
-import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.EmptyRunfilesSupplier;
 import com.google.devtools.build.lib.actions.RunfilesSupplier;
+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.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
@@ -32,14 +34,16 @@ public final class FilesToRunProvider implements TransitiveInfoProvider {
   public static final String SKYLARK_NAME = "files_to_run";
 
   public static final FilesToRunProvider EMPTY =
-      new FilesToRunProvider(ImmutableList.<Artifact>of(), null, null);
+      new FilesToRunProvider(NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER), null, null);
 
-  private final ImmutableList<Artifact> filesToRun;
+  private final NestedSet<Artifact> filesToRun;
   @Nullable private final RunfilesSupport runfilesSupport;
   @Nullable private final Artifact executable;
 
-  public FilesToRunProvider(ImmutableList<Artifact> filesToRun,
-      @Nullable RunfilesSupport runfilesSupport, @Nullable Artifact executable) {
+  public FilesToRunProvider(
+      NestedSet<Artifact> filesToRun,
+      @Nullable RunfilesSupport runfilesSupport,
+      @Nullable Artifact executable) {
     this.filesToRun = filesToRun;
     this.runfilesSupport = runfilesSupport;
     this.executable  = executable;
@@ -49,13 +53,12 @@ public final class FilesToRunProvider implements TransitiveInfoProvider {
    * Creates an instance that contains one single executable and no other files.
    */
   public static FilesToRunProvider fromSingleExecutableArtifact(Artifact artifact) {
-    return new FilesToRunProvider(ImmutableList.of(artifact), null, artifact);
+    return new FilesToRunProvider(
+        NestedSetBuilder.create(Order.STABLE_ORDER, artifact), null, artifact);
   }
 
-  /**
-   * Returns artifacts needed to run the executable for this target.
-   */
-  public ImmutableList<Artifact> getFilesToRun() {
+  /** Returns artifacts needed to run the executable for this target. */
+  public NestedSet<Artifact> getFilesToRun() {
     return filesToRun;
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/LocationExpander.java b/src/main/java/com/google/devtools/build/lib/analysis/LocationExpander.java
index 225b3e14b6..1d068e03bf 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/LocationExpander.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/LocationExpander.java
@@ -313,7 +313,7 @@ public class LocationExpander {
       if (executableArtifact != null) {
         mapGet(locationMap, label).add(executableArtifact);
       } else {
-        mapGet(locationMap, label).addAll(filesToRun.getFilesToRun());
+        Iterables.addAll(mapGet(locationMap, label), filesToRun.getFilesToRun());
       }
     }
     return locationMap;
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
index 552122a3a5..6e28fdc32a 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
@@ -161,16 +161,16 @@ public final class RuleConfiguredTargetBuilder {
   }
 
   /**
-   * Like getFilesToBuild(), except that it also includes the runfiles middleman, if any.
-   * Middlemen are expanded in the SpawnStrategy or by the Distributor.
+   * Like getFilesToBuild(), except that it also includes the runfiles middleman, if any. Middlemen
+   * are expanded in the SpawnStrategy or by the Distributor.
    */
-  private ImmutableList<Artifact> getFilesToRun(
+  private NestedSet<Artifact> getFilesToRun(
       RunfilesSupport runfilesSupport, NestedSet<Artifact> filesToBuild) {
     if (runfilesSupport == null) {
-      return ImmutableList.copyOf(filesToBuild);
+      return filesToBuild;
     } else {
-      ImmutableList.Builder<Artifact> allFilesToBuild = ImmutableList.builder();
-      allFilesToBuild.addAll(filesToBuild);
+      NestedSetBuilder<Artifact> allFilesToBuild = NestedSetBuilder.stableOrder();
+      allFilesToBuild.addTransitive(filesToBuild);
       allFilesToBuild.add(runfilesSupport.getRunfilesMiddleman());
       return allFilesToBuild.build();
     }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java
index 549204bd3f..3b9792cbcf 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java
@@ -355,7 +355,7 @@ public final class RunfilesSupport {
       return runfilesProvider.getDefaultRunfiles();
     } else {
       return new Runfiles.Builder(workspaceName)
-          .addArtifacts(target.getProvider(FilesToRunProvider.class).getFilesToRun())
+          .addTransitiveArtifacts(target.getProvider(FilesToRunProvider.class).getFilesToRun())
           .build();
     }
   }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
index 51178a1d68..8ee9c993ea 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
@@ -772,6 +772,12 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
       return this;
     }
 
+    /** Adds tools to this action. */
+    public Builder addTransitiveTools(NestedSet<Artifact> artifacts) {
+      toolsBuilder.addTransitive(artifacts);
+      return this;
+    }
+
     /**
      * Adds inputs to this action.
      */
@@ -1008,7 +1014,7 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
      * source code).
      */
     public Builder addTool(FilesToRunProvider tool) {
-      addTools(tool.getFilesToRun());
+      addTransitiveTools(tool.getFilesToRun());
       toolRunfilesSuppliers.add(tool.getRunfilesSupplier());
       return this;
     }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplate.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplate.java
index f240cbb048..c25d729b78 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplate.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplate.java
@@ -281,7 +281,7 @@ public final class SpawnActionTemplate implements ActionTemplate<SpawnAction> {
      * for expanded actions.
      */
     public Builder addCommonTool(FilesToRunProvider tool) {
-      toolsBuilder.addAll(tool.getFilesToRun());
+      toolsBuilder.addTransitive(tool.getFilesToRun());
       spawnActionBuilder.addTool(tool);
       return this;
     }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidHostServiceFixture.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidHostServiceFixture.java
index 91075a9d54..2ae4c81e29 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidHostServiceFixture.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidHostServiceFixture.java
@@ -39,7 +39,7 @@ public class AndroidHostServiceFixture implements RuleConfiguredTargetFactory {
     NestedSet<Artifact> filesToBuild =
         NestedSetBuilder.<Artifact>stableOrder()
             .addTransitive(supportApks)
-            .addAll(executable.getFilesToRun())
+            .addTransitive(executable.getFilesToRun())
             .build();
     Runfiles runfiles =
         new Runfiles.Builder(ruleContext.getWorkspaceName())
diff --git a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java
index 1f08b7666f..5f0432ac4a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java
@@ -184,7 +184,7 @@ public abstract class GenRuleBase implements RuleConfiguredTargetFactory {
     inputs.addAll(commandHelper.getResolvedTools());
     FilesToRunProvider genruleSetup =
         ruleContext.getPrerequisite("$genrule_setup", Mode.HOST, FilesToRunProvider.class);
-    inputs.addAll(genruleSetup.getFilesToRun());
+    inputs.addTransitive(genruleSetup.getFilesToRun());
     List<String> argv = commandHelper.buildCommandLine(command, inputs, ".genrule_script.sh",
           ImmutableMap.copyOf(executionInfo));
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java
index e5b280b688..4ccd3d0aaf 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java
@@ -353,6 +353,13 @@ public class ObjcCompileAction extends SpawnAction {
       return this;
     }
 
+    @Override
+    public Builder addTransitiveTools(NestedSet<Artifact> artifacts) {
+      super.addTransitiveTools(artifacts);
+      mandatoryInputs.addTransitive(artifacts);
+      return this;
+    }
+
     /** Sets a .d file that will used to prune input headers */
     public Builder setDotdFile(DotdFile dotdFile) {
       Preconditions.checkNotNull(dotdFile);
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
index 77fcd43d68..c17a916056 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
@@ -1433,16 +1433,16 @@ public abstract class BuildViewTestCase extends FoundationTestCase {
     return target.getProvider(FilesToRunProvider.class).getExecutable();
   }
 
-  protected ImmutableList<Artifact> getFilesToRun(TransitiveInfoCollection target) {
+  protected NestedSet<Artifact> getFilesToRun(TransitiveInfoCollection target) {
     return target.getProvider(FilesToRunProvider.class).getFilesToRun();
   }
 
-  protected ImmutableList<Artifact> getFilesToRun(Label label) throws Exception {
+  protected NestedSet<Artifact> getFilesToRun(Label label) throws Exception {
     return getConfiguredTarget(label, targetConfig)
         .getProvider(FilesToRunProvider.class).getFilesToRun();
   }
 
-  protected ImmutableList<Artifact> getFilesToRun(String label) throws Exception {
+  protected NestedSet<Artifact> getFilesToRun(String label) throws Exception {
     return getConfiguredTarget(label).getProvider(FilesToRunProvider.class).getFilesToRun();
   }
 
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryTest.java b/src/test/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryTest.java
index f9d41460eb..f077c5517b 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryTest.java
@@ -16,12 +16,12 @@ package com.google.devtools.build.lib.bazel.rules.android;
 import static com.google.common.truth.Truth.assertThat;
 import static org.junit.Assert.fail;
 
-import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.FilesToRunProvider;
 import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.packages.AttributeContainer;
 import com.google.devtools.build.lib.packages.BuildFileNotFoundException;
 import com.google.devtools.build.lib.testutil.MoreAsserts;
@@ -69,9 +69,10 @@ public class AndroidNdkRepositoryTest extends BuildViewTestCase {
         ")");
     invalidatePackages();
 
-    ImmutableList<Artifact> x86ClangHighestApiLevelFilesToRun =
+    NestedSet<Artifact> x86ClangHighestApiLevelFilesToRun =
         getConfiguredTarget("@androidndk//:x86-clang3.8-gnu-libstdcpp-all_files")
-            .getProvider(FilesToRunProvider.class).getFilesToRun();
+            .getProvider(FilesToRunProvider.class)
+            .getFilesToRun();
     assertThat(artifactsToStrings(x86ClangHighestApiLevelFilesToRun))
         .contains(
             "src external/androidndk/ndk/platforms/android-24/arch-x86/usr/lib/libandroid.so");
diff --git a/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java b/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java
index 72dc9c3489..08a0bb1928 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java
@@ -31,6 +31,7 @@ import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
+import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.rules.proto.ProtoCompileActionBuilder.ProtoCommandLineArgv;
 import com.google.devtools.build.lib.rules.proto.ProtoCompileActionBuilder.ToolchainInvocation;
 import com.google.devtools.build.lib.util.LazyString;
@@ -52,7 +53,7 @@ public class ProtoCompileActionBuilderTest {
   public void commandLine_basic() throws Exception {
     FilesToRunProvider plugin =
         new FilesToRunProvider(
-            ImmutableList.<Artifact>of(),
+            NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER),
             null /* runfilesSupport */,
             artifact("//:dont-care", "protoc-gen-javalite.exe"));