Skip to content

Commit

Permalink
build runner enhancements in preparation for test-cases
Browse files Browse the repository at this point in the history
 * 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.
  • Loading branch information
andrewrk committed Mar 15, 2023
1 parent 3186658 commit 7cc4a69
Show file tree
Hide file tree
Showing 6 changed files with 173 additions and 49 deletions.
30 changes: 21 additions & 9 deletions lib/build_runner.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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));
}
}

Expand Down Expand Up @@ -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,
};
}
113 changes: 102 additions & 11 deletions lib/std/Build/CompileStep.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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| {
Expand Down Expand Up @@ -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 });
}
20 changes: 15 additions & 5 deletions lib/std/Build/Step.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
29 changes: 16 additions & 13 deletions lib/std/zig/ErrorBundle.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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(' '),
Expand All @@ -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", .{});
Expand Down Expand Up @@ -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);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/Sema.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
Loading

0 comments on commit 7cc4a69

Please sign in to comment.