Support multiple command lines / params files in SpawnAction.

This is necessary for the upcoming Skylark implementation of param files.

PiperOrigin-RevId: 168744486

1 files changed, 205 insertions, 93 deletions

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 8f690a47a4..ac9aa7266a 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
@@ -14,8 +14,6 @@
 
 package com.google.devtools.build.lib.analysis.actions;
 
-import static java.nio.charset.StandardCharsets.ISO_8859_1;
-
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.CharMatcher;
 import com.google.common.collect.ImmutableList;
@@ -31,6 +29,7 @@ import com.google.devtools.build.lib.actions.ActionInput;
 import com.google.devtools.build.lib.actions.ActionInputHelper;
 import com.google.devtools.build.lib.actions.ActionOwner;
 import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
 import com.google.devtools.build.lib.actions.BaseSpawn;
 import com.google.devtools.build.lib.actions.CommandAction;
 import com.google.devtools.build.lib.actions.CommandLineExpansionException;
@@ -38,7 +37,7 @@ import com.google.devtools.build.lib.actions.CompositeRunfilesSupplier;
 import com.google.devtools.build.lib.actions.EmptyRunfilesSupplier;
 import com.google.devtools.build.lib.actions.ExecException;
 import com.google.devtools.build.lib.actions.ExecutionInfoSpecifier;
-import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType;
+import com.google.devtools.build.lib.actions.ParameterFile;
 import com.google.devtools.build.lib.actions.ResourceSet;
 import com.google.devtools.build.lib.actions.RunfilesSupplier;
 import com.google.devtools.build.lib.actions.Spawn;
@@ -62,7 +61,6 @@ import com.google.errorprone.annotations.CompileTimeConstant;
 import com.google.errorprone.annotations.FormatMethod;
 import com.google.errorprone.annotations.FormatString;
 import com.google.protobuf.GeneratedMessage.GeneratedExtension;
-import java.nio.charset.Charset;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.LinkedHashMap;
@@ -521,6 +519,17 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
    */
   public static class Builder {
 
+    private static class CommandLineAndParamFileInfo {
+      private final CommandLine commandLine;
+      @Nullable private final ParamFileInfo paramFileInfo;
+
+      private CommandLineAndParamFileInfo(
+          CommandLine commandLine, @Nullable ParamFileInfo paramFileInfo) {
+        this.commandLine = commandLine;
+        this.paramFileInfo = paramFileInfo;
+      }
+    }
+
     private final NestedSetBuilder<Artifact> toolsBuilder = NestedSetBuilder.stableOrder();
     private final NestedSetBuilder<Artifact> inputsBuilder = NestedSetBuilder.stableOrder();
     private final List<Artifact> outputs = new ArrayList<>();
@@ -535,10 +544,9 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
     private PathFragment executable;
     // executableArgs does not include the executable itself.
     private List<String> executableArgs;
-    @Nullable private CommandLine commandLine;
+    private List<CommandLineAndParamFileInfo> commandLines = new ArrayList<>();
 
     private CharSequence progressMessage;
-    private ParamFileInfo paramFileInfo = null;
     private String mnemonic = "Unknown";
     protected ExtraActionInfoSupplier<?> extraActionInfoSupplier = null;
     private boolean disableSandboxing = false;
@@ -566,9 +574,8 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
       this.executableArgs = (other.executableArgs != null)
           ? Lists.newArrayList(other.executableArgs)
           : null;
-      this.commandLine = other.commandLine;
+      this.commandLines = Lists.newArrayList(other.commandLines);
       this.progressMessage = other.progressMessage;
-      this.paramFileInfo = other.paramFileInfo;
       this.mnemonic = other.mnemonic;
     }
 
@@ -598,37 +605,91 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
     @VisibleForTesting @CheckReturnValue
     public Action[] build(ActionOwner owner, AnalysisEnvironment analysisEnvironment,
         BuildConfiguration configuration) {
-      CommandLine commandLine = this.commandLine != null ? this.commandLine : CommandLine.EMPTY;
-      // Check to see if we need to use param file.
-      Artifact paramsFile = ParamFileHelper.getParamsFileMaybe(
-          buildExecutableArgs(configuration.getShellExecutable()),
-          commandLine,
-          paramFileInfo,
-          configuration,
-          analysisEnvironment,
-          outputs);
-
-      // If param file is to be used, set up the param file write action as well.
-      ParameterFileWriteAction paramFileWriteAction = null;
-      if (paramsFile != null) {
-        paramFileWriteAction =
-            ParamFileHelper.createParameterFileWriteAction(
-                commandLine, owner, paramsFile, paramFileInfo);
+      List<Action> paramFileActions = new ArrayList<>(commandLines.size());
+      CommandLine actualCommandLine =
+          buildCommandLine(owner, analysisEnvironment, configuration, paramFileActions);
+      Action[] actions = new Action[1 + paramFileActions.size()];
+      Action spawnAction =
+          buildSpawnAction(owner, actualCommandLine, configuration.getActionEnvironment());
+      actions[0] = spawnAction;
+      for (int i = 0; i < paramFileActions.size(); ++i) {
+        actions[i + 1] = paramFileActions.get(i);
       }
+      return actions;
+    }
 
-      List<Action> actions = new ArrayList<>(2);
-      actions.add(
-          buildSpawnAction(
-              owner,
-              commandLine,
-              configuration.getActionEnvironment(),
-              configuration.getShellExecutable(),
-              paramsFile));
-      if (paramFileWriteAction != null) {
-        actions.add(paramFileWriteAction);
+    private CommandLine buildCommandLine(
+        ActionOwner owner,
+        AnalysisEnvironment analysisEnvironment,
+        BuildConfiguration configuration,
+        List<Action> paramFileActions) {
+      ImmutableList<String> executableArgs =
+          buildExecutableArgs(configuration.getShellExecutable());
+      boolean hasConditionalParamFile =
+          commandLines.stream().anyMatch(c -> c.paramFileInfo != null && !c.paramFileInfo.always());
+      boolean spillToParamFiles = false;
+      if (hasConditionalParamFile) {
+        int totalLen = getParamFileSize(executableArgs);
+        for (CommandLineAndParamFileInfo commandLineAndParamFileInfo : commandLines) {
+          totalLen += getCommandLineSize(commandLineAndParamFileInfo.commandLine);
+        }
+        // To reduce implementation complexity we either spill all or none of the param files.
+        spillToParamFiles = totalLen > configuration.getMinParamFileSize();
       }
+      // We a name based on the output, starting at <output>-2.params
+      // and then incrementing
+      int paramFileNameSuffix = 2;
+      SpawnActionCommandLine.Builder result = new SpawnActionCommandLine.Builder();
+      result.addExecutableArguments(executableArgs);
+      for (CommandLineAndParamFileInfo commandLineAndParamFileInfo : commandLines) {
+        CommandLine commandLine = commandLineAndParamFileInfo.commandLine;
+        ParamFileInfo paramFileInfo = commandLineAndParamFileInfo.paramFileInfo;
+        boolean useParamsFile =
+            paramFileInfo != null && (paramFileInfo.always() || spillToParamFiles);
+        if (useParamsFile) {
+          Artifact output = Iterables.getFirst(outputs, null);
+          Preconditions.checkNotNull(output);
+          PathFragment paramFilePath =
+              ParameterFile.derivePath(
+                  output.getRootRelativePath(), Integer.toString(paramFileNameSuffix));
+          Artifact paramFile =
+              analysisEnvironment.getDerivedArtifact(paramFilePath, output.getRoot());
+          inputsBuilder.add(paramFile);
+          ParameterFileWriteAction paramFileWriteAction =
+              new ParameterFileWriteAction(
+                  owner,
+                  paramFile,
+                  commandLine,
+                  paramFileInfo.getFileType(),
+                  paramFileInfo.getCharset());
+          paramFileActions.add(paramFileWriteAction);
+          ++paramFileNameSuffix;
+          result.addParamFile(paramFile, paramFileInfo);
+        } else {
+          result.addCommandLine(commandLine);
+        }
+      }
+      return result.build();
+    }
+
+    private static int getCommandLineSize(CommandLine commandLine) {
+      try {
+        Iterable<String> actualArguments = commandLine.arguments();
+        return getParamFileSize(actualArguments);
+      } catch (CommandLineExpansionException e) {
+        // CommandLineExpansionException is thrown deterministically. We can ignore
+        // it here and pretend that a params file is not necessary at this stage,
+        // and an error will be thrown later at execution time.
+        return 0;
+      }
+    }
 
-      return actions.toArray(new Action[actions.size()]);
+    private static int getParamFileSize(Iterable<String> args) {
+      int size = 0;
+      for (String s : args) {
+        size += s.length() + 1; // Account for the space character
+      }
+      return size;
     }
 
     /**
@@ -643,26 +704,11 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
      *
      * @param owner the {@link ActionOwner} for the SpawnAction
      * @param configEnv the config's action environment to use. May be null if not used.
-     * @param defaultShellExecutable the default shell executable path. May be null if not used.
-     * @param paramsFile the parameter file for the SpawnAction. May be null if not used.
      * @return the SpawnAction and any actions required by it, with the first item always being the
      *     SpawnAction itself.
      */
     SpawnAction buildSpawnAction(
-        ActionOwner owner,
-        CommandLine commandLine,
-        @Nullable ActionEnvironment configEnv,
-        @Nullable PathFragment defaultShellExecutable,
-        @Nullable Artifact paramsFile) {
-      ImmutableList<String> argv = buildExecutableArgs(defaultShellExecutable);
-      CommandLine actualCommandLine;
-      if (paramsFile != null) {
-        inputsBuilder.add(paramsFile);
-        actualCommandLine = ParamFileHelper.createWithParamsFile(argv, paramFileInfo, paramsFile);
-      } else {
-        actualCommandLine = ParamFileHelper.createWithoutParamsFile(argv, commandLine);
-      }
-
+        ActionOwner owner, CommandLine commandLine, @Nullable ActionEnvironment configEnv) {
       NestedSet<Artifact> tools = toolsBuilder.build();
 
       // Tools are by definition a subset of the inputs, so make sure they're present there, too.
@@ -692,7 +738,7 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
           inputsAndTools,
           ImmutableList.copyOf(outputs),
           resourceSet,
-          actualCommandLine,
+          commandLine,
           isShellCommand,
           env,
           ImmutableMap.copyOf(executionInfo),
@@ -702,6 +748,21 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
           mnemonic);
     }
 
+    /**
+     * Builds the command line, forcing no params file.
+     *
+     * <p>This method is invoked by {@link SpawnActionTemplate} in the execution phase.
+     */
+    CommandLine buildCommandLineWithoutParamsFiles() {
+      SpawnActionCommandLine.Builder result = new SpawnActionCommandLine.Builder();
+      ImmutableList<String> executableArgs = buildExecutableArgs(null);
+      result.addExecutableArguments(executableArgs);
+      for (CommandLineAndParamFileInfo commandLineAndParamFileInfo : commandLines) {
+        result.addCommandLine(commandLineAndParamFileInfo.commandLine);
+      }
+      return result.build();
+    }
+
     /** Creates a SpawnAction. */
     protected SpawnAction createSpawnAction(
         ActionOwner owner,
@@ -1062,15 +1123,36 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
     }
 
     /**
-     * Sets a delegate to compute the command line at a later time.
+     * Adds a delegate to compute the command line at a later time.
+     *
+     * <p>The arguments are added after the executable arguments. If you add multiple command lines,
+     * they are expanded in the corresponding order.
+     *
+     * <p>The main intention of this method is to save memory by allowing client-controlled sharing
+     * between actions and configured targets. Objects passed to this method MUST be immutable.
+     *
+     * <p>See also {@link CustomCommandLine}.
+     */
+    public Builder addCommandLine(CommandLine commandLine) {
+      this.commandLines.add(new CommandLineAndParamFileInfo(commandLine, null));
+      return this;
+    }
+
+    /**
+     * Adds a delegate to compute the command line at a later time, optionally spilled to a params
+     * file.
+     *
+     * <p>The arguments are added after the executable arguments. If you add multiple command lines,
+     * they are expanded in the corresponding order. If the command line is spilled to a params
+     * file, it is replaced with an argument pointing to the param file.
      *
      * <p>The main intention of this method is to save memory by allowing client-controlled sharing
      * between actions and configured targets. Objects passed to this method MUST be immutable.
      *
      * <p>See also {@link CustomCommandLine}.
      */
-    public Builder setCommandLine(CommandLine commandLine) {
-      this.commandLine = commandLine;
+    public Builder addCommandLine(CommandLine commandLine, @Nullable ParamFileInfo paramFileInfo) {
+      this.commandLines.add(new CommandLineAndParamFileInfo(commandLine, paramFileInfo));
       return this;
     }
 
@@ -1207,50 +1289,80 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
       return this;
     }
 
-    /**
-     * Enable use of a parameter file and set the encoding to ISO-8859-1 (latin1).
-     *
-     * <p>In order to use parameter files, at least one output artifact must be specified.
-     */
-    public Builder useParameterFile(ParameterFileType parameterFileType) {
-      return useParameterFile(parameterFileType, ISO_8859_1, "@");
+    public Builder disableSandboxing() {
+      this.disableSandboxing = true;
+      return this;
     }
+  }
 
-    /**
-     * Force the use of a parameter file and set the encoding to ISO-8859-1 (latin1).
-     *
-     * <p>In order to use parameter files, at least one output artifact must be specified.
-     */
-    public Builder alwaysUseParameterFile(ParameterFileType parameterFileType) {
-      return useParameterFile(parameterFileType, ISO_8859_1, "@", /*always=*/ true);
+  /**
+   * Command line implementation that optimises for containing executable args, command lines, and
+   * command lines spilled to param files.
+   */
+  private static class SpawnActionCommandLine extends CommandLine {
+    private final Object[] values;
+
+    SpawnActionCommandLine(Object[] values) {
+      this.values = values;
     }
 
-    /**
-     * Enable or disable the use of a parameter file, set the encoding to the given value, and
-     * specify the argument prefix to use in passing the parameter file name to the tool.
-     *
-     * <p>The default argument prefix is "@". In order to use parameter files, at least one output
-     * artifact must be specified.
-     */
-    public Builder useParameterFile(
-        ParameterFileType parameterFileType,
-        Charset charset,
-        @CompileTimeConstant String flagPrefix) {
-      return useParameterFile(parameterFileType, charset, flagPrefix, /*always=*/ false);
-    }
-
-    private Builder useParameterFile(
-        ParameterFileType parameterFileType,
-        Charset charset,
-        @CompileTimeConstant String flagPrefix,
-        boolean always) {
-      paramFileInfo = new ParamFileInfo(parameterFileType, charset, flagPrefix, always);
-      return this;
+    @Override
+    public Iterable<String> arguments() throws CommandLineExpansionException {
+      return expandArguments(null);
     }
 
-    public Builder disableSandboxing() {
-      this.disableSandboxing = true;
-      return this;
+    @Override
+    public Iterable<String> arguments(ArtifactExpander artifactExpander)
+        throws CommandLineExpansionException {
+      return expandArguments(artifactExpander);
+    }
+
+    private Iterable<String> expandArguments(@Nullable ArtifactExpander artifactExpander)
+        throws CommandLineExpansionException {
+      ImmutableList.Builder<String> result = ImmutableList.builder();
+      int count = values.length;
+      for (int i = 0; i < count; ++i) {
+        Object value = values[i];
+        if (value instanceof String) {
+          result.add((String) value);
+        } else if (value instanceof Artifact) {
+          Artifact paramFile = (Artifact) value;
+          String flag = (String) values[++i];
+          result.add(flag + paramFile.getExecPathString());
+        } else if (value instanceof CommandLine) {
+          CommandLine commandLine = (CommandLine) value;
+          if (artifactExpander != null) {
+            result.addAll(commandLine.arguments(artifactExpander));
+          } else {
+            result.addAll(commandLine.arguments());
+          }
+        }
+      }
+      return result.build();
+    }
+
+    private static class Builder {
+      private List<Object> values = new ArrayList<>();
+
+      Builder addExecutableArguments(ImmutableList<String> executableArguments) {
+        values.addAll(executableArguments);
+        return this;
+      }
+
+      Builder addParamFile(Artifact paramFile, ParamFileInfo paramFileInfo) {
+        values.add(paramFile);
+        values.add(paramFileInfo.getFlag());
+        return this;
+      }
+
+      Builder addCommandLine(CommandLine commandLine) {
+        values.add(commandLine);
+        return this;
+      }
+
+      SpawnActionCommandLine build() {
+        return new SpawnActionCommandLine(values.toArray());
+      }
     }
   }
 }