Switch Action.getInputs/updateInputs to NestedSet

PiperOrigin-RevId: 286386452

src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java

@@ -22,7 +22,7 @@
 import com.google.devtools.build.lib.actions.extra.ExtraActionInfo;
 import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
 import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.collect.CollectionUtils;
+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;
@@ -97,22 +97,18 @@ public abstract class AbstractAction extends ActionKeyCacher implements Action,
   // The variable inputs is non-final only so that actions that discover their inputs can modify it.
   @GuardedBy("this")
   @VisibleForSerialization
-  protected Iterable<Artifact> inputs;
+  protected NestedSet<Artifact> inputs;
 
   protected final ActionEnvironment env;
   private final RunfilesSupplier runfilesSupplier;
   @VisibleForSerialization protected final ImmutableSet<Artifact> outputs;
 
-  /**
-   * Construct an abstract action with the specified inputs and outputs;
-   */
+  /** Construct an abstract action with the specified inputs and outputs; */
   protected AbstractAction(
-      ActionOwner owner,
-      Iterable<Artifact> inputs,
-      Iterable<Artifact> outputs) {
+      ActionOwner owner, NestedSet<Artifact> inputs, Iterable<Artifact> outputs) {
     this(
         owner,
-        /*tools = */ImmutableList.of(),
+        /*tools = */ NestedSetBuilder.emptySet(Order.STABLE_ORDER),
         inputs,
         EmptyRunfilesSupplier.INSTANCE,
         outputs,
@@ -121,12 +117,12 @@ protected AbstractAction(
 
   protected AbstractAction(
       ActionOwner owner,
-      Iterable<Artifact> inputs,
+      NestedSet<Artifact> inputs,
       Iterable<Artifact> outputs,
       ActionEnvironment env) {
     this(
         owner,
-        /*tools = */ImmutableList.of(),
+        /*tools = */ NestedSetBuilder.emptySet(Order.STABLE_ORDER),
         inputs,
         EmptyRunfilesSupplier.INSTANCE,
         outputs,
@@ -135,15 +131,15 @@ protected AbstractAction(
 
   protected AbstractAction(
       ActionOwner owner,
-      Iterable<Artifact> tools,
-      Iterable<Artifact> inputs,
+      NestedSet<Artifact> tools,
+      NestedSet<Artifact> inputs,
       RunfilesSupplier runfilesSupplier,
       Iterable<? extends Artifact> outputs,
       ActionEnvironment env) {
     Preconditions.checkNotNull(owner);
     this.owner = owner;
-    this.tools = CollectionUtils.makeImmutable(tools);
-    this.inputs = CollectionUtils.makeImmutable(inputs);
+    this.tools = tools;
+    this.inputs = inputs;
     this.env = Preconditions.checkNotNull(env);
     this.outputs = ImmutableSet.copyOf(outputs);
     this.runfilesSupplier = Preconditions.checkNotNull(runfilesSupplier);
@@ -176,12 +172,12 @@ public boolean discoversInputs() {
   /**
    * Run input discovery on the action.
    *
-   * <p>Called by Blaze if {@link #discoversInputs()} returns true. It must return the set of
-   * input artifacts that were not known at analysis time. May also call
-   * {@link #updateInputs(Iterable<Artifact>)}; if it doesn't, the action itself must arrange for
-   * the newly discovered artifacts to be available during action execution, probably by keeping
-   * state in the action instance and using a custom action execution context and for
-   * {@code #updateInputs()} to be called during the execution of the action.
+   * <p>Called by Blaze if {@link #discoversInputs()} returns true. It must return the set of input
+   * artifacts that were not known at analysis time. May also call {@link
+   * #updateInputs(NestedSet<Artifact>)}; if it doesn't, the action itself must arrange for the
+   * newly discovered artifacts to be available during action execution, probably by keeping state
+   * in the action instance and using a custom action execution context and for {@code
+   * #updateInputs()} to be called during the execution of the action.
    *
    * <p>Since keeping state within an action bad, don't do that unless there is a very good reason
    * to do so.
@@ -210,10 +206,10 @@ public Iterable<Artifact> getAllowedDerivedInputs() {
    * itself when an action is loaded from the on-disk action cache.
    */
   @Override
-  public synchronized void updateInputs(Iterable<Artifact> inputs) {
+  public synchronized void updateInputs(NestedSet<Artifact> inputs) {
     Preconditions.checkState(
         discoversInputs(), "Can't update inputs unless discovering: %s %s", this, inputs);
-    this.inputs = CollectionUtils.makeImmutable(inputs);
+    this.inputs = inputs;
     inputsDiscovered = true;
   }
 
@@ -222,11 +218,9 @@ public Iterable<Artifact> getTools() {
     return tools;
   }
 
-  /**
-   * Should not be overridden (it's non-final only for tests)
-   */
+  /** Should not be overridden (it's non-final only for tests) */
   @Override
-  public synchronized Iterable<Artifact> getInputs() {
+  public synchronized NestedSet<Artifact> getInputs() {
     return inputs;
   }
 
@@ -264,15 +258,21 @@ public Artifact getPrimaryOutput() {
   }
 
   @Override
-  public Iterable<Artifact> getMandatoryInputs() {
+  public NestedSet<Artifact> getMandatoryInputs() {
     return getInputs();
   }
 
   @Override
   public String toString() {
-    return prettyPrint() + " (" + getMnemonic() + "[" + ImmutableList.copyOf(getInputs())
+    return prettyPrint()
+        + " ("
+        + getMnemonic()
+        + "["
+        + getInputs().toList()
         + (inputsDiscovered() ? " -> " : ", unknown inputs -> ")
-        + getOutputs() + "]" + ")";
+        + getOutputs()
+        + "]"
+        + ")";
   }
 
   @Override

src/main/java/com/google/devtools/build/lib/actions/Action.java

@@ -15,6 +15,7 @@
 package com.google.devtools.build.lib.actions;
 
 import com.google.devtools.build.lib.actions.extra.ExtraActionInfo;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ConditionallyThreadCompatible;
 import com.google.devtools.build.lib.vfs.Path;
 import java.io.IOException;
@@ -208,10 +209,10 @@ Iterable<Artifact> discoverInputs(ActionExecutionContext actionExecutionContext)
 
   /**
    * Informs the action that its inputs are {@code inputs}, and that its inputs are now known. Can
-   * only be called for actions that discover inputs. After this method is called,
-   * {@link ActionExecutionMetadata#inputsDiscovered} should return true.
+   * only be called for actions that discover inputs. After this method is called, {@link
+   * ActionExecutionMetadata#inputsDiscovered} should return true.
    */
-  void updateInputs(Iterable<Artifact> inputs);
+  void updateInputs(NestedSet<Artifact> inputs);
 
   /**
    * Returns true if the output should bypass output filtering. This is used for test actions.

src/main/java/com/google/devtools/build/lib/actions/ActionAnalysisMetadata.java

@@ -16,6 +16,7 @@
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import java.util.Map;
 import javax.annotation.Nullable;
 
@@ -121,7 +122,7 @@ public interface ActionAnalysisMetadata {
    * AbstractAction, since AbstractAction's implementation of getInputs() returns an immutable
    * iterable.
    */
-  Iterable<Artifact> getInputs();
+  NestedSet<Artifact> getInputs();
 
   /**
    * Returns the environment variables from the client environment that this action depends on. May

src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java

@@ -23,6 +23,9 @@
 import com.google.devtools.build.lib.actions.cache.DigestUtils;
 import com.google.devtools.build.lib.actions.cache.MetadataHandler;
 import com.google.devtools.build.lib.actions.cache.Protos.ActionCacheStatistics.MissReason;
+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.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.events.EventKind;
@@ -265,14 +268,14 @@ public Token getTokenIfNeedToExecute(
     if (!cacheConfig.enabled()) {
       return new Token(getKeyString(action));
     }
-    Iterable<Artifact> actionInputs = action.getInputs();
+    NestedSet<Artifact> actionInputs = action.getInputs();
     // Resolve action inputs from cache, if necessary.
     boolean inputsDiscovered = action.inputsDiscovered();
     if (!inputsDiscovered && resolvedCacheArtifacts != null) {
       // The action doesn't know its inputs, but the caller has a good idea of what they are.
       Preconditions.checkState(action.discoversInputs(),
           "Actions that don't know their inputs must discover them: %s", action);
-      actionInputs = resolvedCacheArtifacts;
+      actionInputs = NestedSetBuilder.wrap(Order.STABLE_ORDER, resolvedCacheArtifacts);
     }
     ActionCache.Entry entry = getCacheEntry(action);
     if (mustExecute(

src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplate.java

@@ -187,7 +187,7 @@ public Iterable<Artifact> getTools() {
   }
 
   @Override
-  public Iterable<Artifact> getInputs() {
+  public NestedSet<Artifact> getInputs() {
     return allInputs;
   }
 

src/main/java/com/google/devtools/build/lib/includescanning/BUILD

@@ -23,6 +23,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity",
         "//src/main/java/com/google/devtools/build/lib/analysis/platform",
         "//src/main/java/com/google/devtools/build/lib/cmdline",
+        "//src/main/java/com/google/devtools/build/lib/collect/nestedset",
         "//src/main/java/com/google/devtools/build/lib/concurrent",
         "//src/main/java/com/google/devtools/build/lib/profiler",
         "//src/main/java/com/google/devtools/build/lib/rules/cpp",

src/main/java/com/google/devtools/build/lib/includescanning/SpawnIncludeScanner.java

@@ -43,6 +43,7 @@
 import com.google.devtools.build.lib.actions.SpawnContinuation;
 import com.google.devtools.build.lib.actions.SpawnResult;
 import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.includescanning.IncludeParser.GrepIncludesFileType;
 import com.google.devtools.build.lib.includescanning.IncludeParser.Inclusion;
 import com.google.devtools.build.lib.util.io.FileOutErr;
@@ -185,7 +186,7 @@ public Iterable<Artifact> getTools() {
     }
 
     @Override
-    public Iterable<Artifact> getInputs() {
+    public NestedSet<Artifact> getInputs() {
       throw new UnsupportedOperationException();
     }
 

src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java

@@ -1204,7 +1204,7 @@ public Iterable<Artifact> getAllowedDerivedInputs() {
    * action cache hit.
    */
   @Override
-  public synchronized void updateInputs(Iterable<Artifact> inputs) {
+  public synchronized void updateInputs(NestedSet<Artifact> inputs) {
     super.updateInputs(inputs);
     if (outputFile.isFileType(CppFileTypes.CPP_MODULE)) {
       discoveredModules =

src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java

@@ -278,7 +278,7 @@ public Iterable<Artifact> getTools() {
   }
 
   @Override
-  public Iterable<Artifact> getInputs() {
+  public NestedSet<Artifact> getInputs() {
     return NestedSetBuilder.<Artifact>stableOrder()
         .add(sourceTreeArtifact)
         .addTransitive(allInputs)

src/test/java/com/google/devtools/build/lib/actions/ActionCacheCheckerTest.java

@@ -32,6 +32,9 @@
 import com.google.devtools.build.lib.actions.util.ActionsTestUtil.MissDetailsBuilder;
 import com.google.devtools.build.lib.actions.util.ActionsTestUtil.NullAction;
 import com.google.devtools.build.lib.clock.Clock;
+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.testutil.ManualClock;
 import com.google.devtools.build.lib.testutil.Scratch;
 import com.google.devtools.build.lib.util.Fingerprint;
@@ -314,11 +317,12 @@ public void testMiddleman_DifferentFiles() throws Exception {
     Action action =
         new NullMiddlemanAction() {
           @Override
-          public synchronized Iterable<Artifact> getInputs() {
+          public synchronized NestedSet<Artifact> getInputs() {
             FileSystem fileSystem = getPrimaryOutput().getPath().getFileSystem();
             Path path = fileSystem.getPath("/input");
             ArtifactRoot root = ArtifactRoot.asSourceRoot(Root.fromPath(fileSystem.getPath("/")));
-            return ImmutableList.of(ActionsTestUtil.createArtifact(root, path));
+            return NestedSetBuilder.create(
+                Order.STABLE_ORDER, ActionsTestUtil.createArtifact(root, path));
           }
         };
     runAction(action);  // Not cached so recorded as different deps.

src/test/java/com/google/devtools/build/lib/actions/ResourceManagerTest.java

@@ -21,6 +21,7 @@
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.actions.ResourceManager.ResourceHandle;
 import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.testutil.TestThread;
 import com.google.devtools.build.lib.testutil.TestUtils;
 import java.util.concurrent.CyclicBarrier;
@@ -432,7 +433,7 @@ public Iterable<Artifact> getTools() {
     }
 
     @Override
-    public Iterable<Artifact> getInputs() {
+    public NestedSet<Artifact> getInputs() {
       throw new IllegalStateException();
     }
 

src/test/java/com/google/devtools/build/lib/exec/util/FakeOwner.java

@@ -26,6 +26,7 @@
 import com.google.devtools.build.lib.actions.RunfilesSupplier;
 import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.packages.AspectDescriptor;
 import javax.annotation.Nullable;
 
@@ -114,7 +115,7 @@ public Iterable<Artifact> getTools() {
   }
 
   @Override
-  public Iterable<Artifact> getInputs() {
+  public NestedSet<Artifact> getInputs() {
     throw new UnsupportedOperationException();
   }