sandbox: Better error messages and the noisy debug logs of the namespace-runner now have to be explicitly activated via --sandbox_debug.

Fixes #424.

--
MOS_MIGRATED_REVID=102566625

5 files changed, 68 insertions, 40 deletions

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 d1a1783e60..3baa7adbf7 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.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Predicates;
-import com.google.common.base.Throwables;
 import com.google.common.collect.ForwardingMap;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.io.Files;
@@ -33,14 +32,8 @@ import com.google.devtools.build.lib.actions.SpawnActionContext;
 import com.google.devtools.build.lib.actions.UserExecException;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.analysis.config.RunUnder;
-import com.google.devtools.build.lib.events.Event;
-import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.rules.cpp.CppCompileAction;
 import com.google.devtools.build.lib.rules.test.TestRunnerAction;
-import com.google.devtools.build.lib.runtime.BlazeRuntime;
-import com.google.devtools.build.lib.shell.AbnormalTerminationException;
-import com.google.devtools.build.lib.shell.CommandException;
-import com.google.devtools.build.lib.shell.TerminationStatus;
 import com.google.devtools.build.lib.standalone.StandaloneSpawnStrategy;
 import com.google.devtools.build.lib.syntax.Label;
 import com.google.devtools.build.lib.unix.FilesystemUtils;
@@ -70,10 +63,11 @@ import java.util.concurrent.atomic.AtomicInteger;
 public class LinuxSandboxedStrategy implements SpawnActionContext {
   private final ExecutorService backgroundWorkers;
 
-  private final BlazeRuntime blazeRuntime;
+  private final ImmutableMap<String, String> clientEnv;
   private final BlazeDirectories blazeDirs;
   private final Path execRoot;
   private final boolean verboseFailures;
+  private final boolean sandboxDebug;
   private final StandaloneSpawnStrategy standaloneStrategy;
   private final UUID uuid = UUID.randomUUID();
   private final AtomicInteger execCounter = new AtomicInteger();
@@ -112,12 +106,17 @@ public class LinuxSandboxedStrategy implements SpawnActionContext {
   }
 
   public LinuxSandboxedStrategy(
-      BlazeRuntime blazeRuntime, boolean verboseFailures, ExecutorService backgroundWorkers) {
-    this.blazeRuntime = blazeRuntime;
-    this.blazeDirs = blazeRuntime.getDirectories();
+      Map<String, String> clientEnv,
+      BlazeDirectories blazeDirs,
+      ExecutorService backgroundWorkers,
+      boolean verboseFailures,
+      boolean sandboxDebug) {
+    this.clientEnv = ImmutableMap.copyOf(clientEnv);
+    this.blazeDirs = blazeDirs;
     this.execRoot = blazeDirs.getExecRoot();
-    this.verboseFailures = verboseFailures;
     this.backgroundWorkers = backgroundWorkers;
+    this.verboseFailures = verboseFailures;
+    this.sandboxDebug = sandboxDebug;
     this.standaloneStrategy = new StandaloneSpawnStrategy(blazeDirs.getExecRoot(), verboseFailures);
   }
 
@@ -163,7 +162,7 @@ public class LinuxSandboxedStrategy implements SpawnActionContext {
 
     try {
       final NamespaceSandboxRunner runner =
-          new NamespaceSandboxRunner(execRoot, sandboxPath, mounts, verboseFailures);
+          new NamespaceSandboxRunner(execRoot, sandboxPath, mounts, verboseFailures, sandboxDebug);
       try {
         runner.run(
             spawn.getArguments(),
@@ -197,18 +196,8 @@ public class LinuxSandboxedStrategy implements SpawnActionContext {
               }
             });
       }
-    } catch (AbnormalTerminationException e) {
-      TerminationStatus status = e.getResult().getTerminationStatus();
-      boolean timedOut = !status.exited() && (status.getTerminatingSignal() == 14 /* SIGALRM */);
-      throw new UserExecException("Error during execution of spawn", e, timedOut);
-    } catch (CommandException e) {
-      throw new UserExecException("Error during execution of spawn", e);
     } catch (IOException e) {
-      EventHandler handler = actionExecutionContext.getExecutor().getEventHandler();
-      handler.handle(
-          Event.error(
-              "I/O error during sandboxed execution:\n" + Throwables.getStackTraceAsString(e)));
-      throw new UserExecException("Could not execute spawn", e);
+      throw new UserExecException("I/O error during sandboxed execution", e);
     }
   }
 
@@ -477,7 +466,7 @@ public class LinuxSandboxedStrategy implements SpawnActionContext {
           source = blazeDirs.getExecRoot().getRelative(sourceFragment);
         } else {
           List<Path> searchPath =
-              SearchPath.parse(blazeDirs.getFileSystem(), blazeRuntime.getClientEnv().get("PATH"));
+              SearchPath.parse(blazeDirs.getFileSystem(), clientEnv.get("PATH"));
           source = SearchPath.which(searchPath, runUnder.getCommand());
         }
         if (source != null) {
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/NamespaceSandboxRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/NamespaceSandboxRunner.java
index 1667a03c54..1c2034a12d 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/NamespaceSandboxRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/NamespaceSandboxRunner.java
@@ -18,11 +18,15 @@ import com.google.common.collect.ImmutableMap;
 import com.google.common.io.ByteStreams;
 import com.google.common.io.Files;
 import com.google.devtools.build.lib.actions.ActionInput;
+import com.google.devtools.build.lib.actions.UserExecException;
 import com.google.devtools.build.lib.analysis.config.BinTools;
 import com.google.devtools.build.lib.runtime.BlazeRuntime;
+import com.google.devtools.build.lib.shell.AbnormalTerminationException;
 import com.google.devtools.build.lib.shell.Command;
 import com.google.devtools.build.lib.shell.CommandException;
+import com.google.devtools.build.lib.shell.TerminationStatus;
 import com.google.devtools.build.lib.unix.FilesystemUtils;
+import com.google.devtools.build.lib.util.CommandFailureUtils;
 import com.google.devtools.build.lib.util.OsUtils;
 import com.google.devtools.build.lib.util.io.FileOutErr;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
@@ -46,15 +50,21 @@ public class NamespaceSandboxRunner {
   private final Path sandboxPath;
   private final Path sandboxExecRoot;
   private final ImmutableMap<Path, Path> mounts;
-  private final boolean debug;
+  private final boolean verboseFailures;
+  private final boolean sandboxDebug;
 
   public NamespaceSandboxRunner(
-      Path execRoot, Path sandboxPath, ImmutableMap<Path, Path> mounts, boolean debug) {
+      Path execRoot,
+      Path sandboxPath,
+      ImmutableMap<Path, Path> mounts,
+      boolean verboseFailures,
+      boolean sandboxDebug) {
     this.execRoot = execRoot;
     this.sandboxPath = sandboxPath;
     this.sandboxExecRoot = sandboxPath.getRelative(execRoot.asFragment().relativeTo("/"));
     this.mounts = mounts;
-    this.debug = debug;
+    this.verboseFailures = verboseFailures;
+    this.sandboxDebug = sandboxDebug;
   }
 
   static boolean isSupported(BlazeRuntime runtime) {
@@ -99,14 +109,14 @@ public class NamespaceSandboxRunner {
       FileOutErr outErr,
       Collection<? extends ActionInput> outputs,
       int timeout)
-      throws IOException, CommandException {
+      throws IOException, UserExecException {
     createFileSystem(outputs);
 
     List<String> args = new ArrayList<>();
 
     args.add(execRoot.getRelative("_bin/namespace-sandbox").getPathString());
 
-    if (debug) {
+    if (sandboxDebug) {
       args.add("-D");
     }
 
@@ -137,12 +147,25 @@ public class NamespaceSandboxRunner {
 
     Command cmd = new Command(args.toArray(new String[0]), env, cwd);
 
-    cmd.execute(
-        /* stdin */ new byte[] {},
-        Command.NO_OBSERVER,
-        outErr.getOutputStream(),
-        outErr.getErrorStream(),
-        /* killSubprocessOnInterrupt */ true);
+    try {
+      cmd.execute(
+          /* stdin */ new byte[] {},
+          Command.NO_OBSERVER,
+          outErr.getOutputStream(),
+          outErr.getErrorStream(),
+          /* killSubprocessOnInterrupt */ true);
+    } catch (CommandException e) {
+      boolean timedOut = false;
+      if (e instanceof AbnormalTerminationException) {
+        TerminationStatus status =
+            ((AbnormalTerminationException) e).getResult().getTerminationStatus();
+        timedOut = !status.exited() && (status.getTerminatingSignal() == 14 /* SIGALRM */);
+      }
+      String message =
+          CommandFailureUtils.describeCommandFailure(
+              verboseFailures, spawnArguments, env, cwd.getPath());
+      throw new UserExecException(message, e, timedOut);
+    }
 
     copyOutputs(outputs);
   }
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 20b64c87ca..61cd2cdbd1 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
@@ -35,10 +35,17 @@ public class SandboxActionContextProvider extends ActionContextProvider {
   public SandboxActionContextProvider(
       BlazeRuntime runtime, BuildRequest buildRequest, ExecutorService backgroundWorkers) {
     boolean verboseFailures = buildRequest.getOptions(ExecutionOptions.class).verboseFailures;
+    boolean sandboxDebug = buildRequest.getOptions(SandboxOptions.class).sandboxDebug;
     Builder<ActionContext> strategies = ImmutableList.builder();
 
     if (OS.getCurrent() == OS.LINUX) {
-      strategies.add(new LinuxSandboxedStrategy(runtime, verboseFailures, backgroundWorkers));
+      strategies.add(
+          new LinuxSandboxedStrategy(
+              runtime.getClientEnv(),
+              runtime.getDirectories(),
+              backgroundWorkers,
+              verboseFailures,
+              sandboxDebug));
     }
 
     this.strategies = strategies.build();
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java
index ad4fa50dc3..4c13ddb7ec 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java
@@ -28,4 +28,14 @@ public class SandboxOptions extends OptionsBase {
     help = "Do not print a warning when sandboxed execution is not supported on this system."
   )
   public boolean ignoreUnsupportedSandboxing;
+
+  @Option(
+    name = "sandbox_debug",
+    defaultValue = "false",
+    category = "strategy",
+    help =
+        "Let the sandbox print debug information on execution. This might help developers of "
+            + "Bazel or Skylark rules with debugging failures due to missing input files, etc."
+  )
+  public boolean sandboxDebug;
 }
diff --git a/src/main/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategy.java
index ed034d5e49..ae03d191ad 100644
--- a/src/main/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategy.java
@@ -41,7 +41,6 @@ import java.util.List;
 @ExecutionStrategy(name = { "standalone" }, contextType = SpawnActionContext.class)
 public class StandaloneSpawnStrategy implements SpawnActionContext {
   private final boolean verboseFailures;
-
   private final Path processWrapper;
 
   public StandaloneSpawnStrategy(Path execRoot, boolean verboseFailures) {
@@ -70,7 +69,7 @@ public class StandaloneSpawnStrategy implements SpawnActionContext {
       try {
         timeout = Integer.parseInt(timeoutStr);
       } catch (NumberFormatException e) {
-        throw new UserExecException("could not parse timeout: " + e);
+        throw new UserExecException("could not parse timeout: ", e);
       }
     }
 
@@ -115,7 +114,7 @@ public class StandaloneSpawnStrategy implements SpawnActionContext {
     } catch (CommandException e) {
       String message = CommandFailureUtils.describeCommandFailure(
           verboseFailures, spawn.getArguments(), spawn.getEnvironment(), cwd);
-      throw new UserExecException(String.format("%s: %s", message, e));
+      throw new UserExecException(message, e);
     }
   }