Relocate the sandbox debug suggestion to the build summary. When a sandboxed action failed, AbstractSandboxSpawnRunner directly emitted an Event.info with the --sandbox_debug suggestion. This caused the hint to appear mid-execution right before the compiler's stderr output, interleaving Bazel advice into the compiler error. Remove the direct reporter call from AbstractSandboxSpawnRunner and instead print the --sandbox_debug suggestion in BuildResultPrinter at the end of the build alongside --verbose_failures. Before: ``` ERROR: /path/to/BUILD:15:11: GoCompilePkg //pkg:go failed: (Exit 1): builder failed: ... Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging pkg/auth.go:6:2: result declared but not used Target //pkg:go failed to build Use --verbose_failures to see the command lines of failed build steps. ``` After: ``` ERROR: /path/to/BUILD:15:11: GoCompilePkg //pkg:go failed: (Exit 1): builder failed: ... pkg/auth.go:6:2: result declared but not used Target //pkg:go failed to build Use --verbose_failures to see the command lines of failed build steps. Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging ``` Part of https://github.com/bazelbuild/bazel/issues/15820 RELNOTES: None. PiperOrigin-RevId: 972632303 Change-Id: I2ee2517c9262eb3af48d7b27b3621702b8e5c0bc
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BUILD b/src/main/java/com/google/devtools/build/lib/buildtool/BUILD index cc645fc..90b0ad5 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/BUILD +++ b/src/main/java/com/google/devtools/build/lib/buildtool/BUILD
@@ -230,6 +230,7 @@ "//src/main/java/com/google/devtools/build/lib/runtime:keep_state_after_build_option", "//src/main/java/com/google/devtools/build/lib/runtime:starlark_options_parser", "//src/main/java/com/google/devtools/build/lib/runtime:ui_options", + "//src/main/java/com/google/devtools/build/lib/sandbox:sandbox_options", "//src/main/java/com/google/devtools/build/lib/skyframe:action_execution_inactivity_watchdog", "//src/main/java/com/google/devtools/build/lib/skyframe:aspect_key_creator", "//src/main/java/com/google/devtools/build/lib/skyframe:build_result_listener",
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildResultPrinter.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildResultPrinter.java index 9118e95..12b4fbe 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildResultPrinter.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildResultPrinter.java
@@ -37,6 +37,7 @@ import com.google.devtools.build.lib.exec.ExecutionOptions; import com.google.devtools.build.lib.runtime.BlazeRuntime; import com.google.devtools.build.lib.runtime.CommandEnvironment; +import com.google.devtools.build.lib.sandbox.SandboxOptions; import com.google.devtools.build.lib.skyframe.AspectKeyCreator.AspectKey; import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey; import com.google.devtools.build.lib.util.io.OutErr; @@ -71,10 +72,20 @@ boolean ok = outputTargets( request, result, configuredTargets, configuredTargetsToSkip, aspects, targetRootCauses); - if (!ok && !request.getOptions(ExecutionOptions.class).getVerboseFailures()) { - request - .getOutErr() - .printErr("Use --verbose_failures to see the command lines of failed build steps.\n"); + if (!ok) { + if (!request.getOptions(ExecutionOptions.class).getVerboseFailures()) { + request + .getOutErr() + .printErr("Use --verbose_failures to see the command lines of failed build steps.\n"); + } + SandboxOptions sandboxOptions = request.getOptions(SandboxOptions.class); + if (sandboxOptions != null && !sandboxOptions.getSandboxDebug()) { + request + .getOutErr() + .printErr( + "Use --sandbox_debug to see verbose messages from the sandbox and retain the" + + " sandbox build root for debugging\n"); + } } }
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 c69456c..125d355 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
@@ -67,10 +67,6 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner { private static final int LOCAL_EXEC_ERROR = -1; - private static final String SANDBOX_DEBUG_SUGGESTION = - "Use --sandbox_debug to see verbose messages from the sandbox " - + "and retain the sandbox build root for debugging"; - private final SandboxOptions sandboxOptions; private final boolean verboseFailures; private final boolean expandParamFiles; @@ -194,7 +190,6 @@ sandbox.getSandboxExecRoot().getPathString(), sandbox); } else { - reporter.handle(Event.info(SANDBOX_DEBUG_SUGGESTION)); return CommandFailureUtils.describeCommandFailure( verboseFailures, expandParamFiles,
diff --git a/src/test/java/com/google/devtools/build/lib/runtime/UiEventHandlerTest.java b/src/test/java/com/google/devtools/build/lib/runtime/UiEventHandlerTest.java index bcd5fad..291709d 100644 --- a/src/test/java/com/google/devtools/build/lib/runtime/UiEventHandlerTest.java +++ b/src/test/java/com/google/devtools/build/lib/runtime/UiEventHandlerTest.java
@@ -52,6 +52,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import net.starlark.java.syntax.Location; import org.junit.Before; import org.junit.Test; import org.junit.experimental.runners.Enclosed; @@ -242,6 +243,56 @@ output.assertFlushed("hello there!\n"); } + @Test + public void handleActionExecutionError_outputsErrorHeaderThenSubprocessStderr() { + Event errorEvent = + Event.error( + Location.fromFileLineColumn("pkg/foo/BUILD.bazel", 10, 5), + "Action failed: command failed"); + Event.ProcessOutput processOutput = + new Event.ProcessOutput() { + @Override + public String getStdOutPath() { + return "stdout.log"; + } + + @Override + public long getStdOutSize() { + return 0; + } + + @Override + public byte[] getStdOut() { + return new byte[0]; + } + + @Override + public String getStdErrPath() { + return "stderr.log"; + } + + @Override + public long getStdErrSize() { + return "compiler error: syntax error\n".length(); + } + + @Override + public byte[] getStdErr() { + return "compiler error: syntax error\n".getBytes(UTF_8); + } + }; + + uiEventHandler.handle(errorEvent.withProcessOutput(processOutput)); + + if (outputKind == EventKind.STDERR) { + output.assertFlushed( + "\033[31m\033[1mERROR: \033[0mpkg/foo/BUILD.bazel:10:5: Action failed: command" + + " failed" + + System.lineSeparator(), + "compiler error: syntax error\n"); + } + } + // This test only exercises progress bar code when testing stderr output, since we don't make // any assertions on stderr (where the progress bar is written) when testing stdout. @Test
diff --git a/src/test/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunnerTest.java index 54b7cb0..ddc6581 100644 --- a/src/test/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunnerTest.java +++ b/src/test/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunnerTest.java
@@ -29,7 +29,6 @@ import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.SpawnResult; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; -import com.google.devtools.build.lib.events.EventKind; import com.google.devtools.build.lib.exec.BinTools; import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext; import com.google.devtools.build.lib.exec.TreeDeleter; @@ -82,7 +81,7 @@ } @Test - public void exec_failingCommand_emitsSandboxDebugSuggestionInfoEvent() throws Exception { + public void exec_failingCommand_doesNotEmitSandboxDebugSuggestionInfoEvent() throws Exception { CommandEnvironment commandEnvironment = createCommandEnvironment(); LinuxSandboxedSpawnRunner runner = setupSandboxAndCreateRunner(commandEnvironment); Spawn spawn = new SpawnBuilder("false").build(); @@ -94,8 +93,7 @@ assertThat(spawnResult.exitCode()).isEqualTo(1); assertThat(spawnResult.getFailureMessage()) .doesNotContain("Use --sandbox_debug to see verbose messages from the sandbox"); - assertContainsEvent( - EventKind.INFO, "Use --sandbox_debug to see verbose messages from the sandbox"); + assertDoesNotContainEvent("Use --sandbox_debug to see verbose messages from the sandbox"); } @Test