CommandBuilder: remove useShell and setWorkingDir

Remove the .useShell method, expect callers to
just pass the shell interpreter if they need it.
This removes the argument vector transformation
heuristic, and stops shelling out to cmd.exe on
Windows.

Also remove the .setWorkingDir method because
callers always had to set the working directory.
Instead, the CommandBuilder constructor takes the
working directory.

Closes #2190. (Test code still shells out to
cmd.exe though.)

Closes #5007.

Change-Id: I545e01c811daaf34913cb585492923da81aa02ee
PiperOrigin-RevId: 192611370

1 files changed, 12 insertions, 48 deletions

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 99ee49d027..9eddb03d00 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,6 +17,7 @@ import static com.google.common.truth.Truth.assertThat;
 import static org.junit.Assert.fail;
 
 import com.google.common.collect.ImmutableList;
+import java.io.File;
 import java.util.Arrays;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -28,12 +29,8 @@ import org.junit.runners.JUnit4;
 @RunWith(JUnit4.class)
 public class CommandBuilderTest {
 
-  private CommandBuilder linuxBuilder() {
-    return new CommandBuilder(OS.LINUX).useTempDir();
-  }
-
-  private CommandBuilder winBuilder() {
-    return new CommandBuilder(OS.WINDOWS).useTempDir();
+  private CommandBuilder builder() {
+    return new CommandBuilder(new File("dummy-workdir"));
   }
 
   private void assertArgv(CommandBuilder builder, String... expected) {
@@ -43,10 +40,6 @@ public class CommandBuilderTest {
         .inOrder();
   }
 
-  private void assertWinCmdArgv(CommandBuilder builder, String expected) {
-    assertArgv(builder, "CMD.EXE", "/S", "/E:ON", "/V:ON", "/D", "/C", expected);
-  }
-
   private void assertFailure(CommandBuilder builder, String expected) {
     try {
       builder.build();
@@ -57,47 +50,18 @@ public class CommandBuilderTest {
   }
 
   @Test
-  public void linuxBuilderTest() {
-    assertArgv(linuxBuilder().addArg("abc"), "abc");
-    assertArgv(linuxBuilder().addArg("abc def"), "abc def");
-    assertArgv(linuxBuilder().addArgs("abc", "def"), "abc", "def");
-    assertArgv(linuxBuilder().addArgs(ImmutableList.of("abc", "def")), "abc", "def");
-    assertArgv(linuxBuilder().addArg("abc").useShell(true), "/bin/sh", "-c", "abc");
-    assertArgv(linuxBuilder().addArg("abc def").useShell(true), "/bin/sh", "-c", "abc def");
-    assertArgv(linuxBuilder().addArgs("abc", "def").useShell(true), "/bin/sh", "-c", "abc def");
-    assertArgv(linuxBuilder().addArgs("/bin/sh", "-c", "abc").useShell(true),
-        "/bin/sh", "-c", "abc");
-    assertArgv(linuxBuilder().addArgs("/bin/sh", "-c"), "/bin/sh", "-c");
-    assertArgv(linuxBuilder().addArgs("/bin/bash", "-c"), "/bin/bash", "-c");
-    assertArgv(linuxBuilder().addArgs("/bin/sh", "-c").useShell(true), "/bin/sh", "-c");
-    assertArgv(linuxBuilder().addArgs("/bin/bash", "-c").useShell(true), "/bin/bash", "-c");
-  }
-
-  @Test
-  public void windowsBuilderTest() {
-    assertArgv(winBuilder().addArg("abc.exe"), "abc.exe");
-    assertArgv(winBuilder().addArg("abc.exe -o"), "abc.exe -o");
-    assertArgv(winBuilder().addArg("ABC.EXE"), "ABC.EXE");
-    assertWinCmdArgv(winBuilder().addArg("abc def.exe"), "abc def.exe");
-    assertArgv(winBuilder().addArgs("abc.exe", "def"), "abc.exe", "def");
-    assertArgv(winBuilder().addArgs(ImmutableList.of("abc.exe", "def")), "abc.exe", "def");
-    assertWinCmdArgv(winBuilder().addArgs("abc.exe", "def").useShell(true), "abc.exe def");
-    assertWinCmdArgv(winBuilder().addArg("abc"), "abc");
-    assertWinCmdArgv(winBuilder().addArgs("abc", "def"), "abc def");
-    assertWinCmdArgv(winBuilder().addArgs("/bin/sh", "-c", "abc", "def"), "abc def");
-    assertWinCmdArgv(winBuilder().addArgs("/bin/sh", "-c"), "");
-    assertWinCmdArgv(winBuilder().addArgs("/bin/bash", "-c"), "");
-    assertWinCmdArgv(winBuilder().addArgs("/bin/sh", "-c").useShell(true), "");
-    assertWinCmdArgv(winBuilder().addArgs("/bin/bash", "-c").useShell(true), "");
+  public void builderTest() {
+    assertArgv(builder().addArg("abc"), "abc");
+    assertArgv(builder().addArg("abc def"), "abc def");
+    assertArgv(builder().addArgs("abc", "def"), "abc", "def");
+    assertArgv(builder().addArgs(ImmutableList.of("abc", "def")), "abc", "def");
+    assertArgv(builder().addArgs("/bin/sh", "-c", "abc"), "/bin/sh", "-c", "abc");
+    assertArgv(builder().addArgs("/bin/sh", "-c"), "/bin/sh", "-c");
+    assertArgv(builder().addArgs("/bin/bash", "-c"), "/bin/bash", "-c");
   }
 
   @Test
   public void failureScenarios() {
-    assertFailure(linuxBuilder(), "At least one argument is expected");
-    assertFailure(new CommandBuilder(OS.UNKNOWN).useTempDir().addArg("a"),
-        "Unidentified operating system");
-    assertFailure(new CommandBuilder(OS.LINUX).addArg("a"),
-        "Working directory must be set");
+    assertFailure(builder(), "At least one argument is expected");
   }
-
 }