Modify --subcommands to allow pretty printing the arguments of a subcommand
as a list, rather than as a single line (i.e., newline delimited rather than
space delimited).
Before:
SUBCOMMAND: # //src/main/java/com/google/devtools/build/lib:string_util [action 'Building src/main/java/com/google/devtools/build/lib/libstring_util.jar (2 source files) [for host]']
(cd /tmp/devbazel_output_base/execroot/io_bazel && \
exec env - \
LC_CTYPE=en_US.UTF-8 \
external/embedded_jdk/bin/java -XX:+UseParallelOldGC -XX:-CompactStrings '--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED' '--add-exports=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED' '--add-exports=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED' '--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED' '--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED' '--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED' '--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED' '--add-opens=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED' '--patch-module=java.compiler=external/bazel_tools/third_party/java/jdk/langtools/java_compiler.jar' '--patch-module=jdk.compiler=external/bazel_tools/third_party/java/jdk/langtools/jdk_compiler.jar' '--add-opens=java.base/java.nio=ALL-UNNAMED' -jar external/bazel_tools/tools/jdk/JavaBuilder_deploy.jar @bazel-out/host/bin/src/main/java/com/google/devtools/build/lib/libstring_util.jar-2.params)
After:
SUBCOMMAND: # //src/main/java/com/google/devtools/build/lib:string_util [action 'Building src/main/java/com/google/devtools/build/lib/libstring_util.jar (2 source files) [for host]']
(cd /tmp/devbazel_output_base/execroot/io_bazel && \
exec env - \
LC_CTYPE=en_US.UTF-8 \
external/embedded_jdk/bin/java \
-XX:+UseParallelOldGC \
-XX:-CompactStrings \
'--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED' \
'--add-exports=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED' \
'--add-exports=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED' \
'--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED' \
'--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED' \
'--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED' \
'--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED' \
'--add-opens=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED' \
'--patch-module=java.compiler=external/bazel_tools/third_party/java/jdk/langtools/java_compiler.jar' \
'--patch-module=jdk.compiler=external/bazel_tools/third_party/java/jdk/langtools/jdk_compiler.jar' \
'--add-opens=java.base/java.nio=ALL-UNNAMED' \
-jar \
external/bazel_tools/tools/jdk/JavaBuilder_deploy.jar \
@bazel-out/host/bin/src/main/java/com/google/devtools/build/lib/libstring_util.jar-2.params)
RELNOTES: --subcommands can now take a "pretty_print" value ("--subcommands=pretty_print") to print the
arguments of subcommands as a list for easier reading.
PiperOrigin-RevId: 206213009
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java
index 2cea715..037f47b 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java
@@ -43,6 +43,19 @@
*/
public class ActionExecutionContext implements Closeable {
+ /** Enum for --subcommands flag */
+ public enum ShowSubcommands {
+ TRUE(true, false), PRETTY_PRINT(true, true), FALSE(false, false);
+
+ private final boolean shouldShowSubcommands;
+ private final boolean prettyPrintArgs;
+
+ private ShowSubcommands(boolean shouldShowSubcommands, boolean prettyPrintArgs) {
+ this.shouldShowSubcommands = shouldShowSubcommands;
+ this.prettyPrintArgs = prettyPrintArgs;
+ }
+ }
+
private final Executor executor;
private final MetadataProvider actionInputFileCache;
private final ActionInputPrefetcher actionInputPrefetcher;
@@ -236,19 +249,15 @@
}
/**
- * Whether this Executor reports subcommands. If not, reportSubcommand has no effect.
- * This is provided so the caller of reportSubcommand can avoid wastefully constructing the
- * subcommand string.
- */
- public boolean reportsSubcommands() {
- return executor.reportsSubcommands();
- }
-
- /**
* Report a subcommand event to this Executor's Reporter and, if action
* logging is enabled, post it on its EventBus.
*/
- public void reportSubcommand(Spawn spawn) {
+ public void maybeReportSubcommand(Spawn spawn) {
+ ShowSubcommands showSubcommands = executor.reportsSubcommands();
+ if (!showSubcommands.shouldShowSubcommands) {
+ return;
+ }
+
String reason;
ActionOwner owner = spawn.getResourceOwner().getOwner();
if (owner == null) {
@@ -257,7 +266,7 @@
reason = Label.print(owner.getLabel())
+ " [" + spawn.getResourceOwner().prettyPrint() + "]";
}
- String message = Spawns.asShellCommand(spawn, getExecRoot());
+ String message = Spawns.asShellCommand(spawn, getExecRoot(), showSubcommands.prettyPrintArgs);
getEventHandler().handle(Event.of(EventKind.SUBCOMMAND, null, "# " + reason + "\n" + message));
}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/Executor.java b/src/main/java/com/google/devtools/build/lib/actions/Executor.java
index a2de3d6..2c90014 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/Executor.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/Executor.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.actions;
import com.google.common.eventbus.EventBus;
+import com.google.devtools.build.lib.actions.ActionExecutionContext.ShowSubcommands;
import com.google.devtools.build.lib.clock.Clock;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.vfs.FileSystem;
@@ -74,7 +75,7 @@
* This is provided so the caller of reportSubcommand can avoid wastefully constructing the
* subcommand string.
*/
- boolean reportsSubcommands();
+ ShowSubcommands reportsSubcommands();
/**
* An event listener to report messages to. Errors that signal a action failure should use
diff --git a/src/main/java/com/google/devtools/build/lib/actions/Spawns.java b/src/main/java/com/google/devtools/build/lib/actions/Spawns.java
index 3174d5b..0ca66b7 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/Spawns.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/Spawns.java
@@ -89,17 +89,29 @@
}
/** Convert a spawn into a Bourne shell command. */
- public static String asShellCommand(Spawn spawn, Path workingDirectory) {
- return asShellCommand(spawn.getArguments(), workingDirectory, spawn.getEnvironment());
+ public static String asShellCommand(Spawn spawn, Path workingDirectory, boolean prettyPrintArgs) {
+ return asShellCommand(
+ spawn.getArguments(),
+ workingDirectory,
+ spawn.getEnvironment(),
+ prettyPrintArgs);
}
/** Convert a working dir + environment map + arg list into a Bourne shell command. */
public static String asShellCommand(
- Collection<String> arguments, Path workingDirectory, Map<String, String> environment) {
+ Collection<String> arguments,
+ Path workingDirectory,
+ Map<String, String> environment,
+ boolean prettyPrintArgs) {
+
// We print this command out in such a way that it can safely be
// copied+pasted as a Bourne shell command. This is extremely valuable for
// debugging.
return CommandFailureUtils.describeCommand(
- CommandDescriptionForm.COMPLETE, arguments, environment, workingDirectory.getPathString());
+ CommandDescriptionForm.COMPLETE,
+ prettyPrintArgs,
+ arguments,
+ environment,
+ workingDirectory.getPathString());
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java
index 0930675..cb40215 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java
@@ -70,9 +70,9 @@
ActionExecutionContext actionExecutionContext,
AtomicReference<Class<? extends SpawnActionContext>> writeOutputFiles)
throws ExecException, InterruptedException {
- if (actionExecutionContext.reportsSubcommands()) {
- actionExecutionContext.reportSubcommand(spawn);
- }
+
+ actionExecutionContext.maybeReportSubcommand(spawn);
+
final Duration timeout = Spawns.getTimeout(spawn);
SpawnExecutionContext context =
new SpawnExecutionContextImpl(spawn, actionExecutionContext, writeOutputFiles, timeout);
diff --git a/src/main/java/com/google/devtools/build/lib/exec/BlazeExecutor.java b/src/main/java/com/google/devtools/build/lib/exec/BlazeExecutor.java
index 5786a48..52b0b35 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/BlazeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/BlazeExecutor.java
@@ -16,6 +16,7 @@
import com.google.common.base.Preconditions;
import com.google.common.eventbus.EventBus;
import com.google.devtools.build.lib.actions.ActionContext;
+import com.google.devtools.build.lib.actions.ActionExecutionContext.ShowSubcommands;
import com.google.devtools.build.lib.actions.Executor;
import com.google.devtools.build.lib.actions.ExecutorInitException;
import com.google.devtools.build.lib.clock.Clock;
@@ -44,7 +45,7 @@
public final class BlazeExecutor implements Executor {
private final boolean verboseFailures;
- private final boolean showSubcommands;
+ private final ShowSubcommands showSubcommands;
private final FileSystem fileSystem;
private final Path execRoot;
private final Reporter reporter;
@@ -123,7 +124,7 @@
}
@Override
- public boolean reportsSubcommands() {
+ public ShowSubcommands reportsSubcommands() {
return showSubcommands;
}
diff --git a/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java b/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java
index 735832b..9536399 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java
@@ -14,11 +14,13 @@
package com.google.devtools.build.lib.exec;
import com.google.common.collect.Iterables;
+import com.google.devtools.build.lib.actions.ActionExecutionContext.ShowSubcommands;
import com.google.devtools.build.lib.actions.ResourceSet;
import com.google.devtools.build.lib.analysis.config.PerLabelOptions;
import com.google.devtools.build.lib.util.OptionsUtils;
import com.google.devtools.build.lib.util.RegexFilter;
import com.google.devtools.build.lib.vfs.PathFragment;
+import com.google.devtools.common.options.BoolOrEnumConverter;
import com.google.devtools.common.options.Option;
import com.google.devtools.common.options.OptionDocumentationCategory;
import com.google.devtools.common.options.OptionEffectTag;
@@ -71,11 +73,12 @@
name = "subcommands",
abbrev = 's',
defaultValue = "false",
+ converter = ShowSubcommandsConverter.class,
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
effectTags = {OptionEffectTag.UNKNOWN},
help = "Display the subcommands executed during a build."
)
- public boolean showSubcommands;
+ public ShowSubcommands showSubcommands;
@Option(
name = "check_up_to_date",
@@ -363,4 +366,13 @@
+ "This flag may be passed more than once";
}
}
+
+ /** Converter for --subcommands */
+ public static class ShowSubcommandsConverter extends BoolOrEnumConverter<ShowSubcommands> {
+ public ShowSubcommandsConverter() {
+ super(
+ ShowSubcommands.class, "subcommand option", ShowSubcommands.TRUE, ShowSubcommands.FALSE);
+ }
+ }
+
}
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 4a5849c..67a00b6 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
@@ -419,7 +419,10 @@
if (runOptions.scriptPath != null) {
String unisolatedCommand = CommandFailureUtils.describeCommand(
CommandDescriptionForm.COMPLETE_UNISOLATED,
- cmdLine, runEnvironment, workingDir.getPathString());
+ /* prettyPrintArgs= */ false,
+ cmdLine,
+ runEnvironment,
+ workingDir.getPathString());
if (writeScript(env, shExecutable, runOptions.scriptPath, unisolatedCommand)) {
return BlazeCommandResult.exitCode(ExitCode.SUCCESS);
} else {
diff --git a/src/main/java/com/google/devtools/build/lib/util/CommandFailureUtils.java b/src/main/java/com/google/devtools/build/lib/util/CommandFailureUtils.java
index dad0d72..5317e81 100644
--- a/src/main/java/com/google/devtools/build/lib/util/CommandFailureUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/util/CommandFailureUtils.java
@@ -139,17 +139,23 @@
* @param form Form of the command to generate; see the documentation of the
* {@link CommandDescriptionForm} values.
*/
- public static String describeCommand(CommandDescriptionForm form,
+ public static String describeCommand(
+ CommandDescriptionForm form,
+ boolean prettyPrintArgs,
Collection<String> commandLineElements,
- @Nullable Map<String, String> environment, @Nullable String cwd) {
+ @Nullable Map<String, String> environment,
+ @Nullable String cwd) {
+
Preconditions.checkNotNull(form);
final int APPROXIMATE_MAXIMUM_MESSAGE_LENGTH = 200;
StringBuilder message = new StringBuilder();
int size = commandLineElements.size();
int numberRemaining = size;
+
if (form == CommandDescriptionForm.COMPLETE) {
describeCommandImpl.describeCommandBeginIsolate(message);
}
+
if (form != CommandDescriptionForm.ABBREVIATED) {
if (cwd != null) {
describeCommandImpl.describeCommandCwd(cwd, message);
@@ -195,6 +201,7 @@
}
}
}
+
for (String commandElement : commandLineElements) {
if (form == CommandDescriptionForm.ABBREVIATED &&
message.length() + commandElement.length() > APPROXIMATE_MAXIMUM_MESSAGE_LENGTH) {
@@ -203,15 +210,17 @@
break;
} else {
if (numberRemaining < size) {
- message.append(' ');
+ message.append(prettyPrintArgs ? " \\\n " : " ");
}
describeCommandImpl.describeCommandElement(message, commandElement);
numberRemaining--;
}
}
+
if (form == CommandDescriptionForm.COMPLETE) {
describeCommandImpl.describeCommandEndIsolate(message);
}
+
return message.toString();
}
@@ -220,14 +229,17 @@
* Currently this returns a message of the form "error executing command foo
* bar baz".
*/
- public static String describeCommandError(boolean verbose,
- Collection<String> commandLineElements,
- Map<String, String> env, String cwd) {
+ public static String describeCommandError(
+ boolean verbose,
+ Collection<String> commandLineElements,
+ Map<String, String> env,
+ String cwd) {
+
CommandDescriptionForm form = verbose
? CommandDescriptionForm.COMPLETE
: CommandDescriptionForm.ABBREVIATED;
return "error executing command " + (verbose ? "\n " : "")
- + describeCommand(form, commandLineElements, env, cwd);
+ + describeCommand(form, /* prettyPrintArgs= */false, commandLineElements, env, cwd);
}
/**
@@ -235,9 +247,12 @@
* Currently this returns a message of the form "foo failed: error executing
* command /dir/foo bar baz".
*/
- public static String describeCommandFailure(boolean verbose,
- Collection<String> commandLineElements,
- Map<String, String> env, String cwd) {
+ public static String describeCommandFailure(
+ boolean verbose,
+ Collection<String> commandLineElements,
+ Map<String, String> env,
+ String cwd) {
+
String commandName = commandLineElements.iterator().next();
// Extract the part of the command name after the last "/", if any.
String shortCommandName = new File(commandName).getName();
diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerKey.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerKey.java
index 4211419..55b0910 100644
--- a/src/main/java/com/google/devtools/build/lib/worker/WorkerKey.java
+++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerKey.java
@@ -123,6 +123,6 @@
@Override
public String toString() {
- return Spawns.asShellCommand(args, execRoot, env);
+ return Spawns.asShellCommand(args, execRoot, env, /* prettyPrintArgs= */ false);
}
}
diff --git a/src/main/java/com/google/devtools/common/options/BoolOrEnumConverter.java b/src/main/java/com/google/devtools/common/options/BoolOrEnumConverter.java
index 3e16f89..17bf7a9 100644
--- a/src/main/java/com/google/devtools/common/options/BoolOrEnumConverter.java
+++ b/src/main/java/com/google/devtools/common/options/BoolOrEnumConverter.java
@@ -17,13 +17,16 @@
import com.google.devtools.common.options.Converters.BooleanConverter;
/**
- * Converter that can also convert from booleans and enumerations.
+ * Converter that can convert both the standard set of boolean string values and enumerations. If
+ * there is an overlap in values, those from the underlying enumeration will be taken.
*
- * <p> This is able to additionally convert from the standard set of
- * boolean string values. If there is an overlap in values, those from
- * the underlying enumeration will be taken.
+ * <p>Note that for the flag to take one of its enum values on the command line, it must be of the
+ * form "--flag=value". That is, "--flag value" and "-f value" (if the flag has a short-form of "f")
+ * will result in "value" being left as residue on the command line. This maintains compatibility
+ * with boolean flags where "--flag true" and "-f true" also leave "true" as residue on the command
+ * line.
*/
-public abstract class BoolOrEnumConverter<T extends Enum<T>> extends EnumConverter<T>{
+public abstract class BoolOrEnumConverter<T extends Enum<T>> extends EnumConverter<T> {
private T falseValue;
private T trueValue;
@@ -55,6 +58,8 @@
boolean value = booleanConverter.convert(input);
return value ? trueValue : falseValue;
} catch (OptionsParsingException eBoolean) {
+ // TODO(b/111883901): Rethrowing the exception from the enum converter does not report the
+ // allowable boolean values.
throw eEnum;
}
}
