diff options
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib/analysis/actions')
4 files changed, 98 insertions, 57 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/CommandLine.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/CommandLine.java index 7bc670569a..1ca9df707a 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/actions/CommandLine.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/CommandLine.java @@ -18,6 +18,7 @@ import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander; +import com.google.devtools.build.lib.actions.CommandLineExpansionException; import com.google.devtools.build.lib.collect.CollectionUtils; /** A representation of a list of arguments, often a command executed by {@link SpawnAction}. */ @@ -30,10 +31,8 @@ public abstract class CommandLine { } }; - /** - * Returns the command line. - */ - public abstract Iterable<String> arguments(); + /** Returns the command line. */ + public abstract Iterable<String> arguments() throws CommandLineExpansionException; /** * Returns the evaluated command line with enclosed artifacts expanded by {@code artifactExpander} @@ -43,7 +42,8 @@ public abstract class CommandLine { * artifact expansion. Subclasses should override this method if they contain TreeArtifacts and * need to expand them for proper argument evaluation. */ - public Iterable<String> arguments(ArtifactExpander artifactExpander) { + public Iterable<String> arguments(ArtifactExpander artifactExpander) + throws CommandLineExpansionException { return arguments(); } @@ -69,12 +69,13 @@ public abstract class CommandLine { } return new CommandLine() { @Override - public Iterable<String> arguments() { + public Iterable<String> arguments() throws CommandLineExpansionException { return Iterables.concat(executableArgs, commandLine.arguments()); } @Override - public Iterable<String> arguments(ArtifactExpander artifactExpander) { + public Iterable<String> arguments(ArtifactExpander artifactExpander) + throws CommandLineExpansionException { return Iterables.concat(executableArgs, commandLine.arguments(artifactExpander)); } }; @@ -91,12 +92,13 @@ public abstract class CommandLine { } return new CommandLine() { @Override - public Iterable<String> arguments() { + public Iterable<String> arguments() throws CommandLineExpansionException { return Iterables.concat(commandLine.arguments(), args); } @Override - public Iterable<String> arguments(ArtifactExpander artifactExpander) { + public Iterable<String> arguments(ArtifactExpander artifactExpander) + throws CommandLineExpansionException { return Iterables.concat(commandLine.arguments(artifactExpander), args); } }; @@ -108,6 +110,10 @@ public abstract class CommandLine { */ @Override public String toString() { - return Joiner.on(' ').join(arguments()); + try { + return Joiner.on(' ').join(arguments()); + } catch (CommandLineExpansionException e) { + return "Error in expanding command line"; + } } } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/ParamFileHelper.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/ParamFileHelper.java index 9803d05ddb..01781d422c 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/actions/ParamFileHelper.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/ParamFileHelper.java @@ -18,6 +18,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.ActionOwner; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.CommandLineExpansionException; import com.google.devtools.build.lib.actions.ParameterFile; import com.google.devtools.build.lib.analysis.AnalysisEnvironment; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; @@ -130,8 +131,15 @@ public final class ParamFileHelper { /** Estimates the params file size for the given arguments. */ private static int getParamFileSize(List<String> executableArgs, CommandLine commandLine) { - Iterable<String> actualArguments = commandLine.arguments(); - return getParamFileSize(executableArgs) + getParamFileSize(actualArguments); + try { + Iterable<String> actualArguments = commandLine.arguments(); + return getParamFileSize(executableArgs) + getParamFileSize(actualArguments); + } catch (CommandLineExpansionException e) { + // CommandLineExpansionException is thrown deterministically. We can ignore + // it here and pretend that a params file is not necessary at this stage, + // and an error will be thrown later at execution time. + return 0; + } } private static int getParamFileSize(Iterable<String> args) { diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/ParameterFileWriteAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/ParameterFileWriteAction.java index 6889e44513..0dc67da467 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/actions/ParameterFileWriteAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/ParameterFileWriteAction.java @@ -21,13 +21,14 @@ 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.Artifact.ArtifactExpander; +import com.google.devtools.build.lib.actions.CommandLineExpansionException; +import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType; -import com.google.devtools.build.lib.analysis.actions.AbstractFileWriteAction.DeterministicWriter; +import com.google.devtools.build.lib.actions.UserExecException; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.util.Fingerprint; import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.util.ShellEscaper; - import java.io.IOException; import java.io.OutputStream; import java.io.OutputStreamWriter; @@ -81,12 +82,11 @@ public final class ParameterFileWriteAction extends AbstractFileWriteAction { } /** - * Returns the list of options written to the parameter file. Don't use this - * method outside tests - the list is often huge, resulting in significant - * garbage collection overhead. + * Returns the list of options written to the parameter file. Don't use this method outside tests + * - the list is often huge, resulting in significant garbage collection overhead. */ @VisibleForTesting - public Iterable<String> getContents() { + public Iterable<String> getContents() throws CommandLineExpansionException { Preconditions.checkState( !hasInputArtifactToExpand, "This action contains a CommandLine with TreeArtifacts: %s, which must be expanded using " @@ -96,34 +96,41 @@ public final class ParameterFileWriteAction extends AbstractFileWriteAction { } @VisibleForTesting - public Iterable<String> getContents(ArtifactExpander artifactExpander) { + public Iterable<String> getContents(ArtifactExpander artifactExpander) + throws CommandLineExpansionException { return commandLine.arguments(artifactExpander); } @Override - public DeterministicWriter newDeterministicWriter(ActionExecutionContext ctx) { - return new ParamFileWriter(Preconditions.checkNotNull(ctx.getArtifactExpander())); + public DeterministicWriter newDeterministicWriter(ActionExecutionContext ctx) + throws ExecException { + final Iterable<String> arguments; + try { + ArtifactExpander artifactExpander = Preconditions.checkNotNull(ctx.getArtifactExpander()); + arguments = commandLine.arguments(artifactExpander); + } catch (CommandLineExpansionException e) { + throw new UserExecException(e); + } + return new ParamFileWriter(arguments); } private class ParamFileWriter implements DeterministicWriter { - private final ArtifactExpander artifactExpander; + private final Iterable<String> arguments; - ParamFileWriter(ArtifactExpander artifactExpander) { - this.artifactExpander = artifactExpander; + ParamFileWriter(Iterable<String> arguments) { + this.arguments = arguments; } @Override public void writeOutputFile(OutputStream out) throws IOException { - Iterable<String> arguments = commandLine.arguments(artifactExpander); - switch (type) { - case SHELL_QUOTED : + case SHELL_QUOTED: writeContentQuoted(out, arguments); break; - case UNQUOTED : + case UNQUOTED: writeContentUnquoted(out, arguments); break; - default : + default: throw new AssertionError(); } } @@ -157,7 +164,7 @@ public final class ParameterFileWriteAction extends AbstractFileWriteAction { } @Override - protected String computeKey() { + protected String computeKey() throws CommandLineExpansionException { Fingerprint f = new Fingerprint(); f.addString(GUID); f.addString(String.valueOf(makeExecutable)); 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 a1db0e05f0..ded0a8e795 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 @@ -33,6 +33,7 @@ import com.google.devtools.build.lib.actions.ActionOwner; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.BaseSpawn; import com.google.devtools.build.lib.actions.CommandAction; +import com.google.devtools.build.lib.actions.CommandLineExpansionException; import com.google.devtools.build.lib.actions.CompositeRunfilesSupplier; import com.google.devtools.build.lib.actions.EmptyRunfilesSupplier; import com.google.devtools.build.lib.actions.ExecException; @@ -203,15 +204,19 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie @Override @VisibleForTesting - public List<String> getArguments() { + public List<String> getArguments() throws CommandLineExpansionException { return ImmutableList.copyOf(argv.arguments()); } @Override - public SkylarkList<String> getSkylarkArgv() { + public SkylarkList<String> getSkylarkArgv() throws CommandLineExpansionException { return SkylarkList.createImmutable(getArguments()); } + protected CommandLine getCommandLine() { + return argv; + } + @Override @VisibleForTesting public Iterable<Artifact> getPossibleInputsForTesting() { @@ -220,16 +225,13 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie /** Returns command argument, argv[0]. */ @VisibleForTesting - public String getCommandFilename() { + public String getCommandFilename() throws CommandLineExpansionException { return Iterables.getFirst(argv.arguments(), null); } - /** - * Returns the (immutable) list of arguments, excluding the command name, - * argv[0]. - */ + /** Returns the (immutable) list of arguments, excluding the command name, argv[0]. */ @VisibleForTesting - public List<String> getRemainingArguments() { + public List<String> getRemainingArguments() throws CommandLineExpansionException { return ImmutableList.copyOf(Iterables.skip(argv.arguments(), 1)); } @@ -253,8 +255,8 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie * * <p>Called by {@link #execute}. */ - protected void internalExecute( - ActionExecutionContext actionExecutionContext) throws ExecException, InterruptedException { + protected void internalExecute(ActionExecutionContext actionExecutionContext) + throws ExecException, InterruptedException, CommandLineExpansionException { getContext(actionExecutionContext) .exec(getSpawn(actionExecutionContext.getClientEnv()), actionExecutionContext); } @@ -265,7 +267,7 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie try { internalExecute(actionExecutionContext); } catch (ExecException e) { - final String failMessage; + String failMessage; if (isShellCommand()) { // The possible reasons it could fail are: shell executable not found, shell // exited non-zero, or shell died from signal. The first is impossible @@ -274,13 +276,24 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie // command that failed. // // 0=shell executable, 1=shell command switch, 2=command - failMessage = "error executing shell command: " + "'" - + truncate(Iterables.get(argv.arguments(), 2), 200) + "'"; + try { + failMessage = + "error executing shell command: " + + "'" + + truncate(Iterables.get(argv.arguments(), 2), 200) + + "'"; + } catch (CommandLineExpansionException commandLineExpansionException) { + failMessage = + "error executing shell command, and error expanding command line: " + + commandLineExpansionException; + } } else { failMessage = getRawProgressMessage(); } throw e.toActionExecutionException( failMessage, actionExecutionContext.getVerboseFailures(), this); + } catch (CommandLineExpansionException e) { + throw new ActionExecutionException(e, this, false); } } @@ -295,14 +308,14 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie } /** - * Returns a Spawn that is representative of the command that this Action - * will execute. This function must not modify any state. + * Returns a Spawn that is representative of the command that this Action will execute. This + * function must not modify any state. * - * This method is final, as it is merely a shorthand use of the generic way to obtain a spawn, + * <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. */ - public final Spawn getSpawn() { + public final Spawn getSpawn() throws CommandLineExpansionException { return getSpawn(null); } @@ -310,12 +323,12 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie * 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) { - return new ActionSpawn(clientEnv); + public Spawn getSpawn(Map<String, String> clientEnv) throws CommandLineExpansionException { + return new ActionSpawn(ImmutableList.copyOf(argv.arguments()), clientEnv); } @Override - protected String computeKey() { + protected String computeKey() throws CommandLineExpansionException { Fingerprint f = new Fingerprint(); f.addString(GUID); f.addStrings(argv.arguments()); @@ -352,9 +365,15 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie message.append(ShellEscaper.escapeString(var)); message.append('\n'); } - for (String argument : ShellEscaper.escapeAll(argv.arguments())) { - message.append(" Argument: "); - message.append(argument); + try { + for (String argument : ShellEscaper.escapeAll(argv.arguments())) { + message.append(" Argument: "); + message.append(argument); + message.append('\n'); + } + } catch (CommandLineExpansionException e) { + message.append("Could not expand command line: "); + message.append(e); message.append('\n'); } return message.toString(); @@ -374,7 +393,7 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie } @Override - public ExtraActionInfo.Builder getExtraActionInfo() { + public ExtraActionInfo.Builder getExtraActionInfo() throws CommandLineExpansionException { ExtraActionInfo.Builder builder = super.getExtraActionInfo(); if (extraActionInfoSupplier == null) { SpawnInfo spawnInfo = getExtraActionSpawnInfo(); @@ -392,7 +411,7 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie * <p>Subclasses of SpawnAction may override this in order to provide action-specific behaviour. * This can be necessary, for example, when the action discovers inputs. */ - protected SpawnInfo getExtraActionSpawnInfo() { + protected SpawnInfo getExtraActionSpawnInfo() throws CommandLineExpansionException { SpawnInfo.Builder info = SpawnInfo.newBuilder(); Spawn spawn = getSpawn(); info.addAllArgument(spawn.getArguments()); @@ -449,8 +468,9 @@ 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(Map<String, String> clientEnv) { - super(ImmutableList.copyOf(argv.arguments()), + protected ActionSpawn(ImmutableList<String> arguments, Map<String, String> clientEnv) { + super( + arguments, ImmutableMap.<String, String>of(), executionInfo, SpawnAction.this.getRunfilesSupplier(), |