Gives 3 levels of sandbox error message under different flags.

 1. no flag: only the direct reason is given (command execution termination status), and also the instruction to use "--verbose_failures"
 2. flag "--verbose_failures": gives failed execution command and the instruction to use "--sandbox_debug --strategy"
 3. flag "--verbose_failures --sandbox_debug": gives failed execution command, debugging message from sandboxing, and the instruction to use "--strategy"

Also removes "cd <sandbox_path>" in given failed command, since debugging is only necessary with flag "--verbose_failures --sandbox_debug" and the path is already given in sandboxing debugging message.

Addresses #2174.

Fixes bazel_sandboxing_test.sh and bazel_test_test.sh for the new error message.
Fixes timeout.

--
PiperOrigin-RevId: 144285435
MOS_MIGRATED_REVID=144285435

6 files changed, 48 insertions, 20 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxRunner.java
index c8387581ff..06f9244ddd 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxRunner.java
@@ -14,6 +14,8 @@
 
 package com.google.devtools.build.lib.sandbox;
 
+import static java.nio.charset.StandardCharsets.UTF_8;
+
 import com.google.common.collect.ImmutableMap;
 import com.google.common.io.ByteStreams;
 import com.google.devtools.build.lib.shell.Command;
@@ -22,8 +24,10 @@ import com.google.devtools.build.lib.shell.KillableObserver;
 import com.google.devtools.build.lib.shell.TimeoutKillableObserver;
 import com.google.devtools.build.lib.util.OS;
 import com.google.devtools.build.lib.vfs.Path;
+import java.io.BufferedWriter;
 import java.io.File;
 import java.io.IOException;
+import java.io.OutputStreamWriter;
 import java.io.PrintWriter;
 import java.util.ArrayList;
 import java.util.List;
@@ -50,7 +54,7 @@ final class DarwinSandboxRunner extends SandboxRunner {
       Set<Path> inaccessiblePaths,
       Path runUnderPath,
       boolean verboseFailures) {
-    super(sandboxExecRoot, verboseFailures);
+    super(verboseFailures);
     this.sandboxExecRoot = sandboxExecRoot;
     this.argumentsFilePath = sandboxPath.getRelative("sandbox.sb");
     this.writableDirs = writableDirs;
@@ -117,7 +121,10 @@ final class DarwinSandboxRunner extends SandboxRunner {
   }
 
   private void writeConfig(boolean allowNetwork) throws IOException {
-    try (PrintWriter out = new PrintWriter(argumentsFilePath.getOutputStream())) {
+    try (PrintWriter out =
+        new PrintWriter(
+            new BufferedWriter(
+                new OutputStreamWriter(argumentsFilePath.getOutputStream(), UTF_8)))) {
       // Note: In Apple's sandbox configuration language, the *last* matching rule wins.
       out.println("(version 1)");
       out.println("(debug deny)");
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxRunner.java
index 97151e24ce..e49fabfc50 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxRunner.java
@@ -60,7 +60,7 @@ final class LinuxSandboxRunner extends SandboxRunner {
       Map<Path, Path> bindMounts,
       boolean verboseFailures,
       boolean sandboxDebug) {
-    super(sandboxExecRoot, verboseFailures);
+    super(verboseFailures);
     this.execRoot = execRoot;
     this.sandboxExecRoot = sandboxExecRoot;
     this.sandboxTempDir = sandboxTempDir;
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 dc4578ef61..e2a81c8af5 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
@@ -167,7 +167,7 @@ public class LinuxSandboxedStrategy extends SandboxStrategy {
           verboseFailures,
           sandboxOptions.sandboxDebug);
     } else {
-      return new ProcessWrapperRunner(execRoot, sandboxPath, sandboxExecRoot, verboseFailures);
+      return new ProcessWrapperRunner(execRoot, sandboxExecRoot, verboseFailures);
     }
   }
 
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 f871ea70a2..723e930759 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
@@ -32,8 +32,8 @@ final class ProcessWrapperRunner extends SandboxRunner {
   private final Path sandboxExecRoot;
 
   ProcessWrapperRunner(
-      Path execRoot, Path sandboxPath, Path sandboxExecRoot, boolean verboseFailures) {
-    super(sandboxExecRoot, verboseFailures);
+      Path execRoot, Path sandboxExecRoot, boolean verboseFailures) {
+    super(verboseFailures);
     this.execRoot = execRoot;
     this.sandboxExecRoot = sandboxExecRoot;
   }
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxRunner.java
index b20da1bd0c..e3643332d5 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxRunner.java
@@ -23,7 +23,6 @@ import com.google.devtools.build.lib.shell.KillableObserver;
 import com.google.devtools.build.lib.shell.TerminationStatus;
 import com.google.devtools.build.lib.util.CommandFailureUtils;
 import com.google.devtools.build.lib.util.io.OutErr;
-import com.google.devtools.build.lib.vfs.Path;
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.List;
@@ -32,11 +31,12 @@ import java.util.Map;
 /** A common interface of all sandbox runners, no matter which platform they're working on. */
 abstract class SandboxRunner {
 
+  private static final String SANDBOX_DEBUG_SUGGESTION =
+      "\n\nUse --sandbox_debug to see verbose messages from the sandbox";
+
   private final boolean verboseFailures;
-  private final Path sandboxExecRoot;
 
-  SandboxRunner(Path sandboxExecRoot, boolean verboseFailures) {
-    this.sandboxExecRoot = sandboxExecRoot;
+  SandboxRunner(boolean verboseFailures) {
     this.verboseFailures = verboseFailures;
   }
 
@@ -48,13 +48,15 @@ abstract class SandboxRunner {
    * @param outErr - error output to capture sandbox's and command's stderr
    * @param timeout - after how many seconds should the process be killed
    * @param allowNetwork - whether networking should be allowed for the process
+   * @param sandboxDebug - whether debugging message should be printed
    */
   void run(
       List<String> arguments,
       Map<String, String> environment,
       OutErr outErr,
       int timeout,
-      boolean allowNetwork)
+      boolean allowNetwork,
+      boolean sandboxDebug)
       throws ExecException {
     Command cmd;
     try {
@@ -63,6 +65,7 @@ abstract class SandboxRunner {
       throw new UserExecException("I/O error during sandboxed execution", e);
     }
 
+    TerminationStatus status = null;
     try {
       cmd.execute(
           /* stdin */ new byte[] {},
@@ -73,17 +76,28 @@ abstract class SandboxRunner {
     } catch (CommandException e) {
       boolean timedOut = false;
       if (e instanceof AbnormalTerminationException) {
-        TerminationStatus status =
-            ((AbnormalTerminationException) e).getResult().getTerminationStatus();
+        status = ((AbnormalTerminationException) e).getResult().getTerminationStatus();
         timedOut = !status.exited() && (status.getTerminatingSignal() == getSignalOnTimeout());
       }
-      String message =
+      String statusMessage = status + " [sandboxed]";
+      if (!verboseFailures) {
+        // simplest error message
+        throw new UserExecException(statusMessage, e, timedOut);
+      }
+      List<String> commandList;
+      if (!sandboxDebug) {
+        commandList = arguments;
+      } else {
+        commandList = Arrays.asList(cmd.getCommandLineElements());
+      }
+      String commandFailureMessage =
           CommandFailureUtils.describeCommandFailure(
-              verboseFailures,
-              Arrays.asList(cmd.getCommandLineElements()),
+              true,
+              commandList,
               environment,
-              sandboxExecRoot.getPathString());
-      throw new UserExecException(message, e, timedOut);
+              null)
+          + (sandboxDebug ? "" : SANDBOX_DEBUG_SUGGESTION);
+      throw new UserExecException(commandFailureMessage, e, timedOut);
     }
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxStrategy.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxStrategy.java
index b91d3dd57e..18df3d0387 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxStrategy.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.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
+import com.google.devtools.build.lib.util.io.OutErr;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.io.IOException;
@@ -69,13 +70,15 @@ abstract class SandboxStrategy implements SandboxedSpawnActionContext {
       throws ExecException, InterruptedException {
     EventHandler eventHandler = actionExecutionContext.getExecutor().getEventHandler();
     ExecException execException = null;
+    OutErr outErr = actionExecutionContext.getFileOutErr();
     try {
       runner.run(
           spawn.getArguments(),
           spawnEnvironment,
-          actionExecutionContext.getFileOutErr(),
+          outErr,
           Spawns.getTimeoutSeconds(spawn),
-          SandboxHelpers.shouldAllowNetwork(buildRequest, spawn));
+          SandboxHelpers.shouldAllowNetwork(buildRequest, spawn),
+          sandboxOptions.sandboxDebug);
     } catch (ExecException e) {
       execException = e;
     }
@@ -102,6 +105,10 @@ abstract class SandboxStrategy implements SandboxedSpawnActionContext {
     }
 
     if (execException != null) {
+      outErr.printErr(
+          "Use --strategy="
+          + spawn.getMnemonic()
+          + "=standalone to disable sandboxing for the failing actions.\n");
       throw execException;
     }
   }