diff options
Diffstat (limited to 'src')
-rw-r--r-- | src/main/java/com/google/devtools/build/lib/actions/CommandLines.java | 186 | ||||
-rw-r--r-- | src/test/java/com/google/devtools/build/lib/actions/CommandLinesTest.java | 68 |
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( |