Commit 7cc4a6965c

Andrew Kelley <andrew@ziglang.org>
2023-03-10 02:13:55
build runner enhancements in preparation for test-cases
* std.zig.ErrorBundle: support rendering options for whether to include the reference trace, whether to include the source line, and TTY configuration. * build runner: don't print progress in dumb terminals * std.Build.CompileStep: - add a way to expect compilation errors via the new `expect_errors` field. This is an advanced setting that can change the intent of the CompileStep. If this slice has nonzero length, it means that the CompileStep exists to check for compile errors and return *success* if they match, and failure otherwise. - remove the object format parameter from `checkObject`. The object format is known based on the CompileStep's target. - Avoid passing -L and -I flags for nonexistent directories within search_prefixes. This prevents a warning, that should probably be upgraded to an error in Zig's CLI parsing code, when the linker sees an -L directory that does not exist. * std.Build.Step: - When spawning the zig compiler process, takes advantage of the new `std.Progress.Node.setName` API to avoid ticking up a meaningless number at every progress update.
1 parent 3186658
lib/std/Build/CompileStep.zig
@@ -207,6 +207,12 @@ want_lto: ?bool = null,
 use_llvm: ?bool = null,
 use_lld: ?bool = null,
 
+/// This is an advanced setting that can change the intent of this CompileStep.
+/// If this slice has nonzero length, it means that this CompileStep exists to
+/// check for compile errors and return *success* if they match, and failure
+/// otherwise.
+expect_errors: []const []const u8 = &.{},
+
 output_path_source: GeneratedFile,
 output_lib_path_source: GeneratedFile,
 output_h_path_source: GeneratedFile,
@@ -552,8 +558,8 @@ pub fn run(cs: *CompileStep) *RunStep {
     return cs.step.owner.addRunArtifact(cs);
 }
 
-pub fn checkObject(self: *CompileStep, obj_format: std.Target.ObjectFormat) *CheckObjectStep {
-    return CheckObjectStep.create(self.step.owner, self.getOutputSource(), obj_format);
+pub fn checkObject(self: *CompileStep) *CheckObjectStep {
+    return CheckObjectStep.create(self.step.owner, self.getOutputSource(), self.target_info.target.ofmt);
 }
 
 pub fn setLinkerScriptPath(self: *CompileStep, source: FileSource) void {
@@ -1838,14 +1844,38 @@ fn make(step: *Step, prog_node: *std.Progress.Node) !void {
     }
 
     for (b.search_prefixes.items) |search_prefix| {
-        try zig_args.append("-L");
-        try zig_args.append(b.pathJoin(&.{
-            search_prefix, "lib",
-        }));
-        try zig_args.append("-I");
-        try zig_args.append(b.pathJoin(&.{
-            search_prefix, "include",
-        }));
+        var prefix_dir = fs.cwd().openDir(search_prefix, .{}) catch |err| {
+            return step.fail("unable to open prefix directory '{s}': {s}", .{
+                search_prefix, @errorName(err),
+            });
+        };
+        defer prefix_dir.close();
+
+        // Avoid passing -L and -I flags for nonexistent directories.
+        // This prevents a warning, that should probably be upgraded to an error in Zig's
+        // CLI parsing code, when the linker sees an -L directory that does not exist.
+
+        if (prefix_dir.accessZ("lib", .{})) |_| {
+            try zig_args.appendSlice(&.{
+                "-L", try fs.path.join(b.allocator, &.{ search_prefix, "lib" }),
+            });
+        } else |err| switch (err) {
+            error.FileNotFound => {},
+            else => |e| return step.fail("unable to access '{s}/lib' directory: {s}", .{
+                search_prefix, @errorName(e),
+            }),
+        }
+
+        if (prefix_dir.accessZ("include", .{})) |_| {
+            try zig_args.appendSlice(&.{
+                "-I", try fs.path.join(b.allocator, &.{ search_prefix, "include" }),
+            });
+        } else |err| switch (err) {
+            error.FileNotFound => {},
+            else => |e| return step.fail("unable to access '{s}/include' directory: {s}", .{
+                search_prefix, @errorName(e),
+            }),
+        }
     }
 
     try addFlag(&zig_args, "valgrind", self.valgrind_support);
@@ -1943,7 +1973,14 @@ fn make(step: *Step, prog_node: *std.Progress.Node) !void {
         try zig_args.append(resolved_args_file);
     }
 
-    const output_bin_path = try step.evalZigProcess(zig_args.items, prog_node);
+    const output_bin_path = step.evalZigProcess(zig_args.items, prog_node) catch |err| switch (err) {
+        error.NeedCompileErrorCheck => {
+            assert(self.expect_errors.len != 0);
+            try checkCompileErrors(self);
+            return;
+        },
+        else => |e| return e,
+    };
     const build_output_dir = fs.path.dirname(output_bin_path).?;
 
     if (self.output_dir) |output_dir| {
@@ -2178,3 +2215,57 @@ const TransitiveDeps = struct {
         }
     }
 };
+
+fn checkCompileErrors(self: *CompileStep) !void {
+    // Clear this field so that it does not get printed by the build runner.
+    const actual_eb = self.step.result_error_bundle;
+    self.step.result_error_bundle = std.zig.ErrorBundle.empty;
+
+    const arena = self.step.owner.allocator;
+
+    var actual_stderr_list = std.ArrayList(u8).init(arena);
+    try actual_eb.renderToWriter(.{
+        .ttyconf = .no_color,
+        .include_reference_trace = false,
+        .include_source_line = false,
+    }, actual_stderr_list.writer());
+    const actual_stderr = try actual_stderr_list.toOwnedSlice();
+
+    // Render the expected lines into a string that we can compare verbatim.
+    var expected_generated = std.ArrayList(u8).init(arena);
+
+    var actual_line_it = mem.split(u8, actual_stderr, "\n");
+    for (self.expect_errors) |expect_line| {
+        const actual_line = actual_line_it.next() orelse {
+            try expected_generated.appendSlice(expect_line);
+            try expected_generated.append('\n');
+            continue;
+        };
+        if (mem.endsWith(u8, actual_line, expect_line)) {
+            try expected_generated.appendSlice(actual_line);
+            try expected_generated.append('\n');
+            continue;
+        }
+        if (mem.startsWith(u8, expect_line, ":?:?: ")) {
+            if (mem.endsWith(u8, actual_line, expect_line[":?:?: ".len..])) {
+                try expected_generated.appendSlice(actual_line);
+                try expected_generated.append('\n');
+                continue;
+            }
+        }
+        try expected_generated.appendSlice(expect_line);
+        try expected_generated.append('\n');
+    }
+
+    if (mem.eql(u8, expected_generated.items, actual_stderr)) return;
+
+    // TODO merge this with the testing.expectEqualStrings logic, and also CheckFile
+    return self.step.fail(
+        \\
+        \\========= expected: =====================
+        \\{s}
+        \\========= but found: ====================
+        \\{s}
+        \\=========================================
+    , .{ expected_generated.items, actual_stderr });
+}
lib/std/Build/Step.zig
@@ -295,8 +295,8 @@ pub fn evalZigProcess(
 
     var node_name: std.ArrayListUnmanaged(u8) = .{};
     defer node_name.deinit(gpa);
-    var sub_prog_node: ?std.Progress.Node = null;
-    defer if (sub_prog_node) |*n| n.end();
+    var sub_prog_node = prog_node.start("", 0);
+    defer sub_prog_node.end();
 
     const stdout = poller.fifo(.stdout);
 
@@ -336,11 +336,9 @@ pub fn evalZigProcess(
                     };
                 },
                 .progress => {
-                    if (sub_prog_node) |*n| n.end();
                     node_name.clearRetainingCapacity();
                     try node_name.appendSlice(gpa, body);
-                    sub_prog_node = prog_node.start(node_name.items, 0);
-                    sub_prog_node.?.activate();
+                    sub_prog_node.setName(node_name.items);
                 },
                 .emit_bin_path => {
                     const EbpHdr = std.zig.Server.Message.EmitBinPath;
@@ -371,6 +369,18 @@ pub fn evalZigProcess(
     s.result_duration_ns = timer.read();
     s.result_peak_rss = child.resource_usage_statistics.getMaxRss() orelse 0;
 
+    // Special handling for CompileStep that is expecting compile errors.
+    if (s.cast(Build.CompileStep)) |compile| switch (term) {
+        .Exited => {
+            // Note that the exit code may be 0 in this case due to the
+            // compiler server protocol.
+            if (compile.expect_errors.len != 0 and s.result_error_bundle.errorMessageCount() > 0) {
+                return error.NeedCompileErrorCheck;
+            }
+        },
+        else => {},
+    };
+
     try handleChildProcessTerm(s, term, null, argv);
 
     if (s.result_error_bundle.errorMessageCount() > 0) {
lib/std/zig/ErrorBundle.zig
@@ -141,32 +141,35 @@ pub fn nullTerminatedString(eb: ErrorBundle, index: usize) [:0]const u8 {
     return string_bytes[index..end :0];
 }
 
-pub fn renderToStdErr(eb: ErrorBundle, ttyconf: std.debug.TTY.Config) void {
+pub const RenderOptions = struct {
+    ttyconf: std.debug.TTY.Config,
+    include_reference_trace: bool = true,
+    include_source_line: bool = true,
+};
+
+pub fn renderToStdErr(eb: ErrorBundle, options: RenderOptions) void {
     std.debug.getStderrMutex().lock();
     defer std.debug.getStderrMutex().unlock();
     const stderr = std.io.getStdErr();
-    return renderToWriter(eb, ttyconf, stderr.writer()) catch return;
+    return renderToWriter(eb, options, stderr.writer()) catch return;
 }
 
-pub fn renderToWriter(
-    eb: ErrorBundle,
-    ttyconf: std.debug.TTY.Config,
-    writer: anytype,
-) anyerror!void {
+pub fn renderToWriter(eb: ErrorBundle, options: RenderOptions, writer: anytype) anyerror!void {
     for (eb.getMessages()) |err_msg| {
-        try renderErrorMessageToWriter(eb, err_msg, ttyconf, writer, "error", .Red, 0);
+        try renderErrorMessageToWriter(eb, options, err_msg, writer, "error", .Red, 0);
     }
 }
 
 fn renderErrorMessageToWriter(
     eb: ErrorBundle,
+    options: RenderOptions,
     err_msg_index: MessageIndex,
-    ttyconf: std.debug.TTY.Config,
     stderr: anytype,
     kind: []const u8,
     color: std.debug.TTY.Color,
     indent: usize,
 ) anyerror!void {
+    const ttyconf = options.ttyconf;
     var counting_writer = std.io.countingWriter(stderr);
     const counting_stderr = counting_writer.writer();
     const err_msg = eb.getErrorMessage(err_msg_index);
@@ -196,7 +199,7 @@ fn renderErrorMessageToWriter(
             try stderr.print(" ({d} times)\n", .{err_msg.count});
         }
         try ttyconf.setColor(stderr, .Reset);
-        if (src.data.source_line != 0) {
+        if (src.data.source_line != 0 and options.include_source_line) {
             const line = eb.nullTerminatedString(src.data.source_line);
             for (line) |b| switch (b) {
                 '\t' => try stderr.writeByte(' '),
@@ -216,9 +219,9 @@ fn renderErrorMessageToWriter(
             try ttyconf.setColor(stderr, .Reset);
         }
         for (eb.getNotes(err_msg_index)) |note| {
-            try renderErrorMessageToWriter(eb, note, ttyconf, stderr, "note", .Cyan, indent);
+            try renderErrorMessageToWriter(eb, options, note, stderr, "note", .Cyan, indent);
         }
-        if (src.data.reference_trace_len > 0) {
+        if (src.data.reference_trace_len > 0 and options.include_reference_trace) {
             try ttyconf.setColor(stderr, .Reset);
             try ttyconf.setColor(stderr, .Dim);
             try stderr.print("referenced by:\n", .{});
@@ -266,7 +269,7 @@ fn renderErrorMessageToWriter(
         }
         try ttyconf.setColor(stderr, .Reset);
         for (eb.getNotes(err_msg_index)) |note| {
-            try renderErrorMessageToWriter(eb, note, ttyconf, stderr, "note", .Cyan, indent + 4);
+            try renderErrorMessageToWriter(eb, options, note, stderr, "note", .Cyan, indent + 4);
         }
     }
 }
lib/build_runner.zig
@@ -84,8 +84,6 @@ pub fn main() !void {
     );
     defer builder.destroy();
 
-    const Color = enum { auto, off, on };
-
     var targets = ArrayList([]const u8).init(arena);
     var debug_log_scopes = ArrayList([]const u8).init(arena);
     var thread_pool_options: std.Thread.Pool.Options = .{ .allocator = arena };
@@ -273,13 +271,9 @@ pub fn main() !void {
     }
 
     const stderr = std.io.getStdErr();
-    const ttyconf: std.debug.TTY.Config = switch (color) {
-        .auto => std.debug.detectTTYConfig(stderr),
-        .on => .escape_codes,
-        .off => .no_color,
-    };
+    const ttyconf = get_tty_conf(color, stderr);
 
-    var progress: std.Progress = .{};
+    var progress: std.Progress = .{ .dont_print_on_dumb = true };
     const main_progress_node = progress.start("", 0);
 
     builder.debug_log_scopes = debug_log_scopes.items;
@@ -498,7 +492,7 @@ fn runStepNames(
     if (total_compile_errors > 0) {
         for (compile_error_steps.items) |s| {
             if (s.result_error_bundle.errorMessageCount() > 0) {
-                s.result_error_bundle.renderToStdErr(ttyconf);
+                s.result_error_bundle.renderToStdErr(renderOptions(ttyconf));
             }
         }
 
@@ -961,3 +955,21 @@ fn cleanExit() void {
     // of calling exit.
     process.exit(0);
 }
+
+const Color = enum { auto, off, on };
+
+fn get_tty_conf(color: Color, stderr: std.fs.File) std.debug.TTY.Config {
+    return switch (color) {
+        .auto => std.debug.detectTTYConfig(stderr),
+        .on => .escape_codes,
+        .off => .no_color,
+    };
+}
+
+fn renderOptions(ttyconf: std.debug.TTY.Config) std.zig.ErrorBundle.RenderOptions {
+    return .{
+        .ttyconf = ttyconf,
+        .include_source_line = ttyconf != .no_color,
+        .include_reference_trace = ttyconf != .no_color,
+    };
+}
src/main.zig
@@ -4082,7 +4082,7 @@ fn updateModule(gpa: Allocator, comp: *Compilation, hook: AfterUpdateHook) !void
     defer errors.deinit(comp.gpa);
 
     if (errors.errorMessageCount() > 0) {
-        errors.renderToStdErr(get_tty_conf(comp.color));
+        errors.renderToStdErr(renderOptions(comp.color));
         const log_text = comp.getCompileLogOutput();
         if (log_text.len != 0) {
             std.debug.print("\nCompile Log Output:\n{s}", .{log_text});
@@ -4711,7 +4711,7 @@ pub fn cmdBuild(gpa: Allocator, arena: Allocator, args: []const []const u8) !voi
             if (wip_errors.root_list.items.len > 0) {
                 var errors = try wip_errors.toOwnedBundle();
                 defer errors.deinit(gpa);
-                errors.renderToStdErr(get_tty_conf(color));
+                errors.renderToStdErr(renderOptions(color));
                 process.exit(1);
             }
             try fetch_result;
@@ -4974,7 +4974,7 @@ pub fn cmdFmt(gpa: Allocator, arena: Allocator, args: []const []const u8) !void
                 try Compilation.addZirErrorMessages(&wip_errors, &file);
                 var error_bundle = try wip_errors.toOwnedBundle();
                 defer error_bundle.deinit(gpa);
-                error_bundle.renderToStdErr(get_tty_conf(color));
+                error_bundle.renderToStdErr(renderOptions(color));
                 process.exit(2);
             }
         } else if (tree.errors.len != 0) {
@@ -5180,7 +5180,7 @@ fn fmtPathFile(
             try Compilation.addZirErrorMessages(&wip_errors, &file);
             var error_bundle = try wip_errors.toOwnedBundle();
             defer error_bundle.deinit(gpa);
-            error_bundle.renderToStdErr(get_tty_conf(fmt.color));
+            error_bundle.renderToStdErr(renderOptions(fmt.color));
             fmt.any_error = true;
         }
     }
@@ -5217,7 +5217,7 @@ fn printAstErrorsToStderr(gpa: Allocator, tree: Ast, path: []const u8, color: Co
 
     var error_bundle = try wip_errors.toOwnedBundle();
     defer error_bundle.deinit(gpa);
-    error_bundle.renderToStdErr(get_tty_conf(color));
+    error_bundle.renderToStdErr(renderOptions(color));
 }
 
 pub fn putAstErrorsIntoBundle(
@@ -5834,7 +5834,7 @@ pub fn cmdAstCheck(
         try Compilation.addZirErrorMessages(&wip_errors, &file);
         var error_bundle = try wip_errors.toOwnedBundle();
         defer error_bundle.deinit(gpa);
-        error_bundle.renderToStdErr(get_tty_conf(color));
+        error_bundle.renderToStdErr(renderOptions(color));
         process.exit(1);
     }
 
@@ -5892,6 +5892,7 @@ pub fn cmdChangelist(
     arena: Allocator,
     args: []const []const u8,
 ) !void {
+    const color: Color = .auto;
     const Zir = @import("Zir.zig");
 
     const old_source_file = args[0];
@@ -5948,10 +5949,9 @@ pub fn cmdChangelist(
         try wip_errors.init(gpa);
         defer wip_errors.deinit();
         try Compilation.addZirErrorMessages(&wip_errors, &file);
-        const ttyconf = std.debug.detectTTYConfig(std.io.getStdErr());
         var error_bundle = try wip_errors.toOwnedBundle();
         defer error_bundle.deinit(gpa);
-        error_bundle.renderToStdErr(ttyconf);
+        error_bundle.renderToStdErr(renderOptions(color));
         process.exit(1);
     }
 
@@ -5984,10 +5984,9 @@ pub fn cmdChangelist(
         try wip_errors.init(gpa);
         defer wip_errors.deinit();
         try Compilation.addZirErrorMessages(&wip_errors, &file);
-        const ttyconf = std.debug.detectTTYConfig(std.io.getStdErr());
         var error_bundle = try wip_errors.toOwnedBundle();
         defer error_bundle.deinit(gpa);
-        error_bundle.renderToStdErr(ttyconf);
+        error_bundle.renderToStdErr(renderOptions(color));
         process.exit(1);
     }
 
@@ -6256,3 +6255,12 @@ fn get_tty_conf(color: Color) std.debug.TTY.Config {
         .off => .no_color,
     };
 }
+
+fn renderOptions(color: Color) std.zig.ErrorBundle.RenderOptions {
+    const ttyconf = get_tty_conf(color);
+    return .{
+        .ttyconf = ttyconf,
+        .include_source_line = ttyconf != .no_color,
+        .include_reference_trace = ttyconf != .no_color,
+    };
+}
src/Sema.zig
@@ -2220,7 +2220,7 @@ fn failWithOwnedErrorMsg(sema: *Sema, err_msg: *Module.ErrorMsg) CompileError {
         Compilation.addModuleErrorMsg(&wip_errors, err_msg.*) catch unreachable;
         std.debug.print("compile error during Sema:\n", .{});
         var error_bundle = wip_errors.toOwnedBundle() catch unreachable;
-        error_bundle.renderToStdErr(.no_color);
+        error_bundle.renderToStdErr(.{ .ttyconf = .no_color });
         crash_report.compilerPanic("unexpected compile error occurred", null, null);
     }