Allow CommandLine expansion to throw an exception.
This paves the way for Skylark-side compact command lines that can fail during expansion.
In general, expansion happens in these scenarios:
* Action execution expands the command line to execute it. This is fine since we are well equipped already to handle failing actions.
* In the analysis phase we expand command lines to investigate whether we need a params file. This could be moved to the execution phase, which would have the benefit of getting params files out of the action graph and saving memory.
* Key computation expands the command line. This could be fixed by allowing command lines to compute their own keys (which wouldn't try to expand the command line). This could have the benefit of being more efficient.
* Extra actions expand the command line to construct the extra action proto. This could maybe be deferred to the execution phase (writing the extra action), which would also be more memory efficient.
For failed key computations, we return a singleton "error" key. This means multiple actions that will fail will map to the same key. These actions will necessarily fail if executed, so we should not need to worry about these ending up in the action cache. If we do manage to make the command line compute its own keys then this will become moot in the future.
RELNOTES: None
PiperOrigin-RevId: 166266385
diff --git a/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java b/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
index 9db7d38..e19fdcc 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
@@ -294,16 +294,20 @@
public abstract String getMnemonic();
/**
- * See the javadoc for {@link com.google.devtools.build.lib.actions.Action} and
- * {@link com.google.devtools.build.lib.actions.ActionExecutionMetadata#getKey()} for the contract
- * for {@link #computeKey()}.
+ * See the javadoc for {@link com.google.devtools.build.lib.actions.Action} and {@link
+ * com.google.devtools.build.lib.actions.ActionExecutionMetadata#getKey()} for the contract for
+ * {@link #computeKey()}.
*/
- protected abstract String computeKey();
+ protected abstract String computeKey() throws CommandLineExpansionException;
@Override
public final synchronized String getKey() {
if (cachedKey == null) {
- cachedKey = computeKey();
+ try {
+ cachedKey = computeKey();
+ } catch (CommandLineExpansionException e) {
+ cachedKey = KEY_ERROR;
+ }
}
return cachedKey;
}
@@ -483,7 +487,7 @@
}
@Override
- public ExtraActionInfo.Builder getExtraActionInfo() {
+ public ExtraActionInfo.Builder getExtraActionInfo() throws CommandLineExpansionException {
ActionOwner owner = getOwner();
ExtraActionInfo.Builder result =
ExtraActionInfo.newBuilder()
@@ -565,15 +569,17 @@
}
@SkylarkCallable(
- name = "argv",
- doc = "For actions created by <a href=\"actions.html#run\">ctx.actions.run()</a> "
- + "or <a href=\"actions.html#run_shell\">ctx.actions.run_shell()</a> an immutable "
- + "list of the arguments for the command line to be executed. Note that "
- + "for shell actions the first two arguments will be the shell path "
- + "and <code>\"-c\"</code>.",
- structField = true,
- allowReturnNones = true)
- public SkylarkList<String> getSkylarkArgv() {
+ name = "argv",
+ doc =
+ "For actions created by <a href=\"actions.html#run\">ctx.actions.run()</a> "
+ + "or <a href=\"actions.html#run_shell\">ctx.actions.run_shell()</a> an immutable "
+ + "list of the arguments for the command line to be executed. Note that "
+ + "for shell actions the first two arguments will be the shell path "
+ + "and <code>\"-c\"</code>.",
+ structField = true,
+ allowReturnNones = true
+ )
+ public SkylarkList<String> getSkylarkArgv() throws CommandLineExpansionException {
return null;
}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/Action.java b/src/main/java/com/google/devtools/build/lib/actions/Action.java
index e27b122..9faddfe 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/Action.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/Action.java
@@ -197,8 +197,8 @@
* Called by {@link com.google.devtools.build.lib.analysis.extra.ExtraAction} at execution time to
* extract information from this action into a protocol buffer to be used by extra_action rules.
*
- * <p>As this method is called from the ExtraAction, make sure it is ok to call this method from
- * a different thread than the one this action is executed on.
+ * <p>As this method is called from the ExtraAction, make sure it is ok to call this method from a
+ * different thread than the one this action is executed on.
*/
- ExtraActionInfo.Builder getExtraActionInfo();
+ ExtraActionInfo.Builder getExtraActionInfo() throws CommandLineExpansionException;
}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionMetadata.java b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionMetadata.java
index 6184954..7f4b569 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionMetadata.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionMetadata.java
@@ -24,6 +24,17 @@
* other than that all methods with side effects must belong to the former.
*/
public interface ActionExecutionMetadata extends ActionAnalysisMetadata {
+
+ /**
+ * Return this key to signify a failed key computation.
+ *
+ * <p>Actions that return this value should fail to execute.
+ *
+ * <p>Consumers must either gracefully handle multiple failed actions having the same key,
+ * (recommended), or check against this value explicitly.
+ */
+ String KEY_ERROR = "1ea50e01-0349-4552-80cf-76cf520e8592";
+
/**
* If this executable can supply verbose information, returns a string that can be used as a
* progress message while this executable is running. A return value of {@code null} indicates no
diff --git a/src/main/java/com/google/devtools/build/lib/actions/CommandAction.java b/src/main/java/com/google/devtools/build/lib/actions/CommandAction.java
index 9c2d31e..e8d5a93 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/CommandAction.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/CommandAction.java
@@ -24,7 +24,7 @@
public interface CommandAction extends Action, ExecutionInfoSpecifier {
/** Returns a list of command line arguments that implements this action. */
- List<String> getArguments();
+ List<String> getArguments() throws CommandLineExpansionException;
/**
* Returns a map of command line variables to their values that constitute the environment
diff --git a/src/main/java/com/google/devtools/build/lib/actions/CommandLineExpansionException.java b/src/main/java/com/google/devtools/build/lib/actions/CommandLineExpansionException.java
new file mode 100644
index 0000000..91eebee
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/actions/CommandLineExpansionException.java
@@ -0,0 +1,29 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.actions;
+
+/**
+ * Thrown by command line classes during expansion.
+ *
+ * <p>This exception should be thrown deterministically, i.e. the command line should fail to expand
+ * in exactly the same way every time at attempt is made. An example would be an illegal format
+ * string, or a failure in Skylark evaluation of a compact command line.
+ */
+public final class CommandLineExpansionException extends Exception {
+ /** @param userVisibleErrorMessage An error string that will be displayed to the user. */
+ public CommandLineExpansionException(String userVisibleErrorMessage) {
+ super(userVisibleErrorMessage);
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/PseudoAction.java b/src/main/java/com/google/devtools/build/lib/analysis/PseudoAction.java
index 37ba410..d37770f 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/PseudoAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/PseudoAction.java
@@ -19,6 +19,7 @@
import com.google.devtools.build.lib.actions.ActionExecutionException;
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.extra.ExtraActionInfo;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.util.Fingerprint;
@@ -71,7 +72,11 @@
@Override
public ExtraActionInfo.Builder getExtraActionInfo() {
- return super.getExtraActionInfo().setExtension(infoExtension, getInfo());
+ try {
+ return super.getExtraActionInfo().setExtension(infoExtension, getInfo());
+ } catch (CommandLineExpansionException e) {
+ throw new AssertionError("PsedoAction command line expansion cannot fail");
+ }
}
public static Artifact getDummyOutput(RuleContext ruleContext) {
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 7bc6705..1ca9df7 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.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 @@
}
};
- /**
- * 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 @@
* 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 @@
}
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 @@
}
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 @@
*/
@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 9803d05..01781d4 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.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 @@
/** 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 6889e44..0dc67da 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.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 @@
}
/**
- * 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 @@
}
@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 @@
}
@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 a1db0e0..ded0a8e 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.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 @@
@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 @@
/** 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 @@
*
* <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 @@
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 @@
// 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 @@
}
/**
- * 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 @@
* 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 @@
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 @@
}
@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 @@
* <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 @@
* <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(),
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/extra/ExtraActionInfoFileWriteAction.java b/src/main/java/com/google/devtools/build/lib/analysis/extra/ExtraActionInfoFileWriteAction.java
index 1357c10..e9f21ec 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/extra/ExtraActionInfoFileWriteAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/extra/ExtraActionInfoFileWriteAction.java
@@ -18,7 +18,9 @@
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.CommandLineExpansionException;
import com.google.devtools.build.lib.actions.ExecException;
+import com.google.devtools.build.lib.actions.UserExecException;
import com.google.devtools.build.lib.analysis.actions.AbstractFileWriteAction;
import com.google.devtools.build.lib.analysis.actions.ProtoDeterministicWriter;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
@@ -47,11 +49,15 @@
@Override
public DeterministicWriter newDeterministicWriter(ActionExecutionContext ctx)
throws IOException, InterruptedException, ExecException {
- return new ProtoDeterministicWriter(shadowedAction.getExtraActionInfo().build());
+ try {
+ return new ProtoDeterministicWriter(shadowedAction.getExtraActionInfo().build());
+ } catch (CommandLineExpansionException e) {
+ throw new UserExecException(e);
+ }
}
@Override
- protected String computeKey() {
+ protected String computeKey() throws CommandLineExpansionException {
Fingerprint f = new Fingerprint();
f.addString(UUID);
f.addString(shadowedAction.getKey());
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java
index bb57220..f7d2fc1 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java
@@ -26,6 +26,7 @@
import com.google.devtools.build.lib.actions.ActionInputHelper;
import com.google.devtools.build.lib.actions.ActionOwner;
import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.CommandLineExpansionException;
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.NotifyOnActionCacheHit;
import com.google.devtools.build.lib.actions.UserExecException;
@@ -226,7 +227,7 @@
}
@Override
- protected String computeKey() {
+ protected String computeKey() throws CommandLineExpansionException {
Fingerprint f = new Fingerprint();
f.addString(GUID);
f.addStrings(executionSettings.getArgs().arguments());
diff --git a/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java
index 04affbb..78a8786 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java
@@ -23,7 +23,9 @@
import com.google.common.io.Closeables;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.CommandLineExpansionException;
import com.google.devtools.build.lib.actions.ExecException;
+import com.google.devtools.build.lib.actions.UserExecException;
import com.google.devtools.build.lib.analysis.config.BinTools;
import com.google.devtools.build.lib.analysis.test.TestActionContext;
import com.google.devtools.build.lib.analysis.test.TestResult;
@@ -183,7 +185,11 @@
// Execute the test using the alias in the runfiles tree, as mandated by the Test Encyclopedia.
args.add(execSettings.getExecutable().getRootRelativePath().getCallablePathString());
- Iterables.addAll(args, execSettings.getArgs().arguments());
+ try {
+ Iterables.addAll(args, execSettings.getArgs().arguments());
+ } catch (CommandLineExpansionException e) {
+ throw new UserExecException(e);
+ }
return ImmutableList.copyOf(args);
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
index 2862ade..ed46336 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
@@ -32,6 +32,7 @@
import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
import com.google.devtools.build.lib.actions.ArtifactResolver;
import com.google.devtools.build.lib.actions.CommandAction;
+import com.google.devtools.build.lib.actions.CommandLineExpansionException;
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.ExecutionInfoSpecifier;
import com.google.devtools.build.lib.actions.ExecutionRequirements;
@@ -789,8 +790,11 @@
.build());
}
- return super.getExtraActionInfo()
- .setExtension(CppCompileInfo.cppCompileInfo, info.build());
+ try {
+ return super.getExtraActionInfo().setExtension(CppCompileInfo.cppCompileInfo, info.build());
+ } catch (CommandLineExpansionException e) {
+ throw new AssertionError("CppCompileAction command line expansion cannot fail.");
+ }
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java
index 5ceaf00..296befb 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java
@@ -28,6 +28,7 @@
import com.google.devtools.build.lib.actions.ActionOwner;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.CommandAction;
+import com.google.devtools.build.lib.actions.CommandLineExpansionException;
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.ExecutionInfoSpecifier;
import com.google.devtools.build.lib.actions.ExecutionRequirements;
@@ -427,8 +428,11 @@
Artifact.toExecPaths(getLinkCommandLine().getBuildInfoHeaderArtifacts()));
info.addAllLinkOpt(getLinkCommandLine().getRawLinkArgv());
- return super.getExtraActionInfo()
- .setExtension(CppLinkInfo.cppLinkInfo, info.build());
+ try {
+ return super.getExtraActionInfo().setExtension(CppLinkInfo.cppLinkInfo, info.build());
+ } catch (CommandLineExpansionException e) {
+ throw new AssertionError("CppLinkAction command line expansion cannot fail.");
+ }
}
@Override
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 d305317..b53d38b 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
@@ -22,6 +22,7 @@
import com.google.devtools.build.lib.actions.ActionExecutionException;
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.ResourceSet;
import com.google.devtools.build.lib.actions.RunfilesSupplier;
import com.google.devtools.build.lib.analysis.actions.CommandLine;
@@ -169,7 +170,11 @@
protected String computeKey() {
Fingerprint f = new Fingerprint();
f.addString(GUID);
- f.addStrings(getArguments());
+ try {
+ f.addStrings(getArguments());
+ } catch (CommandLineExpansionException e) {
+ throw new AssertionError("LtoBackendAction command line expansion cannot fail");
+ }
f.addString(getMnemonic());
f.addPaths(getRunfilesSupplier().getRunfilesDirs());
ImmutableList<Artifact> runfilesManifests = getRunfilesSupplier().getManifests();
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 65236c9..5190829 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
@@ -19,6 +19,7 @@
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.CommandLineExpansionException;
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.ResourceSet;
import com.google.devtools.build.lib.actions.RunfilesSupplier;
@@ -65,11 +66,15 @@
}
@Override
- protected void internalExecute(
- ActionExecutionContext actionExecutionContext) throws ExecException, InterruptedException {
+ protected void internalExecute(ActionExecutionContext actionExecutionContext)
+ throws ExecException, InterruptedException {
EventHandler reporter = actionExecutionContext.getEventHandler();
checkInputsForDirectories(reporter, actionExecutionContext.getMetadataHandler());
- super.internalExecute(actionExecutionContext);
+ try {
+ super.internalExecute(actionExecutionContext);
+ } catch (CommandLineExpansionException e) {
+ throw new AssertionError("GenRuleAction command line expansion cannot fail");
+ }
checkOutputsForDirectories(reporter);
}
}
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 567e66e..c0c8ae4 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
@@ -32,6 +32,7 @@
import com.google.devtools.build.lib.actions.ActionOwner;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ArtifactOwner;
+import com.google.devtools.build.lib.actions.CommandLineExpansionException;
import com.google.devtools.build.lib.actions.EmptyRunfilesSupplier;
import com.google.devtools.build.lib.actions.ParameterFile;
import com.google.devtools.build.lib.actions.ResourceSet;
@@ -320,28 +321,38 @@
}
/**
- * Constructs a command line that can be used to invoke the
- * JavaBuilder.
+ * Constructs a command line that can be used to invoke the JavaBuilder.
*
- * <p>Do not use this method, except for testing (and for the in-process
- * strategy).
+ * <p>Do not use this method, except for testing (and for the in-process strategy).
*/
@VisibleForTesting
public Iterable<String> buildCommandLine() {
- return javaCompileCommandLine.arguments();
+ try {
+ return javaCompileCommandLine.arguments();
+ } catch (CommandLineExpansionException e) {
+ throw new AssertionError("JavaCompileAction command line expansion cannot fail");
+ }
}
/** Returns the command and arguments for a java compile action. */
public List<String> getCommand() {
- return ImmutableList.copyOf(commandLine.arguments());
+ try {
+ return ImmutableList.copyOf(commandLine.arguments());
+ } catch (CommandLineExpansionException e) {
+ throw new AssertionError("JavaCompileAction command line expansion cannot fail");
+ }
}
@Override
public String toString() {
- StringBuilder result = new StringBuilder();
- result.append("JavaBuilder ");
- Joiner.on(' ').appendTo(result, commandLine.arguments());
- return result.toString();
+ try {
+ StringBuilder result = new StringBuilder();
+ result.append("JavaBuilder ");
+ Joiner.on(' ').appendTo(result, commandLine.arguments());
+ return result.toString();
+ } catch (CommandLineExpansionException e) {
+ return "Error expanding command line";
+ }
}
@Override
@@ -356,8 +367,11 @@
info.addAllProcessorpath(Artifact.toExecPaths(getProcessorpath()));
info.setOutputjar(getOutputJar().getExecPathString());
- return super.getExtraActionInfo()
- .setExtension(JavaCompileInfo.javaCompileInfo, info.build());
+ try {
+ return super.getExtraActionInfo().setExtension(JavaCompileInfo.javaCompileInfo, info.build());
+ } catch (CommandLineExpansionException e) {
+ throw new AssertionError("JavaCompileAction command line expansion cannot fail");
+ }
}
/**
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 6f9fe8e..16c8823 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
@@ -29,12 +29,14 @@
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.CommandLineExpansionException;
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.ParameterFile;
import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType;
import com.google.devtools.build.lib.actions.ResourceSet;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnActionContext;
+import com.google.devtools.build.lib.actions.UserExecException;
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.actions.CommandLine;
import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
@@ -121,11 +123,14 @@
@Override
protected String computeKey() {
- return new Fingerprint()
- .addString(GUID)
- .addString(super.computeKey())
- .addStrings(directCommandLine.arguments())
- .hexDigestAndReset();
+ Fingerprint f = new Fingerprint().addString(GUID);
+ try {
+ f.addString(super.computeKey());
+ f.addStrings(directCommandLine.arguments());
+ } catch (CommandLineExpansionException e) {
+ throw new AssertionError("JavaHeaderCompileAction command line expansion cannot fail");
+ }
+ return f.hexDigestAndReset();
}
@Override
@@ -137,7 +142,11 @@
} catch (ExecException e) {
// if the direct input spawn failed, try again with transitive inputs to produce better
// better messages
- context.exec(getSpawn(actionExecutionContext.getClientEnv()), actionExecutionContext);
+ try {
+ context.exec(getSpawn(actionExecutionContext.getClientEnv()), actionExecutionContext);
+ } catch (CommandLineExpansionException commandLineExpansionException) {
+ throw new UserExecException(commandLineExpansionException);
+ }
// The compilation should never fail with direct deps but succeed with transitive inputs
// unless it failed due to a strict deps error, in which case fall back to the transitive
// classpath may allow it to succeed (Strict Java Deps errors are reported by javac,
@@ -146,17 +155,21 @@
}
private final Spawn getDirectSpawn() {
- return new BaseSpawn(
- ImmutableList.copyOf(directCommandLine.arguments()),
- ImmutableMap.<String, String>of() /*environment*/,
- ImmutableMap.<String, String>of() /*executionInfo*/,
- this,
- LOCAL_RESOURCES) {
- @Override
- public Iterable<? extends ActionInput> getInputFiles() {
- return directInputs;
- }
- };
+ try {
+ return new BaseSpawn(
+ ImmutableList.copyOf(directCommandLine.arguments()),
+ ImmutableMap.<String, String>of() /*environment*/,
+ ImmutableMap.<String, String>of() /*executionInfo*/,
+ this,
+ LOCAL_RESOURCES) {
+ @Override
+ public Iterable<? extends ActionInput> getInputFiles() {
+ return directInputs;
+ }
+ };
+ } catch (CommandLineExpansionException e) {
+ throw new AssertionError("JavaHeaderCompileAction command line expansion cannot fail");
+ }
}
/** Builder class to construct Java header compilation actions. */
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/LegacyCompilationSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/LegacyCompilationSupport.java
index dc7326d..3b09fc9 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/LegacyCompilationSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/LegacyCompilationSupport.java
@@ -44,6 +44,7 @@
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
+import com.google.devtools.build.lib.actions.CommandLineExpansionException;
import com.google.devtools.build.lib.analysis.OutputGroupProvider;
import com.google.devtools.build.lib.analysis.PrerequisiteArtifacts;
import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
@@ -590,9 +591,17 @@
}
private StrippingType getStrippingType(CommandLine commandLine) {
- return Iterables.contains(commandLine.arguments(), "-dynamiclib")
- ? StrippingType.DYNAMIC_LIB
- : StrippingType.DEFAULT;
+ try {
+ return Iterables.contains(commandLine.arguments(), "-dynamiclib")
+ ? StrippingType.DYNAMIC_LIB
+ : StrippingType.DEFAULT;
+ } catch (CommandLineExpansionException e) {
+ // TODO(b/64941219): This code should be rewritten to not expand the command line
+ // in the analysis phase
+ // This can't actually happen, because the command lines used by this class do
+ // not throw.
+ throw new AssertionError("Cannot fail to expand command line but did.", e);
+ }
}
private void registerLinkAction(
@@ -789,7 +798,7 @@
}
@Override
- public Iterable<String> arguments() {
+ public Iterable<String> arguments() throws CommandLineExpansionException {
return ImmutableList.of(Joiner.on(' ').join(original.arguments()));
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java
index dc8e89d..bfa06e2 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java
@@ -29,6 +29,7 @@
import com.google.devtools.build.lib.actions.ActionOwner;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ArtifactResolver;
+import com.google.devtools.build.lib.actions.CommandLineExpansionException;
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.ResourceSet;
import com.google.devtools.build.lib.actions.RunfilesSupplier;
@@ -78,8 +79,8 @@
*/
public class ObjcCompileActionSpawn extends ActionSpawn {
- public ObjcCompileActionSpawn(Map<String, String> clientEnv) {
- super(clientEnv);
+ public ObjcCompileActionSpawn(ImmutableList<String> arguments, Map<String, String> clientEnv) {
+ super(arguments, clientEnv);
}
@Override
@@ -172,7 +173,12 @@
@Override
public final Spawn getSpawn(Map<String, String> clientEnv) {
- return new ObjcCompileActionSpawn(clientEnv);
+ try {
+ return new ObjcCompileActionSpawn(
+ ImmutableList.copyOf(getCommandLine().arguments()), clientEnv);
+ } catch (CommandLineExpansionException e) {
+ throw new AssertionError("ObjcCompileAction command line expansion cannot fail");
+ }
}
@Override
@@ -296,7 +302,12 @@
@Override
protected SpawnInfo getExtraActionSpawnInfo() {
- SpawnInfo.Builder info = SpawnInfo.newBuilder(super.getExtraActionSpawnInfo());
+ SpawnInfo.Builder info = null;
+ try {
+ info = SpawnInfo.newBuilder(super.getExtraActionSpawnInfo());
+ } catch (CommandLineExpansionException e) {
+ throw new AssertionError("ObjcCompileAction command line expansion cannot fail");
+ }
if (!inputsDiscovered()) {
for (Artifact headerArtifact : filterHeaderFiles()) {
// As in SpawnAction#getExtraActionSpawnInfo explicitly ignore middleman artifacts here.
@@ -312,7 +323,11 @@
public String computeKey() {
Fingerprint f = new Fingerprint();
f.addString(GUID);
- f.addString(super.computeKey());
+ try {
+ f.addString(super.computeKey());
+ } catch (CommandLineExpansionException e) {
+ throw new AssertionError("ObjcCompileAction command line expansion cannot fail");
+ }
f.addBoolean(dotdFile == null || dotdFile.artifact() == null);
f.addBoolean(dotdPruningPlan == HeaderDiscovery.DotdPruningMode.USE);
f.addBoolean(headersListFile == null);
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/PrintActionCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/PrintActionCommand.java
index 5745706..184c48d 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/PrintActionCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/PrintActionCommand.java
@@ -20,6 +20,7 @@
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata.MiddlemanType;
import com.google.devtools.build.lib.actions.ActionGraph;
import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.CommandLineExpansionException;
import com.google.devtools.build.lib.actions.extra.DetailedExtraActionInfo;
import com.google.devtools.build.lib.actions.extra.ExtraActionSummary;
import com.google.devtools.build.lib.analysis.BuildView;
@@ -177,10 +178,15 @@
outputGroupProvider.getOutputGroup(OutputGroupProvider.FILES_TO_COMPILE);
}
if (!filesToCompile.isEmpty()) {
- if (compileOneDependency) {
- gatherActionsForFiles(configuredTarget, actionGraph, targets);
- } else {
- gatherActionsForTarget(configuredTarget, actionGraph);
+ try {
+ if (compileOneDependency) {
+ gatherActionsForFiles(configuredTarget, actionGraph, targets);
+ } else {
+ gatherActionsForTarget(configuredTarget, actionGraph);
+ }
+ } catch (CommandLineExpansionException e) {
+ env.getReporter().handle(Event.error(null, "Error expanding command line: " + e));
+ return null;
}
} else {
// TODO(rbraunstein): If a source is a member of a genrule and a cc_library don't
@@ -194,7 +200,8 @@
}
private BuildResult gatherActionsForFiles(
- ConfiguredTarget configuredTarget, ActionGraph actionGraph, List<String> files) {
+ ConfiguredTarget configuredTarget, ActionGraph actionGraph, List<String> files)
+ throws CommandLineExpansionException {
Set<String> filesDesired = new LinkedHashSet<>(files);
ActionFilter filter = new DefaultActionFilter(filesDesired, actionMnemonicMatcher);
@@ -202,8 +209,8 @@
return null;
}
- private void gatherActionsForTarget(ConfiguredTarget configuredTarget,
- ActionGraph actionGraph) {
+ private void gatherActionsForTarget(ConfiguredTarget configuredTarget, ActionGraph actionGraph)
+ throws CommandLineExpansionException {
if (!(configuredTarget.getTarget() instanceof Rule)) {
return;
}
@@ -227,12 +234,13 @@
}
}
- /**
+ /**
* Looks for files to compile in the given configured target and outputs the corresponding
* extra_action if the filter evaluates to {@code true}.
*/
- private void gatherActionsForFile(ConfiguredTarget configuredTarget, ActionFilter filter,
- ActionGraph actionGraph) {
+ private void gatherActionsForFile(
+ ConfiguredTarget configuredTarget, ActionFilter filter, ActionGraph actionGraph)
+ throws CommandLineExpansionException {
NestedSet<Artifact> artifacts = OutputGroupProvider.get(configuredTarget)
.getOutputGroup(OutputGroupProvider.FILES_TO_COMPILE);
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java
index 57b8f1d..c48ad81 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java
@@ -19,6 +19,7 @@
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.CommandLineExpansionException;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.FilesToRunProvider;
import com.google.devtools.build.lib.analysis.RunfilesSupport;
@@ -237,7 +238,12 @@
RunfilesSupport runfilesSupport = provider == null ? null : provider.getRunfilesSupport();
if (runfilesSupport != null && runfilesSupport.getArgs() != null) {
CommandLine targetArgs = runfilesSupport.getArgs();
- Iterables.addAll(args, targetArgs.arguments());
+ try {
+ Iterables.addAll(args, targetArgs.arguments());
+ } catch (CommandLineExpansionException e) {
+ env.getReporter().handle(Event.error("Could not expand target command line: " + e));
+ return ExitCode.ANALYSIS_FAILURE;
+ }
}
args.addAll(runTargetArgs);
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
index c00b06e..79f0d8c 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
@@ -36,6 +36,7 @@
import com.google.devtools.build.lib.actions.ActionLookupValue;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ArtifactOwner;
+import com.google.devtools.build.lib.actions.CommandLineExpansionException;
import com.google.devtools.build.lib.actions.MapBasedActionGraph;
import com.google.devtools.build.lib.actions.MiddlemanFactory;
import com.google.devtools.build.lib.actions.MutableActionGraph;
@@ -691,7 +692,8 @@
return (SpawnAction) getGeneratingAction(artifact);
}
- protected final List<String> getGeneratingSpawnActionArgs(Artifact artifact) {
+ protected final List<String> getGeneratingSpawnActionArgs(Artifact artifact)
+ throws CommandLineExpansionException {
SpawnAction a = getGeneratingSpawnAction(artifact);
ParameterFileWriteAction p = findParamsFileAction(a);
return p == null
diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBuildViewTestCase.java
index 978fb93..c38263e 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBuildViewTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBuildViewTestCase.java
@@ -25,6 +25,7 @@
import com.google.common.collect.Sets;
import com.google.devtools.build.lib.actions.Action;
import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.CommandLineExpansionException;
import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.FileProvider;
@@ -122,7 +123,8 @@
.isNotNull();
}
- protected List<String> resourceArguments(ResourceContainer resource) {
+ protected List<String> resourceArguments(ResourceContainer resource)
+ throws CommandLineExpansionException {
return getGeneratingSpawnActionArgs(resource.getApk());
}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidDataBindingTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidDataBindingTest.java
index cac7c2a..5dd1e92 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidDataBindingTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidDataBindingTest.java
@@ -18,6 +18,7 @@
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.CommandLineExpansionException;
import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.actions.ParameterFileWriteAction;
@@ -91,10 +92,9 @@
"package android.binary; public class MyApp {};");
}
- /**
- * Returns the .params file contents of a {@link JavaCompileAction}
- */
- private Iterable<String> getParamFileContents(JavaCompileAction action) {
+ /** Returns the .params file contents of a {@link JavaCompileAction} */
+ private Iterable<String> getParamFileContents(JavaCompileAction action)
+ throws CommandLineExpansionException {
Artifact paramFile = getFirstArtifactEndingWith(action.getInputs(), "-2.params");
return ((ParameterFileWriteAction) getGeneratingAction(paramFile)).getContents();
}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/IosExtensionTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/IosExtensionTest.java
index af7f0f1..7f9f9d6 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/objc/IosExtensionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/objc/IosExtensionTest.java
@@ -28,6 +28,7 @@
import com.google.devtools.build.lib.actions.Action;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.CommandAction;
+import com.google.devtools.build.lib.actions.CommandLineExpansionException;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.rules.apple.AppleConfiguration.ConfigurationDistinguisher;
@@ -496,7 +497,8 @@
}
}
- private String generatingArgumentsToString(CommandAction generatingAction) {
+ private String generatingArgumentsToString(CommandAction generatingAction)
+ throws CommandLineExpansionException {
return Joiner.on(' ').join(generatingAction.getArguments());
}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java
index bbe7aef..e0d84bc 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java
@@ -541,8 +541,8 @@
* @param objlistName the path suffix of the filelist artifact
* @param inputArchives path suffixes of the expected contents of the filelist
*/
- protected void verifyObjlist(Action originalAction, String objlistName,
- String... inputArchives) {
+ protected void verifyObjlist(Action originalAction, String objlistName, String... inputArchives)
+ throws Exception {
Artifact filelistArtifact =
getFirstArtifactEndingWith(originalAction.getInputs(), objlistName);
@@ -567,9 +567,14 @@
* @param importedFrameworks custom framework path fragments
* @param extraLinkArgs extra link arguments expected on the link action
*/
- protected void verifyLinkAction(Artifact binArtifact, Artifact filelistArtifact, String arch,
- List<String> inputArchives, List<PathFragment> importedFrameworks,
- ExtraLinkArgs extraLinkArgs) {
+ protected void verifyLinkAction(
+ Artifact binArtifact,
+ Artifact filelistArtifact,
+ String arch,
+ List<String> inputArchives,
+ List<PathFragment> importedFrameworks,
+ ExtraLinkArgs extraLinkArgs)
+ throws Exception {
final CommandAction binAction = (CommandAction) getGeneratingAction(binArtifact);
for (String inputArchive : inputArchives) {
@@ -1489,7 +1494,7 @@
.doesNotContain(protoHeaderB);
}
- private void assertCoptsAndDefinesForBundlingTarget(ConfiguredTarget topTarget) {
+ private void assertCoptsAndDefinesForBundlingTarget(ConfiguredTarget topTarget) throws Exception {
Artifact protoObject =
getBinArtifact("_objs/x/x/_generated_protos/x/protos/DataA.pbobjc.o", topTarget);
CommandAction protoObjectAction = (CommandAction) getGeneratingAction(protoObject);
@@ -2141,9 +2146,8 @@
}
private void checkRegistersStoryboardCompileActions(
- ConfiguredTarget target,
- DottedVersion minimumOsVersion,
- ImmutableList<String> targetDevices) {
+ ConfiguredTarget target, DottedVersion minimumOsVersion, ImmutableList<String> targetDevices)
+ throws Exception {
Artifact storyboardZip = getBinArtifact("x/1.storyboard.zip", target);
CommandAction compileAction = (CommandAction) getGeneratingAction(storyboardZip);
assertThat(compileAction.getInputs()).containsExactly(
