aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/main/java/com/google/devtools/build/lib/actions/CommandLines.java186
-rw-r--r--src/test/java/com/google/devtools/build/lib/actions/CommandLinesTest.java68
2 files changed, 180 insertions, 74 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/actions/CommandLines.java b/src/main/java/com/google/devtools/build/lib/actions/CommandLines.java
index 51423a8f75..5c4f27f7a3 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/CommandLines.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/CommandLines.java
@@ -45,8 +45,17 @@ public class CommandLines {
private static final UUID PARAM_FILE_UUID =
UUID.fromString("106c1389-88d7-4cc1-8f05-f8a61fd8f7b1");
+ /** Command line OS limitations, such as the max length. */
+ public static class CommandLineLimits {
+ public final int maxLength;
+
+ public CommandLineLimits(int maxLength) {
+ this.maxLength = maxLength;
+ }
+ }
+
/** A simple tuple of a {@link CommandLine} and a {@link ParamFileInfo}. */
- static class CommandLineAndParamFileInfo {
+ public static class CommandLineAndParamFileInfo {
private final CommandLine commandLine;
@Nullable private final ParamFileInfo paramFileInfo;
@@ -68,26 +77,8 @@ public class CommandLines {
*/
private final Object commandLines;
- CommandLines(List<CommandLineAndParamFileInfo> commandLines) {
- if (commandLines.size() == 1) {
- CommandLineAndParamFileInfo pair = commandLines.get(0);
- if (pair.paramFileInfo != null) {
- this.commandLines = pair;
- } else {
- this.commandLines = pair.commandLine;
- }
- } else {
- Object[] result = new Object[commandLines.size()];
- for (int i = 0; i < commandLines.size(); ++i) {
- CommandLineAndParamFileInfo pair = commandLines.get(i);
- if (pair.paramFileInfo != null) {
- result[i] = pair;
- } else {
- result[i] = pair.commandLine;
- }
- }
- this.commandLines = result;
- }
+ private CommandLines(Object commandLines) {
+ this.commandLines = commandLines;
}
/**
@@ -95,51 +86,63 @@ public class CommandLines {
* is expected to write these param files prior to execution of an action.
*
* @param artifactExpander The artifact expander to use.
- * @param primaryOutput The primary output of the action. Used to derive param file names.
- * @param maxLength The maximum command line length the executing host system can tolerate.
+ * @param paramFileBasePath Used to derive param file names. Often the first output of an action.
+ * @param limits The command line limits the host OS can support.
* @return The expanded command line and its param files (if any).
*/
public ExpandedCommandLines expand(
- ArtifactExpander artifactExpander, Artifact primaryOutput, int maxLength)
+ ArtifactExpander artifactExpander, PathFragment paramFileBasePath, CommandLineLimits limits)
throws CommandLineExpansionException {
- return expand(
- artifactExpander, primaryOutput.getExecPath(), maxLength, PARAM_FILE_ARG_LENGTH_ESTIMATE);
+ return expand(artifactExpander, paramFileBasePath, limits, PARAM_FILE_ARG_LENGTH_ESTIMATE);
+ }
+
+ /**
+ * Returns all arguments, including ones inside of param files.
+ *
+ * <p>Suitable for debugging and printing messages to users. This expands all command lines, so it
+ * is potentially expensive.
+ */
+ public ImmutableList<String> allArguments() throws CommandLineExpansionException {
+ ImmutableList.Builder<String> arguments = ImmutableList.builder();
+ for (CommandLineAndParamFileInfo pair : getCommandLines()) {
+ arguments.addAll(pair.commandLine.arguments());
+ }
+ return arguments.build();
}
@VisibleForTesting
ExpandedCommandLines expand(
ArtifactExpander artifactExpander,
- PathFragment primaryOutputExecPath,
- int maxLength,
+ PathFragment paramFileBasePath,
+ CommandLineLimits limits,
int paramFileArgLengthEstimate)
throws CommandLineExpansionException {
// Optimize for simple case of single command line
if (commandLines instanceof CommandLine) {
CommandLine commandLine = (CommandLine) commandLines;
- return new ExpandedCommandLines(commandLine.arguments(artifactExpander), ImmutableList.of());
+ Iterable<String> arguments = commandLine.arguments(artifactExpander);
+ return new ExpandedCommandLines(arguments, arguments, ImmutableList.of());
}
- List<Object> commandLines =
- (this.commandLines instanceof CommandLineAndParamFileInfo)
- ? ImmutableList.of(this.commandLines)
- : Arrays.asList((Object[]) this.commandLines);
+ List<CommandLineAndParamFileInfo> commandLines = getCommandLines();
IterablesChain.Builder<String> arguments = IterablesChain.builder();
+ IterablesChain.Builder<String> allArguments = IterablesChain.builder();
ArrayList<ParamFileActionInput> paramFiles = new ArrayList<>(commandLines.size());
- int conservativeMaxLength = maxLength - commandLines.size() * paramFileArgLengthEstimate;
+ int conservativeMaxLength = limits.maxLength - commandLines.size() * paramFileArgLengthEstimate;
int cmdLineLength = 0;
// We name based on the output, starting at <output>-0.params and then incrementing
int paramFileNameSuffix = 0;
- for (Object object : commandLines) {
- if (object instanceof CommandLine) {
- CommandLine commandLine = (CommandLine) object;
+ for (CommandLineAndParamFileInfo pair : commandLines) {
+ CommandLine commandLine = pair.commandLine;
+ ParamFileInfo paramFileInfo = pair.paramFileInfo;
+ if (paramFileInfo == null) {
Iterable<String> args = commandLine.arguments(artifactExpander);
+ allArguments.add(args);
arguments.add(args);
cmdLineLength += totalArgLen(args);
} else {
- CommandLineAndParamFileInfo pair = (CommandLineAndParamFileInfo) object;
- CommandLine commandLine = pair.commandLine;
- ParamFileInfo paramFileInfo = pair.paramFileInfo;
Preconditions.checkNotNull(paramFileInfo); // If null, we would have just had a CommandLine
Iterable<String> args = commandLine.arguments(artifactExpander);
+ allArguments.add(args);
boolean useParamFile = true;
if (!paramFileInfo.always()) {
int tentativeCmdLineLength = cmdLineLength + totalArgLen(args);
@@ -151,8 +154,7 @@ public class CommandLines {
}
if (useParamFile) {
PathFragment paramFileExecPath =
- ParameterFile.derivePath(
- primaryOutputExecPath, Integer.toString(paramFileNameSuffix));
+ ParameterFile.derivePath(paramFileBasePath, Integer.toString(paramFileNameSuffix));
++paramFileNameSuffix;
String paramArg =
@@ -165,7 +167,7 @@ public class CommandLines {
}
}
}
- return new ExpandedCommandLines(arguments.build(), paramFiles);
+ return new ExpandedCommandLines(arguments.build(), allArguments.build(), paramFiles);
}
public void addToFingerprint(ActionKeyContext actionKeyContext, Fingerprint fingerprint)
@@ -176,20 +178,12 @@ public class CommandLines {
commandLine.addToFingerprint(actionKeyContext, fingerprint);
return;
}
- List<Object> commandLines =
- (this.commandLines instanceof CommandLineAndParamFileInfo)
- ? ImmutableList.of(this.commandLines)
- : Arrays.asList((Object[]) this.commandLines);
- for (Object object : commandLines) {
- if (object instanceof CommandLine) {
- CommandLine commandLine = (CommandLine) object;
- commandLine.addToFingerprint(actionKeyContext, fingerprint);
- } else {
- CommandLineAndParamFileInfo pair = (CommandLineAndParamFileInfo) object;
- CommandLine commandLine = pair.commandLine;
- commandLine.addToFingerprint(actionKeyContext, fingerprint);
- ParamFileInfo paramFileInfo = pair.paramFileInfo;
- Preconditions.checkNotNull(paramFileInfo); // If null, we would have just had a CommandLine
+ List<CommandLineAndParamFileInfo> commandLines = getCommandLines();
+ for (CommandLineAndParamFileInfo pair : commandLines) {
+ CommandLine commandLine = pair.commandLine;
+ ParamFileInfo paramFileInfo = pair.paramFileInfo;
+ commandLine.addToFingerprint(actionKeyContext, fingerprint);
+ if (paramFileInfo != null) {
addParamFileInfoToFingerprint(paramFileInfo, fingerprint);
}
}
@@ -203,10 +197,15 @@ public class CommandLines {
*/
public static class ExpandedCommandLines {
private final Iterable<String> arguments;
+ private final Iterable<String> allArguments;
private final List<ParamFileActionInput> paramFiles;
- ExpandedCommandLines(Iterable<String> arguments, List<ParamFileActionInput> paramFiles) {
+ ExpandedCommandLines(
+ Iterable<String> arguments,
+ Iterable<String> allArguments,
+ List<ParamFileActionInput> paramFiles) {
this.arguments = arguments;
+ this.allArguments = allArguments;
this.paramFiles = paramFiles;
}
@@ -215,6 +214,15 @@ public class CommandLines {
return arguments;
}
+ /**
+ * Returns all arguments, including ones inside of param files.
+ *
+ * <p>Suitable for debugging and printing messages to users.
+ */
+ public Iterable<String> allArguments() {
+ return allArguments;
+ }
+
/** Returns the param file action inputs needed to execute the command. */
public List<ParamFileActionInput> getParamFiles() {
return paramFiles;
@@ -266,6 +274,28 @@ public class CommandLines {
}
}
+ // Helper function to unpack the optimized storage format into a list
+ @SuppressWarnings("unchecked")
+ private List<CommandLineAndParamFileInfo> getCommandLines() {
+ if (commandLines instanceof CommandLine) {
+ return ImmutableList.of(new CommandLineAndParamFileInfo((CommandLine) commandLines, null));
+ } else if (commandLines instanceof CommandLineAndParamFileInfo) {
+ return ImmutableList.of((CommandLineAndParamFileInfo) commandLines);
+ } else {
+ List<Object> commandLines = Arrays.asList((Object[]) this.commandLines);
+ ImmutableList.Builder<CommandLineAndParamFileInfo> result =
+ ImmutableList.builderWithExpectedSize(commandLines.size());
+ for (Object commandLine : commandLines) {
+ if (commandLine instanceof CommandLine) {
+ result.add(new CommandLineAndParamFileInfo((CommandLine) commandLine, null));
+ } else {
+ result.add((CommandLineAndParamFileInfo) commandLine);
+ }
+ }
+ return result.build();
+ }
+ }
+
private static int totalArgLen(Iterable<String> args) {
int result = 0;
for (String s : args) {
@@ -286,6 +316,20 @@ public class CommandLines {
return new Builder();
}
+ /** Returns an instance with a single trivial command line. */
+ public static CommandLines fromArguments(Iterable<String> args) {
+ return new CommandLines(CommandLine.of(args));
+ }
+
+ public static CommandLines concat(CommandLine commandLine, CommandLines commandLines) {
+ Builder builder = builder();
+ builder.addCommandLine(commandLine);
+ for (CommandLineAndParamFileInfo pair : commandLines.getCommandLines()) {
+ builder.addCommandLine(pair);
+ }
+ return builder.build();
+ }
+
/** Builder for {@link CommandLines}. */
public static class Builder {
List<CommandLineAndParamFileInfo> commandLines = new ArrayList<>();
@@ -296,11 +340,35 @@ public class CommandLines {
}
public Builder addCommandLine(CommandLine commandLine, ParamFileInfo paramFileInfo) {
- commandLines.add(new CommandLineAndParamFileInfo(commandLine, paramFileInfo));
+ return addCommandLine(new CommandLineAndParamFileInfo(commandLine, paramFileInfo));
+ }
+
+ public Builder addCommandLine(CommandLineAndParamFileInfo pair) {
+ commandLines.add(pair);
return this;
}
public CommandLines build() {
+ final Object commandLines;
+ if (this.commandLines.size() == 1) {
+ CommandLineAndParamFileInfo pair = this.commandLines.get(0);
+ if (pair.paramFileInfo != null) {
+ commandLines = pair;
+ } else {
+ commandLines = pair.commandLine;
+ }
+ } else {
+ Object[] result = new Object[this.commandLines.size()];
+ for (int i = 0; i < this.commandLines.size(); ++i) {
+ CommandLineAndParamFileInfo pair = this.commandLines.get(i);
+ if (pair.paramFileInfo != null) {
+ result[i] = pair;
+ } else {
+ result[i] = pair.commandLine;
+ }
+ }
+ commandLines = result;
+ }
return new CommandLines(commandLines);
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/actions/CommandLinesTest.java b/src/test/java/com/google/devtools/build/lib/actions/CommandLinesTest.java
index 4d57313faf..ab5b4c54a4 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/CommandLinesTest.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/CommandLinesTest.java
@@ -17,6 +17,7 @@ import static com.google.common.truth.Truth.assertThat;
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
+import com.google.devtools.build.lib.actions.CommandLines.CommandLineLimits;
import com.google.devtools.build.lib.actions.CommandLines.ExpandedCommandLines;
import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
@@ -34,27 +35,55 @@ public class CommandLinesTest {
private final ArtifactExpander artifactExpander = null;
private final PathFragment execPath = PathFragment.create("output.txt");
+ private static final CommandLineLimits NO_LIMIT = new CommandLineLimits(10000);
@Test
public void testSimpleCommandLine() throws Exception {
- ExpandedCommandLines expanded =
+ CommandLines commandLines =
CommandLines.builder()
.addCommandLine(CommandLine.of(ImmutableList.of("--foo", "--bar")))
- .build()
- .expand(artifactExpander, execPath, 1024, 0);
+ .build();
+ ExpandedCommandLines expanded = commandLines.expand(artifactExpander, execPath, NO_LIMIT, 0);
+ assertThat(commandLines.allArguments()).containsExactly("--foo", "--bar");
+ assertThat(expanded.allArguments()).containsExactly("--foo", "--bar");
+ assertThat(expanded.arguments()).containsExactly("--foo", "--bar");
+ assertThat(expanded.getParamFiles()).isEmpty();
+ }
+
+ @Test
+ public void testFromArguments() throws Exception {
+ CommandLines commandLines = CommandLines.fromArguments(ImmutableList.of("--foo", "--bar"));
+ ExpandedCommandLines expanded = commandLines.expand(artifactExpander, execPath, NO_LIMIT, 0);
+ assertThat(commandLines.allArguments()).containsExactly("--foo", "--bar");
+ assertThat(expanded.allArguments()).containsExactly("--foo", "--bar");
assertThat(expanded.arguments()).containsExactly("--foo", "--bar");
assertThat(expanded.getParamFiles()).isEmpty();
}
@Test
+ public void testConcat() throws Exception {
+ CommandLines commandLines =
+ CommandLines.concat(
+ CommandLine.of(ImmutableList.of("--before")),
+ CommandLines.fromArguments(ImmutableList.of("--foo", "--bar")));
+ ExpandedCommandLines expanded = commandLines.expand(artifactExpander, execPath, NO_LIMIT, 0);
+ assertThat(commandLines.allArguments()).containsExactly("--before", "--foo", "--bar");
+ assertThat(expanded.allArguments()).containsExactly("--before", "--foo", "--bar");
+ assertThat(expanded.arguments()).containsExactly("--before", "--foo", "--bar");
+ assertThat(expanded.getParamFiles()).isEmpty();
+ }
+
+ @Test
public void testSimpleParamFileUseAlways() throws Exception {
- ExpandedCommandLines expanded =
+ CommandLines commandLines =
CommandLines.builder()
.addCommandLine(
CommandLine.of(ImmutableList.of("--foo", "--bar")),
ParamFileInfo.builder(ParameterFileType.UNQUOTED).setUseAlways(true).build())
- .build()
- .expand(artifactExpander, execPath, 1024, 0);
+ .build();
+ ExpandedCommandLines expanded = commandLines.expand(artifactExpander, execPath, NO_LIMIT, 0);
+ assertThat(commandLines.allArguments()).containsExactly("--foo", "--bar");
+ assertThat(expanded.allArguments()).containsExactly("--foo", "--bar");
assertThat(expanded.arguments()).containsExactly("@output.txt-0.params");
assertThat(expanded.getParamFiles()).hasSize(1);
assertThat(expanded.getParamFiles().get(0).arguments).containsExactly("--foo", "--bar");
@@ -69,12 +98,12 @@ public class CommandLinesTest {
ParamFileInfo.builder(ParameterFileType.UNQUOTED).setUseAlways(false).build())
.build();
// Set max length to longer than command line, no param file needed
- ExpandedCommandLines expanded = commandLines.expand(artifactExpander, execPath, 1024, 0);
+ ExpandedCommandLines expanded = commandLines.expand(artifactExpander, execPath, NO_LIMIT, 0);
assertThat(expanded.arguments()).containsExactly("--foo", "--bar");
assertThat(expanded.getParamFiles()).isEmpty();
// Set max length to 0, spill to param file is forced
- expanded = commandLines.expand(artifactExpander, execPath, 0, 0);
+ expanded = commandLines.expand(artifactExpander, execPath, new CommandLineLimits(0), 0);
assertThat(expanded.arguments()).containsExactly("@output.txt-0.params");
assertThat(expanded.getParamFiles()).hasSize(1);
assertThat(expanded.getParamFiles().get(0).arguments).containsExactly("--foo", "--bar");
@@ -82,7 +111,7 @@ public class CommandLinesTest {
@Test
public void testMixOfCommandLinesAndParamFiles() throws Exception {
- ExpandedCommandLines expanded =
+ CommandLines commandLines =
CommandLines.builder()
.addCommandLine(CommandLine.of(ImmutableList.of("a", "b")))
.addCommandLine(
@@ -92,8 +121,10 @@ public class CommandLinesTest {
.addCommandLine(
CommandLine.of(ImmutableList.of("g", "h")),
ParamFileInfo.builder(ParameterFileType.UNQUOTED).setUseAlways(true).build())
- .build()
- .expand(artifactExpander, execPath, 1024, 0);
+ .build();
+ ExpandedCommandLines expanded = commandLines.expand(artifactExpander, execPath, NO_LIMIT, 0);
+ assertThat(commandLines.allArguments()).containsExactly("a", "b", "c", "d", "e", "f", "g", "h");
+ assertThat(expanded.allArguments()).containsExactly("a", "b", "c", "d", "e", "f", "g", "h");
assertThat(expanded.arguments())
.containsExactly("a", "b", "@output.txt-0.params", "e", "f", "@output.txt-1.params");
assertThat(expanded.getParamFiles()).hasSize(2);
@@ -107,7 +138,7 @@ public class CommandLinesTest {
@Test
public void testFirstParamFilePassesButSecondFailsLengthTest() throws Exception {
- ExpandedCommandLines expanded =
+ CommandLines commandLines =
CommandLines.builder()
.addCommandLine(
CommandLine.of(ImmutableList.of("a", "b")),
@@ -115,8 +146,11 @@ public class CommandLinesTest {
.addCommandLine(
CommandLine.of(ImmutableList.of("c", "d")),
ParamFileInfo.builder(ParameterFileType.UNQUOTED).setUseAlways(false).build())
- .build()
- .expand(artifactExpander, execPath, 4, 0);
+ .build();
+ ExpandedCommandLines expanded =
+ commandLines.expand(artifactExpander, execPath, new CommandLineLimits(4), 0);
+ assertThat(commandLines.allArguments()).containsExactly("a", "b", "c", "d");
+ assertThat(expanded.allArguments()).containsExactly("a", "b", "c", "d");
assertThat(expanded.arguments()).containsExactly("a", "b", "@output.txt-0.params");
assertThat(expanded.getParamFiles()).hasSize(1);
assertThat(expanded.getParamFiles().get(0).arguments).containsExactly("c", "d");
@@ -137,7 +171,11 @@ public class CommandLinesTest {
Path execRoot = inMemoryFileSystem.getPath("/exec");
execRoot.createDirectoryAndParents();
ExpandedCommandLines expanded =
- commandLines.expand(artifactExpander, PathFragment.create("my/param/file/out"), 0, 0);
+ commandLines.expand(
+ artifactExpander,
+ PathFragment.create("my/param/file/out"),
+ new CommandLineLimits(0),
+ 0);
expanded.writeParamFiles(execRoot);
assertThat(
generated by cgit v1.2.3 (git 2.39.1) at 2024-06-30 03:11:41 +0000