@AutoCodec CppLinkAction.

PiperOrigin-RevId: 187212799

4 files changed, 131 insertions, 49 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java
index abb041cefc..69ec47e29f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java
@@ -49,11 +49,9 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
 import com.google.devtools.build.lib.rules.cpp.CcLinkParams.Linkstamp;
-import com.google.devtools.build.lib.rules.cpp.CppConfiguration.Tool;
 import com.google.devtools.build.lib.rules.cpp.Link.LinkStaticness;
 import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType;
 import com.google.devtools.build.lib.rules.cpp.LinkerInputs.LibraryToLink;
-import com.google.devtools.build.lib.skyframe.serialization.ObjectCodec;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization;
 import com.google.devtools.build.lib.util.Fingerprint;
@@ -66,10 +64,9 @@ import java.util.LinkedHashMap;
 import java.util.List;
 import javax.annotation.Nullable;
 
-/** 
- * Action that represents a linking step. 
- */
+/** Action that represents a linking step. */
 @ThreadCompatible
+@AutoCodec
 public final class CppLinkAction extends AbstractAction
     implements ExecutionInfoSpecifier, CommandAction {
 
@@ -155,7 +152,7 @@ public final class CppLinkAction extends AbstractAction
       ActionOwner owner,
       String mnemonic,
       Iterable<Artifact> inputs,
-      ImmutableList<Artifact> outputs,
+      ImmutableSet<Artifact> outputs,
       LibraryToLink outputLibrary,
       Artifact linkOutput,
       LibraryToLink interfaceOutputLibrary,
@@ -167,7 +164,9 @@ public final class CppLinkAction extends AbstractAction
       ImmutableMap<String, String> actionEnv,
       ImmutableMap<String, String> toolchainEnv,
       ImmutableSet<String> executionRequirements,
-      CcToolchainProvider toolchain) {
+      PathFragment ldExecutable,
+      String hostSystemName,
+      String targetCpu) {
     super(owner, inputs, outputs);
     if (mnemonic == null) {
       this.mnemonic = (isLtoIndexing) ? "CppLTOIndexing" : "CppLink";
@@ -186,9 +185,9 @@ public final class CppLinkAction extends AbstractAction
     this.actionEnv = actionEnv;
     this.toolchainEnv = toolchainEnv;
     this.executionRequirements = executionRequirements;
-    this.ldExecutable = toolchain.getToolPathFragment(Tool.LD);
-    this.hostSystemName = toolchain.getHostSystemName();
-    this.targetCpu = toolchain.getTargetCpu();
+    this.ldExecutable = ldExecutable;
+    this.hostSystemName = hostSystemName;
+    this.targetCpu = targetCpu;
   }
 
   @VisibleForTesting
@@ -541,8 +540,6 @@ public final class CppLinkAction extends AbstractAction
   @ThreadSafe
   @AutoCodec
   public static final class Context implements TransitiveInfoProvider {
-    public static final ObjectCodec<Context> CODEC = new CppLinkAction_Context_AutoCodec();
-
     // Morally equivalent with {@link Builder}, except these are immutable.
     // Keep these in sync with {@link Builder}.
     final ImmutableSet<LinkerInput> objectFiles;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java
index 2b9447a755..6a0ce57f52 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java
@@ -47,6 +47,7 @@ import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.LibraryToLinkValue;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.SequenceBuilder;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.VariablesExtension;
+import com.google.devtools.build.lib.rules.cpp.CppConfiguration.Tool;
 import com.google.devtools.build.lib.rules.cpp.CppLinkAction.Context;
 import com.google.devtools.build.lib.rules.cpp.CppLinkAction.LinkArtifactFactory;
 import com.google.devtools.build.lib.rules.cpp.Link.LinkStaticness;
@@ -928,9 +929,9 @@ public class CppLinkActionBuilder {
           linkArtifactFactory.create(ruleContext, configuration, thinltoMergedObjectFileRootPath);
     }
 
-    final ImmutableList<Artifact> actionOutputs;
+    final ImmutableSet<Artifact> actionOutputs;
     if (isLtoIndexing) {
-      ImmutableList.Builder<Artifact> builder = ImmutableList.builder();
+      ImmutableSet.Builder<Artifact> builder = ImmutableSet.builder();
       for (LtoBackendArtifacts ltoA : allLtoArtifacts) {
         ltoA.addIndexingOutputs(builder);
       }
@@ -1253,7 +1254,9 @@ public class CppLinkActionBuilder {
         configuration.getLocalShellEnvironment(),
         toolchainEnv,
         executionRequirements.build(),
-        toolchain);
+        toolchain.getToolPathFragment(Tool.LD),
+        toolchain.getHostSystemName(),
+        toolchain.getTargetCpu());
   }
 
   private boolean shouldUseLinkDynamicLibraryTool() {
@@ -1280,9 +1283,9 @@ public class CppLinkActionBuilder {
         && sharedLinkopts;
   }
 
-  private static ImmutableList<Artifact> constructOutputs(
+  private static ImmutableSet<Artifact> constructOutputs(
       Artifact primaryOutput, Iterable<Artifact> outputList, Artifact... outputs) {
-    return new ImmutableList.Builder<Artifact>()
+    return new ImmutableSet.Builder<Artifact>()
         .add(primaryOutput)
         .addAll(outputList)
         .addAll(CollectionUtils.asSetWithoutNulls(outputs))
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java
index 3539126e2d..389176cf0e 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java
@@ -28,6 +28,8 @@ import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables;
 import com.google.devtools.build.lib.rules.cpp.Link.LinkStaticness;
 import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType;
 import com.google.devtools.build.lib.rules.cpp.Link.Staticness;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization;
 import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.ArrayList;
@@ -35,9 +37,10 @@ import java.util.List;
 import javax.annotation.Nullable;
 
 /**
- * Represents the command line of a linker invocation. It supports executables and dynamic
- * libraries as well as static libraries.
+ * Represents the command line of a linker invocation. It supports executables and dynamic libraries
+ * as well as static libraries.
  */
+@AutoCodec
 @Immutable
 public final class LinkCommandLine extends CommandLine {
   private final String actionName;
@@ -47,8 +50,8 @@ public final class LinkCommandLine extends CommandLine {
   // The feature config can be null for tests.
   @Nullable private final FeatureConfiguration featureConfiguration;
   private final ImmutableList<Artifact> buildInfoHeaderArtifacts;
-  private final Iterable<? extends LinkerInput> linkerInputs;
-  private final Iterable<? extends LinkerInput> runtimeInputs;
+  private final Iterable<LinkerInput> linkerInputs;
+  private final Iterable<LinkerInput> runtimeInputs;
   private final LinkTargetType linkTargetType;
   private final LinkStaticness linkStaticness;
   private final ImmutableList<String> linkopts;
@@ -58,13 +61,14 @@ public final class LinkCommandLine extends CommandLine {
 
   @Nullable private final Artifact paramFile;
 
-  private LinkCommandLine(
+  @VisibleForSerialization
+  LinkCommandLine(
       String actionName,
       String forcedToolPath,
       PathFragment crosstoolTopPathFragment,
       ImmutableList<Artifact> buildInfoHeaderArtifacts,
-      Iterable<? extends LinkerInput> linkerInputs,
-      Iterable<? extends LinkerInput> runtimeInputs,
+      Iterable<LinkerInput> linkerInputs,
+      Iterable<LinkerInput> runtimeInputs,
       LinkTargetType linkTargetType,
       LinkStaticness linkStaticness,
       ImmutableList<String> linkopts,
@@ -102,17 +106,13 @@ public final class LinkCommandLine extends CommandLine {
     return buildInfoHeaderArtifacts;
   }
 
-  /**
-   * Returns the (ordered, immutable) list of paths to the linker's input files.
-   */
-  public Iterable<? extends LinkerInput> getLinkerInputs() {
+  /** Returns the (ordered, immutable) list of paths to the linker's input files. */
+  public Iterable<LinkerInput> getLinkerInputs() {
     return linkerInputs;
   }
 
-  /**
-   * Returns the runtime inputs to the linker.
-   */
-  public Iterable<? extends LinkerInput> getRuntimeInputs() {
+  /** Returns the runtime inputs to the linker. */
+  public Iterable<LinkerInput> getRuntimeInputs() {
     return runtimeInputs;
   }
 
@@ -183,13 +183,19 @@ public final class LinkCommandLine extends CommandLine {
    */
   @VisibleForTesting
   final Pair<List<String>, List<String>> splitCommandline() {
-    return splitCommandline(null);
+    return splitCommandline(paramFile, getRawLinkArgv(null), linkTargetType);
   }
 
   @VisibleForTesting
   final Pair<List<String>, List<String>> splitCommandline(@Nullable ArtifactExpander expander) {
+    return splitCommandline(paramFile, getRawLinkArgv(expander), linkTargetType);
+  }
+
+  private static Pair<List<String>, List<String>> splitCommandline(
+      Artifact paramFile,
+      List<String> args,
+      LinkTargetType linkTargetType) {
     Preconditions.checkNotNull(paramFile);
-    List<String> args = getRawLinkArgv(expander);
     if (linkTargetType.staticness() == Staticness.STATIC) {
       // Ar link commands can also generate huge command lines.
       List<String> paramFileArgs = new ArrayList<>();
@@ -209,21 +215,79 @@ public final class LinkCommandLine extends CommandLine {
   }
 
   /**
+   * A {@link CommandLine} implementation that returns the command line args pertaining to the
+   * .params file.
+   */
+  @AutoCodec
+  @VisibleForSerialization
+  static class ParamFileCommandLine extends CommandLine {
+    private final Artifact paramsFile;
+    private final LinkTargetType linkTargetType;
+    private final String forcedToolPath;
+    private final FeatureConfiguration featureConfiguration;
+    private final String actionName;
+    private final PathFragment crosstoolTopPathFragment;
+    private final Variables variables;
+
+    public ParamFileCommandLine(
+        Artifact paramsFile,
+        LinkTargetType linkTargetType,
+        String forcedToolPath,
+        FeatureConfiguration featureConfiguration,
+        String actionName,
+        PathFragment crosstoolTopPathFragment,
+        Variables variables) {
+      this.paramsFile = paramsFile;
+      this.linkTargetType = linkTargetType;
+      this.forcedToolPath = forcedToolPath;
+      this.featureConfiguration = featureConfiguration;
+      this.actionName = actionName;
+      this.crosstoolTopPathFragment = crosstoolTopPathFragment;
+      this.variables = variables;
+    }
+
+    @Override
+    public Iterable<String> arguments() {
+      List<String> argv =
+          getRawLinkArgv(
+              null,
+              forcedToolPath,
+              featureConfiguration,
+              actionName,
+              linkTargetType,
+              crosstoolTopPathFragment,
+              variables);
+      return splitCommandline(paramsFile, argv, linkTargetType).getSecond();
+    }
+
+    @Override
+    public Iterable<String> arguments(ArtifactExpander expander) {
+      List<String> argv =
+          getRawLinkArgv(
+              expander,
+              forcedToolPath,
+              featureConfiguration,
+              actionName,
+              linkTargetType,
+              crosstoolTopPathFragment,
+              variables);
+      return splitCommandline(paramsFile, argv, linkTargetType).getSecond();
+    }
+  }
+
+  /**
    * Returns just the .params file portion of the command-line as a {@link CommandLine}.
    */
   CommandLine paramCmdLine() {
     Preconditions.checkNotNull(paramFile);
-    return new CommandLine() {
-      @Override
-      public Iterable<String> arguments() {
-        return splitCommandline(null).getSecond();
-      }
-
-      @Override
-      public Iterable<String> arguments(ArtifactExpander expander) {
-        return splitCommandline(expander).getSecond();
-      }
-    };
+    return new ParamFileCommandLine(
+        paramFile,
+        linkTargetType,
+        forcedToolPath,
+        featureConfiguration,
+        actionName,
+        crosstoolTopPathFragment,
+        variables);
   }
 
   public static void extractArgumentsForStaticLinkParamFile(
@@ -306,6 +370,24 @@ public final class LinkCommandLine extends CommandLine {
    * @return raw link command line.
    */
   public List<String> getRawLinkArgv(@Nullable ArtifactExpander expander) {
+    return getRawLinkArgv(
+        expander,
+        forcedToolPath,
+        featureConfiguration,
+        actionName,
+        linkTargetType,
+        crosstoolTopPathFragment,
+        variables);
+  }
+
+  private static List<String> getRawLinkArgv(
+      @Nullable ArtifactExpander expander,
+      String forcedToolPath,
+      FeatureConfiguration featureConfiguration,
+      String actionName,
+      LinkTargetType linkTargetType,
+      PathFragment crosstoolTopPathFragment,
+      Variables variables) {
     List<String> argv = new ArrayList<>();
     if (forcedToolPath != null) {
       argv.add(forcedToolPath);
@@ -350,8 +432,8 @@ public final class LinkCommandLine extends CommandLine {
     private final RuleContext ruleContext;
     private String forcedToolPath;
     private ImmutableList<Artifact> buildInfoHeaderArtifacts = ImmutableList.of();
-    private Iterable<? extends LinkerInput> linkerInputs = ImmutableList.of();
-    private Iterable<? extends LinkerInput> runtimeInputs = ImmutableList.of();
+    private Iterable<LinkerInput> linkerInputs = ImmutableList.of();
+    private Iterable<LinkerInput> runtimeInputs = ImmutableList.of();
     @Nullable private LinkTargetType linkTargetType;
     private LinkStaticness linkStaticness = LinkStaticness.FULLY_STATIC;
     private ImmutableList<String> linkopts = ImmutableList.of();
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendArtifacts.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendArtifacts.java
index 883ca4d38b..2dc2996484 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendArtifacts.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendArtifacts.java
@@ -15,7 +15,7 @@
 package com.google.devtools.build.lib.rules.cpp;
 
 import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
@@ -166,7 +166,7 @@ public final class LtoBackendArtifacts {
     return dwoFile;
   }
 
-  public void addIndexingOutputs(ImmutableList.Builder<Artifact> builder) {
+  public void addIndexingOutputs(ImmutableSet.Builder<Artifact> builder) {
     builder.add(imports);
     builder.add(index);
   }