Rollback https://github.com/bazelbuild/bazel/pull/5007

It breaks downstream rules_nodejs. See https://github.com/bazelbuild/bazel/issues/5028 for details.

RELNOTES: None.
PiperOrigin-RevId: 193074798

6 files changed, 135 insertions, 29 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelWorkspaceStatusModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelWorkspaceStatusModule.java
index ef7ecdf4cb..2434bff695 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/BazelWorkspaceStatusModule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelWorkspaceStatusModule.java
@@ -109,12 +109,14 @@ public class BazelWorkspaceStatusModule extends BlazeModule {
       this.getWorkspaceStatusCommand =
           options.workspaceStatusCommand.equals(PathFragment.EMPTY_FRAGMENT)
               ? null
-              : new CommandBuilder(workspace)
-                  .addArg(options.workspaceStatusCommand.toString())
+              : new CommandBuilder()
+                  .addArgs(options.workspaceStatusCommand.toString())
                   // Pass client env, because certain SCM client(like
                   // perforce, git) relies on environment variables to work
                   // correctly.
                   .setEnv(clientEnv)
+                  .setWorkingDir(workspace)
+                  .useShell(true)
                   .build();
       this.workspace = workspace;
     }
diff --git a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java
index 2eaaf499c5..5f231e4fd0 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java
@@ -17,6 +17,7 @@ import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Lists;
 import com.google.devtools.build.lib.actions.ActionExecutionContext;
 import com.google.devtools.build.lib.actions.ActionExecutionMetadata;
 import com.google.devtools.build.lib.actions.ActionInput;
@@ -36,7 +37,6 @@ import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.io.IOException;
-import java.util.ArrayList;
 import java.util.List;
 
 /**
@@ -90,7 +90,10 @@ public final class SymlinkTreeHelper {
       Path execRoot, BuildConfiguration config, BinTools binTools)
           throws CommandException {
     List<String> argv = getSpawnArgumentList(execRoot, binTools.getExecPath(BUILD_RUNFILES));
-    new CommandBuilder(execRoot).addArgs(argv).build().execute();
+    CommandBuilder builder = new CommandBuilder();
+    builder.addArgs(argv);
+    builder.setWorkingDir(execRoot);
+    builder.build().execute();
   }
 
   /**
@@ -145,7 +148,7 @@ public final class SymlinkTreeHelper {
     ActionInput buildRunfiles = binTools.getActionInput(BUILD_RUNFILES);
     return new SimpleSpawn(
         owner,
-        ImmutableList.copyOf(getSpawnArgumentList(execRoot, buildRunfiles.getExecPath())),
+        getSpawnArgumentList(execRoot, buildRunfiles.getExecPath()),
         environment,
         ImmutableMap.of(
             ExecutionRequirements.LOCAL, "",
@@ -156,9 +159,11 @@ public final class SymlinkTreeHelper {
         RESOURCE_SET);
   }
 
-  /** Returns the complete argument list build-runfiles has to be called with. */
-  private ArrayList<String> getSpawnArgumentList(Path execRoot, PathFragment buildRunfiles) {
-    ArrayList<String> args = new ArrayList<>(filesetTree ? 5 : 3);
+  /**
+   * Returns the complete argument list build-runfiles has to be called with.
+   */
+  private ImmutableList<String> getSpawnArgumentList(Path execRoot, PathFragment buildRunfiles) {
+    List<String> args = Lists.newArrayList();
     args.add(buildRunfiles.getPathString());
 
     if (filesetTree) {
@@ -169,6 +174,6 @@ public final class SymlinkTreeHelper {
     args.add(inputManifest.relativeTo(execRoot).getPathString());
     args.add(symlinkTreeRoot.relativeTo(execRoot).getPathString());
 
-    return args;
+    return ImmutableList.copyOf(args);
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/CleanCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/CleanCommand.java
index c9c91f3f48..d848d47469 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/CleanCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/CleanCommand.java
@@ -194,10 +194,10 @@ public final class CleanCommand implements BlazeCommand {
     logger.info("Executing shell commmand " + ShellEscaper.escapeString(command));
 
     // Doesn't throw iff command exited and was successful.
-    new CommandBuilder(tempPath.getParentDirectory())
-        // TODO(laszlocsomor): implement the async directory tree deleter as a native, embedded
-        // binary, and stop relying on the shell. See GitHub issue #4319.
-        .addArgs("/bin/sh", "-c", command)
+    new CommandBuilder()
+        .addArg(command)
+        .useShell(true)
+        .setWorkingDir(tempPath.getParentDirectory())
         .build()
         .execute();
   }
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 c9030b54e0..10b437107f 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
@@ -246,8 +246,8 @@ public class RunCommand implements BlazeCommand  {
         null, "Running command line: " + ShellEscaper.escapeJoinAll(prettyCmdLine)));
 
     try {
-      com.google.devtools.build.lib.shell.Command command =
-          new CommandBuilder(workingDir).addArgs(cmdLine).setEnv(env.getClientEnv()).build();
+      com.google.devtools.build.lib.shell.Command command = new CommandBuilder()
+          .addArgs(cmdLine).setEnv(env.getClientEnv()).setWorkingDir(workingDir).build();
 
       // Restore a raw EventHandler if it is registered. This allows for blaze run to produce the
       // actual output of the command being run even if --color=no is specified.
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/mobileinstall/MobileInstallCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/mobileinstall/MobileInstallCommand.java
index 0d43d8f95a..6484bbe3f2 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/mobileinstall/MobileInstallCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/mobileinstall/MobileInstallCommand.java
@@ -285,7 +285,11 @@ public class MobileInstallCommand implements BlazeCommand {
 
     Path workingDir = env.getBlazeWorkspace().getOutputPath().getParentDirectory();
     com.google.devtools.build.lib.shell.Command command =
-        new CommandBuilder(workingDir).addArgs(cmdLine).setEnv(env.getClientEnv()).build();
+        new CommandBuilder()
+            .addArgs(cmdLine)
+            .setEnv(env.getClientEnv())
+            .setWorkingDir(workingDir)
+            .build();
 
     try {
       // Restore a raw EventHandler if it is registered. This allows for blaze run to produce the
diff --git a/src/main/java/com/google/devtools/build/lib/util/CommandBuilder.java b/src/main/java/com/google/devtools/build/lib/util/CommandBuilder.java
index 9bff531521..d60fd94dc4 100644
--- a/src/main/java/com/google/devtools/build/lib/util/CommandBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/util/CommandBuilder.java
@@ -14,44 +14,65 @@
 
 package com.google.devtools.build.lib.util;
 
+import static com.google.common.base.StandardSystemProperty.JAVA_IO_TMPDIR;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.CharMatcher;
+import com.google.common.base.Joiner;
 import com.google.common.base.Preconditions;
+import com.google.common.base.Splitter;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
 import com.google.devtools.build.lib.shell.Command;
 import com.google.devtools.build.lib.vfs.Path;
 import java.io.File;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 
 /**
- * Implements {@link Command} builder.
+ * Implements OS aware {@link Command} builder. At this point only Linux, Mac
+ * and Windows XP are supported.
+ *
+ * <p>Builder will also apply heuristic to identify trivial cases where
+ * unix-like command lines could be automatically converted into the
+ * Windows-compatible form.
  *
- * <p>TODO(bazel-team): (2010) Some of the code here is very similar to the {@link
- * com.google.devtools.build.lib.shell.Shell} class. This should be looked at.
+ * <p>TODO(bazel-team): (2010) Some of the code here is very similar to the
+ * {@link com.google.devtools.build.lib.shell.Shell} class. This should be looked at.
  */
 public final class CommandBuilder {
 
-  private final ArrayList<String> argv = new ArrayList<>();
-  private final HashMap<String, String> env = new HashMap<>();
-  private final File workingDir;
+  private static final ImmutableList<String> SHELLS = ImmutableList.of("/bin/sh", "/bin/bash");
+
+  private static final Splitter ARGV_SPLITTER = Splitter.on(CharMatcher.anyOf(" \t"));
 
-  public CommandBuilder(Path workingDir) {
-    this(workingDir.getPathFile());
+  private final OS system;
+  private final List<String> argv = new ArrayList<>();
+  private final Map<String, String> env = new HashMap<>();
+  private File workingDir = null;
+  private boolean useShell = false;
+
+  public CommandBuilder() {
+    this(OS.getCurrent());
   }
 
-  public CommandBuilder(File workingDir) {
-    this.workingDir = Preconditions.checkNotNull(workingDir);
+  @VisibleForTesting
+  CommandBuilder(OS system) {
+    this.system = system;
   }
 
   public CommandBuilder addArg(String arg) {
-    Preconditions.checkNotNull(arg);
+    Preconditions.checkNotNull(arg, "Argument must not be null");
     argv.add(arg);
     return this;
   }
 
   public CommandBuilder addArgs(Iterable<String> args) {
-    Preconditions.checkArgument(!Iterables.contains(args, null));
+    Preconditions.checkArgument(!Iterables.contains(args, null), "Arguments must not be null");
     Iterables.addAll(argv, args);
     return this;
   }
@@ -77,8 +98,82 @@ public final class CommandBuilder {
     return this;
   }
 
+  public CommandBuilder setWorkingDir(Path path) {
+    Preconditions.checkNotNull(path);
+    workingDir = path.getPathFile();
+    return this;
+  }
+
+  public CommandBuilder useTempDir() {
+    workingDir = new File(JAVA_IO_TMPDIR.value());
+    return this;
+  }
+
+  public CommandBuilder useShell(boolean useShell) {
+    this.useShell = useShell;
+    return this;
+  }
+
+  private boolean argvStartsWithSh() {
+    return argv.size() >= 2 && SHELLS.contains(argv.get(0)) && "-c".equals(argv.get(1));
+  }
+
+  private String[] transformArgvForLinux() {
+    // If command line already starts with "/bin/sh -c", ignore useShell attribute.
+    if (useShell && !argvStartsWithSh()) {
+      // c.g.io.base.shell.Shell.shellify() actually concatenates argv into the space-separated
+      // string here. Not sure why, but we will do the same.
+      return new String[] { "/bin/sh", "-c", Joiner.on(' ').join(argv) };
+    }
+    return argv.toArray(new String[argv.size()]);
+  }
+
+  private String[] transformArgvForWindows() {
+    List<String> modifiedArgv;
+    // Heuristic: replace "/bin/sh -c" with something more appropriate for Windows.
+    if (argvStartsWithSh()) {
+      useShell = true;
+      modifiedArgv = Lists.newArrayList(argv.subList(2, argv.size()));
+    } else {
+      modifiedArgv = Lists.newArrayList(argv);
+    }
+
+    if (!modifiedArgv.isEmpty()) {
+      // args can contain whitespace, so figure out the first word
+      String argv0 = modifiedArgv.get(0);
+      String command = ARGV_SPLITTER.split(argv0).iterator().next();
+
+      // Automatically enable CMD.EXE use if we are executing something else besides "*.exe" file.
+      // When use CMD.EXE to invoke a bat/cmd file, the file path must have '\' instead of '/'
+      if (!command.toLowerCase().endsWith(".exe")) {
+        useShell = true;
+        modifiedArgv.set(0, argv0.replace('/', '\\'));
+      }
+    } else {
+      // This is degenerate "/bin/sh -c" case. We ensure that Windows behavior is identical
+      // to the Linux - call shell that will do nothing.
+      useShell = true;
+    }
+    if (useShell) {
+      // /S - strip first and last quotes and execute everything else as is.
+      // /E:ON - enable extended command set.
+      // /V:ON - enable delayed variable expansion
+      // /D - ignore AutoRun registry entries.
+      // /C - execute command. This must be the last option before the command itself.
+      return new String[] { "CMD.EXE", "/S", "/E:ON", "/V:ON", "/D", "/C",
+          Joiner.on(' ').join(modifiedArgv) };
+    } else {
+      return modifiedArgv.toArray(new String[argv.size()]);
+    }
+  }
+
   public Command build() {
+    Preconditions.checkState(system != OS.UNKNOWN, "Unidentified operating system");
+    Preconditions.checkNotNull(workingDir, "Working directory must be set");
     Preconditions.checkState(!argv.isEmpty(), "At least one argument is expected");
-    return new Command(argv.toArray(new String[0]), env, workingDir);
+
+    return new Command(
+        system == OS.WINDOWS ? transformArgvForWindows() : transformArgvForLinux(),
+        env, workingDir);
   }
 }