diff options
Diffstat (limited to 'src')
13 files changed, 314 insertions, 219 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 5c4f27f7a3..fef7c2d31f 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 @@ -20,7 +20,6 @@ import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander; import com.google.devtools.build.lib.actions.cache.VirtualActionInput; import com.google.devtools.build.lib.collect.IterablesChain; import com.google.devtools.build.lib.util.Fingerprint; -import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.protobuf.ByteString; import java.io.IOException; @@ -47,6 +46,14 @@ public class CommandLines { /** Command line OS limitations, such as the max length. */ public static class CommandLineLimits { + /** + * "Unlimited" command line limits. + * + * <p>Use these limits when you want to prohibit param files, or you don't use param files so + * you don't care what the limit is. + */ + public static final CommandLineLimits UNLIMITED = new CommandLineLimits(Integer.MAX_VALUE); + public final int maxLength; public CommandLineLimits(int maxLength) { @@ -56,10 +63,10 @@ public class CommandLines { /** A simple tuple of a {@link CommandLine} and a {@link ParamFileInfo}. */ public static class CommandLineAndParamFileInfo { - private final CommandLine commandLine; - @Nullable private final ParamFileInfo paramFileInfo; + public final CommandLine commandLine; + @Nullable public final ParamFileInfo paramFileInfo; - private CommandLineAndParamFileInfo( + public CommandLineAndParamFileInfo( CommandLine commandLine, @Nullable ParamFileInfo paramFileInfo) { this.commandLine = commandLine; this.paramFileInfo = paramFileInfo; @@ -121,11 +128,10 @@ public class CommandLines { if (commandLines instanceof CommandLine) { CommandLine commandLine = (CommandLine) commandLines; Iterable<String> arguments = commandLine.arguments(artifactExpander); - return new ExpandedCommandLines(arguments, arguments, ImmutableList.of()); + return new ExpandedCommandLines(arguments, ImmutableList.of()); } 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 = limits.maxLength - commandLines.size() * paramFileArgLengthEstimate; int cmdLineLength = 0; @@ -136,13 +142,11 @@ public class CommandLines { ParamFileInfo paramFileInfo = pair.paramFileInfo; if (paramFileInfo == null) { Iterable<String> args = commandLine.arguments(artifactExpander); - allArguments.add(args); arguments.add(args); cmdLineLength += totalArgLen(args); } else { 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); @@ -167,7 +171,7 @@ public class CommandLines { } } } - return new ExpandedCommandLines(arguments.build(), allArguments.build(), paramFiles); + return new ExpandedCommandLines(arguments.build(), paramFiles); } public void addToFingerprint(ActionKeyContext actionKeyContext, Fingerprint fingerprint) @@ -197,15 +201,12 @@ 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, - Iterable<String> allArguments, List<ParamFileActionInput> paramFiles) { this.arguments = arguments; - this.allArguments = allArguments; this.paramFiles = paramFiles; } @@ -214,28 +215,10 @@ 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; } - - /** Convenience function to write all param files locally under the given exec root. */ - public void writeParamFiles(Path execRoot) throws IOException { - for (ParamFileActionInput actionInput : paramFiles) { - Path paramFilePath = execRoot.getRelative(actionInput.paramFileExecPath); - paramFilePath.getParentDirectory().createDirectoryAndParents(); - actionInput.writeTo(paramFilePath.getOutputStream()); - } - } } static final class ParamFileActionInput implements VirtualActionInput { @@ -276,7 +259,7 @@ public class CommandLines { // Helper function to unpack the optimized storage format into a list @SuppressWarnings("unchecked") - private List<CommandLineAndParamFileInfo> getCommandLines() { + public List<CommandLineAndParamFileInfo> getCommandLines() { if (commandLines instanceof CommandLine) { return ImmutableList.of(new CommandLineAndParamFileInfo((CommandLine) commandLines, null)); } else if (commandLines instanceof CommandLineAndParamFileInfo) { @@ -316,8 +299,17 @@ public class CommandLines { return new Builder(); } + public static Builder builder(Builder other) { + return new Builder(other); + } + + /** Returns an instance with a single command line. */ + public static CommandLines of(CommandLine commandLine) { + return new CommandLines(commandLine); + } + /** Returns an instance with a single trivial command line. */ - public static CommandLines fromArguments(Iterable<String> args) { + public static CommandLines of(Iterable<String> args) { return new CommandLines(CommandLine.of(args)); } @@ -332,7 +324,15 @@ public class CommandLines { /** Builder for {@link CommandLines}. */ public static class Builder { - List<CommandLineAndParamFileInfo> commandLines = new ArrayList<>(); + private final List<CommandLineAndParamFileInfo> commandLines; + + Builder() { + commandLines = new ArrayList<>(); + } + + Builder(Builder other) { + commandLines = new ArrayList<>(other.commandLines); + } public Builder addCommandLine(CommandLine commandLine) { commandLines.add(new CommandLineAndParamFileInfo(commandLine, null)); 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 c9a5a2dddc..5532b2223d 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 @@ -40,6 +40,10 @@ import com.google.devtools.build.lib.actions.CommandAction; import com.google.devtools.build.lib.actions.CommandLine; import com.google.devtools.build.lib.actions.CommandLineExpansionException; import com.google.devtools.build.lib.actions.CommandLineItemSimpleFormatter; +import com.google.devtools.build.lib.actions.CommandLines; +import com.google.devtools.build.lib.actions.CommandLines.CommandLineAndParamFileInfo; +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.CompositeRunfilesSupplier; import com.google.devtools.build.lib.actions.EmptyRunfilesSupplier; import com.google.devtools.build.lib.actions.ExecException; @@ -91,8 +95,8 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie private static final String GUID = "ebd6fce3-093e-45ee-adb6-bf513b602f0d"; - @VisibleForSerialization - protected final CommandLine argv; + @VisibleForSerialization protected final CommandLines commandLines; + @VisibleForSerialization protected final CommandLineLimits commandLineLimits; private final boolean executeUnconditionally; private final boolean isShellCommand; @@ -103,6 +107,7 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie private final ImmutableMap<String, String> executionInfo; private final ExtraActionInfoSupplier extraActionInfoSupplier; + private final Artifact primaryOutput; /** * Constructs a SpawnAction using direct initialization arguments. @@ -114,11 +119,12 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie * @param inputs the set of all files potentially read by this action; must not be subsequently * modified. * @param outputs the set of all files written by this action; must not be subsequently modified. + * @param primaryOutput the primary output of this action * @param resourceSet the resources consumed by executing this Action * @param env the action environment - * @param argv the command line to execute. This is merely a list of options to the executable, - * and is uninterpreted by the build tool for the purposes of dependency checking; typically - * it may include the names of input and output files, but this is not necessary. + * @param commandLines the command lines to execute. This includes the main argv vector and any + * param file-backed command lines. + * @param commandLineLimits the command line limits, from the build configuration * @param isShellCommand Whether the command line represents a shell command with the given shell * executable. This is used to give better error messages. * @param progressMessage the message printed during the progression of the build. @@ -130,8 +136,10 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie Iterable<Artifact> tools, Iterable<Artifact> inputs, Iterable<Artifact> outputs, + Artifact primaryOutput, ResourceSet resourceSet, - CommandLine argv, + CommandLines commandLines, + CommandLineLimits commandLineLimits, boolean isShellCommand, ActionEnvironment env, CharSequence progressMessage, @@ -141,8 +149,10 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie tools, inputs, outputs, + primaryOutput, resourceSet, - argv, + commandLines, + commandLineLimits, isShellCommand, env, ImmutableMap.<String, String>of(), @@ -164,13 +174,13 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie * @param inputs the set of all files potentially read by this action; must not be subsequently * modified * @param outputs the set of all files written by this action; must not be subsequently modified. + * @param primaryOutput the primary output of this action * @param resourceSet the resources consumed by executing this Action * @param env the action's environment * @param executionInfo out-of-band information for scheduling the spawn - * @param argv the argv array (including argv[0]) of arguments to pass. This is merely a list of - * options to the executable, and is uninterpreted by the build tool for the purposes of - * dependency checking; typically it may include the names of input and output files, but this - * is not necessary. + * @param commandLines the command lines to execute. This includes the main argv vector and any + * param file-backed command lines. + * @param commandLineLimits the command line limits, from the build configuration * @param isShellCommand Whether the command line represents a shell command with the given shell * executable. This is used to give better error messages. * @param progressMessage the message printed during the progression of the build @@ -182,8 +192,10 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie Iterable<Artifact> tools, Iterable<Artifact> inputs, Iterable<Artifact> outputs, + Artifact primaryOutput, ResourceSet resourceSet, - CommandLine argv, + CommandLines commandLines, + CommandLineLimits commandLineLimits, boolean isShellCommand, ActionEnvironment env, ImmutableMap<String, String> executionInfo, @@ -193,9 +205,11 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie boolean executeUnconditionally, ExtraActionInfoSupplier extraActionInfoSupplier) { super(owner, tools, inputs, runfilesSupplier, outputs, env); + this.primaryOutput = primaryOutput; this.resourceSet = resourceSet; this.executionInfo = executionInfo; - this.argv = argv; + this.commandLines = commandLines; + this.commandLineLimits = commandLineLimits; this.isShellCommand = isShellCommand; this.progressMessage = progressMessage; this.mnemonic = mnemonic; @@ -204,9 +218,14 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie } @Override + public Artifact getPrimaryOutput() { + return primaryOutput; + } + + @Override @VisibleForTesting public List<String> getArguments() throws CommandLineExpansionException { - return ImmutableList.copyOf(argv.arguments()); + return ImmutableList.copyOf(commandLines.allArguments()); } @Override @@ -214,10 +233,6 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie return SkylarkList.createImmutable(getArguments()); } - protected CommandLine getCommandLine() { - return argv; - } - @Override @VisibleForTesting public Iterable<Artifact> getPossibleInputsForTesting() { @@ -227,13 +242,13 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie /** Returns command argument, argv[0]. */ @VisibleForTesting public String getCommandFilename() throws CommandLineExpansionException { - return Iterables.getFirst(argv.arguments(), null); + return Iterables.getFirst(getArguments(), null); } /** Returns the (immutable) list of arguments, excluding the command name, argv[0]. */ @VisibleForTesting public List<String> getRemainingArguments() throws CommandLineExpansionException { - return ImmutableList.copyOf(Iterables.skip(argv.arguments(), 1)); + return ImmutableList.copyOf(Iterables.skip(getArguments(), 1)); } @VisibleForTesting @@ -258,7 +273,7 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie */ protected List<SpawnResult> internalExecute(ActionExecutionContext actionExecutionContext) throws ExecException, InterruptedException, CommandLineExpansionException { - Spawn spawn = getSpawn(actionExecutionContext.getClientEnv()); + Spawn spawn = getSpawn(actionExecutionContext); return getContext(actionExecutionContext, spawn).exec(spawn, actionExecutionContext); } @@ -281,7 +296,7 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie failMessage = "error executing shell command: " + "'" - + truncate(Iterables.get(argv.arguments(), 2), 200) + + truncate(Iterables.get(getArguments(), 2), 200) + "'"; } catch (CommandLineExpansionException commandLineExpansionException) { failMessage = @@ -314,25 +329,37 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie * * <p>This method is final, as it is merely a shorthand use of the generic way to obtain a spawn, * which also depends on the client environment. Subclasses that which to override the way to get - * a spawn should override {@link #getSpawn(Map)} instead. + * a spawn should override {@link #getSpawn()} instead. */ public final Spawn getSpawn() throws CommandLineExpansionException { - return getSpawn(null); + return new ActionSpawn(commandLines.allArguments(), null, ImmutableList.of()); } /** * Return a spawn that is representative of the command that this Action will execute in the given * client environment. */ - public Spawn getSpawn(Map<String, String> clientEnv) throws CommandLineExpansionException { - return new ActionSpawn(ImmutableList.copyOf(argv.arguments()), clientEnv); + public Spawn getSpawn(ActionExecutionContext actionExecutionContext) + throws CommandLineExpansionException { + return getSpawn( + actionExecutionContext.getArtifactExpander(), actionExecutionContext.getClientEnv()); + } + + Spawn getSpawn(ArtifactExpander artifactExpander, Map<String, String> clientEnv) + throws CommandLineExpansionException { + ExpandedCommandLines expandedCommandLines = + commandLines.expand(artifactExpander, getPrimaryOutput().getExecPath(), commandLineLimits); + return new ActionSpawn( + ImmutableList.copyOf(expandedCommandLines.arguments()), + clientEnv, + expandedCommandLines.getParamFiles()); } @Override protected void computeKey(ActionKeyContext actionKeyContext, Fingerprint fp) throws CommandLineExpansionException { fp.addString(GUID); - argv.addToFingerprint(actionKeyContext, fp); + commandLines.addToFingerprint(actionKeyContext, fp); fp.addString(getMnemonic()); // We don't need the toolManifests here, because they are a subset of the inputManifests by // definition and the output of an action shouldn't change whether something is considered a @@ -366,7 +393,7 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie message.append('\n'); } try { - for (String argument : ShellEscaper.escapeAll(argv.arguments())) { + for (String argument : ShellEscaper.escapeAll(getArguments())) { message.append(" Argument: "); message.append(argument); message.append('\n'); @@ -455,11 +482,10 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie return actionExecutionContext.getSpawnActionContext(spawn); } - /** - * A spawn instance that is tied to a specific SpawnAction. - */ - public class ActionSpawn extends BaseSpawn { + /** A spawn instance that is tied to a specific SpawnAction. */ + private class ActionSpawn extends BaseSpawn { + private final ImmutableList<ActionInput> inputs; private final ImmutableList<Artifact> filesets; private final ImmutableMap<String, String> effectiveEnvironment; @@ -469,7 +495,10 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie * <p>Subclasses of ActionSpawn may subclass in order to provide action-specific values for * environment variables or action inputs. */ - protected ActionSpawn(ImmutableList<String> arguments, Map<String, String> clientEnv) { + private ActionSpawn( + ImmutableList<String> arguments, + Map<String, String> clientEnv, + Iterable<? extends ActionInput> additionalInputs) { super( arguments, ImmutableMap.<String, String>of(), @@ -477,13 +506,19 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie SpawnAction.this.getRunfilesSupplier(), SpawnAction.this, resourceSet); - ImmutableList.Builder<Artifact> builder = ImmutableList.builder(); + ImmutableList.Builder<ActionInput> inputs = ImmutableList.builder(); + ImmutableList.Builder<Artifact> filesets = ImmutableList.builder(); + ImmutableList<Artifact> manifests = getRunfilesSupplier().getManifests(); for (Artifact input : getInputs()) { if (input.isFileset()) { - builder.add(input); + filesets.add(input); + } else if (!manifests.contains(input)) { + inputs.add(input); } } - filesets = builder.build(); + inputs.addAll(additionalInputs); + this.filesets = filesets.build(); + this.inputs = inputs.build(); LinkedHashMap<String, String> env = new LinkedHashMap<>(SpawnAction.this.getEnvironment()); if (clientEnv != null) { for (String var : SpawnAction.this.getClientEnvironmentVariables()) { @@ -511,20 +546,7 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie @Override @SuppressWarnings("unchecked") public Iterable<? extends ActionInput> getInputFiles() { - Iterable<? extends ActionInput> inputs = getInputs(); - ImmutableList<Artifact> manifests = getRunfilesSupplier().getManifests(); - if (filesets.isEmpty() && manifests.isEmpty()) { - return inputs; - } - // TODO(buchgr): These optimizations shouldn't exists. Instead getInputFiles() should - // directly return a NestedSet. In order to do this, rules need to be updated to - // provide inputs as nestedsets and store manifest files and filesets separately. - if (inputs instanceof NestedSet) { - return new FilesetAndManifestFilteringNestedSetView<>( - (NestedSet<ActionInput>) inputs, filesets, manifests); - } else { - return new FilesetAndManifestFilteringIterable<>(inputs, filesets, manifests); - } + return inputs; } } @@ -575,17 +597,6 @@ 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<>(); @@ -630,7 +641,7 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie this.executableArgs = (other.executableArgs != null) ? Lists.newArrayList(other.executableArgs) : null; - this.commandLines = Lists.newArrayList(other.commandLines); + this.commandLines = new ArrayList<>(other.commandLines); this.progressMessage = other.progressMessage; this.mnemonic = other.mnemonic; } @@ -661,25 +672,65 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie @VisibleForTesting @CheckReturnValue public Action[] build(ActionOwner owner, AnalysisEnvironment analysisEnvironment, BuildConfiguration configuration) { - List<Action> paramFileActions = new ArrayList<>(commandLines.size()); - CommandLine actualCommandLine = - buildCommandLine(owner, analysisEnvironment, configuration, paramFileActions); - Action[] actions = new Action[1 + paramFileActions.size()]; + final Action[] actions; + final CommandLines commandLines; + ImmutableList<String> executableArgs = buildExecutableArgs(); + if (configuration.deferParamFiles()) { + actions = new Action[1]; + CommandLines.Builder result = CommandLines.builder(); + result.addCommandLine(CommandLine.of(executableArgs)); + for (CommandLineAndParamFileInfo pair : this.commandLines) { + result.addCommandLine(pair); + } + commandLines = result.build(); + } else { + List<Action> paramFileActions = new ArrayList<>(this.commandLines.size()); + CommandLine commandLine = + buildCommandLinesAndParamFileActions( + owner, + analysisEnvironment, + configuration, + executableArgs, + this.commandLines, + paramFileActions); + commandLines = CommandLines.of(commandLine); + actions = new Action[1 + paramFileActions.size()]; + for (int i = 0; i < paramFileActions.size(); ++i) { + actions[i + 1] = paramFileActions.get(i); + } + } + ActionEnvironment env = + useDefaultShellEnvironment + ? configuration.getActionEnvironment() + : ActionEnvironment.create(this.environment); Action spawnAction = - buildSpawnAction(owner, actualCommandLine, configuration.getActionEnvironment()); + buildSpawnAction(owner, commandLines, configuration.getCommandLineLimits(), env); actions[0] = spawnAction; - for (int i = 0; i < paramFileActions.size(); ++i) { - actions[i + 1] = paramFileActions.get(i); - } return actions; } - private CommandLine buildCommandLine( + @CheckReturnValue + SpawnAction buildForActionTemplate(ActionOwner owner) { + CommandLines.Builder result = CommandLines.builder(); + ImmutableList<String> executableArgs = buildExecutableArgs(); + result.addCommandLine(CommandLine.of(executableArgs)); + for (CommandLineAndParamFileInfo pair : commandLines) { + result.addCommandLine(pair.commandLine); + } + return buildSpawnAction( + owner, + result.build(), + CommandLineLimits.UNLIMITED, + ActionEnvironment.create(this.environment)); + } + + private CommandLine buildCommandLinesAndParamFileActions( ActionOwner owner, AnalysisEnvironment analysisEnvironment, BuildConfiguration configuration, + ImmutableList<String> executableArgs, + List<CommandLineAndParamFileInfo> commandLines, List<Action> paramFileActions) { - ImmutableList<String> executableArgs = buildExecutableArgs(); boolean hasConditionalParamFile = commandLines.stream().anyMatch(c -> c.paramFileInfo != null && !c.paramFileInfo.always()); boolean spillToParamFiles = false; @@ -689,7 +740,7 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie totalLen += getCommandLineSize(commandLineAndParamFileInfo.commandLine); } // To reduce implementation complexity we either spill all or none of the param files. - spillToParamFiles = totalLen > configuration.getMinParamFileSize(); + spillToParamFiles = totalLen >= configuration.getCommandLineLimits().maxLength; } // We a name based on the output, starting at <output>-2.params // and then incrementing @@ -752,18 +803,12 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie * * <p>This method makes a copy of all the collections, so it is safe to reuse the builder after * this method returns. - * - * <p>This method is invoked by {@link SpawnActionTemplate} in the execution phase. It is - * important that analysis-phase objects (RuleContext, Configuration, etc.) are not directly - * referenced in this function to prevent them from bleeding into the execution phase. - * - * @param owner the {@link ActionOwner} for the SpawnAction - * @param configEnv the config's action environment to use. 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) { + private SpawnAction buildSpawnAction( + ActionOwner owner, + CommandLines commandLines, + CommandLineLimits commandLineLimits, + ActionEnvironment env) { NestedSet<Artifact> tools = toolsBuilder.build(); // Tools are by definition a subset of the inputs, so make sure they're present there, too. @@ -773,13 +818,6 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie .addTransitive(tools) .build(); - ActionEnvironment env; - if (useDefaultShellEnvironment) { - env = Preconditions.checkNotNull(configEnv); - } else { - env = ActionEnvironment.create(this.environment); - } - if (disableSandboxing) { ImmutableMap.Builder<String, String> builder = ImmutableMap.builder(); builder.putAll(executionInfo); @@ -792,8 +830,10 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie tools, inputsAndTools, ImmutableList.copyOf(outputs), + outputs.get(0), resourceSet, - commandLine, + commandLines, + commandLineLimits, isShellCommand, env, ImmutableMap.copyOf(executionInfo), @@ -803,29 +843,16 @@ 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(); - result.addExecutableArguments(executableArgs); - for (CommandLineAndParamFileInfo commandLineAndParamFileInfo : commandLines) { - result.addCommandLine(commandLineAndParamFileInfo.commandLine); - } - return result.build(); - } - /** Creates a SpawnAction. */ protected SpawnAction createSpawnAction( ActionOwner owner, NestedSet<Artifact> tools, NestedSet<Artifact> inputsAndTools, ImmutableList<Artifact> outputs, + Artifact primaryOutput, ResourceSet resourceSet, - CommandLine actualCommandLine, + CommandLines commandLines, + CommandLineLimits commandLineLimits, boolean isShellCommand, ActionEnvironment env, ImmutableMap<String, String> executionInfo, @@ -837,8 +864,10 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie tools, inputsAndTools, outputs, + primaryOutput, resourceSet, - actualCommandLine, + commandLines, + commandLineLimits, isShellCommand, env, executionInfo, diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplate.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplate.java index c8c7176dd9..fcb1766176 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplate.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplate.java @@ -129,8 +129,7 @@ public final class SpawnActionTemplate implements ActionTemplate<SpawnAction> { // Note that we pass in nulls below because SpawnActionTemplate does not support param file, and // it does not use any default value for executable or shell environment. They must be set // explicitly via builder method #setExecutable and #setEnvironment. - return actionBuilder.buildSpawnAction( - getOwner(), actionBuilder.buildCommandLineWithoutParamsFiles(), /*configEnv=*/ null); + return actionBuilder.buildForActionTemplate(getOwner()); } /** @@ -204,9 +203,7 @@ public final class SpawnActionTemplate implements ActionTemplate<SpawnAction> { @Override public Iterable<String> getClientEnvironmentVariables() { - return spawnActionBuilder - .buildSpawnAction(getOwner(), CommandLine.of(ImmutableList.of()), null) - .getClientEnvironmentVariables(); + return spawnActionBuilder.buildForActionTemplate(getOwner()).getClientEnvironmentVariables(); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java index 80f35abd4f..9c33df8865 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java @@ -34,6 +34,7 @@ import com.google.common.collect.MutableClassToInstanceMap; import com.google.devtools.build.lib.actions.ActionEnvironment; import com.google.devtools.build.lib.actions.ArtifactRoot; import com.google.devtools.build.lib.actions.BuildConfigurationEvent; +import com.google.devtools.build.lib.actions.CommandLines.CommandLineLimits; import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider; import com.google.devtools.build.lib.analysis.actions.FileWriteAction; @@ -424,6 +425,21 @@ public class BuildConfiguration { public int minParamFileSize; @Option( + name = "defer_param_files", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, + effectTags = { + OptionEffectTag.LOADING_AND_ANALYSIS, + OptionEffectTag.EXECUTION, + OptionEffectTag.ACTION_COMMAND_LINES + }, + help = + "Whether to use deferred param files. WHen set, param files will not be " + + "added to the action graph. Instead, they will be added as virtual action inputs " + + "and written at the same time as the action executes.") + public boolean deferParamFiles; + + @Option( name = "experimental_extended_sanity_checks", defaultValue = "false", documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, @@ -993,6 +1009,7 @@ public class BuildConfiguration { private final String repositoryName; private final RepositoryName mainRepositoryName; private final ImmutableSet<String> reservedActionMnemonics; + private CommandLineLimits commandLineLimits; /** * Directories in the output tree. @@ -1316,6 +1333,7 @@ public class BuildConfiguration { this.reservedActionMnemonics = reservedActionMnemonics; this.buildEventSupplier = Suppliers.memoize(this::createBuildEvent); + this.commandLineLimits = new CommandLineLimits(options.minParamFileSize); } /** @@ -1752,8 +1770,12 @@ public class BuildConfiguration { return testEnv; } - public int getMinParamFileSize() { - return options.minParamFileSize; + public CommandLineLimits getCommandLineLimits() { + return commandLineLimits; + } + + public boolean deferParamFiles() { + return options.deferParamFiles; } @SkylarkCallable(name = "coverage_enabled", structField = true, diff --git a/src/main/java/com/google/devtools/build/lib/analysis/extra/ExtraAction.java b/src/main/java/com/google/devtools/build/lib/analysis/extra/ExtraAction.java index 7248874cdd..c3f573d935 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/extra/ExtraAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/extra/ExtraAction.java @@ -19,6 +19,7 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; import com.google.common.collect.Sets; import com.google.devtools.build.lib.actions.AbstractAction; import com.google.devtools.build.lib.actions.Action; @@ -28,6 +29,8 @@ import com.google.devtools.build.lib.actions.ActionExecutionException; import com.google.devtools.build.lib.actions.ActionResult; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.CommandLine; +import com.google.devtools.build.lib.actions.CommandLines; +import com.google.devtools.build.lib.actions.CommandLines.CommandLineLimits; import com.google.devtools.build.lib.actions.CompositeRunfilesSupplier; import com.google.devtools.build.lib.actions.RunfilesSupplier; import com.google.devtools.build.lib.actions.SpawnActionContext; @@ -78,8 +81,10 @@ public final class ExtraAction extends SpawnAction { ImmutableList.<Artifact>of(), createInputs(shadowedAction.getInputs(), ImmutableList.<Artifact>of(), extraActionInputs), outputs, + Iterables.getFirst(outputs, null), AbstractAction.DEFAULT_RESOURCE_SET, - argv, + CommandLines.of(argv), + CommandLineLimits.UNLIMITED, false, env, ImmutableMap.copyOf(executionInfo), diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendAction.java index ce171002b6..857e457a22 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendAction.java @@ -24,8 +24,9 @@ import com.google.devtools.build.lib.actions.ActionExecutionException; import com.google.devtools.build.lib.actions.ActionKeyContext; import com.google.devtools.build.lib.actions.ActionOwner; import com.google.devtools.build.lib.actions.Artifact; -import com.google.devtools.build.lib.actions.CommandLine; import com.google.devtools.build.lib.actions.CommandLineExpansionException; +import com.google.devtools.build.lib.actions.CommandLines; +import com.google.devtools.build.lib.actions.CommandLines.CommandLineLimits; import com.google.devtools.build.lib.actions.ResourceSet; import com.google.devtools.build.lib.actions.RunfilesSupplier; import com.google.devtools.build.lib.analysis.actions.SpawnAction; @@ -67,8 +68,10 @@ public final class LtoBackendAction extends SpawnAction { Map<PathFragment, Artifact> allBitcodeFiles, Artifact importsFile, Collection<Artifact> outputs, + Artifact primaryOutput, ActionOwner owner, - CommandLine argv, + CommandLines argv, + CommandLineLimits commandLineLimits, boolean isShellCommand, ActionEnvironment env, Map<String, String> executionInfo, @@ -80,8 +83,10 @@ public final class LtoBackendAction extends SpawnAction { ImmutableList.<Artifact>of(), inputs, outputs, + primaryOutput, AbstractAction.DEFAULT_RESOURCE_SET, argv, + commandLineLimits, isShellCommand, env, ImmutableMap.copyOf(executionInfo), @@ -220,8 +225,10 @@ public final class LtoBackendAction extends SpawnAction { NestedSet<Artifact> tools, NestedSet<Artifact> inputsAndTools, ImmutableList<Artifact> outputs, + Artifact primaryOutput, ResourceSet resourceSet, - CommandLine actualCommandLine, + CommandLines commandLines, + CommandLineLimits commandLineLimits, boolean isShellCommand, ActionEnvironment env, ImmutableMap<String, String> executionInfo, @@ -233,8 +240,10 @@ public final class LtoBackendAction extends SpawnAction { bitcodeFiles, imports, outputs, + primaryOutput, owner, - actualCommandLine, + commandLines, + commandLineLimits, isShellCommand, env, executionInfo, diff --git a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleAction.java b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleAction.java index ec9bcc9daa..6f8e75fb65 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleAction.java @@ -16,12 +16,14 @@ package com.google.devtools.build.lib.rules.genrule; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.ActionEnvironment; import com.google.devtools.build.lib.actions.ActionExecutionContext; import com.google.devtools.build.lib.actions.ActionOwner; import com.google.devtools.build.lib.actions.Artifact; -import com.google.devtools.build.lib.actions.CommandLine; import com.google.devtools.build.lib.actions.CommandLineExpansionException; +import com.google.devtools.build.lib.actions.CommandLines; +import com.google.devtools.build.lib.actions.CommandLines.CommandLineLimits; import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.ResourceSet; import com.google.devtools.build.lib.actions.RunfilesSupplier; @@ -47,7 +49,7 @@ public class GenRuleAction extends SpawnAction { Iterable<Artifact> tools, Iterable<Artifact> inputs, Iterable<Artifact> outputs, - CommandLine argv, + CommandLines commandLines, ActionEnvironment env, ImmutableMap<String, String> executionInfo, RunfilesSupplier runfilesSupplier, @@ -57,8 +59,10 @@ public class GenRuleAction extends SpawnAction { tools, inputs, outputs, + Iterables.getFirst(outputs, null), GENRULE_RESOURCES, - argv, + commandLines, + CommandLineLimits.UNLIMITED, false, env, executionInfo, diff --git a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java index 33e52c01b9..7f133d75a9 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java +++ b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java @@ -20,7 +20,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; import com.google.common.collect.Maps; import com.google.devtools.build.lib.actions.Artifact; -import com.google.devtools.build.lib.actions.CommandLine; +import com.google.devtools.build.lib.actions.CommandLines; import com.google.devtools.build.lib.actions.CompositeRunfilesSupplier; import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException; import com.google.devtools.build.lib.analysis.AliasProvider; @@ -222,7 +222,7 @@ public abstract class GenRuleBase implements RuleConfiguredTargetFactory { ImmutableList.copyOf(commandHelper.getResolvedTools()), inputs.build(), filesToBuild, - CommandLine.of(argv), + CommandLines.of(argv), ruleContext.getConfiguration().getActionEnvironment(), ImmutableMap.copyOf(executionInfo), new CompositeRunfilesSupplier(commandHelper.getToolsRunfilesSuppliers()), diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java index 1c02c96a08..b60cb2c30e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java @@ -31,6 +31,8 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ArtifactRoot; import com.google.devtools.build.lib.actions.CommandLine; import com.google.devtools.build.lib.actions.CommandLineExpansionException; +import com.google.devtools.build.lib.actions.CommandLines; +import com.google.devtools.build.lib.actions.CommandLines.CommandLineLimits; import com.google.devtools.build.lib.actions.ParameterFile; import com.google.devtools.build.lib.actions.ResourceSet; import com.google.devtools.build.lib.actions.RunfilesSupplier; @@ -180,6 +182,7 @@ public final class JavaCompileAction extends SpawnAction { Collection<Artifact> outputs, CommandLine javaCompileCommandLine, CommandLine commandLine, + CommandLineLimits commandLineLimits, PathFragment classDirectory, Artifact outputJar, NestedSet<Artifact> classpathEntries, @@ -203,8 +206,10 @@ public final class JavaCompileAction extends SpawnAction { tools, inputs, outputs, + outputJar, LOCAL_RESOURCES, - commandLine, + CommandLines.of(commandLine), + commandLineLimits, false, // TODO(#3320): This is missing the configuration's action environment! UTF8_ACTION_ENVIRONMENT, @@ -601,6 +606,7 @@ public final class JavaCompileAction extends SpawnAction { outputs, paramFileContents, javaBuilderCommandLine.build(), + configuration.getCommandLineLimits(), classDirectory, outputJar, classpathEntries, diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java index 51f91c2634..119dbcaafa 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java @@ -32,6 +32,8 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.BaseSpawn; import com.google.devtools.build.lib.actions.CommandLine; import com.google.devtools.build.lib.actions.CommandLineExpansionException; +import com.google.devtools.build.lib.actions.CommandLines; +import com.google.devtools.build.lib.actions.CommandLines.CommandLineLimits; import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.ParamFileInfo; import com.google.devtools.build.lib.actions.ParameterFile; @@ -97,8 +99,10 @@ public class JavaHeaderCompileAction extends SpawnAction { * @param directInputs the set of direct input artifacts of the compile action * @param inputs the set of transitive input artifacts of the compile action * @param outputs the outputs of the action + * @param primaryOutput the output jar + * @param commandLines the transitive command line arguments for the java header compiler * @param directCommandLine the direct command line arguments for the java header compiler - * @param argv the transitive command line arguments for the java header compiler + * @param commandLineLimits the command line limits * @param progressMessage the message printed during the progression of the build */ protected JavaHeaderCompileAction( @@ -107,16 +111,20 @@ public class JavaHeaderCompileAction extends SpawnAction { Iterable<Artifact> directInputs, Iterable<Artifact> inputs, Iterable<Artifact> outputs, + Artifact primaryOutput, + CommandLines commandLines, CommandLine directCommandLine, - CommandLine argv, + CommandLineLimits commandLineLimits, CharSequence progressMessage) { super( owner, tools, inputs, outputs, + primaryOutput, LOCAL_RESOURCES, - argv, + commandLines, + commandLineLimits, false, // TODO(#3320): This is missing the config's action environment. JavaCompileAction.UTF8_ACTION_ENVIRONMENT, @@ -148,9 +156,7 @@ public class JavaHeaderCompileAction extends SpawnAction { // if the direct input spawn failed, try again with transitive inputs to produce better // better messages try { - return context.exec( - getSpawn(actionExecutionContext.getClientEnv()), - actionExecutionContext); + return context.exec(getSpawn(actionExecutionContext), actionExecutionContext); } catch (CommandLineExpansionException commandLineExpansionException) { throw new UserExecException(commandLineExpansionException); } @@ -459,8 +465,10 @@ public class JavaHeaderCompileAction extends SpawnAction { tools, transitiveInputs, outputs, + outputJar, LOCAL_RESOURCES, - transitiveCommandLine, + CommandLines.of(transitiveCommandLine), + ruleContext.getConfiguration().getCommandLineLimits(), false, // TODO(b/63280599): This is missing the config's action environment. JavaCompileAction.UTF8_ACTION_ENVIRONMENT, @@ -489,8 +497,10 @@ public class JavaHeaderCompileAction extends SpawnAction { directInputs, transitiveInputs, outputs, + outputJar, + CommandLines.of(transitiveCommandLine), directCommandLine, - transitiveCommandLine, + ruleContext.getConfiguration().getCommandLineLimits(), getProgressMessage()) }; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/ResourceJarActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/java/ResourceJarActionBuilder.java index 5120208f11..d5931af5be 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/ResourceJarActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/ResourceJarActionBuilder.java @@ -135,7 +135,7 @@ public class ResourceJarActionBuilder { // the params file in situations where it is required for --min_param_file_size. if (sizeGreaterThanOrEqual( Iterables.concat(messages, resources.values(), resourceJars, classpathResources), 10) - || ruleContext.getConfiguration().getMinParamFileSize() < 10000) { + || ruleContext.getConfiguration().getCommandLineLimits().maxLength < 10000) { paramFileInfo = ParamFileInfo.builder(ParameterFileType.SHELL_QUOTED).build(); } ruleContext.registerAction( 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 ab5b4c54a4..c51e236b6f 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 @@ -20,11 +20,7 @@ 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; -import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; -import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; -import java.nio.charset.StandardCharsets; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -45,17 +41,15 @@ public class CommandLinesTest { .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")); + CommandLines commandLines = CommandLines.of(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(); } @@ -65,10 +59,9 @@ public class CommandLinesTest { CommandLines commandLines = CommandLines.concat( CommandLine.of(ImmutableList.of("--before")), - CommandLines.fromArguments(ImmutableList.of("--foo", "--bar"))); + CommandLines.of(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(); } @@ -83,7 +76,6 @@ public class CommandLinesTest { .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"); @@ -124,7 +116,6 @@ public class CommandLinesTest { .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); @@ -150,41 +141,8 @@ public class CommandLinesTest { 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"); } - - @Test - public void testWriteParamFiles() throws Exception { - CommandLines commandLines = - CommandLines.builder() - .addCommandLine( - CommandLine.of(ImmutableList.of("--foo", "--bar")), - ParamFileInfo.builder(ParameterFileType.UNQUOTED).setUseAlways(true).build()) - .addCommandLine( - CommandLine.of(ImmutableList.of("--baz")), - ParamFileInfo.builder(ParameterFileType.UNQUOTED).setUseAlways(true).build()) - .build(); - InMemoryFileSystem inMemoryFileSystem = new InMemoryFileSystem(); - Path execRoot = inMemoryFileSystem.getPath("/exec"); - execRoot.createDirectoryAndParents(); - ExpandedCommandLines expanded = - commandLines.expand( - artifactExpander, - PathFragment.create("my/param/file/out"), - new CommandLineLimits(0), - 0); - expanded.writeParamFiles(execRoot); - - assertThat( - FileSystemUtils.readLines( - execRoot.getRelative("my/param/file/out-0.params"), StandardCharsets.ISO_8859_1)) - .containsExactly("--foo", "--bar"); - assertThat( - FileSystemUtils.readLines( - execRoot.getRelative("my/param/file/out-1.params"), StandardCharsets.ISO_8859_1)) - .containsExactly("--baz"); - } } diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTest.java index 6cf8622e32..3cbe6b59d5 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTest.java @@ -26,11 +26,14 @@ import com.google.common.collect.ImmutableSet; import com.google.common.eventbus.EventBus; import com.google.devtools.build.lib.actions.AbstractAction; import com.google.devtools.build.lib.actions.Action; +import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.CommandLine; import com.google.devtools.build.lib.actions.ParamFileInfo; import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType; import com.google.devtools.build.lib.actions.RunfilesSupplier; +import com.google.devtools.build.lib.actions.Spawn; +import com.google.devtools.build.lib.actions.cache.VirtualActionInput; import com.google.devtools.build.lib.actions.extra.EnvironmentVariable; import com.google.devtools.build.lib.actions.extra.ExtraActionInfo; import com.google.devtools.build.lib.actions.extra.SpawnInfo; @@ -47,6 +50,8 @@ import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; +import java.util.stream.StreamSupport; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -161,8 +166,58 @@ public class SpawnActionTest extends BuildViewTestCase { } @Test + public void testBuilderWithJavaExecutableAndParameterFile2() throws Exception { + useConfiguration("--min_param_file_size=0", "--defer_param_files"); + collectingAnalysisEnvironment = + new AnalysisTestUtil.CollectingAnalysisEnvironment(getTestAnalysisEnvironment()); + Artifact output = getBinArtifactWithNoOwner("output"); + Action[] actions = + builder() + .addOutput(output) + .setJavaExecutable( + scratch.file("/bin/java").asFragment(), + jarArtifact, + "MyMainClass", + asList("-jvmarg")) + .addCommandLine( + CustomCommandLine.builder().add("-X").build(), + ParamFileInfo.builder(ParameterFileType.UNQUOTED).build()) + .build(ActionsTestUtil.NULL_ACTION_OWNER, collectingAnalysisEnvironment, targetConfig); + SpawnAction action = (SpawnAction) actions[0]; + + // The action reports all arguments, including those inside the param file + assertThat(action.getArguments()) + .containsExactly( + "/bin/java", "-Xverify:none", "-jvmarg", "-cp", "pkg/exe.jar", "MyMainClass", "-X") + .inOrder(); + + Spawn spawn = action.getSpawn((artifact, outputs) -> outputs.add(artifact), ImmutableMap.of()); + String paramFileName = output.getExecPathString() + "-0.params"; + // The spawn's primary arguments should reference the param file + assertThat(spawn.getArguments()) + .containsExactly( + "/bin/java", + "-Xverify:none", + "-jvmarg", + "-cp", + "pkg/exe.jar", + "MyMainClass", + "@" + paramFileName) + .inOrder(); + + // Asserts that the inputs contain the param file virtual input + Optional<? extends ActionInput> input = + StreamSupport.stream(spawn.getInputFiles().spliterator(), false) + .filter(i -> i instanceof VirtualActionInput) + .findFirst(); + assertThat(input.isPresent()).isTrue(); + VirtualActionInput paramFile = (VirtualActionInput) input.get(); + assertThat(paramFile.getBytes().toString(ISO_8859_1).trim()).isEqualTo("-X"); + } + + @Test public void testBuilderWithJavaExecutableAndParameterFile() throws Exception { - useConfiguration("--min_param_file_size=0"); + useConfiguration("--min_param_file_size=0", "--nodefer_param_files"); collectingAnalysisEnvironment = new AnalysisTestUtil.CollectingAnalysisEnvironment( getTestAnalysisEnvironment()); Artifact output = getBinArtifactWithNoOwner("output"); @@ -205,7 +260,7 @@ public class SpawnActionTest extends BuildViewTestCase { @Test public void testBuilderWithJavaExecutableAndParameterFileAndParameterFileFlag() throws Exception { - useConfiguration("--min_param_file_size=0"); + useConfiguration("--min_param_file_size=0", "--nodefer_param_files"); collectingAnalysisEnvironment = new AnalysisTestUtil.CollectingAnalysisEnvironment( getTestAnalysisEnvironment()); @@ -279,7 +334,7 @@ public class SpawnActionTest extends BuildViewTestCase { @Test public void testBuilderWithExtraExecutableArgumentsAndParameterFile() throws Exception { - useConfiguration("--min_param_file_size=0"); + useConfiguration("--min_param_file_size=0", "--nodefer_param_files"); collectingAnalysisEnvironment = new AnalysisTestUtil.CollectingAnalysisEnvironment( getTestAnalysisEnvironment()); Artifact output = getBinArtifactWithNoOwner("output"); @@ -338,7 +393,7 @@ public class SpawnActionTest extends BuildViewTestCase { Artifact paramFile = getBinArtifactWithNoOwner("output1-2.params"); PathFragment executable = PathFragment.create("/bin/executable"); - useConfiguration("--min_param_file_size=500"); + useConfiguration("--min_param_file_size=500", "--nodefer_param_files"); String longOption = Strings.repeat("x", 1000); SpawnAction spawnAction = @@ -355,7 +410,7 @@ public class SpawnActionTest extends BuildViewTestCase { assertThat(spawnAction.getRemainingArguments()).containsExactly( "@" + paramFile.getExecPathString()).inOrder(); - useConfiguration("--min_param_file_size=1500"); + useConfiguration("--min_param_file_size=1500", "--nodefer_param_files"); spawnAction = ((SpawnAction) builder() @@ -388,7 +443,7 @@ public class SpawnActionTest extends BuildViewTestCase { @Test public void testMultipleParameterFiles() throws Exception { - useConfiguration("--min_param_file_size=0"); + useConfiguration("--min_param_file_size=0", "--nodefer_param_files"); Artifact input = getSourceArtifact("input"); Artifact output = getBinArtifactWithNoOwner("output"); Artifact paramFile1 = getBinArtifactWithNoOwner("output-2.params"); |