From f60c045cef7bafdd3eac285f82d5a310daadaec5 Mon Sep 17 00:00:00 2001
From: mlugg <mlugg@mlugg.co.uk>
Date: Tue, 20 Aug 2024 18:09:23 +0100
Subject: [PATCH 1/6] tests: add `test-incremental` step

This is contained in the `test` step, so is tested by CI.

This commit also includes some enhancements to the `incr-check` tool to
make this work correctly.
---
 build.zig            |  4 ++++
 test/tests.zig       | 28 ++++++++++++++++++++++++++++
 tools/incr-check.zig |  4 +++-
 3 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/build.zig b/build.zig
index 731b825c07..a5f4cd5675 100644
--- a/build.zig
+++ b/build.zig
@@ -577,6 +577,10 @@ pub fn build(b: *std.Build) !void {
     } else {
         update_mingw_step.dependOn(&b.addFail("The -Dmingw-src=... option is required for this step").step);
     }
+
+    const test_incremental_step = b.step("test-incremental", "Run the incremental compilation test cases");
+    try tests.addIncrementalTests(b, test_incremental_step);
+    test_step.dependOn(test_incremental_step);
 }
 
 fn addWasiUpdateStep(b: *std.Build, version: [:0]const u8) !void {
diff --git a/test/tests.zig b/test/tests.zig
index 1e98a48489..017f9478a9 100644
--- a/test/tests.zig
+++ b/test/tests.zig
@@ -1509,3 +1509,31 @@ pub fn addDebuggerTests(b: *std.Build, options: DebuggerContext.Options) ?*Step
     });
     return step;
 }
+
+pub fn addIncrementalTests(b: *std.Build, test_step: *Step) !void {
+    const incr_check = b.addExecutable(.{
+        .name = "incr-check",
+        .root_source_file = b.path("tools/incr-check.zig"),
+        .target = b.graph.host,
+        .optimize = .Debug,
+    });
+
+    var dir = try b.build_root.handle.openDir("test/incremental", .{ .iterate = true });
+    defer dir.close();
+
+    var it = try dir.walk(b.graph.arena);
+    while (try it.next()) |entry| {
+        if (entry.kind != .file) continue;
+
+        const run = b.addRunArtifact(incr_check);
+        run.setName(b.fmt("incr-check '{s}'", .{entry.basename}));
+
+        run.addArg(b.graph.zig_exe);
+        run.addFileArg(b.path("test/incremental/").path(b, entry.path));
+        run.addArgs(&.{ "--zig-lib-dir", b.fmt("{}", .{b.graph.zig_lib_directory}) });
+
+        run.addCheck(.{ .expect_term = .{ .Exited = 0 } });
+
+        test_step.dependOn(&run.step);
+    }
+}
diff --git a/tools/incr-check.zig b/tools/incr-check.zig
index 047b129276..0a3081d210 100644
--- a/tools/incr-check.zig
+++ b/tools/incr-check.zig
@@ -68,7 +68,9 @@ pub fn main() !void {
     const debug_log_verbose = debug_zcu or debug_link;
 
     for (case.targets) |target| {
-        std.log.scoped(.status).info("target: '{s}-{s}'", .{ target.query, @tagName(target.backend) });
+        if (debug_log_verbose) {
+            std.log.scoped(.status).info("target: '{s}-{s}'", .{ target.query, @tagName(target.backend) });
+        }
 
         var child_args: std.ArrayListUnmanaged([]const u8) = .empty;
         try child_args.appendSlice(arena, &.{

From 5ce962eb690aa094521b0ec5f3f466192c42318a Mon Sep 17 00:00:00 2001
From: mlugg <mlugg@mlugg.co.uk>
Date: Thu, 26 Sep 2024 03:58:39 +0100
Subject: [PATCH 2/6] incr-check: better progress output, support external
 executors

If no external executor is available for a successful binary, its
execution is silently skipped. This allows the CI to test, to the
fullest extent possible, incremental cross-compilation to targets whose
binaries can't be executed on the host.
---
 tools/incr-check.zig | 83 +++++++++++++++++++++++++++++++++++---------
 1 file changed, 67 insertions(+), 16 deletions(-)

diff --git a/tools/incr-check.zig b/tools/incr-check.zig
index 0a3081d210..f33acc1e27 100644
--- a/tools/incr-check.zig
+++ b/tools/incr-check.zig
@@ -55,9 +55,6 @@ pub fn main() !void {
     const tmp_dir_path = "tmp_" ++ std.fmt.hex(rand_int);
     const tmp_dir = try std.fs.cwd().makeOpenPath(tmp_dir_path, .{});
 
-    const child_prog_node = prog_node.start("zig build-exe", 0);
-    defer child_prog_node.end();
-
     // Convert paths to be relative to the cwd of the subprocess.
     const resolved_zig_exe = try std.fs.path.relative(arena, tmp_dir_path, zig_exe);
     const opt_resolved_lib_dir = if (opt_lib_dir) |lib_dir|
@@ -65,9 +62,18 @@ pub fn main() !void {
     else
         null;
 
+    const host = try std.zig.system.resolveTargetQuery(.{});
+
     const debug_log_verbose = debug_zcu or debug_link;
 
     for (case.targets) |target| {
+        const target_prog_node = node: {
+            var name_buf: [std.Progress.Node.max_name_len]u8 = undefined;
+            const name = std.fmt.bufPrint(&name_buf, "{s}-{s}", .{ target.query, @tagName(target.backend) }) catch &name_buf;
+            break :node prog_node.start(name, case.updates.len);
+        };
+        defer target_prog_node.end();
+
         if (debug_log_verbose) {
             std.log.scoped(.status).info("target: '{s}-{s}'", .{ target.query, @tagName(target.backend) });
         }
@@ -102,11 +108,14 @@ pub fn main() !void {
             try child_args.appendSlice(arena, &.{ "--debug-log", "link", "--debug-log", "link_state", "--debug-log", "link_relocs" });
         }
 
+        const zig_prog_node = target_prog_node.start("zig build-exe", 0);
+        defer zig_prog_node.end();
+
         var child = std.process.Child.init(child_args.items, arena);
         child.stdin_behavior = .Pipe;
         child.stdout_behavior = .Pipe;
         child.stderr_behavior = .Pipe;
-        child.progress_node = child_prog_node;
+        child.progress_node = zig_prog_node;
         child.cwd_dir = tmp_dir;
         child.cwd = tmp_dir_path;
 
@@ -131,6 +140,7 @@ pub fn main() !void {
         var eval: Eval = .{
             .arena = arena,
             .case = case,
+            .host = host,
             .target = target,
             .tmp_dir = tmp_dir,
             .tmp_dir_path = tmp_dir_path,
@@ -148,7 +158,7 @@ pub fn main() !void {
         defer poller.deinit();
 
         for (case.updates) |update| {
-            var update_node = prog_node.start(update.name, 0);
+            var update_node = target_prog_node.start(update.name, 0);
             defer update_node.end();
 
             if (debug_log_verbose) {
@@ -168,6 +178,7 @@ pub fn main() !void {
 
 const Eval = struct {
     arena: Allocator,
+    host: std.Target,
     case: Case,
     target: Case.Target,
     tmp_dir: std.fs.Dir,
@@ -270,14 +281,7 @@ const Eval = struct {
                     const name = std.fs.path.stem(std.fs.path.basename(eval.case.root_source_file));
                     const bin_name = try std.zig.binNameAlloc(arena, .{
                         .root_name = name,
-                        .target = try std.zig.system.resolveTargetQuery(try std.Build.parseTargetQuery(.{
-                            .arch_os_abi = eval.target.query,
-                            .object_format = switch (eval.target.backend) {
-                                .sema => unreachable,
-                                .selfhosted, .llvm => null,
-                                .cbe => "c",
-                            },
-                        })),
+                        .target = eval.target.resolved,
                         .output_mode = .Exe,
                     });
                     const bin_path = try std.fs.path.join(arena, &.{ result_dir, bin_name });
@@ -346,9 +350,41 @@ const Eval = struct {
             },
         };
 
+        var argv_buf: [2][]const u8 = undefined;
+        const argv: []const []const u8, const ignore_stderr: bool = switch (std.zig.system.getExternalExecutor(
+            eval.host,
+            &eval.target.resolved,
+            .{ .link_libc = eval.target.backend == .cbe },
+        )) {
+            .bad_dl, .bad_os_or_cpu => {
+                // This binary cannot be executed on this host.
+                if (eval.allow_stderr) {
+                    std.log.warn("skipping execution because host '{s}' cannot execute binaries for foreign target '{s}'", .{
+                        try eval.host.zigTriple(eval.arena),
+                        try eval.target.resolved.zigTriple(eval.arena),
+                    });
+                }
+                return;
+            },
+            .native, .rosetta => argv: {
+                argv_buf[0] = binary_path;
+                break :argv .{ argv_buf[0..1], false };
+            },
+            .qemu, .wine, .wasmtime, .darling => |executor_cmd| argv: {
+                argv_buf[0] = executor_cmd;
+                argv_buf[1] = binary_path;
+                // Some executors (looking at you, Wine) like throwing some stderr in, just for fun.
+                // Therefore, we'll ignore stderr when using a foreign executor.
+                break :argv .{ argv_buf[0..2], true };
+            },
+        };
+
+        const run_prog_node = prog_node.start("run generated executable", 0);
+        defer run_prog_node.end();
+
         const result = std.process.Child.run(.{
             .allocator = eval.arena,
-            .argv = &.{binary_path},
+            .argv = argv,
             .cwd_dir = eval.tmp_dir,
             .cwd = eval.tmp_dir_path,
         }) catch |err| {
@@ -356,7 +392,7 @@ const Eval = struct {
                 update.name, binary_path, @errorName(err),
             });
         };
-        if (result.stderr.len != 0) {
+        if (!ignore_stderr and result.stderr.len != 0) {
             std.log.err("update '{s}': generated executable '{s}' had unexpected stderr:\n{s}", .{
                 update.name, binary_path, result.stderr,
             });
@@ -380,7 +416,7 @@ const Eval = struct {
                 });
             },
         }
-        if (result.stderr.len != 0) std.process.exit(1);
+        if (!ignore_stderr and result.stderr.len != 0) std.process.exit(1);
     }
 
     fn requestUpdate(eval: *Eval) !void {
@@ -468,6 +504,7 @@ const Case = struct {
 
     const Target = struct {
         query: []const u8,
+        resolved: std.Target,
         backend: Backend,
         const Backend = enum {
             /// Run semantic analysis only. Runtime output will not be tested, but we still verify
@@ -529,12 +566,26 @@ const Case = struct {
                 } else if (std.mem.eql(u8, key, "target")) {
                     const split_idx = std.mem.lastIndexOfScalar(u8, val, '-') orelse
                         fatal("line {d}: target does not include backend", .{line_n});
+
                     const query = val[0..split_idx];
+
                     const backend_str = val[split_idx + 1 ..];
                     const backend: Target.Backend = std.meta.stringToEnum(Target.Backend, backend_str) orelse
                         fatal("line {d}: invalid backend '{s}'", .{ line_n, backend_str });
+
+                    const parsed_query = std.Build.parseTargetQuery(.{
+                        .arch_os_abi = query,
+                        .object_format = switch (backend) {
+                            .sema, .selfhosted, .llvm => null,
+                            .cbe => "c",
+                        },
+                    }) catch fatal("line {d}: invalid target query '{s}'", .{ line_n, query });
+
+                    const resolved = try std.zig.system.resolveTargetQuery(parsed_query);
+
                     try targets.append(arena, .{
                         .query = query,
+                        .resolved = resolved,
                         .backend = backend,
                     });
                 } else if (std.mem.eql(u8, key, "update")) {

From dfc0a27090b1f15aae429801cdded14ab0a80595 Mon Sep 17 00:00:00 2001
From: mlugg <mlugg@mlugg.co.uk>
Date: Thu, 26 Sep 2024 04:21:58 +0100
Subject: [PATCH 3/6] incr-check: clean up temporary directory by default

The new `--preserve-tmp` flag can be used to preserve the temporary
directory for debugging purposes.
---
 tools/incr-check.zig | 94 ++++++++++++++++++++++++++++++--------------
 1 file changed, 64 insertions(+), 30 deletions(-)

diff --git a/tools/incr-check.zig b/tools/incr-check.zig
index f33acc1e27..473066eabe 100644
--- a/tools/incr-check.zig
+++ b/tools/incr-check.zig
@@ -1,11 +1,12 @@
 const std = @import("std");
-const fatal = std.process.fatal;
 const Allocator = std.mem.Allocator;
 const Cache = std.Build.Cache;
 
-const usage = "usage: incr-check <zig binary path> <input file> [--zig-lib-dir lib] [--debug-zcu] [--debug-link] [--zig-cc-binary /path/to/zig]";
+const usage = "usage: incr-check <zig binary path> <input file> [--zig-lib-dir lib] [--debug-zcu] [--debug-link] [--preserve-tmp] [--zig-cc-binary /path/to/zig]";
 
 pub fn main() !void {
+    const fatal = std.process.fatal;
+
     var arena_instance = std.heap.ArenaAllocator.init(std.heap.page_allocator);
     defer arena_instance.deinit();
     const arena = arena_instance.allocator();
@@ -16,6 +17,7 @@ pub fn main() !void {
     var opt_cc_zig: ?[]const u8 = null;
     var debug_zcu = false;
     var debug_link = false;
+    var preserve_tmp = false;
 
     var arg_it = try std.process.argsWithAllocator(arena);
     _ = arg_it.skip();
@@ -27,6 +29,8 @@ pub fn main() !void {
                 debug_zcu = true;
             } else if (std.mem.eql(u8, arg, "--debug-link")) {
                 debug_link = true;
+            } else if (std.mem.eql(u8, arg, "--preserve-tmp")) {
+                preserve_tmp = true;
             } else if (std.mem.eql(u8, arg, "--zig-cc-binary")) {
                 opt_cc_zig = arg_it.next() orelse fatal("expect arg after '--zig-cc-binary'\n{s}", .{usage});
             } else {
@@ -48,12 +52,29 @@ pub fn main() !void {
     const input_file_bytes = try std.fs.cwd().readFileAlloc(arena, input_file_name, std.math.maxInt(u32));
     const case = try Case.parse(arena, input_file_bytes);
 
+    // Check now: if there are any targets using the `cbe` backend, we need the lib dir.
+    if (opt_lib_dir == null) {
+        for (case.targets) |target| {
+            if (target.backend == .cbe) {
+                fatal("'--zig-lib-dir' requried when using backend 'cbe'", .{});
+            }
+        }
+    }
+
     const prog_node = std.Progress.start(.{});
     defer prog_node.end();
 
     const rand_int = std.crypto.random.int(u64);
     const tmp_dir_path = "tmp_" ++ std.fmt.hex(rand_int);
-    const tmp_dir = try std.fs.cwd().makeOpenPath(tmp_dir_path, .{});
+    var tmp_dir = try std.fs.cwd().makeOpenPath(tmp_dir_path, .{});
+    defer {
+        tmp_dir.close();
+        if (!preserve_tmp) {
+            std.fs.cwd().deleteTree(tmp_dir_path) catch |err| {
+                std.log.warn("failed to delete tree '{s}': {s}", .{ tmp_dir_path, @errorName(err) });
+            };
+        }
+    }
 
     // Convert paths to be relative to the cwd of the subprocess.
     const resolved_zig_exe = try std.fs.path.relative(arena, tmp_dir_path, zig_exe);
@@ -132,7 +153,7 @@ pub fn main() !void {
                 "-target",
                 target.query,
                 "-I",
-                opt_resolved_lib_dir orelse fatal("'--zig-lib-dir' required when using backend 'cbe'", .{}),
+                opt_resolved_lib_dir.?, // verified earlier
                 "-o",
             });
         }
@@ -146,6 +167,7 @@ pub fn main() !void {
             .tmp_dir_path = tmp_dir_path,
             .child = &child,
             .allow_stderr = debug_log_verbose,
+            .preserve_tmp_on_fatal = preserve_tmp,
             .cc_child_args = &cc_child_args,
         };
 
@@ -172,7 +194,7 @@ pub fn main() !void {
 
         try eval.end(&poller);
 
-        waitChild(&child);
+        waitChild(&child, &eval);
     }
 }
 
@@ -185,6 +207,7 @@ const Eval = struct {
     tmp_dir_path: []const u8,
     child: *std.process.Child,
     allow_stderr: bool,
+    preserve_tmp_on_fatal: bool,
     /// When `target.backend == .cbe`, this contains the first few arguments to `zig cc` to build the generated binary.
     /// The arguments `out.c in.c` must be appended before spawning the subprocess.
     cc_child_args: *std.ArrayListUnmanaged([]const u8),
@@ -199,12 +222,12 @@ const Eval = struct {
                 .sub_path = full_contents.name,
                 .data = full_contents.bytes,
             }) catch |err| {
-                fatal("failed to update '{s}': {s}", .{ full_contents.name, @errorName(err) });
+                eval.fatal("failed to update '{s}': {s}", .{ full_contents.name, @errorName(err) });
             };
         }
         for (update.deletes) |doomed_name| {
             eval.tmp_dir.deleteFile(doomed_name) catch |err| {
-                fatal("failed to delete '{s}': {s}", .{ doomed_name, @errorName(err) });
+                eval.fatal("failed to delete '{s}': {s}", .{ doomed_name, @errorName(err) });
             };
         }
     }
@@ -246,7 +269,7 @@ const Eval = struct {
                         if (eval.allow_stderr) {
                             std.log.info("error_bundle included stderr:\n{s}", .{stderr_data});
                         } else {
-                            fatal("error_bundle included unexpected stderr:\n{s}", .{stderr_data});
+                            eval.fatal("error_bundle included unexpected stderr:\n{s}", .{stderr_data});
                         }
                     }
                     if (result_error_bundle.errorMessageCount() != 0) {
@@ -265,7 +288,7 @@ const Eval = struct {
                         if (eval.allow_stderr) {
                             std.log.info("emit_digest included stderr:\n{s}", .{stderr_data});
                         } else {
-                            fatal("emit_digest included unexpected stderr:\n{s}", .{stderr_data});
+                            eval.fatal("emit_digest included unexpected stderr:\n{s}", .{stderr_data});
                         }
                     }
 
@@ -302,16 +325,15 @@ const Eval = struct {
             if (eval.allow_stderr) {
                 std.log.info("update '{s}' included stderr:\n{s}", .{ update.name, stderr_data });
             } else {
-                fatal("update '{s}' failed:\n{s}", .{ update.name, stderr_data });
+                eval.fatal("update '{s}' failed:\n{s}", .{ update.name, stderr_data });
             }
         }
 
-        waitChild(eval.child);
-        fatal("update '{s}': compiler failed to send error_bundle or emit_bin_path", .{update.name});
+        waitChild(eval.child, eval);
+        eval.fatal("update '{s}': compiler failed to send error_bundle or emit_bin_path", .{update.name});
     }
 
     fn checkErrorOutcome(eval: *Eval, update: Case.Update, error_bundle: std.zig.ErrorBundle) !void {
-        _ = eval;
         switch (update.outcome) {
             .unknown => return,
             .compile_errors => |expected_errors| {
@@ -323,7 +345,7 @@ const Eval = struct {
             .stdout, .exit_code => {
                 const color: std.zig.Color = .auto;
                 error_bundle.renderToStdErr(color.renderOptions());
-                fatal("update '{s}': unexpected compile errors", .{update.name});
+                eval.fatal("update '{s}': unexpected compile errors", .{update.name});
             },
         }
     }
@@ -331,7 +353,7 @@ const Eval = struct {
     fn checkSuccessOutcome(eval: *Eval, update: Case.Update, opt_emitted_path: ?[]const u8, prog_node: std.Progress.Node) !void {
         switch (update.outcome) {
             .unknown => return,
-            .compile_errors => fatal("expected compile errors but compilation incorrectly succeeded", .{}),
+            .compile_errors => eval.fatal("expected compile errors but compilation incorrectly succeeded", .{}),
             .stdout, .exit_code => {},
         }
         const emitted_path = opt_emitted_path orelse {
@@ -388,7 +410,7 @@ const Eval = struct {
             .cwd_dir = eval.tmp_dir,
             .cwd = eval.tmp_dir_path,
         }) catch |err| {
-            fatal("update '{s}': failed to run the generated executable '{s}': {s}", .{
+            eval.fatal("update '{s}': failed to run the generated executable '{s}': {s}", .{
                 update.name, binary_path, @errorName(err),
             });
         };
@@ -402,7 +424,7 @@ const Eval = struct {
                 .unknown, .compile_errors => unreachable,
                 .stdout => |expected_stdout| {
                     if (code != 0) {
-                        fatal("update '{s}': generated executable '{s}' failed with code {d}", .{
+                        eval.fatal("update '{s}': generated executable '{s}' failed with code {d}", .{
                             update.name, binary_path, code,
                         });
                     }
@@ -411,7 +433,7 @@ const Eval = struct {
                 .exit_code => |expected_code| try std.testing.expectEqual(expected_code, result.term.Exited),
             },
             .Signal, .Stopped, .Unknown => {
-                fatal("update '{s}': generated executable '{s}' terminated unexpectedly", .{
+                eval.fatal("update '{s}': generated executable '{s}' terminated unexpectedly", .{
                     update.name, binary_path,
                 });
             },
@@ -428,7 +450,7 @@ const Eval = struct {
     }
 
     fn end(eval: *Eval, poller: *Poller) !void {
-        requestExit(eval.child);
+        requestExit(eval.child, eval);
 
         const Header = std.zig.Server.Message.Header;
         const stdout = poller.fifo(.stdout);
@@ -448,7 +470,7 @@ const Eval = struct {
 
         if (stderr.readableLength() > 0) {
             const stderr_data = try stderr.toOwnedSlice();
-            fatal("unexpected stderr:\n{s}", .{stderr_data});
+            eval.fatal("unexpected stderr:\n{s}", .{stderr_data});
         }
     }
 
@@ -468,7 +490,7 @@ const Eval = struct {
             .cwd = eval.tmp_dir_path,
             .progress_node = child_prog_node,
         }) catch |err| {
-            fatal("update '{s}': failed to spawn zig cc for '{s}': {s}", .{
+            eval.fatal("update '{s}': failed to spawn zig cc for '{s}': {s}", .{
                 update.name, c_path, @errorName(err),
             });
         };
@@ -479,7 +501,7 @@ const Eval = struct {
                         update.name, result.stderr,
                     });
                 }
-                fatal("update '{s}': zig cc for '{s}' failed with code {d}", .{
+                eval.fatal("update '{s}': zig cc for '{s}' failed with code {d}", .{
                     update.name, c_path, code,
                 });
             },
@@ -489,12 +511,22 @@ const Eval = struct {
                         update.name, result.stderr,
                     });
                 }
-                fatal("update '{s}': zig cc for '{s}' terminated unexpectedly", .{
+                eval.fatal("update '{s}': zig cc for '{s}' terminated unexpectedly", .{
                     update.name, c_path,
                 });
             },
         }
     }
+
+    fn fatal(eval: *Eval, comptime fmt: []const u8, args: anytype) noreturn {
+        eval.tmp_dir.close();
+        if (!eval.preserve_tmp_on_fatal) {
+            std.fs.cwd().deleteTree(eval.tmp_dir_path) catch |err| {
+                std.log.warn("failed to delete tree '{s}': {s}", .{ eval.tmp_dir_path, @errorName(err) });
+            };
+        }
+        std.process.fatal(fmt, args);
+    }
 };
 
 const Case = struct {
@@ -550,6 +582,8 @@ const Case = struct {
     };
 
     fn parse(arena: Allocator, bytes: []const u8) !Case {
+        const fatal = std.process.fatal;
+
         var targets: std.ArrayListUnmanaged(Target) = .empty;
         var updates: std.ArrayListUnmanaged(Update) = .empty;
         var changes: std.ArrayListUnmanaged(FullContents) = .empty;
@@ -656,7 +690,7 @@ const Case = struct {
     }
 };
 
-fn requestExit(child: *std.process.Child) void {
+fn requestExit(child: *std.process.Child, eval: *Eval) void {
     if (child.stdin == null) return;
 
     const header: std.zig.Client.Message.Header = .{
@@ -665,7 +699,7 @@ fn requestExit(child: *std.process.Child) void {
     };
     child.stdin.?.writeAll(std.mem.asBytes(&header)) catch |err| switch (err) {
         error.BrokenPipe => {},
-        else => fatal("failed to send exit: {s}", .{@errorName(err)}),
+        else => eval.fatal("failed to send exit: {s}", .{@errorName(err)}),
     };
 
     // Send EOF to stdin.
@@ -673,11 +707,11 @@ fn requestExit(child: *std.process.Child) void {
     child.stdin = null;
 }
 
-fn waitChild(child: *std.process.Child) void {
-    requestExit(child);
-    const term = child.wait() catch |err| fatal("child process failed: {s}", .{@errorName(err)});
+fn waitChild(child: *std.process.Child, eval: *Eval) void {
+    requestExit(child, eval);
+    const term = child.wait() catch |err| eval.fatal("child process failed: {s}", .{@errorName(err)});
     switch (term) {
-        .Exited => |code| if (code != 0) fatal("compiler failed with code {d}", .{code}),
-        .Signal, .Stopped, .Unknown => fatal("compiler terminated unexpectedly", .{}),
+        .Exited => |code| if (code != 0) eval.fatal("compiler failed with code {d}", .{code}),
+        .Signal, .Stopped, .Unknown => eval.fatal("compiler terminated unexpectedly", .{}),
     }
 }

From 14ccbbef9f4e34ed2311b96b771bbb452dccf9dc Mon Sep 17 00:00:00 2001
From: mlugg <mlugg@mlugg.co.uk>
Date: Thu, 26 Sep 2024 04:26:18 +0100
Subject: [PATCH 4/6] test: add x86_64-windows-cbe target to incremental cases

Throw another target in there just to spice things up a little!

Running the incremental cases with the C backend is pretty slow due to
the need to recompile the whole output from scratch on every update; for
this reason, we probably don't want to keep many of these targeting CBE
long-term. However, for now, while we have relatively few tests and
things are still changing quite a lot, it's better to have this little
bit of extra test coverage.
---
 test/incremental/add_decl                   | 1 +
 test/incremental/add_decl_namespaced        | 1 +
 test/incremental/delete_comptime_decls      | 1 +
 test/incremental/hello                      | 1 +
 test/incremental/modify_inline_fn           | 1 +
 test/incremental/move_src                   | 1 +
 test/incremental/remove_enum_field          | 1 +
 test/incremental/type_becomes_comptime_only | 1 +
 test/incremental/unreferenced_error         | 1 +
 9 files changed, 9 insertions(+)

diff --git a/test/incremental/add_decl b/test/incremental/add_decl
index 1f9788f481..6b3a0dad84 100644
--- a/test/incremental/add_decl
+++ b/test/incremental/add_decl
@@ -1,5 +1,6 @@
 #target=x86_64-linux-selfhosted
 #target=x86_64-linux-cbe
+#target=x86_64-windows-cbe
 #update=initial version
 #file=main.zig
 const std = @import("std");
diff --git a/test/incremental/add_decl_namespaced b/test/incremental/add_decl_namespaced
index 7063e3fa74..48ed5cfd2e 100644
--- a/test/incremental/add_decl_namespaced
+++ b/test/incremental/add_decl_namespaced
@@ -1,5 +1,6 @@
 #target=x86_64-linux-selfhosted
 #target=x86_64-linux-cbe
+#target=x86_64-windows-cbe
 #update=initial version
 #file=main.zig
 const std = @import("std");
diff --git a/test/incremental/delete_comptime_decls b/test/incremental/delete_comptime_decls
index 54411797cf..03ddc68128 100644
--- a/test/incremental/delete_comptime_decls
+++ b/test/incremental/delete_comptime_decls
@@ -1,5 +1,6 @@
 #target=x86_64-linux-selfhosted
 #target=x86_64-linux-cbe
+#target=x86_64-windows-cbe
 #update=initial version
 #file=main.zig
 pub fn main() void {}
diff --git a/test/incremental/hello b/test/incremental/hello
index 9419b3d186..3526b47f7c 100644
--- a/test/incremental/hello
+++ b/test/incremental/hello
@@ -1,5 +1,6 @@
 #target=x86_64-linux-selfhosted
 #target=x86_64-linux-cbe
+#target=x86_64-windows-cbe
 #update=initial version
 #file=main.zig
 const std = @import("std");
diff --git a/test/incremental/modify_inline_fn b/test/incremental/modify_inline_fn
index 69ab1f429a..cd5361fb17 100644
--- a/test/incremental/modify_inline_fn
+++ b/test/incremental/modify_inline_fn
@@ -1,5 +1,6 @@
 #target=x86_64-linux-selfhosted
 #target=x86_64-linux-cbe
+#target=x86_64-windows-cbe
 #update=initial version
 #file=main.zig
 const std = @import("std");
diff --git a/test/incremental/move_src b/test/incremental/move_src
index 8a3a24a269..908135485c 100644
--- a/test/incremental/move_src
+++ b/test/incremental/move_src
@@ -1,5 +1,6 @@
 #target=x86_64-linux-selfhosted
 #target=x86_64-linux-cbe
+#target=x86_64-windows-cbe
 #update=initial version
 #file=main.zig
 const std = @import("std");
diff --git a/test/incremental/remove_enum_field b/test/incremental/remove_enum_field
index c163e5e168..3a882ae0f1 100644
--- a/test/incremental/remove_enum_field
+++ b/test/incremental/remove_enum_field
@@ -1,5 +1,6 @@
 #target=x86_64-linux-selfhosted
 #target=x86_64-linux-cbe
+#target=x86_64-windows-cbe
 #update=initial version
 #file=main.zig
 const MyEnum = enum(u8) {
diff --git a/test/incremental/type_becomes_comptime_only b/test/incremental/type_becomes_comptime_only
index 3878e0c61b..2da31ec5f2 100644
--- a/test/incremental/type_becomes_comptime_only
+++ b/test/incremental/type_becomes_comptime_only
@@ -1,5 +1,6 @@
 #target=x86_64-linux-selfhosted
 #target=x86_64-linux-cbe
+#target=x86_64-windows-cbe
 #update=initial version
 #file=main.zig
 const SomeType = u32;
diff --git a/test/incremental/unreferenced_error b/test/incremental/unreferenced_error
index 2031975986..3dfe0ab758 100644
--- a/test/incremental/unreferenced_error
+++ b/test/incremental/unreferenced_error
@@ -1,5 +1,6 @@
 #target=x86_64-linux-selfhosted
 #target=x86_64-linux-cbe
+#target=x86_64-windows-cbe
 #update=initial version
 #file=main.zig
 const std = @import("std");

From ada60616b37b76004237f4e13de8b552c16dc773 Mon Sep 17 00:00:00 2001
From: mlugg <mlugg@mlugg.co.uk>
Date: Thu, 26 Sep 2024 19:35:14 +0100
Subject: [PATCH 5/6] incr-check: minor fixes

* fix inconsistency in global cache directory name
* don't error if spawning external executor fails
* handle CRLF correctly
---
 tools/incr-check.zig | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/tools/incr-check.zig b/tools/incr-check.zig
index 473066eabe..b5329f134e 100644
--- a/tools/incr-check.zig
+++ b/tools/incr-check.zig
@@ -110,7 +110,7 @@ pub fn main() !void {
             "--cache-dir",
             ".local-cache",
             "--global-cache-dir",
-            ".global_cache",
+            ".global-cache",
             "--listen=-",
         });
         if (opt_resolved_lib_dir) |resolved_lib_dir| {
@@ -373,7 +373,7 @@ const Eval = struct {
         };
 
         var argv_buf: [2][]const u8 = undefined;
-        const argv: []const []const u8, const ignore_stderr: bool = switch (std.zig.system.getExternalExecutor(
+        const argv: []const []const u8, const is_foreign: bool = switch (std.zig.system.getExternalExecutor(
             eval.host,
             &eval.target.resolved,
             .{ .link_libc = eval.target.backend == .cbe },
@@ -395,8 +395,6 @@ const Eval = struct {
             .qemu, .wine, .wasmtime, .darling => |executor_cmd| argv: {
                 argv_buf[0] = executor_cmd;
                 argv_buf[1] = binary_path;
-                // Some executors (looking at you, Wine) like throwing some stderr in, just for fun.
-                // Therefore, we'll ignore stderr when using a foreign executor.
                 break :argv .{ argv_buf[0..2], true };
             },
         };
@@ -410,15 +408,31 @@ const Eval = struct {
             .cwd_dir = eval.tmp_dir,
             .cwd = eval.tmp_dir_path,
         }) catch |err| {
+            if (is_foreign) {
+                // Chances are the foreign executor isn't available. Skip this evaluation.
+                if (eval.allow_stderr) {
+                    std.log.warn("update '{s}': skipping execution of '{s}' via executor for foreign target '{s}': {s}", .{
+                        update.name,
+                        binary_path,
+                        try eval.target.resolved.zigTriple(eval.arena),
+                        @errorName(err),
+                    });
+                }
+                return;
+            }
             eval.fatal("update '{s}': failed to run the generated executable '{s}': {s}", .{
                 update.name, binary_path, @errorName(err),
             });
         };
-        if (!ignore_stderr and result.stderr.len != 0) {
+
+        // Some executors (looking at you, Wine) like throwing some stderr in, just for fun.
+        // Therefore, we'll ignore stderr when using a foreign executor.
+        if (!is_foreign and result.stderr.len != 0) {
             std.log.err("update '{s}': generated executable '{s}' had unexpected stderr:\n{s}", .{
                 update.name, binary_path, result.stderr,
             });
         }
+
         switch (result.term) {
             .Exited => |code| switch (update.outcome) {
                 .unknown, .compile_errors => unreachable,
@@ -438,7 +452,8 @@ const Eval = struct {
                 });
             },
         }
-        if (!ignore_stderr and result.stderr.len != 0) std.process.exit(1);
+
+        if (!is_foreign and result.stderr.len != 0) std.process.exit(1);
     }
 
     fn requestUpdate(eval: *Eval) !void {
@@ -594,7 +609,7 @@ const Case = struct {
             if (std.mem.startsWith(u8, line, "#")) {
                 var line_it = std.mem.splitScalar(u8, line, '=');
                 const key = line_it.first()[1..];
-                const val = line_it.rest();
+                const val = std.mem.trimRight(u8, line_it.rest(), "\r"); // windows moment
                 if (val.len == 0) {
                     fatal("line {d}: missing value", .{line_n});
                 } else if (std.mem.eql(u8, key, "target")) {

From 90db7677212f8331733a661615490d37c7bf75d2 Mon Sep 17 00:00:00 2001
From: mlugg <mlugg@mlugg.co.uk>
Date: Sun, 29 Sep 2024 00:03:46 +0100
Subject: [PATCH 6/6] std: async read into small temporary buffer between
 `poll` calls on Windows

This commit changes how `std.io.poll` is implemented on Windows. The new
implementation unfortunately incurs a little extra system call overhead,
but fixes several bugs in the old implementation:

* The `lpNumberOfBytesRead` parameter of `ReadFile` was used with
  overlapped I/O. This is explicitly disallowed by the documentation, as
  the value written to this pointer is "potentially erroneous"; instead,
  `GetOverlappedResult` must always be used, even if the operation
  immediately returns. Documentation states that `lpNumberOfBytesRead`
  cannot be passed as null on Windows 7, so for compatibility, the
  parameter is passed as a pointer to a dummy global.
* If the initial `ReadFile` returned data, and the next read returned
  `BROKEN_PIPE`, the received data was silently ignored in the sense
  that `pollWindows` did not `return`, instead waiting for data to come
  in on another file (or for all files to close).
* The asynchronous `ReadFile` calls which were left pending between
  calls to `pollWindows` pointed to a potentially unstable buffer, since
  the user of `poll` may use part of the `LinearFifo` API which rotate
  its ring buffer. This race condition was causing CI failures in some
  uses of the compiler server protocol.

These issues are all resolved. Now, `pollWindows` will queue an initial
read to a small (128-byte) stable buffer per file. When this read is
completed, reads directly into the FIFO's writable slice are performed
until one is left pending, at which point that read is cancelled (with a
check to see if it was completed between the `ReadFile` and `CancelIo`
calls) and the next read into the small stable buffer is queued. These
small buffer reads are the ones left pending between `pollWindows`
calls, avoiding the race condition described above.

Related: #21565
---
 lib/std/io.zig | 192 +++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 162 insertions(+), 30 deletions(-)

diff --git a/lib/std/io.zig b/lib/std/io.zig
index 6455693d67..0a9b2bf867 100644
--- a/lib/std/io.zig
+++ b/lib/std/io.zig
@@ -442,6 +442,7 @@ pub fn poll(
         .overlapped = [1]windows.OVERLAPPED{
             mem.zeroes(windows.OVERLAPPED),
         } ** enum_fields.len,
+        .small_bufs = undefined,
         .active = .{
             .count = 0,
             .handles_buf = undefined,
@@ -481,6 +482,7 @@ pub fn Poller(comptime StreamEnum: type) type {
         windows: if (is_windows) struct {
             first_read_done: bool,
             overlapped: [enum_fields.len]windows.OVERLAPPED,
+            small_bufs: [enum_fields.len][128]u8,
             active: struct {
                 count: math.IntFittingRange(0, enum_fields.len),
                 handles_buf: [enum_fields.len]windows.HANDLE,
@@ -534,24 +536,31 @@ pub fn Poller(comptime StreamEnum: type) type {
             const bump_amt = 512;
 
             if (!self.windows.first_read_done) {
-                // Windows Async IO requires an initial call to ReadFile before waiting on the handle
+                var already_read_data = false;
                 for (0..enum_fields.len) |i| {
                     const handle = self.windows.active.handles_buf[i];
-                    switch (try windowsAsyncRead(
+                    switch (try windowsAsyncReadToFifoAndQueueSmallRead(
                         handle,
                         &self.windows.overlapped[i],
                         &self.fifos[i],
+                        &self.windows.small_bufs[i],
                         bump_amt,
                     )) {
-                        .pending => {
+                        .populated, .empty => |state| {
+                            if (state == .populated) already_read_data = true;
                             self.windows.active.handles_buf[self.windows.active.count] = handle;
                             self.windows.active.stream_map[self.windows.active.count] = @as(StreamEnum, @enumFromInt(i));
                             self.windows.active.count += 1;
                         },
                         .closed => {}, // don't add to the wait_objects list
+                        .closed_populated => {
+                            // don't add to the wait_objects list, but we did already get data
+                            already_read_data = true;
+                        },
                     }
                 }
                 self.windows.first_read_done = true;
+                if (already_read_data) return true;
             }
 
             while (true) {
@@ -576,32 +585,35 @@ pub fn Poller(comptime StreamEnum: type) type {
 
                 const active_idx = status - windows.WAIT_OBJECT_0;
 
-                const handle = self.windows.active.handles_buf[active_idx];
                 const stream_idx = @intFromEnum(self.windows.active.stream_map[active_idx]);
-                var read_bytes: u32 = undefined;
-                if (0 == windows.kernel32.GetOverlappedResult(
-                    handle,
-                    &self.windows.overlapped[stream_idx],
-                    &read_bytes,
-                    0,
-                )) switch (windows.GetLastError()) {
-                    .BROKEN_PIPE => {
+                const handle = self.windows.active.handles_buf[active_idx];
+
+                const overlapped = &self.windows.overlapped[stream_idx];
+                const stream_fifo = &self.fifos[stream_idx];
+                const small_buf = &self.windows.small_bufs[stream_idx];
+
+                const num_bytes_read = switch (try windowsGetReadResult(handle, overlapped, false)) {
+                    .success => |n| n,
+                    .closed => {
                         self.windows.active.removeAt(active_idx);
                         continue;
                     },
-                    else => |err| return windows.unexpectedError(err),
+                    .aborted => unreachable,
                 };
+                try stream_fifo.write(small_buf[0..num_bytes_read]);
 
-                self.fifos[stream_idx].update(read_bytes);
-
-                switch (try windowsAsyncRead(
+                switch (try windowsAsyncReadToFifoAndQueueSmallRead(
                     handle,
-                    &self.windows.overlapped[stream_idx],
-                    &self.fifos[stream_idx],
+                    overlapped,
+                    stream_fifo,
+                    small_buf,
                     bump_amt,
                 )) {
-                    .pending => {},
-                    .closed => self.windows.active.removeAt(active_idx),
+                    .empty => {}, // irrelevant, we already got data from the small buffer
+                    .populated => {},
+                    .closed,
+                    .closed_populated, // identical, since we already got data from the small buffer
+                    => self.windows.active.removeAt(active_idx),
                 }
                 return true;
             }
@@ -654,25 +666,145 @@ pub fn Poller(comptime StreamEnum: type) type {
     };
 }
 
-fn windowsAsyncRead(
+/// The `ReadFile` docuementation states that `lpNumberOfBytesRead` does not have a meaningful
+/// result when using overlapped I/O, but also that it cannot be `null` on Windows 7. For
+/// compatibility, we point it to this dummy variables, which we never otherwise access.
+/// See: https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-readfile
+var win_dummy_bytes_read: u32 = undefined;
+
+/// Read as much data as possible from `handle` with `overlapped`, and write it to the FIFO. Before
+/// returning, queue a read into `small_buf` so that `WaitForMultipleObjects` returns when more data
+/// is available. `handle` must have no pending asynchronous operation.
+fn windowsAsyncReadToFifoAndQueueSmallRead(
     handle: windows.HANDLE,
     overlapped: *windows.OVERLAPPED,
     fifo: *PollFifo,
+    small_buf: *[128]u8,
     bump_amt: usize,
-) !enum { pending, closed } {
+) !enum { empty, populated, closed_populated, closed } {
+    var read_any_data = false;
     while (true) {
-        const buf = try fifo.writableWithSize(bump_amt);
-        var read_bytes: u32 = undefined;
-        const read_result = windows.kernel32.ReadFile(handle, buf.ptr, math.cast(u32, buf.len) orelse math.maxInt(u32), &read_bytes, overlapped);
-        if (read_result == 0) return switch (windows.GetLastError()) {
-            .IO_PENDING => .pending,
-            .BROKEN_PIPE => .closed,
-            else => |err| windows.unexpectedError(err),
+        const fifo_read_pending = while (true) {
+            const buf = try fifo.writableWithSize(bump_amt);
+            const buf_len = math.cast(u32, buf.len) orelse math.maxInt(u32);
+
+            if (0 == windows.kernel32.ReadFile(
+                handle,
+                buf.ptr,
+                buf_len,
+                &win_dummy_bytes_read,
+                overlapped,
+            )) switch (windows.GetLastError()) {
+                .IO_PENDING => break true,
+                .BROKEN_PIPE => return if (read_any_data) .closed_populated else .closed,
+                else => |err| return windows.unexpectedError(err),
+            };
+
+            const num_bytes_read = switch (try windowsGetReadResult(handle, overlapped, false)) {
+                .success => |n| n,
+                .closed => return if (read_any_data) .closed_populated else .closed,
+                .aborted => unreachable,
+            };
+
+            read_any_data = true;
+            fifo.update(num_bytes_read);
+
+            if (num_bytes_read == buf_len) {
+                // We filled the buffer, so there's probably more data available.
+                continue;
+            } else {
+                // We didn't fill the buffer, so assume we're out of data.
+                // There is no pending read.
+                break false;
+            }
         };
-        fifo.update(read_bytes);
+
+        if (fifo_read_pending) cancel_read: {
+            // Cancel the pending read into the FIFO.
+            _ = windows.kernel32.CancelIo(handle);
+
+            // We have to wait for the handle to be signalled, i.e. for the cancellation to complete.
+            switch (windows.kernel32.WaitForSingleObject(handle, windows.INFINITE)) {
+                windows.WAIT_OBJECT_0 => {},
+                windows.WAIT_FAILED => return windows.unexpectedError(windows.GetLastError()),
+                else => unreachable,
+            }
+
+            // If it completed before we canceled, make sure to tell the FIFO!
+            const num_bytes_read = switch (try windowsGetReadResult(handle, overlapped, true)) {
+                .success => |n| n,
+                .closed => return if (read_any_data) .closed_populated else .closed,
+                .aborted => break :cancel_read,
+            };
+            read_any_data = true;
+            fifo.update(num_bytes_read);
+        }
+
+        // Try to queue the 1-byte read.
+        if (0 == windows.kernel32.ReadFile(
+            handle,
+            small_buf,
+            small_buf.len,
+            &win_dummy_bytes_read,
+            overlapped,
+        )) switch (windows.GetLastError()) {
+            .IO_PENDING => {
+                // 1-byte read pending as intended
+                return if (read_any_data) .populated else .empty;
+            },
+            .BROKEN_PIPE => return if (read_any_data) .closed_populated else .closed,
+            else => |err| return windows.unexpectedError(err),
+        };
+
+        // We got data back this time. Write it to the FIFO and run the main loop again.
+        const num_bytes_read = switch (try windowsGetReadResult(handle, overlapped, false)) {
+            .success => |n| n,
+            .closed => return if (read_any_data) .closed_populated else .closed,
+            .aborted => unreachable,
+        };
+        try fifo.write(small_buf[0..num_bytes_read]);
+        read_any_data = true;
     }
 }
 
+/// Simple wrapper around `GetOverlappedResult` to determine the result of a `ReadFile` operation.
+/// If `!allow_aborted`, then `aborted` is never returned (`OPERATION_ABORTED` is considered unexpected).
+///
+/// The `ReadFile` documentation states that the number of bytes read by an overlapped `ReadFile` must be determined using `GetOverlappedResult`, even if the
+/// operation immediately returns data:
+/// "Use NULL for [lpNumberOfBytesRead] if this is an asynchronous operation to avoid potentially
+/// erroneous results."
+/// "If `hFile` was opened with `FILE_FLAG_OVERLAPPED`, the following conditions are in effect: [...]
+/// The lpNumberOfBytesRead parameter should be set to NULL. Use the GetOverlappedResult function to
+/// get the actual number of bytes read."
+/// See: https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-readfile
+fn windowsGetReadResult(
+    handle: windows.HANDLE,
+    overlapped: *windows.OVERLAPPED,
+    allow_aborted: bool,
+) !union(enum) {
+    success: u32,
+    closed,
+    aborted,
+} {
+    var num_bytes_read: u32 = undefined;
+    if (0 == windows.kernel32.GetOverlappedResult(
+        handle,
+        overlapped,
+        &num_bytes_read,
+        0,
+    )) switch (windows.GetLastError()) {
+        .BROKEN_PIPE => return .closed,
+        .OPERATION_ABORTED => |err| if (allow_aborted) {
+            return .aborted;
+        } else {
+            return windows.unexpectedError(err);
+        },
+        else => |err| return windows.unexpectedError(err),
+    };
+    return .{ .success = num_bytes_read };
+}
+
 /// Given an enum, returns a struct with fields of that enum, each field
 /// representing an I/O stream for polling.
 pub fn PollFiles(comptime StreamEnum: type) type {