From b39c7e61ef3d0b2ac28a2ce70ef549f2fac834b1 Mon Sep 17 00:00:00 2001 From: Manlio Perillo Date: Tue, 18 Apr 2023 12:22:30 +0200 Subject: [PATCH 1/3] build: restore compatibility support again Commit e214c44 (build: update ZiglingStep to the new API) broke again the compatibility support for old compilers, due to the use of the multi-object for loop syntax. Move the Zig IPC support code to src/ipc.zig. Use the while statement, instead of the for statement. --- build.zig | 38 +++++------------------------- src/ipc.zig | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 32 deletions(-) create mode 100644 src/ipc.zig diff --git a/build.zig b/build.zig index c8d8f35..d512e25 100644 --- a/build.zig +++ b/build.zig @@ -1,6 +1,7 @@ const std = @import("std"); const builtin = @import("builtin"); const compat = @import("src/compat.zig"); +const ipc = @import("src/ipc.zig"); const tests = @import("test/tests.zig"); const Build = compat.Build; @@ -878,8 +879,8 @@ const ZiglingStep = struct { }); defer poller.deinit(); - try sendMessage(child.stdin.?, .update); - try sendMessage(child.stdin.?, .exit); + try ipc.sendMessage(child.stdin.?, .update); + try ipc.sendMessage(child.stdin.?, .exit); const Header = std.zig.Server.Message.Header; var result: ?[]const u8 = null; @@ -907,21 +908,7 @@ const ZiglingStep = struct { return error.ZigVersionMismatch; }, .error_bundle => { - const EbHdr = std.zig.Server.Message.ErrorBundle; - const eb_hdr = @ptrCast(*align(1) const EbHdr, body); - const extra_bytes = - body[@sizeOf(EbHdr)..][0 .. @sizeOf(u32) * eb_hdr.extra_len]; - const string_bytes = - body[@sizeOf(EbHdr) + extra_bytes.len ..][0..eb_hdr.string_bytes_len]; - // TODO: use @ptrCast when the compiler supports it - const unaligned_extra = std.mem.bytesAsSlice(u32, extra_bytes); - const extra_array = try allocator.alloc(u32, unaligned_extra.len); - // TODO: use @memcpy when it supports slices - for (extra_array, unaligned_extra) |*dst, src| dst.* = src; - const error_bundle: std.zig.ErrorBundle = .{ - .string_bytes = try allocator.dupe(u8, string_bytes), - .extra = extra_array, - }; + const error_bundle = try ipc.parseErrorBundle(allocator, body); // Print the compiler error bundle now. // TODO: use the same ttyconf from the builder. @@ -939,13 +926,8 @@ const ZiglingStep = struct { sub_prog_node.setName(node_name.items); }, .emit_bin_path => { - const EbpHdr = std.zig.Server.Message.EmitBinPath; - - // TODO: add cache support? - //const ebp_hdr = @ptrCast(*align(1) const EbpHdr, body); - //s.result_cached = ebp_hdr.flags.cache_hit; - - result = try allocator.dupe(u8, body[@sizeOf(EbpHdr)..]); + const emit_bin = try ipc.parseEmitBinPath(allocator, body); + result = emit_bin.path; }, else => {}, // ignore other messages } @@ -985,14 +967,6 @@ const ZiglingStep = struct { } }; -fn sendMessage(file: std.fs.File, tag: std.zig.Client.Message.Tag) !void { - const header: std.zig.Client.Message.Header = .{ - .tag = tag, - .bytes_len = 0, - }; - try file.writeAll(std.mem.asBytes(&header)); -} - // Print a message to stderr. const PrintStep = struct { step: Step, diff --git a/src/ipc.zig b/src/ipc.zig new file mode 100644 index 0000000..9eaaa13 --- /dev/null +++ b/src/ipc.zig @@ -0,0 +1,68 @@ +/// Client side support for Zig IPC. +const std = @import("std"); +const debug = std.debug; +const fs = std.fs; +const mem = std.mem; + +const Allocator = mem.Allocator; +const Client = std.zig.Client; +const ErrorBundle = std.zig.ErrorBundle; +const Server = std.zig.Server; + +/// This data structure must be kept in sync with zig.Server.Message.EmitBinPath. +const EmitBinPath = struct { + flags: Flags, + path: []const u8, + + pub const Flags = Server.Message.EmitBinPath.Flags; + + pub fn deinit(self: *EmitBinPath, allocator: Allocator) void { + allocator.free(self.path); + self.* = undefined; + } +}; + +pub fn parseErrorBundle(allocator: Allocator, data: []const u8) !ErrorBundle { + const EbHdr = Server.Message.ErrorBundle; + const eb_hdr = @ptrCast(*align(1) const EbHdr, data); + const extra_bytes = + data[@sizeOf(EbHdr)..][0 .. @sizeOf(u32) * eb_hdr.extra_len]; + const string_bytes = + data[@sizeOf(EbHdr) + extra_bytes.len ..][0..eb_hdr.string_bytes_len]; + + // TODO: use @ptrCast when the compiler supports it + const unaligned_extra = std.mem.bytesAsSlice(u32, extra_bytes); + const extra_array = try allocator.alloc(u32, unaligned_extra.len); + // TODO: use @memcpy when it supports slices + // + // Don't use the "multi-object for loop" syntax, in + // order to avoid a syntax error with old Zig compilers. + var i: usize = 0; + while (i < extra_array.len) : (i += 1) { + extra_array[i] = unaligned_extra[i]; + } + + return .{ + .string_bytes = try allocator.dupe(u8, string_bytes), + .extra = extra_array, + }; +} + +pub fn parseEmitBinPath(allocator: Allocator, data: []const u8) !EmitBinPath { + const EbpHdr = Server.Message.EmitBinPath; + const ebp_hdr = @ptrCast(*align(1) const EbpHdr, data); + const path = try allocator.dupe(u8, data[@sizeOf(EbpHdr)..]); + + return .{ + .flags = ebp_hdr.flags, + .path = path, + }; +} + +pub fn sendMessage(file: fs.File, tag: Client.Message.Tag) !void { + const header: Client.Message.Header = .{ + .tag = tag, + .bytes_len = 0, + }; + try file.writeAll(mem.asBytes(&header)); +} From c6e055dd8340981b4eedec5b6a41b944caf9c722 Mon Sep 17 00:00:00 2001 From: Manlio Perillo Date: Mon, 17 Apr 2023 15:52:57 +0200 Subject: [PATCH 2/3] build: don't print errors in ZiglingStep.eval Move the code for printing compiler errors and messages to the new ZiglingStep.printErrors method. Call printErrors in the Zigling.doCompile method, both in the normal and error flow. When handling an error from the Zig IPC, add the case when the compiler was unable to return the executable path. Before using the IPC, the error was "The following command exited with error code 1" now it is "The following command failed to communicate the compilation result" --- build.zig | 47 +++++++++++++++++++++++++++++++++-------------- 1 file changed, 33 insertions(+), 14 deletions(-) diff --git a/build.zig b/build.zig index d512e25..e906ee4 100644 --- a/build.zig +++ b/build.zig @@ -690,6 +690,9 @@ const ZiglingStep = struct { builder: *Build, use_healed: bool, + result_messages: []const u8 = "", + result_error_bundle: std.zig.ErrorBundle = std.zig.ErrorBundle.empty, + pub fn create(builder: *Build, exercise: Exercise, use_healed: bool) *@This() { const self = builder.allocator.create(@This()) catch unreachable; self.* = .{ @@ -829,6 +832,8 @@ const ZiglingStep = struct { const argv = zig_args.items; var code: u8 = undefined; const file_name = self.eval(argv, &code, prog_node) catch |err| { + self.printErrors(); + switch (err) { error.FileNotFound => { print("{s}{s}: Unable to spawn the following command: file not found{s}\n", .{ red_text, self.exercise.main_file, reset_text }); @@ -845,11 +850,21 @@ const ZiglingStep = struct { for (argv) |v| print("{s} ", .{v}); print("\n", .{}); }, + error.ZigIPCError => { + print("{s}{s}: The following command failed to communicate the compilation result:{s}\n", .{ + red_text, + self.exercise.main_file, + reset_text, + }); + for (argv) |v| print("{s} ", .{v}); + print("\n", .{}); + }, else => {}, } return err; }; + self.printErrors(); return file_name; } @@ -908,17 +923,7 @@ const ZiglingStep = struct { return error.ZigVersionMismatch; }, .error_bundle => { - const error_bundle = try ipc.parseErrorBundle(allocator, body); - - // Print the compiler error bundle now. - // TODO: use the same ttyconf from the builder. - const ttyconf: std.debug.TTY.Config = if (use_color_escapes) - .escape_codes - else - .no_color; - error_bundle.renderToStdErr( - .{ .ttyconf = ttyconf }, - ); + self.result_error_bundle = try ipc.parseErrorBundle(allocator, body); }, .progress => { node_name.clearRetainingCapacity(); @@ -937,9 +942,7 @@ const ZiglingStep = struct { const stderr = poller.fifo(.stderr); if (stderr.readableLength() > 0) { - // Print the additional log and verbose messages now. - const messages = try stderr.toOwnedSlice(); - print("{s}\n", .{messages}); + self.result_messages = try stderr.toOwnedSlice(); } // Send EOF to stdin. @@ -965,6 +968,22 @@ const ZiglingStep = struct { return result orelse return error.ZigIPCError; } + + fn printErrors(self: *ZiglingStep) void { + // Print the additional log and verbose messages. + // TODO: use colors? + if (self.result_messages.len > 0) print("{s}", .{self.result_messages}); + + // Print the compiler errors. + // TODO: use the same ttyconf from the builder. + const ttyconf: std.debug.TTY.Config = if (use_color_escapes) + .escape_codes + else + .no_color; + if (self.result_error_bundle.errorMessageCount() > 0) { + self.result_error_bundle.renderToStdErr(.{ .ttyconf = ttyconf }); + } + } }; // Print a message to stderr. From 30db9d105ab7370bf67938c048714fe2cc3c9c71 Mon Sep 17 00:00:00 2001 From: Manlio Perillo Date: Mon, 17 Apr 2023 19:09:30 +0200 Subject: [PATCH 3/3] build: avoid intermixed messages on the same line In same cases, the progress messages from the compiler are intermixed with the messages printed by ZiglingStep. This intermixing appears in two cases: - when printing, e.g., the message "Checking 0_arrays2.zig..." - when printing the compiler errors Closes #230 --- build.zig | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/build.zig b/build.zig index e906ee4..226d38c 100644 --- a/build.zig +++ b/build.zig @@ -728,6 +728,7 @@ const ZiglingStep = struct { const exe_file = try self.doCompile(prog_node); + resetLine(); print("Checking {s}...\n", .{self.exercise.main_file}); const cwd = self.builder.build_root.path.?; @@ -970,6 +971,8 @@ const ZiglingStep = struct { } fn printErrors(self: *ZiglingStep) void { + resetLine(); + // Print the additional log and verbose messages. // TODO: use colors? if (self.result_messages.len > 0) print("{s}", .{self.result_messages}); @@ -986,6 +989,12 @@ const ZiglingStep = struct { } }; +// Clear the entire line and move the cursor to column zero. +// Used for clearing the compiler and build_runner progress messages. +fn resetLine() void { + if (use_color_escapes) print("{s}", .{"\x1b[2K\r"}); +} + // Print a message to stderr. const PrintStep = struct { step: Step,