build runner: remove --prominent-compile-errors, introduce --error-style

The new `--error-style` option decides how build failures are printed.
The default mode "verbose" prints all context including the step graph
fragment and the failed command (if any). The alternative mode "minimal"
prints only the failed step itself, and does not print the failed
command. There are also "verbose_clear" and "minimal_clear" modes, which
have the distinction that the output is cleared (through ANSI escape
codes) between updates, preventing different updates from being confused
in the output. If `--error-style` is not specified, the environment
variable `ZIG_BUILD_ERROR_STYLE` is checked before falling back to the
default of "verbose"; this means the value can effectively be chosen
system-wide since it is generally a personal preference.

Also introduced is a `--multiline-errors` option which decides how to
print errors which span multiple lines. By default, non-initial lines
are indented to align with the first. Alternatively, a leading newline
can be printed to align everyting on the first column, or no special
treatment can be applied, resulting in misaligned output. Again, there
is an environment variable (`ZIG_BUILD_MULTILINE_ERRORS`) to specify a
preferred default if the option is not explicitly provided.

Resolves: #23472

lib/compiler/build_runner.zig

@@ -103,12 +103,13 @@ pub fn main() !void {
 
     var install_prefix: ?[]const u8 = null;
     var dir_list = std.Build.DirList{};
+    var error_style: ErrorStyle = .verbose;
+    var multiline_errors: MultilineErrors = .indent;
     var summary: ?Summary = null;
     var max_rss: u64 = 0;
     var skip_oom_steps = false;
     var test_timeout_ms: ?u64 = null;
     var color: Color = .auto;
-    var prominent_compile_errors = false;
     var help_menu = false;
     var steps_menu = false;
     var output_tmp_nonce: ?[16]u8 = null;
@@ -117,6 +118,18 @@ pub fn main() !void {
     var debounce_interval_ms: u16 = 50;
     var webui_listen: ?std.net.Address = null;
 
+    if (try std.zig.EnvVar.ZIG_BUILD_ERROR_STYLE.get(arena)) |str| {
+        if (std.meta.stringToEnum(ErrorStyle, str)) |style| {
+            error_style = style;
+        }
+    }
+
+    if (try std.zig.EnvVar.ZIG_BUILD_MULTILINE_ERRORS.get(arena)) |str| {
+        if (std.meta.stringToEnum(MultilineErrors, str)) |style| {
+            multiline_errors = style;
+        }
+    }
+
     while (nextArg(args, &arg_idx)) |arg| {
         if (mem.startsWith(u8, arg, "-Z")) {
             if (arg.len != 18) fatalWithHint("bad argument: '{s}'", .{arg});
@@ -197,11 +210,23 @@ pub fn main() !void {
                         arg, next_arg,
                     });
                 };
+            } else if (mem.eql(u8, arg, "--error-style")) {
+                const next_arg = nextArg(args, &arg_idx) orelse
+                    fatalWithHint("expected style after '{s}'", .{arg});
+                error_style = std.meta.stringToEnum(ErrorStyle, next_arg) orelse {
+                    fatalWithHint("expected style after '{s}', found '{s}'", .{ arg, next_arg });
+                };
+            } else if (mem.eql(u8, arg, "--multiline-errors")) {
+                const next_arg = nextArg(args, &arg_idx) orelse
+                    fatalWithHint("expected style after '{s}'", .{arg});
+                multiline_errors = std.meta.stringToEnum(MultilineErrors, next_arg) orelse {
+                    fatalWithHint("expected style after '{s}', found '{s}'", .{ arg, next_arg });
+                };
             } else if (mem.eql(u8, arg, "--summary")) {
                 const next_arg = nextArg(args, &arg_idx) orelse
-                    fatalWithHint("expected [all|new|failures|none] after '{s}'", .{arg});
+                    fatalWithHint("expected [all|new|failures|line|none] after '{s}'", .{arg});
                 summary = std.meta.stringToEnum(Summary, next_arg) orelse {
-                    fatalWithHint("expected [all|new|failures|none] after '{s}', found '{s}'", .{
+                    fatalWithHint("expected [all|new|failures|line|none] after '{s}', found '{s}'", .{
                         arg, next_arg,
                     });
                 };
@@ -273,8 +298,6 @@ pub fn main() !void {
                 builder.verbose_cc = true;
             } else if (mem.eql(u8, arg, "--verbose-llvm-cpu-features")) {
                 builder.verbose_llvm_cpu_features = true;
-            } else if (mem.eql(u8, arg, "--prominent-compile-errors")) {
-                prominent_compile_errors = true;
             } else if (mem.eql(u8, arg, "--watch")) {
                 watch = true;
             } else if (mem.eql(u8, arg, "--time-report")) {
@@ -466,10 +489,11 @@ pub fn main() !void {
         .web_server = undefined, // set after `prepare`
         .memory_blocked_steps = .empty,
         .step_stack = .empty,
-        .prominent_compile_errors = prominent_compile_errors,
 
         .claimed_rss = 0,
-        .summary = summary orelse if (watch) .new else .failures,
+        .error_style = error_style,
+        .multiline_errors = multiline_errors,
+        .summary = summary orelse if (watch or webui_listen != null) .line else .failures,
         .ttyconf = ttyconf,
         .stderr = stderr,
         .thread_pool = undefined,
@@ -485,8 +509,14 @@ pub fn main() !void {
     }
 
     prepare(arena, builder, targets.items, &run, graph.random_seed) catch |err| switch (err) {
-        error.UncleanExit => process.exit(1),
+        error.DependencyLoopDetected => {
-        else => return err,
+            // Perhaps in the future there could be an Advanced Options flag such as
+            // --debug-build-runner-leaks which would make this code return instead of
+            // calling exit.
+            std.debug.lockStdErr();
+            process.exit(1);
+        },
+        else => |e| return e,
     };
 
     var w: Watch = w: {
@@ -516,22 +546,20 @@ pub fn main() !void {
         ws.start() catch |err| fatal("failed to start web server: {s}", .{@errorName(err)});
     }
 
-    rebuild: while (true) {
+    rebuild: while (true) : (if (run.error_style.clearOnUpdate()) {
+        const bw = std.debug.lockStderrWriter(&stdio_buffer_allocation);
+        defer std.debug.unlockStderrWriter();
+        try bw.writeAll("\x1B[2J\x1B[3J\x1B[H");
+    }) {
         if (run.web_server) |*ws| ws.startBuild();
 
-        runStepNames(
+        try runStepNames(
             builder,
             targets.items,
             main_progress_node,
             &run,
             fuzz,
-        ) catch |err| switch (err) {
+        );
-            error.UncleanExit => {
-                assert(!run.watch and run.web_server == null);
-                process.exit(1);
-            },
-            else => return err,
-        };
 
         if (run.web_server) |*web_server| {
             if (fuzz) |mode| if (mode != .forever) fatal(
@@ -542,10 +570,6 @@ pub fn main() !void {
             web_server.finishBuild(.{ .fuzz = fuzz != null });
         }
 
-        if (!watch and run.web_server == null) {
-            return cleanExit();
-        }
-
         if (run.web_server) |*ws| {
             assert(!watch); // fatal error after CLI parsing
             while (true) switch (ws.wait()) {
@@ -626,18 +650,14 @@ const Run = struct {
     memory_blocked_steps: std.ArrayListUnmanaged(*Step),
     /// Allocated into `gpa`.
     step_stack: std.AutoArrayHashMapUnmanaged(*Step, void),
-    prominent_compile_errors: bool,
     thread_pool: std.Thread.Pool,
 
     claimed_rss: usize,
+    error_style: ErrorStyle,
+    multiline_errors: MultilineErrors,
     summary: Summary,
     ttyconf: tty.Config,
     stderr: File,
-
-    fn cleanExit(run: Run) void {
-        if (run.watch or run.web_server != null) return;
-        return runner.cleanExit();
-    }
 };
 
 fn prepare(
@@ -671,10 +691,7 @@ fn prepare(
     rand.shuffle(*Step, starting_steps);
 
     for (starting_steps) |s| {
-        constructGraphAndCheckForDependencyLoop(gpa, b, s, &run.step_stack, rand) catch |err| switch (err) {
+        try constructGraphAndCheckForDependencyLoop(gpa, b, s, &run.step_stack, rand);
-            error.DependencyLoopDetected => return uncleanExit(),
-            else => |e| return e,
-        };
     }
 
     {
@@ -827,26 +844,25 @@ fn runStepNames(
     // Every test has a state
     assert(test_pass_count + test_skip_count + test_fail_count + test_crash_count + test_timeout_count == test_count);
 
-    // A proper command line application defaults to silently succeeding.
-    // The user may request verbose mode if they have a different preference.
-    const failures_only = switch (run.summary) {
-        .failures, .none => true,
-        else => false,
-    };
     if (failure_count == 0) {
         std.Progress.setStatus(.success);
-        if (failures_only) return run.cleanExit();
     } else {
         std.Progress.setStatus(.failure);
     }
 
-    if (run.summary != .none) {
+    summary: {
+        switch (run.summary) {
+            .all, .new, .line => {},
+            .failures => if (failure_count == 0) break :summary,
+            .none => break :summary,
+        }
+
         const w = std.debug.lockStderrWriter(&stdio_buffer_allocation);
         defer std.debug.unlockStderrWriter();
 
         const total_count = success_count + failure_count + pending_count + skipped_count;
         ttyconf.setColor(w, .cyan) catch {};
-        w.writeAll("\nBuild Summary:") catch {};
+        w.writeAll("Build Summary:") catch {};
         ttyconf.setColor(w, .reset) catch {};
         w.print(" {d}/{d} steps succeeded", .{ success_count, total_count }) catch {};
         if (skipped_count > 0) w.print(", {d} skipped", .{skipped_count}) catch {};
@@ -862,6 +878,8 @@ fn runStepNames(
 
         w.writeAll("\n") catch {};
 
+        if (run.summary == .line) break :summary;
+
         // Print a fancy tree with build results.
         var step_stack_copy = try step_stack.clone(gpa);
         defer step_stack_copy.deinit(gpa);
@@ -877,7 +895,7 @@ fn runStepNames(
                     i -= 1;
                     const step = b.top_level_steps.get(step_names[i]).?.step;
                     const found = switch (run.summary) {
-                        .all, .none => unreachable,
+                        .all, .line, .none => unreachable,
                         .failures => step.state != .success,
                         .new => !step.result_cached,
                     };
@@ -894,28 +912,19 @@ fn runStepNames(
         w.writeByte('\n') catch {};
     }
 
-    if (failure_count == 0) {
+    if (run.watch or run.web_server != null) return;
-        return run.cleanExit();
-    }
 
-    // Finally, render compile errors at the bottom of the terminal.
+    // Perhaps in the future there could be an Advanced Options flag such as
-    if (run.prominent_compile_errors and total_compile_errors > 0) {
+    // --debug-build-runner-leaks which would make this code return instead of
-        for (step_stack.keys()) |s| {
+    // calling exit.
-            if (s.result_error_bundle.errorMessageCount() > 0) {
-                s.result_error_bundle.renderToStdErr(.{ .ttyconf = ttyconf });
-            }
-        }
 
-        if (!run.watch and run.web_server == null) {
+    const code: u8 = code: {
-            // Signal to parent process that we have printed compile errors. The
+        if (failure_count == 0) break :code 0; // success
-            // parent process may choose to omit the "following command failed"
+        if (run.error_style.verboseContext()) break :code 1; // failure; print build command
-            // line in this case.
+        break :code 2; // failure; do not print build command
+    };
     std.debug.lockStdErr();
-            process.exit(2);
+    process.exit(code);
-        }
-    }
-
-    if (!run.watch and run.web_server == null) return uncleanExit();
 }
 
 const PrintNode = struct {
@@ -1124,7 +1133,7 @@ fn printTreeStep(
     const first = step_stack.swapRemove(s);
     const summary = run.summary;
     const skip = switch (summary) {
-        .none => unreachable,
+        .none, .line => unreachable,
         .all => false,
         .new => s.result_cached,
         .failures => s.state == .success,
@@ -1157,7 +1166,7 @@ fn printTreeStep(
 
                 const step = s.dependencies.items[i];
                 const found = switch (summary) {
-                    .all, .none => unreachable,
+                    .all, .line, .none => unreachable,
                     .failures => step.state != .success,
                     .new => !step.result_cached,
                 };
@@ -1316,15 +1325,13 @@ fn workerMakeOneStep(
     });
 
     // No matter the result, we want to display error/warning messages.
-    const show_compile_errors = !run.prominent_compile_errors and
+    const show_compile_errors = s.result_error_bundle.errorMessageCount() > 0;
-        s.result_error_bundle.errorMessageCount() > 0;
     const show_error_msgs = s.result_error_msgs.items.len > 0;
     const show_stderr = s.result_stderr.len > 0;
-
     if (show_error_msgs or show_compile_errors or show_stderr) {
         const bw = std.debug.lockStderrWriter(&stdio_buffer_allocation);
         defer std.debug.unlockStderrWriter();
-        printErrorMessages(run.gpa, s, .{ .ttyconf = run.ttyconf }, bw, run.prominent_compile_errors) catch {};
+        printErrorMessages(run.gpa, s, .{ .ttyconf = run.ttyconf }, bw, run.error_style, run.multiline_errors) catch {};
     }
 
     handle_result: {
@@ -1388,11 +1395,14 @@ pub fn printErrorMessages(
     failing_step: *Step,
     options: std.zig.ErrorBundle.RenderOptions,
     stderr: *Writer,
-    prominent_compile_errors: bool,
+    error_style: ErrorStyle,
+    multiline_errors: MultilineErrors,
 ) !void {
+    const ttyconf = options.ttyconf;
+
+    if (error_style.verboseContext()) {
         // Provide context for where these error messages are coming from by
         // printing the corresponding Step subtree.
-
         var step_stack: std.ArrayListUnmanaged(*Step) = .empty;
         defer step_stack.deinit(gpa);
         try step_stack.append(gpa, failing_step);
@@ -1401,7 +1411,6 @@ pub fn printErrorMessages(
         }
 
         // Now, `step_stack` has the subtree that we want to print, in reverse order.
-    const ttyconf = options.ttyconf;
         try ttyconf.setColor(stderr, .dim);
         var indent: usize = 0;
         while (step_stack.pop()) |s| : (indent += 1) {
@@ -1419,6 +1428,13 @@ pub fn printErrorMessages(
             }
         }
         try ttyconf.setColor(stderr, .reset);
+    } else {
+        // Just print the failing step itself.
+        try ttyconf.setColor(stderr, .dim);
+        try stderr.writeAll(failing_step.name);
+        try printStepFailure(failing_step, stderr, ttyconf);
+        try ttyconf.setColor(stderr, .reset);
+    }
 
     if (failing_step.result_stderr.len > 0) {
         try stderr.writeAll(failing_step.result_stderr);
@@ -1427,23 +1443,28 @@ pub fn printErrorMessages(
         }
     }
 
-    if (!prominent_compile_errors and failing_step.result_error_bundle.errorMessageCount() > 0) {
     try failing_step.result_error_bundle.renderToWriter(options, stderr);
-    }
 
     for (failing_step.result_error_msgs.items) |msg| {
         try ttyconf.setColor(stderr, .red);
         try stderr.writeAll("error:");
         try ttyconf.setColor(stderr, .reset);
-        // If the message has multiple lines, indent the non-initial ones to align them with the 'error:' text.
+        if (std.mem.indexOfScalar(u8, msg, '\n') == null) {
+            try stderr.print(" {s}\n", .{msg});
+        } else switch (multiline_errors) {
+            .indent => {
                 var it = std.mem.splitScalar(u8, msg, '\n');
-        try stderr.writeAll(it.first());
+                try stderr.print(" {s}\n", .{it.first()});
                 while (it.next()) |line| {
-            try stderr.print("\n       {s}", .{line});
+                    try stderr.print("       {s}\n", .{line});
+                }
+            },
+            .newline => try stderr.print("\n{s}\n", .{msg}),
+            .none => try stderr.print(" {s}\n", .{msg}),
         }
-        try stderr.writeAll("\n");
     }
 
+    if (error_style.verboseContext()) {
         if (failing_step.result_failed_command) |cmd_str| {
             try ttyconf.setColor(stderr, .red);
             try stderr.writeAll("failed command: ");
@@ -1453,6 +1474,9 @@ pub fn printErrorMessages(
         }
     }
 
+    try stderr.writeByte('\n');
+}
+
 fn printSteps(builder: *std.Build, w: *Writer) !void {
     const arena = builder.graph.arena;
     for (builder.top_level_steps.values()) |top_level_step| {
@@ -1505,11 +1529,20 @@ fn printUsage(b: *std.Build, w: *Writer) !void {
         \\  -l, --list-steps             Print available steps
         \\  --verbose                    Print commands before executing them
         \\  --color [auto|off|on]        Enable or disable colored error messages
-        \\  --prominent-compile-errors   Buffer compile errors and display at end
+        \\  --error-style [style]        Control how build errors are printed
+        \\    verbose                    (Default) Report errors with full context
+        \\    minimal                    Report errors after summary, excluding context like command lines
+        \\    verbose_clear              Like 'verbose', but clear the terminal at the start of each update
+        \\    minimal_clear              Like 'minimal', but clear the terminal at the start of each update
+        \\  --multiline-errors [style]   Control how multi-line error messages are printed
+        \\    indent                     (Default) Indent non-initial lines to align with initial line
+        \\    newline                    Include a leading newline so that the error message is on its own lines
+        \\    none                       Print as usual so the first line is misaligned
         \\  --summary [mode]             Control the printing of the build summary
         \\    all                        Print the build summary in its entirety
         \\    new                        Omit cached steps
-        \\    failures                   (Default) Only print failed steps
+        \\    failures                   (Default if short-lived) Only print failed steps
+        \\    line                       (Default if long-lived) Only print the single-line summary
         \\    none                       Do not print the build summary
         \\  -j<N>                        Limit concurrent jobs (default is to use all CPU cores)
         \\  --maxrss <bytes>             Limit memory usage (default is to use available memory)
@@ -1633,24 +1666,27 @@ fn argsRest(args: []const [:0]const u8, idx: usize) ?[]const [:0]const u8 {
     return args[idx..];
 }
 
-/// Perhaps in the future there could be an Advanced Options flag such as
-/// --debug-build-runner-leaks which would make this function return instead of
-/// calling exit.
-fn cleanExit() void {
-    std.debug.lockStdErr();
-    process.exit(0);
-}
-
-/// Perhaps in the future there could be an Advanced Options flag such as
-/// --debug-build-runner-leaks which would make this function return instead of
-/// calling exit.
-fn uncleanExit() error{UncleanExit} {
-    std.debug.lockStdErr();
-    process.exit(1);
-}
-
 const Color = std.zig.Color;
-const Summary = enum { all, new, failures, none };
+const ErrorStyle = enum {
+    verbose,
+    minimal,
+    verbose_clear,
+    minimal_clear,
+    fn verboseContext(s: ErrorStyle) bool {
+        return switch (s) {
+            .verbose, .verbose_clear => true,
+            .minimal, .minimal_clear => false,
+        };
+    }
+    fn clearOnUpdate(s: ErrorStyle) bool {
+        return switch (s) {
+            .verbose, .minimal => false,
+            .verbose_clear, .minimal_clear => true,
+        };
+    }
+};
+const MultilineErrors = enum { indent, newline, none };
+const Summary = enum { all, new, failures, line, none };
 
 fn get_tty_conf(color: Color, stderr: File) tty.Config {
     return switch (color) {

lib/std/Build/Fuzz.zig

@@ -178,7 +178,7 @@ fn rebuildTestsWorkerRunFallible(run: *Step.Run, gpa: Allocator, ttyconf: std.Io
         var buf: [256]u8 = undefined;
         const w = std.debug.lockStderrWriter(&buf);
         defer std.debug.unlockStderrWriter();
-        build_runner.printErrorMessages(gpa, &compile.step, .{ .ttyconf = ttyconf }, w, false) catch {};
+        build_runner.printErrorMessages(gpa, &compile.step, .{ .ttyconf = ttyconf }, w, .verbose, .indent) catch {};
     }
 
     const rebuilt_bin_path = result catch |err| switch (err) {
@@ -204,7 +204,7 @@ fn fuzzWorkerRun(
             var buf: [256]u8 = undefined;
             const w = std.debug.lockStderrWriter(&buf);
             defer std.debug.unlockStderrWriter();
-            build_runner.printErrorMessages(gpa, &run.step, .{ .ttyconf = fuzz.ttyconf }, w, false) catch {};
+            build_runner.printErrorMessages(gpa, &run.step, .{ .ttyconf = fuzz.ttyconf }, w, .verbose, .indent) catch {};
             return;
         },
         else => {

lib/std/zig.zig

@@ -697,6 +697,8 @@ pub const EnvVar = enum {
     ZIG_LIB_DIR,
     ZIG_LIBC,
     ZIG_BUILD_RUNNER,
+    ZIG_BUILD_ERROR_STYLE,
+    ZIG_BUILD_MULTILINE_ERRORS,
     ZIG_VERBOSE_LINK,
     ZIG_VERBOSE_CC,
     ZIG_BTRFS_WORKAROUND,