Refactor BuildRequest out of sandboxed spawn runner initialization.

Remove BuildRequest as parameter to sandboxed spawn runner
constructors. Previously, the build request was used to obtain some
options, but those can be extricated from a CommandEnvironment, which
is passed in, too.

Also, remove LinuxSandboxedSpawnRunner's aliased sandboxOptions member
variable. It can just use the superclass's.

Change-Id: I1ef1a45cbf7e800d0809f05673f097a148289740
PiperOrigin-RevId: 164257471

7 files changed, 34 insertions, 51 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java
index 8a4d9c4b62..f4fbeebbde 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java
@@ -52,12 +52,9 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner {
   private final SandboxOptions sandboxOptions;
   private final ImmutableSet<Path> inaccessiblePaths;
 
-  public AbstractSandboxSpawnRunner(
-      CommandEnvironment cmdEnv,
-      Path sandboxBase,
-      SandboxOptions sandboxOptions) {
+  public AbstractSandboxSpawnRunner(CommandEnvironment cmdEnv, Path sandboxBase) {
     this.sandboxBase = sandboxBase;
-    this.sandboxOptions = sandboxOptions;
+    this.sandboxOptions = cmdEnv.getOptions().getOptions(SandboxOptions.class);
     this.inaccessiblePaths =
         sandboxOptions.getInaccessiblePaths(cmdEnv.getDirectories().getFileSystem());
   }
@@ -224,5 +221,9 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner {
     return inaccessiblePaths;
   }
 
+  protected SandboxOptions getSandboxOptions() {
+    return sandboxOptions;
+  }
+
   protected abstract String getName();
 }
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java
index 1c9f098c41..2344516ac7 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java
@@ -23,7 +23,6 @@ import com.google.devtools.build.lib.actions.ExecException;
 import com.google.devtools.build.lib.actions.ExecutionStrategy;
 import com.google.devtools.build.lib.actions.Spawn;
 import com.google.devtools.build.lib.actions.SpawnActionContext;
-import com.google.devtools.build.lib.buildtool.BuildRequest;
 import com.google.devtools.build.lib.exec.SpawnResult;
 import com.google.devtools.build.lib.exec.apple.XCodeLocalEnvProvider;
 import com.google.devtools.build.lib.exec.local.LocalEnvProvider;
@@ -103,15 +102,11 @@ final class DarwinSandboxedSpawnRunner extends AbstractSandboxSpawnRunner {
 
   DarwinSandboxedSpawnRunner(
       CommandEnvironment cmdEnv,
-      BuildRequest buildRequest,
       Path sandboxBase,
       String productName,
       int timeoutGraceSeconds)
       throws IOException {
-    super(
-        cmdEnv,
-        sandboxBase,
-        buildRequest.getOptions(SandboxOptions.class));
+    super(cmdEnv, sandboxBase);
     this.execRoot = cmdEnv.getExecRoot();
     this.allowNetwork = SandboxHelpers.shouldAllowNetwork(cmdEnv.getOptions());
     this.productName = productName;
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java
index 88ec144a9f..7ea2120441 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java
@@ -22,7 +22,6 @@ import com.google.devtools.build.lib.actions.ExecException;
 import com.google.devtools.build.lib.actions.Spawn;
 import com.google.devtools.build.lib.actions.UserExecException;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
-import com.google.devtools.build.lib.buildtool.BuildRequest;
 import com.google.devtools.build.lib.exec.SpawnResult;
 import com.google.devtools.build.lib.runtime.CommandEnvironment;
 import com.google.devtools.build.lib.shell.Command;
@@ -86,7 +85,6 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner {
     return execPath != null ? cmdEnv.getExecRoot().getRelative(execPath) : null;
   }
 
-  private final SandboxOptions sandboxOptions;
   private final BlazeDirectories blazeDirs;
   private final Path execRoot;
   private final boolean allowNetwork;
@@ -97,16 +95,11 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner {
 
   LinuxSandboxedSpawnRunner(
       CommandEnvironment cmdEnv,
-      BuildRequest buildRequest,
       Path sandboxBase,
       Path inaccessibleHelperFile,
       Path inaccessibleHelperDir,
       int timeoutGraceSeconds) {
-    super(
-        cmdEnv,
-        sandboxBase,
-        buildRequest.getOptions(SandboxOptions.class));
-    this.sandboxOptions = cmdEnv.getOptions().getOptions(SandboxOptions.class);
+    super(cmdEnv, sandboxBase);
     this.blazeDirs = cmdEnv.getDirectories();
     this.execRoot = cmdEnv.getExecRoot();
     this.allowNetwork = SandboxHelpers.shouldAllowNetwork(cmdEnv.getOptions());
@@ -157,7 +150,7 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner {
     List<String> commandLineArgs = new ArrayList<>();
     commandLineArgs.add(linuxSandbox.getPathString());
 
-    if (sandboxOptions.sandboxDebug) {
+    if (getSandboxOptions().sandboxDebug) {
       commandLineArgs.add("-D");
     }
 
@@ -199,12 +192,12 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner {
       commandLineArgs.add("-N");
     }
 
-    if (sandboxOptions.sandboxFakeHostname) {
+    if (getSandboxOptions().sandboxFakeHostname) {
       // Use a fake hostname ("localhost") inside the sandbox.
       commandLineArgs.add("-H");
     }
 
-    if (sandboxOptions.sandboxFakeUsername) {
+    if (getSandboxOptions().sandboxFakeUsername) {
       // Use a fake username ("nobody") inside the sandbox.
       commandLineArgs.add("-U");
     }
@@ -234,7 +227,7 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner {
 
   private ImmutableSet<Path> getTmpfsPaths() {
     ImmutableSet.Builder<Path> tmpfsPaths = ImmutableSet.builder();
-    for (String tmpfsPath : sandboxOptions.sandboxTmpfsPath) {
+    for (String tmpfsPath : getSandboxOptions().sandboxTmpfsPath) {
       tmpfsPaths.add(blazeDirs.getFileSystem().getPath(tmpfsPath));
     }
     return tmpfsPaths.build();
@@ -251,7 +244,7 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner {
       bindMounts.put(blazeDirs.getOutputBase(), blazeDirs.getOutputBase());
     }
     for (ImmutableMap.Entry<String, String> additionalMountPath :
-        sandboxOptions.sandboxAdditionalMounts) {
+        getSandboxOptions().sandboxAdditionalMounts) {
       try {
         final Path mountTarget = blazeDirs.getFileSystem().getPath(additionalMountPath.getValue());
         // If source path is relative, treat it as a relative path inside the execution root
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 f344708d90..c809c2d5f0 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
@@ -16,7 +16,6 @@ package com.google.devtools.build.lib.sandbox;
 
 import com.google.devtools.build.lib.actions.ExecutionStrategy;
 import com.google.devtools.build.lib.actions.SpawnActionContext;
-import com.google.devtools.build.lib.buildtool.BuildRequest;
 import com.google.devtools.build.lib.exec.AbstractSpawnStrategy;
 import com.google.devtools.build.lib.exec.SpawnRunner;
 import com.google.devtools.build.lib.runtime.CommandEnvironment;
@@ -42,7 +41,6 @@ public final class LinuxSandboxedStrategy extends AbstractSpawnStrategy {
 
   static LinuxSandboxedSpawnRunner create(
       CommandEnvironment cmdEnv,
-      BuildRequest buildRequest,
       Path sandboxBase,
       int timeoutGraceSeconds)
           throws IOException {
@@ -60,7 +58,6 @@ public final class LinuxSandboxedStrategy extends AbstractSpawnStrategy {
 
     return new LinuxSandboxedSpawnRunner(
         cmdEnv,
-        buildRequest,
         sandboxBase,
         inaccessibleHelperFile,
         inaccessibleHelperDir,
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java
index 4a6e5a87b3..5e98898030 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java
@@ -16,7 +16,6 @@ package com.google.devtools.build.lib.sandbox;
 
 import com.google.devtools.build.lib.actions.ExecException;
 import com.google.devtools.build.lib.actions.Spawn;
-import com.google.devtools.build.lib.buildtool.BuildRequest;
 import com.google.devtools.build.lib.exec.SpawnResult;
 import com.google.devtools.build.lib.exec.apple.XCodeLocalEnvProvider;
 import com.google.devtools.build.lib.exec.local.LocalEnvProvider;
@@ -43,14 +42,10 @@ final class ProcessWrapperSandboxedSpawnRunner extends AbstractSandboxSpawnRunne
 
   ProcessWrapperSandboxedSpawnRunner(
       CommandEnvironment cmdEnv,
-      BuildRequest buildRequest,
       Path sandboxBase,
       String productName,
       int timeoutGraceSeconds) {
-    super(
-        cmdEnv,
-        sandboxBase,
-        buildRequest.getOptions(SandboxOptions.class));
+    super(cmdEnv, sandboxBase);
     this.execRoot = cmdEnv.getExecRoot();
     this.productName = productName;
     this.timeoutGraceSeconds = timeoutGraceSeconds;
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 55f755da4c..a17639eef8 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
@@ -19,7 +19,6 @@ import com.google.devtools.build.lib.actions.ActionContext;
 import com.google.devtools.build.lib.actions.ExecException;
 import com.google.devtools.build.lib.actions.ResourceManager;
 import com.google.devtools.build.lib.actions.Spawn;
-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.exec.SpawnResult;
@@ -31,6 +30,7 @@ import com.google.devtools.build.lib.exec.local.LocalSpawnRunner;
 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 com.google.devtools.common.options.OptionsProvider;
 import java.io.IOException;
 
 /**
@@ -43,39 +43,42 @@ final class SandboxActionContextProvider extends ActionContextProvider {
     this.contexts = contexts;
   }
 
-  public static SandboxActionContextProvider create(
-      CommandEnvironment cmdEnv, BuildRequest buildRequest, Path sandboxBase) throws IOException {
+  public static SandboxActionContextProvider create(CommandEnvironment cmdEnv, Path sandboxBase)
+      throws IOException {
     ImmutableList.Builder<ActionContext> contexts = ImmutableList.builder();
 
+    OptionsProvider options = cmdEnv.getOptions();
     int timeoutGraceSeconds =
-        buildRequest.getOptions(LocalExecutionOptions.class).localSigkillGraceSeconds;
-    boolean verboseFailures = buildRequest.getOptions(ExecutionOptions.class).verboseFailures;
+        options.getOptions(LocalExecutionOptions.class).localSigkillGraceSeconds;
+    boolean verboseFailures = options.getOptions(ExecutionOptions.class).verboseFailures;
     String productName = cmdEnv.getRuntime().getProductName();
 
     // 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 (ProcessWrapperSandboxedSpawnRunner.isSupported(cmdEnv)) {
-      SpawnRunner spawnRunner = withFallback(
-          cmdEnv,
-          new ProcessWrapperSandboxedSpawnRunner(
-              cmdEnv, buildRequest, sandboxBase, productName, timeoutGraceSeconds));
+      SpawnRunner spawnRunner =
+          withFallback(
+              cmdEnv,
+              new ProcessWrapperSandboxedSpawnRunner(
+                  cmdEnv, sandboxBase, productName, timeoutGraceSeconds));
       contexts.add(new ProcessWrapperSandboxedStrategy(verboseFailures, spawnRunner));
     }
 
     // This is the preferred sandboxing strategy on Linux.
     if (LinuxSandboxedSpawnRunner.isSupported(cmdEnv)) {
-      SpawnRunner spawnRunner = withFallback(
-          cmdEnv,
-          LinuxSandboxedStrategy.create(cmdEnv, buildRequest, sandboxBase, timeoutGraceSeconds));
+      SpawnRunner spawnRunner =
+          withFallback(
+              cmdEnv, LinuxSandboxedStrategy.create(cmdEnv, sandboxBase, timeoutGraceSeconds));
       contexts.add(new LinuxSandboxedStrategy(verboseFailures, spawnRunner));
     }
 
     // This is the preferred sandboxing strategy on macOS.
     if (DarwinSandboxedSpawnRunner.isSupported(cmdEnv)) {
-      SpawnRunner spawnRunner = withFallback(
-          cmdEnv,
-          new DarwinSandboxedSpawnRunner(
-              cmdEnv, buildRequest, sandboxBase, productName, timeoutGraceSeconds));
+      SpawnRunner spawnRunner =
+          withFallback(
+              cmdEnv,
+              new DarwinSandboxedSpawnRunner(
+                  cmdEnv, sandboxBase, productName, timeoutGraceSeconds));
       contexts.add(new DarwinSandboxedStrategy(verboseFailures, spawnRunner));
     }
 
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxModule.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxModule.java
index 18caee78da..5003b698d9 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxModule.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxModule.java
@@ -64,8 +64,7 @@ public final class SandboxModule extends BlazeModule {
 
     try {
       FileSystemUtils.createDirectoryAndParents(sandboxBase);
-      builder.addActionContextProvider(
-          SandboxActionContextProvider.create(cmdEnv, request, sandboxBase));
+      builder.addActionContextProvider(SandboxActionContextProvider.create(cmdEnv, sandboxBase));
     } catch (IOException e) {
       throw new IllegalStateException(e);
     }