Update CppLinkAction to use NestedSet for inputs (part 5)

This is in preparation for making NestedSet not implement Iterable. Unfortunately, doing so means that there is no longer a common super-type for NestedSet and Collection, which means that we have to use NestedSet for all action inputs.

PiperOrigin-RevId: 285925773

src/main/java/com/google/devtools/build/lib/collect/CollectionUtils.java

@@ -130,7 +130,6 @@ public static <T> boolean isImmutable(Iterable<T> iterable) {
     return iterable instanceof ImmutableList<?>
         || iterable instanceof ImmutableSet<?>
         || iterable instanceof IterablesChain<?>
-        || iterable instanceof DedupingIterable<?>
         || iterable instanceof NestedSet<?>
         || iterable instanceof ImmutableIterable<?>;
   }

src/main/java/com/google/devtools/build/lib/collect/IterableCodecs.java

@@ -67,26 +67,6 @@ public IterablesChain deserialize(DeserializationContext context, CodedInputStre
     }
   }
 
-  static class DedupingIterableCodec implements ObjectCodec<DedupingIterable> {
-    @Override
-    public Class<DedupingIterable> getEncodedClass() {
-      return DedupingIterable.class;
-    }
-
-    @Override
-    public void serialize(
-        SerializationContext context, DedupingIterable obj, CodedOutputStream codedOut)
-        throws SerializationException, IOException {
-      IterableCodecs.serialize(context, obj, codedOut);
-    }
-
-    @Override
-    public DedupingIterable deserialize(DeserializationContext context, CodedInputStream codedIn)
-        throws SerializationException, IOException {
-      return new DedupingIterable<>(IterableCodecs.deserialize(context, codedIn));
-    }
-  }
-
   private static void serialize(
       SerializationContext context, Iterable obj, CodedOutputStream codedOut)
       throws SerializationException, IOException {

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

@@ -25,7 +25,6 @@
 import com.google.devtools.build.lib.analysis.actions.ActionConstructionContext;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.collect.CollectionUtils;
 import com.google.devtools.build.lib.collect.IterablesChain;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
@@ -580,9 +579,8 @@ public CppCompileActionBuilder setBuiltinIncludeFiles(
   }
 
   public CppCompileActionBuilder setInputsForInvalidation(
-      Iterable<Artifact> inputsForInvalidation) {
-    this.inputsForInvalidation =
-        Preconditions.checkNotNull(CollectionUtils.makeImmutable(inputsForInvalidation));
+      NestedSet<Artifact> inputsForInvalidation) {
+    this.inputsForInvalidation = inputsForInvalidation;
     return this;
   }
 

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

@@ -54,6 +54,7 @@
 import com.google.devtools.build.lib.analysis.skylark.Args;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.collect.CollectionUtils;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible;
 import com.google.devtools.build.lib.rules.cpp.Link.LinkingMode;
 import com.google.devtools.build.lib.rules.cpp.LinkerInputs.LibraryToLink;
@@ -136,8 +137,6 @@ public Artifact create(
   private final String hostSystemName;
   private final String targetCpu;
 
-  private final Iterable<Artifact> mandatoryInputs;
-
   // Linking uses a lot of memory; estimate 1 MB per input file, min 1.5 Gib. It is vital to not
   // underestimate too much here, because running too many concurrent links can thrash the machine
   // to the point where it stops responding to keystrokes or mouse clicks. This is primarily a
@@ -163,7 +162,7 @@ public Artifact create(
   CppLinkAction(
       ActionOwner owner,
       String mnemonic,
-      Iterable<Artifact> inputs,
+      NestedSet<Artifact> inputs,
       ImmutableSet<Artifact> outputs,
       LibraryToLink outputLibrary,
       Artifact linkOutput,
@@ -181,7 +180,6 @@ public Artifact create(
       String targetCpu) {
     super(owner, inputs, outputs, env);
     this.mnemonic = getMnemonic(mnemonic, isLtoIndexing);
-    this.mandatoryInputs = inputs;
     this.outputLibrary = outputLibrary;
     this.linkOutput = linkOutput;
     this.interfaceOutputLibrary = interfaceOutputLibrary;
@@ -333,7 +331,7 @@ private Spawn createSpawn(ActionExecutionContext actionExecutionContext)
           ImmutableList.copyOf(getCommandLine(actionExecutionContext.getArtifactExpander())),
           getEnvironment(actionExecutionContext.getClientEnv()),
           getExecutionInfo(),
-          ImmutableList.copyOf(getMandatoryInputs()),
+          ImmutableList.copyOf(getInputs()),
           getOutputs().asList(),
           estimateResourceConsumptionLocal());
     } catch (CommandLineExpansionException e) {
@@ -547,11 +545,6 @@ public ResourceSet estimateResourceConsumptionLocal() {
     );
   }
 
-  @Override
-  public Iterable<Artifact> getMandatoryInputs() {
-    return mandatoryInputs;
-  }
-
   private final class CppLinkActionContinuation extends ActionContinuationOrResult {
     private final ActionExecutionContext actionExecutionContext;
     private final SpawnContinuation spawnContinuation;

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

@@ -34,7 +34,6 @@
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.collect.CollectionUtils;
-import com.google.devtools.build.lib.collect.DedupingIterable;
 import com.google.devtools.build.lib.collect.IterablesChain;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
@@ -718,9 +717,9 @@ public CppLinkAction build() throws InterruptedException, RuleErrorException {
         ltoMapping.put(a.getBitcodeFile(), a.getObjectFile());
       }
     }
-    ImmutableSet<Artifact> objectArtifacts =
+    NestedSet<Artifact> objectArtifacts =
         getArtifactsPossiblyLtoMapped(objectFileInputs, ltoMapping);
-    ImmutableSet<Artifact> linkstampObjectArtifacts =
+    NestedSet<Artifact> linkstampObjectArtifacts =
         getArtifactsPossiblyLtoMapped(linkstampObjectFileInputs, ltoMapping);
 
     ImmutableSet<Artifact> combinedObjectArtifacts =
@@ -863,15 +862,10 @@ public CppLinkAction build() throws InterruptedException, RuleErrorException {
     CollectedLibrariesToLink collectedLibrariesToLink =
         librariesToLinkCollector.collectLibrariesToLink();
 
-    ImmutableSet<Artifact> expandedLinkerArtifacts =
+    NestedSet<Artifact> expandedLinkerArtifacts =
         getArtifactsPossiblyLtoMapped(
             collectedLibrariesToLink.getExpandedLinkerInputs(), ltoMapping);
 
-    // Remove the linkstamp objects from inputs so that createLinkstampCompileAction doesn't cause a
-    // circular dependency.
-    ImmutableSet<Artifact> expandedLinkerArtifactsNoLinkstamps =
-        Sets.difference(expandedLinkerArtifacts, linkstampObjectArtifacts).immutableCopy();
-
     CcToolchainVariables variables;
     try {
       ImmutableList.Builder<String> userLinkFlags =
@@ -985,20 +979,31 @@ public CppLinkAction build() throws InterruptedException, RuleErrorException {
       dependencyInputsBuilder.add(defFile);
     }
 
+    NestedSet<Artifact> nonCodeInputsAsNestedSet =
+        NestedSetBuilder.wrap(Order.STABLE_ORDER, nonCodeInputs);
+
+    // Remove the linkstamp objects from inputs so that createLinkstampCompileAction doesn't cause a
+    // circular dependency.
+    NestedSet<Artifact> expandedLinkerArtifactsNoLinkstamps =
+        NestedSetBuilder.<Artifact>stableOrder()
+            .addAll(
+                Sets.difference(expandedLinkerArtifacts.toSet(), linkstampObjectArtifacts.toSet()))
+            .build();
+
     // getPrimaryInput returns the first element, and that is a public interface - therefore the
     // order here is important.
-    IterablesChain.Builder<Artifact> inputsBuilder =
-        IterablesChain.<Artifact>builder()
-            .add(objectArtifacts)
-            .add(ImmutableList.copyOf(nonCodeInputs))
-            .add(dependencyInputsBuilder.build())
-            .add(expandedLinkerArtifactsNoLinkstamps);
+    NestedSetBuilder<Artifact> inputsBuilder =
+        NestedSetBuilder.<Artifact>stableOrder()
+            .addTransitive(objectArtifacts)
+            .addTransitive(nonCodeInputsAsNestedSet)
+            .addTransitive(dependencyInputsBuilder.build())
+            .addTransitive(expandedLinkerArtifactsNoLinkstamps);
 
     if (thinltoParamFile != null && !isLtoIndexing) {
-      inputsBuilder.add(ImmutableList.of(thinltoParamFile));
+      inputsBuilder.add(thinltoParamFile);
     }
     if (linkCommandLine.getParamFile() != null) {
-      inputsBuilder.add(ImmutableList.of(linkCommandLine.getParamFile()));
+      inputsBuilder.add(linkCommandLine.getParamFile());
       // Pass along tree artifacts, so they can be properly expanded.
       NestedSet<Artifact> paramFileActionInputs =
           NestedSetBuilder.<Artifact>stableOrder()
@@ -1043,8 +1048,8 @@ public CppLinkAction build() throws InterruptedException, RuleErrorException {
                 linkstampEntry.getKey().getArtifact(),
                 linkstampEntry.getValue(),
                 linkstampEntry.getKey().getDeclaredIncludeSrcs(),
-                ImmutableSet.copyOf(nonCodeInputs),
-                DedupingIterable.of(inputsBuilder.build()),
+                nonCodeInputsAsNestedSet,
+                inputsBuilder.build(),
                 buildInfoHeaderArtifacts,
                 additionalLinkstampDefines,
                 toolchain,
@@ -1064,10 +1069,10 @@ public CppLinkAction build() throws InterruptedException, RuleErrorException {
                 cppSemantics));
       }
 
-      inputsBuilder.add(linkstampMap.values());
+      inputsBuilder.addAll(linkstampMap.values());
     }
 
-    inputsBuilder.add(linkstampObjectArtifacts);
+    inputsBuilder.addTransitive(linkstampObjectArtifacts);
 
     ImmutableSet<Artifact> fakeLinkerInputArtifacts =
         collectedLibrariesToLink.getExpandedLinkerInputs().stream()
@@ -1078,7 +1083,7 @@ public CppLinkAction build() throws InterruptedException, RuleErrorException {
     return new CppLinkAction(
         getOwner(),
         mnemonic,
-        DedupingIterable.of(inputsBuilder.build()),
+        inputsBuilder.build(),
         actionOutputs,
         outputLibrary,
         output,
@@ -1103,10 +1108,10 @@ private boolean isFinalLinkOfLtoBuild() {
     return !isLtoIndexing && allLtoArtifacts != null;
   }
 
-  private ImmutableSet<Artifact> getArtifactsPossiblyLtoMapped(
+  private static NestedSet<Artifact> getArtifactsPossiblyLtoMapped(
       Iterable<LinkerInput> inputs, Map<Artifact, Artifact> ltoMapping) {
     Preconditions.checkNotNull(ltoMapping);
-    ImmutableSet.Builder<Artifact> result = ImmutableSet.builder();
+    NestedSetBuilder<Artifact> result = NestedSetBuilder.stableOrder();
     Iterable<Artifact> artifacts = LinkerInputs.toLibraryArtifacts(inputs);
     for (Artifact a : artifacts) {
       Artifact renamed = ltoMapping.get(a);

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

@@ -21,6 +21,7 @@
 import com.google.devtools.build.lib.analysis.actions.ActionConstructionContext;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.packages.RuleErrorConsumer;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -44,7 +45,7 @@ public static CppCompileAction createLinkstampCompileAction(
       Artifact outputFile,
       Iterable<Artifact> compilationInputs,
       Iterable<Artifact> nonCodeInputs,
-      Iterable<Artifact> inputsForInvalidation,
+      NestedSet<Artifact> inputsForInvalidation,
       ImmutableList<Artifact> buildInfoHeaderArtifacts,
       Iterable<String> additionalLinkstampDefines,
       CcToolchainProvider ccToolchainProvider,