Use getopt to parse process-wrapper's command-line.

This will allow us to add new and optional flags like selecting a strategy used to spawn / wait for the child process.

No one except Bazel should be calling "process-wrapper" and I couldn't find any references, so this breaking change should be fine.

PiperOrigin-RevId: 159685867

9 files changed, 38 insertions, 43 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/exec/local/LocalExecutionOptions.java b/src/main/java/com/google/devtools/build/lib/exec/local/LocalExecutionOptions.java
index 14fe8b1fdc..565631e24f 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/local/LocalExecutionOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/local/LocalExecutionOptions.java
@@ -33,7 +33,7 @@ public class LocalExecutionOptions extends OptionsBase {
         "Time to wait between terminating a local process due to timeout and forcefully "
             + "shutting it down."
   )
-  public double localSigkillGraceSeconds;
+  public int localSigkillGraceSeconds;
 
   @Option(
     name = "allowed_local_actions_regex",
diff --git a/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java
index 5cd191be3b..e77c656a6b 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java
@@ -81,6 +81,10 @@ public final class LocalSpawnRunner implements SpawnRunner {
   private final String productName;
   private final LocalEnvProvider localEnvProvider;
 
+  private static Path getProcessWrapper(Path execRoot, OS localOs) {
+    return execRoot.getRelative("_bin/process-wrapper" + OsUtils.executableExtension(localOs));
+  }
+
   public LocalSpawnRunner(
       Logger logger,
       AtomicInteger execCount,
@@ -95,10 +99,7 @@ public final class LocalSpawnRunner implements SpawnRunner {
     this.logger = logger;
     this.execRoot = execRoot;
     this.actionInputPrefetcher = Preconditions.checkNotNull(actionInputPrefetcher);
-    this.processWrapper =
-        execRoot
-            .getRelative("_bin/process-wrapper" + OsUtils.executableExtension(localOs))
-            .getPathString();
+    this.processWrapper = getProcessWrapper(execRoot, localOs).getPathString();
     this.localExecutionOptions = Preconditions.checkNotNull(localExecutionOptions);
     this.hostName = NetUtil.findShortHostName();
     this.execCount = execCount;
@@ -122,7 +123,7 @@ public final class LocalSpawnRunner implements SpawnRunner {
         actionInputPrefetcher,
         localExecutionOptions,
         resourceManager,
-        /*useProcessWrapper=*/OS.getCurrent() != OS.WINDOWS,
+        OS.getCurrent() != OS.WINDOWS && getProcessWrapper(execRoot, OS.getCurrent()).exists(),
         OS.getCurrent(),
         productName,
         localEnvProvider);
@@ -256,10 +257,10 @@ public final class LocalSpawnRunner implements SpawnRunner {
       if (useProcessWrapper) {
         List<String> cmdLine = new ArrayList<>();
         cmdLine.add(processWrapper);
-        cmdLine.add(Float.toString(timeoutSeconds));
-        cmdLine.add(Double.toString(localExecutionOptions.localSigkillGraceSeconds));
-        cmdLine.add(getPathOrDevNull(outErr.getOutputPath()));
-        cmdLine.add(getPathOrDevNull(outErr.getErrorPath()));
+        cmdLine.add("--timeout=" + timeoutSeconds);
+        cmdLine.add("--kill_delay=" + localExecutionOptions.localSigkillGraceSeconds);
+        cmdLine.add("--stdout=" + getPathOrDevNull(outErr.getOutputPath()));
+        cmdLine.add("--stderr=" + getPathOrDevNull(outErr.getErrorPath()));
         cmdLine.addAll(spawn.getArguments());
         cmd = new Command(
             cmdLine.toArray(new String[0]),
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 6f29578dd8..6a6dbbc66a 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
@@ -255,10 +255,6 @@ public class RunCommand implements BlazeCommand  {
       Preconditions.checkNotNull(
           processWrapperPath, PROCESS_WRAPPER + " not found in embedded tools");
       cmdLine.add(env.getExecRoot().getRelative(processWrapperPath).getPathString());
-      cmdLine.add("-1");
-      cmdLine.add("15");
-      cmdLine.add("-");
-      cmdLine.add("-");
     }
     List<String> prettyCmdLine = new ArrayList<>();
     // Insert the command prefix specified by the "--run_under=<command-prefix>" option
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java
index e96c417980..9d8e5acfe9 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java
@@ -31,6 +31,7 @@ import com.google.devtools.build.lib.runtime.CommandEnvironment;
 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 com.google.devtools.build.lib.util.OS;
 import com.google.devtools.build.lib.vfs.FileSystem;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
@@ -48,6 +49,10 @@ import java.util.concurrent.atomic.AtomicReference;
 )
 public class DarwinSandboxedStrategy extends SandboxStrategy {
 
+  public static boolean isSupported(CommandEnvironment cmdEnv) {
+    return OS.getCurrent() == OS.DARWIN && DarwinSandboxRunner.isSupported(cmdEnv);
+  }
+
   private final Path execRoot;
   private final boolean sandboxDebug;
   private final boolean verboseFailures;
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java
index ec4a5e5d9b..1d6e5ca4fd 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java
@@ -28,6 +28,7 @@ import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.buildtool.BuildRequest;
 import com.google.devtools.build.lib.exec.SpawnInputExpander;
 import com.google.devtools.build.lib.runtime.CommandEnvironment;
+import com.google.devtools.build.lib.util.OS;
 import com.google.devtools.build.lib.vfs.FileSystem;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
@@ -47,7 +48,7 @@ import java.util.concurrent.atomic.AtomicReference;
 public class LinuxSandboxedStrategy extends SandboxStrategy {
 
   public static boolean isSupported(CommandEnvironment cmdEnv) {
-    return LinuxSandboxRunner.isSupported(cmdEnv);
+    return OS.getCurrent() == OS.LINUX && LinuxSandboxRunner.isSupported(cmdEnv);
   }
 
   private final SandboxOptions sandboxOptions;
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperRunner.java
index 71ceebac73..e6a3061da5 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperRunner.java
@@ -66,10 +66,8 @@ final class ProcessWrapperRunner extends SandboxRunner {
       CommandEnvironment cmdEnv, List<String> spawnArguments, int timeout) {
     List<String> commandLineArgs = new ArrayList<>(5 + spawnArguments.size());
     commandLineArgs.add(getProcessWrapper(cmdEnv).getPathString());
-    commandLineArgs.add(Integer.toString(timeout));
-    commandLineArgs.add("5"); /* kill delay: give some time to print stacktraces and whatnot. */
-    commandLineArgs.add("-"); /* stdout. */
-    commandLineArgs.add("-"); /* stderr. */
+    commandLineArgs.add("--timeout=" + timeout);
+    commandLineArgs.add("--kill_delay=5"); /* give some time to print stacktraces and whatnot. */
     commandLineArgs.addAll(spawnArguments);
     return commandLineArgs;
   }
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedStrategy.java b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedStrategy.java
index 977916cb54..0ccdd989e3 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedStrategy.java
@@ -43,7 +43,7 @@ import java.util.concurrent.atomic.AtomicReference;
 public class ProcessWrapperSandboxedStrategy extends SandboxStrategy {
 
   public static boolean isSupported(CommandEnvironment cmdEnv) {
-    return ProcessWrapperRunner.isSupported(cmdEnv);
+    return OS.isPosixCompatible() && ProcessWrapperRunner.isSupported(cmdEnv);
   }
 
   private final SandboxOptions sandboxOptions;
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextConsumer.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextConsumer.java
index 1bbeacf863..653c9dafa7 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextConsumer.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextConsumer.java
@@ -21,7 +21,6 @@ import com.google.devtools.build.lib.actions.ActionContext;
 import com.google.devtools.build.lib.actions.SpawnActionContext;
 import com.google.devtools.build.lib.exec.ActionContextConsumer;
 import com.google.devtools.build.lib.runtime.CommandEnvironment;
-import com.google.devtools.build.lib.util.OS;
 
 /**
  * {@link ActionContextConsumer} that requests the action contexts necessary for sandboxed
@@ -37,9 +36,9 @@ final class SandboxActionContextConsumer implements ActionContextConsumer {
         ImmutableMultimap.builder();
     ImmutableMap.Builder<String, String> spawnContexts = ImmutableMap.builder();
 
-    if ((OS.getCurrent() == OS.LINUX && LinuxSandboxedStrategy.isSupported(cmdEnv))
-        || (OS.getCurrent() == OS.DARWIN && DarwinSandboxRunner.isSupported(cmdEnv))
-        || (OS.isPosixCompatible() && ProcessWrapperSandboxedStrategy.isSupported(cmdEnv))) {
+    if (LinuxSandboxedStrategy.isSupported(cmdEnv)
+        || DarwinSandboxedStrategy.isSupported(cmdEnv)
+        || ProcessWrapperSandboxedStrategy.isSupported(cmdEnv)) {
       // This makes the "sandboxed" strategy available via --spawn_strategy=sandboxed,
       // but it is not necessarily the default.
       contexts.put(SpawnActionContext.class, "sandboxed");
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java
index adcc21462f..d9829a2cb7 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java
@@ -20,7 +20,6 @@ import com.google.devtools.build.lib.buildtool.BuildRequest;
 import com.google.devtools.build.lib.exec.ActionContextProvider;
 import com.google.devtools.build.lib.exec.ExecutionOptions;
 import com.google.devtools.build.lib.runtime.CommandEnvironment;
-import com.google.devtools.build.lib.util.OS;
 import com.google.devtools.build.lib.vfs.Path;
 import java.io.IOException;
 
@@ -41,29 +40,25 @@ final class SandboxActionContextProvider extends ActionContextProvider {
     boolean verboseFailures = buildRequest.getOptions(ExecutionOptions.class).verboseFailures;
     String productName = cmdEnv.getRuntime().getProductName();
 
-    // The ProcessWrapperSandboxedStrategy works on all POSIX-compatible operating systems.
-    if (OS.isPosixCompatible()) {
+    // This works on most platforms, but isn't the best choice, so we put it first and let later
+    // platform-specific sandboxing strategies become the default.
+    if (ProcessWrapperSandboxedStrategy.isSupported(cmdEnv)) {
       contexts.add(
           new ProcessWrapperSandboxedStrategy(
               cmdEnv, buildRequest, sandboxBase, verboseFailures, productName));
     }
 
-    switch (OS.getCurrent()) {
-      case LINUX:
-        if (LinuxSandboxedStrategy.isSupported(cmdEnv)) {
-          contexts.add(
-              LinuxSandboxedStrategy.create(cmdEnv, buildRequest, sandboxBase, verboseFailures));
-        }
-        break;
-      case DARWIN:
-        if (DarwinSandboxRunner.isSupported(cmdEnv)) {
-          contexts.add(
-              DarwinSandboxedStrategy.create(
-                  cmdEnv, buildRequest, sandboxBase, verboseFailures, productName));
-        }
-        break;
-      default:
-        // No additional platform-specific sandboxing available.
+    // This is the preferred sandboxing strategy on Linux.
+    if (LinuxSandboxedStrategy.isSupported(cmdEnv)) {
+      contexts.add(
+          LinuxSandboxedStrategy.create(cmdEnv, buildRequest, sandboxBase, verboseFailures));
+    }
+
+    // This is the preferred sandboxing strategy on macOS.
+    if (DarwinSandboxedStrategy.isSupported(cmdEnv)) {
+      contexts.add(
+          DarwinSandboxedStrategy.create(
+              cmdEnv, buildRequest, sandboxBase, verboseFailures, productName));
     }
 
     return new SandboxActionContextProvider(contexts.build());