Update Command to implement DescribableExecutionUnit.
PiperOrigin-RevId: 398330385
diff --git a/src/main/java/com/google/devtools/build/lib/shell/BUILD b/src/main/java/com/google/devtools/build/lib/shell/BUILD
index 985c866..9f27331 100644
--- a/src/main/java/com/google/devtools/build/lib/shell/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/shell/BUILD
@@ -17,6 +17,7 @@
name = "shell",
srcs = glob(["*.java"]),
deps = [
+ "//src/main/java/com/google/devtools/build/lib/util:describable_execution_unit",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/protobuf:execution_statistics_java_proto",
"//third_party:auto_value",
diff --git a/src/main/java/com/google/devtools/build/lib/shell/Command.java b/src/main/java/com/google/devtools/build/lib/shell/Command.java
index 83c3915..7ca600c 100644
--- a/src/main/java/com/google/devtools/build/lib/shell/Command.java
+++ b/src/main/java/com/google/devtools/build/lib/shell/Command.java
@@ -16,16 +16,17 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.common.flogger.GoogleLogger;
import com.google.common.flogger.LazyArgs;
import com.google.common.io.ByteStreams;
import com.google.devtools.build.lib.shell.Consumers.OutErrConsumers;
+import com.google.devtools.build.lib.util.DescribableExecutionUnit;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.time.Duration;
-import java.util.List;
import java.util.Map;
import javax.annotation.Nullable;
@@ -122,7 +123,7 @@
* lookup, output redirection, pipelines, etc) are needed, call it directly without using a shell,
* as in the {@code du(1)} example above.
*/
-public final class Command {
+public final class Command implements DescribableExecutionUnit {
private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();
@@ -206,13 +207,15 @@
}
/** Returns the raw command line elements to be executed */
- public String[] getCommandLineElements() {
- final List<String> elements = subprocessBuilder.getArgv();
- return elements.toArray(new String[elements.size()]);
+ @Override
+ public ImmutableList<String> getArguments() {
+ return subprocessBuilder.getArgv();
}
/** Returns an (unmodifiable) {@link Map} view of command's environment variables or null. */
- @Nullable public Map<String, String> getEnvironmentVariables() {
+ @Override
+ @Nullable
+ public ImmutableMap<String, String> getEnvironment() {
return subprocessBuilder.getEnv();
}
diff --git a/src/main/java/com/google/devtools/build/lib/util/CommandUtils.java b/src/main/java/com/google/devtools/build/lib/util/CommandUtils.java
index 7082cbc..3bec531 100644
--- a/src/main/java/com/google/devtools/build/lib/util/CommandUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/util/CommandUtils.java
@@ -19,9 +19,6 @@
import com.google.devtools.build.lib.shell.Command;
import com.google.devtools.build.lib.shell.CommandException;
import com.google.devtools.build.lib.shell.CommandResult;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.Map;
/**
* Utility methods relating to the {@link Command} class.
@@ -31,16 +28,6 @@
private CommandUtils() {} // Prevent instantiation.
@VisibleForTesting
- static Collection<String> commandLine(Command command) {
- return Arrays.asList(command.getCommandLineElements());
- }
-
- @VisibleForTesting
- static Map<String, String> env(Command command) {
- return command.getEnvironmentVariables();
- }
-
- @VisibleForTesting
static String cwd(Command command) {
return command.getWorkingDirectory() == null ? null : command.getWorkingDirectory().getPath();
}
@@ -48,22 +35,15 @@
/**
* Construct an error message that describes a failed command invocation.
* Currently this returns a message of the form "foo failed: error executing
- * command /dir/foo bar baz".
- */
- public static String describeCommandFailure(boolean verbose, Command command) {
- return CommandFailureUtils.describeCommandFailure(
- verbose, commandLine(command), env(command), cwd(command), null);
- }
-
- /**
- * Construct an error message that describes a failed command invocation.
- * Currently this returns a message of the form "foo failed: error executing
* command /dir/foo bar baz: exception message", with the
* command's stdout and stderr output appended if available.
*/
public static String describeCommandFailure(boolean verbose, CommandException exception) {
- String message = describeCommandFailure(verbose, exception.getCommand()) + ": "
- + exception.getMessage();
+ Command command = exception.getCommand();
+ String message =
+ CommandFailureUtils.describeCommandFailure(verbose, cwd(command), command)
+ + ": "
+ + exception.getMessage();
if (exception instanceof AbnormalTerminationException) {
CommandResult result = ((AbnormalTerminationException) exception).getResult();
try {
diff --git a/src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeHelperTest.java b/src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeHelperTest.java
index 8bb5fa8..a15756c 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeHelperTest.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeHelperTest.java
@@ -45,13 +45,13 @@
Command command =
new SymlinkTreeHelper(inputManifestPath, execRoot.getRelative("output/MANIFEST"), false)
.createCommand(execRoot, binTools, ImmutableMap.of());
- assertThat(command.getEnvironmentVariables()).isEmpty();
+ assertThat(command.getEnvironment()).isEmpty();
assertThat(command.getWorkingDirectory()).isEqualTo(execRoot.getPathFile());
- String[] commandLine = command.getCommandLineElements();
- assertThat(commandLine).hasLength(3);
- assertThat(commandLine[0]).endsWith(SymlinkTreeHelper.BUILD_RUNFILES);
- assertThat(commandLine[1]).isEqualTo("input_manifest");
- assertThat(commandLine[2]).isEqualTo("output/MANIFEST");
+ ImmutableList<String> commandLine = command.getArguments();
+ assertThat(commandLine).hasSize(3);
+ assertThat(commandLine.get(0)).endsWith(SymlinkTreeHelper.BUILD_RUNFILES);
+ assertThat(commandLine.get(1)).isEqualTo("input_manifest");
+ assertThat(commandLine.get(2)).isEqualTo("output/MANIFEST");
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/shell/CommandTest.java b/src/test/java/com/google/devtools/build/lib/shell/CommandTest.java
index 0543e07..7091291 100644
--- a/src/test/java/com/google/devtools/build/lib/shell/CommandTest.java
+++ b/src/test/java/com/google/devtools/build/lib/shell/CommandTest.java
@@ -15,7 +15,6 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
-import static com.google.devtools.build.lib.shell.TestUtil.assertArrayEquals;
import static org.junit.Assert.assertThrows;
import com.google.common.collect.ImmutableMap;
@@ -71,9 +70,9 @@
Map<String, String> env = Collections.singletonMap("foo", "bar");
String[] commandArgs = new String[] { "command" };
Command command = new Command(commandArgs, env, workingDir);
- assertArrayEquals(commandArgs, command.getCommandLineElements());
+ assertThat(command.getArguments()).containsExactlyElementsIn(commandArgs);
for (String key : env.keySet()) {
- assertThat(command.getEnvironmentVariables()).containsEntry(key, env.get(key));
+ assertThat(command.getEnvironment()).containsEntry(key, env.get(key));
}
assertThat(command.getWorkingDirectory()).isEqualTo(workingDir);
}
@@ -403,7 +402,7 @@
private static void checkCommandElements(CommandException e,
String... expected) {
- assertArrayEquals(expected, e.getCommand().getCommandLineElements());
+ assertThat(e.getCommand().getArguments()).containsExactlyElementsIn(expected);
}
private static void checkATE(final AbnormalTerminationException ate) {
@@ -423,7 +422,7 @@
Command command = new Command(new String[]{"relative/path/to/binary"},
ImmutableMap.<String, String>of(),
new File("/working/directory"));
- assertThat(command.getCommandLineElements()[0])
+ assertThat(command.getArguments().get(0))
.isEqualTo("/working/directory/relative/path/to/binary");
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/util/CommandBuilderTest.java b/src/test/java/com/google/devtools/build/lib/util/CommandBuilderTest.java
index 18293c8..a3f5b0b 100644
--- a/src/test/java/com/google/devtools/build/lib/util/CommandBuilderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/util/CommandBuilderTest.java
@@ -17,7 +17,6 @@
import static org.junit.Assert.assertThrows;
import com.google.common.collect.ImmutableList;
-import java.util.Arrays;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -37,10 +36,7 @@
}
private void assertArgv(CommandBuilder builder, String... expected) {
- assertThat(builder.build().getCommandLineElements())
- .asList()
- .containsExactlyElementsIn(Arrays.asList(expected))
- .inOrder();
+ assertThat(builder.build().getArguments()).containsExactlyElementsIn(expected).inOrder();
}
private void assertWinCmdArgv(CommandBuilder builder, String expected) {
diff --git a/src/test/java/com/google/devtools/build/lib/util/CommandUtilsTest.java b/src/test/java/com/google/devtools/build/lib/util/CommandUtilsTest.java
index 52ae6ec..8693b98 100644
--- a/src/test/java/com/google/devtools/build/lib/util/CommandUtilsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/util/CommandUtilsTest.java
@@ -28,8 +28,7 @@
@RunWith(JUnit4.class)
public class CommandUtilsTest {
- @Test
- public void longCommand() throws Exception {
+ private static Command buildLongCommand() {
String[] args = new String[40];
args[0] = "this_command_will_not_be_found";
for (int i = 1; i < args.length; i++) {
@@ -39,35 +38,33 @@
env.put("PATH", "/usr/bin:/bin:/sbin");
env.put("FOO", "foo");
File directory = new File("/tmp");
- CommandException exception =
- assertThrows(CommandException.class, () -> new Command(args, env, directory).execute());
- Command command1 = exception.getCommand();
+ return new Command(args, env, directory);
+ }
+
+ @Test
+ public void longCommand() throws Exception {
+ Command command = buildLongCommand();
String message =
- CommandFailureUtils.describeCommandError(
- false,
- CommandUtils.commandLine(command1),
- CommandUtils.env(command1),
- CommandUtils.cwd(command1),
- null);
- Command command = exception.getCommand();
- String verboseMessage =
- CommandFailureUtils.describeCommandError(
- true,
- CommandUtils.commandLine(command),
- CommandUtils.env(command),
- CommandUtils.cwd(command),
- null);
+ CommandFailureUtils.describeCommandFailure(false, CommandUtils.cwd(command), command);
assertThat(message)
.isEqualTo(
- "error executing command this_command_will_not_be_found arg1 "
+ "this_command_will_not_be_found failed: "
+ + "error executing command this_command_will_not_be_found arg1 "
+ "arg2 arg3 arg4 arg5 arg6 arg7 arg8 arg9 arg10 "
+ "arg11 arg12 arg13 arg14 arg15 arg16 arg17 arg18 "
+ "arg19 arg20 arg21 arg22 arg23 arg24 arg25 arg26 "
+ "arg27 arg28 arg29 arg30 "
+ "... (remaining 9 arguments skipped)");
+ }
+
+ @Test
+ public void longCommand_verbose() throws Exception {
+ Command command = buildLongCommand();
+ String verboseMessage =
+ CommandFailureUtils.describeCommandFailure(true, CommandUtils.cwd(command), command);
assertThat(verboseMessage)
.isEqualTo(
- "error executing command \n"
+ "this_command_will_not_be_found failed: error executing command \n"
+ " (cd /tmp && \\\n"
+ " exec env - \\\n"
+ " FOO=foo \\\n"
