From feee0704b9a2654e1b2a9d99ec9f2fd590f8b23f Mon Sep 17 00:00:00 2001 From: Yue Gan Date: Mon, 9 Jan 2017 09:34:10 +0000 Subject: 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 " 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. -- PiperOrigin-RevId: 143937589 MOS_MIGRATED_REVID=143937589 --- .../build/lib/sandbox/DarwinSandboxRunner.java | 11 +++++-- .../build/lib/sandbox/LinuxSandboxRunner.java | 2 +- .../build/lib/sandbox/LinuxSandboxedStrategy.java | 2 +- .../build/lib/sandbox/ProcessWrapperRunner.java | 4 +-- .../devtools/build/lib/sandbox/SandboxRunner.java | 38 +++++++++++++++------- .../build/lib/sandbox/SandboxStrategy.java | 11 +++++-- 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 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 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..aa9334cbb2 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 arguments, Map 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); + } + List 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; } } -- cgit v1.2.3