aboutsummaryrefslogtreecommitdiffhomepage
path: root/src/main/java/com/google/devtools
diff options
context:
space:
mode:
authorGravatar tomlu <tomlu@google.com>2017-08-24 00:44:45 +0200
committerGravatar Damien Martin-Guillerez <dmarting@google.com>2017-08-24 14:00:00 +0200
commit3fb6ac385df80a5f583072c0b84247d503e330b7 (patch)
treef708d2e1132a71236307bb1bce3f2179b7701bad /src/main/java/com/google/devtools
parent4e08dfcf563f1ef036a1561bd5312ad469077049 (diff)
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
Diffstat (limited to 'src/main/java/com/google/devtools')
-rw-r--r--src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java36
-rw-r--r--src/main/java/com/google/devtools/build/lib/actions/Action.java6
-rw-r--r--src/main/java/com/google/devtools/build/lib/actions/ActionExecutionMetadata.java11
-rw-r--r--src/main/java/com/google/devtools/build/lib/actions/CommandAction.java2
-rw-r--r--src/main/java/com/google/devtools/build/lib/actions/CommandLineExpansionException.java29
-rw-r--r--src/main/java/com/google/devtools/build/lib/analysis/PseudoAction.java7
-rw-r--r--src/main/java/com/google/devtools/build/lib/analysis/actions/CommandLine.java26
-rw-r--r--src/main/java/com/google/devtools/build/lib/analysis/actions/ParamFileHelper.java12
-rw-r--r--src/main/java/com/google/devtools/build/lib/analysis/actions/ParameterFileWriteAction.java43
-rw-r--r--src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java74
-rw-r--r--src/main/java/com/google/devtools/build/lib/analysis/extra/ExtraActionInfoFileWriteAction.java10
-rw-r--r--src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java3
-rw-r--r--src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java8
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java8
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java8
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendAction.java7
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleAction.java11
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java38
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java47
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/objc/LegacyCompilationSupport.java17
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java25
-rw-r--r--src/main/java/com/google/devtools/build/lib/runtime/commands/PrintActionCommand.java28
-rw-r--r--src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java8
23 files changed, 326 insertions, 138 deletions
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 9db7d38f2e..e19fdcc4aa 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 class AbstractAction implements Action, SkylarkValue {
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 @@ public abstract class AbstractAction implements Action, SkylarkValue {
}
@Override
- public ExtraActionInfo.Builder getExtraActionInfo() {
+ public ExtraActionInfo.Builder getExtraActionInfo() throws CommandLineExpansionException {
ActionOwner owner = getOwner();
ExtraActionInfo.Builder result =
ExtraActionInfo.newBuilder()
@@ -565,15 +569,17 @@ public abstract class AbstractAction implements Action, SkylarkValue {
}
@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 e27b122cb8..9faddfea0c 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 @@ public interface Action extends ActionExecutionMetadata, Describable {
* 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 6184954699..7f4b569b13 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 @@ import javax.annotation.Nullable;
* 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 9c2d31ec86..e8d5a93d1c 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 @@ import java.util.List;
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 0000000000..91eebee4da
--- /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 37ba410a96..d37770fd77 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.ActionExecutionContext;
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 @@ public class PseudoAction<InfoType extends MessageLite> extends AbstractAction {
@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 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(),
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 1357c106ac..e9f21ec651 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.Action;
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 @@ public final class ExtraActionInfoFileWriteAction extends AbstractFileWriteActio
@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 bb572202cd..f7d2fc184e 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.ActionInput;
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 @@ public class TestRunnerAction extends AbstractAction implements NotifyOnActionCa
}
@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 04affbbd02..78a878657f 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.ByteStreams;
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 @@ public abstract class TestStrategy implements TestActionContext {
// 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 2862ade4e6..ed46336d53 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;
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 @@ public class CppCompileAction extends AbstractAction
.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 5ceaf00cf0..296befba8a 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.ActionExecutionException;
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 @@ public final class CppLinkAction extends AbstractAction
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 d305317c6b..b53d38bbfd 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.ActionExecutionContext;
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 @@ public final class LtoBackendAction extends SpawnAction {
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 65236c93ee..5190829987 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.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.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 @@ public class GenRuleAction extends SpawnAction {
}
@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 567e66e438..c0c8ae4e80 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.ActionEnvironment;
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 @@ public final class JavaCompileAction extends SpawnAction {
}
/**
- * 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 @@ public final class JavaCompileAction extends SpawnAction {
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 6f9fe8e11f..16c8823f62 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.ActionInput;
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 @@ public class JavaHeaderCompileAction extends SpawnAction {
@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 @@ public class JavaHeaderCompileAction extends SpawnAction {
} 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 @@ public class JavaHeaderCompileAction extends SpawnAction {
}
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 dc7326d56e..3b09fc991f 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.ImmutableSet;
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 @@ public class LegacyCompilationSupport extends CompilationSupport {
}
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 @@ public class LegacyCompilationSupport extends CompilationSupport {
}
@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 dc8e89d56b..bfa06e21f5 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.ActionInput;
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 ObjcCompileAction extends SpawnAction {
*/
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 @@ public class ObjcCompileAction extends SpawnAction {
@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 @@ public class ObjcCompileAction extends SpawnAction {
@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 class ObjcCompileAction extends SpawnAction {
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 57457062b6..184c48d8f1 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;
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 @@ public final class PrintActionCommand implements BlazeCommand {
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 @@ public final class PrintActionCommand implements BlazeCommand {
}
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 @@ public final class PrintActionCommand implements BlazeCommand {
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 @@ public final class PrintActionCommand implements BlazeCommand {
}
}
- /**
+ /**
* 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 57b8f1dc20..c48ad81b3d 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.ImmutableList;
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 @@ public class RunCommand implements BlazeCommand {
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);