From 2e77ff7ca9c25a262969cf55992d0bf2ecaa483d Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Wed, 12 Aug 2026 21:45:43 -0700 Subject: [PATCH 1/5] zig build: lock stderr before replacing process image fixes not clearing `std.Progress` output before executing maker --- src/main.zig | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main.zig b/src/main.zig index f0850ffa08462b1841362e76b602ad11d8012529..c577bf1079e218b0ae2a2ca281f89001f1d4e52f 100644 --- a/src/main.zig +++ b/src/main.zig @@ -5134,6 +5134,7 @@ fn jitCmdInner( } if (process.can_replace and options.capture == null) { + _ = try io.lockStderr(&.{}, .no_color); const err = process.replace(io, .{ .argv = child_argv.items, .environ_map = environ_map }); const cmd = try std.mem.join(arena, " ", child_argv.items); fatal("the following command failed to execve with {t}:\n{s}", .{ err, cmd }); -- 2.54.0 From a68b5ee372365034289308967ff488b194aa9fa9 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Wed, 12 Aug 2026 21:52:32 -0700 Subject: [PATCH 2/5] std.Build.Cache: introduce API for taking the files ability to take ownership of the set of input files prior to deinitializing a Manifest --- lib/std/Build/Cache.zig | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/lib/std/Build/Cache.zig b/lib/std/Build/Cache.zig index cf01a5ff8691d55e26353bdfd3fc446042e4ed6e..a66f2bb543a162ba879174434ccc10dce52597de 100644 --- a/lib/std/Build/Cache.zig +++ b/lib/std/Build/Cache.zig @@ -1,7 +1,7 @@ -//! Manages `zig-cache` directories. -//! This is not a general-purpose cache. It is designed to be fast and simple, -//! not to withstand attacks using specially-crafted input. - +//! Tracks metadata of file inputs associated with Zig compiler and build +//! system artifacts in order to determine whether those artifacts must be +//! produced again, or may be retrieved from the cache directory on the +//! filesystem. const Cache = @This(); const builtin = @import("builtin"); @@ -1236,19 +1236,32 @@ pub const Manifest = struct { /// Obtain only the data needed to maintain a lock on the manifest file. /// The `Manifest` remains safe to deinit. + /// /// Don't forget to call `writeManifest` before this! pub fn toOwnedLock(self: *Manifest) Lock { defer self.manifest_file = null; return .{ .manifest_file = self.manifest_file.? }; } + pub fn takeFiles(man: *Manifest) Files { + defer man.files = .empty; + return man.files; + } + + pub fn freeFiles(gpa: Allocator, files: *Files) void { + for (files.keys()) |*file| file.deinit(gpa); + files.deinit(gpa); + } + /// Releases the manifest file and frees any memory the Manifest was using. /// `Manifest.hit` must be called first. + /// /// Don't forget to call `writeManifest` before this! - pub fn deinit(self: *Manifest) void { - const io = self.cache.io; + pub fn deinit(man: *Manifest) void { + const io = man.cache.io; + const gpa = man.cache.gpa; - if (self.manifest_file) |file| { + if (man.manifest_file) |file| { if (builtin.os.tag == .windows) { // See Lock.release for why this is required on Windows file.unlock(io); @@ -1256,10 +1269,8 @@ pub const Manifest = struct { file.close(io); } - for (self.files.keys()) |*file| { - file.deinit(self.cache.gpa); - } - self.files.deinit(self.cache.gpa); + freeFiles(gpa, &man.files); + man.* = undefined; } pub fn populateFileSystemInputs(man: *Manifest, buf: *std.ArrayList(u8)) Allocator.Error!void { -- 2.54.0 From 470f77600d79a5c17b3734f97eaf2462c452718e Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Wed, 12 Aug 2026 21:59:34 -0700 Subject: [PATCH 3/5] Maker: detect modifications to configurer and recompile it including when using `--watch`. This is done by adding an extra auto-generated placeholder Step at the end of `Maker.steps` that contains the file system inputs for the configurer. It is done this way so that the hot path of file system watching does not need to make any special cases, and to avoid more OS-specific logic in file system watching implementation. closes #20602 closes #35460 --- lib/compiler/Maker.zig | 134 ++++++++++++++++++++++++++--------- lib/compiler/Maker/Step.zig | 14 +++- lib/compiler/Maker/Watch.zig | 61 ++++++---------- 3 files changed, 133 insertions(+), 76 deletions(-) diff --git a/lib/compiler/Maker.zig b/lib/compiler/Maker.zig index 51195aaf23bea8bb551d175b61cc57804b3a50e1..0b10b8fdacfa220a87b9e078d2067c9fb468a99d 100644 --- a/lib/compiler/Maker.zig +++ b/lib/compiler/Maker.zig @@ -44,6 +44,10 @@ gpa: Allocator, graph: *Graph, install_paths: InstallPaths, scanned_config: *const ScannedConfig, +/// Includes an extra auto-generated placeholder Step at the end that indicates +/// configure must be rerun. It is done this way so that the hot path of file +/// system watching does not need to make any special cases, and to avoid more +/// OS-specific logic in file system watching implementation. steps: []Step, generated_files: []Path, run_args: ?[]const []const u8, @@ -708,7 +712,12 @@ pub fn main(init: process.Init.Minimal) !void { break :s &protocol_server_allocation; } else null; - while (true) { + configure: while (true) { + // Set of files that, if modified, imply that recompiling and rerunning + // configurer is needed. + var configure_source_files: Cache.Manifest.Files = .empty; + defer Cache.Manifest.freeFiles(gpa, &configure_source_files); + // If this fails, we can still start the server and wait for user // to request a rebuild. If it returns error.FailedButCacheIntact // we can even still do file system watching and automatically @@ -730,6 +739,7 @@ pub fn main(init: process.Init.Minimal) !void { .fetch_only = fetch_only, .print_configuration = print_configuration, .forks = forks.items, + .src_files = &configure_source_files, })) |scanned_config| { if (help_menu) { scanned_config.printUsage(&graph, initStdoutWriter(io)) catch |err| switch (err) { @@ -766,7 +776,9 @@ pub fn main(init: process.Init.Minimal) !void { .include = install_include_path, }, - .steps = try arena.alloc(Step, scanned_config.configuration.steps.len), + // Extra step at the end which is the autogenerated placeholder + // step which indicates that we need to reconfigure. + .steps = try arena.alloc(Step, scanned_config.configuration.steps.len + 1), .generated_files = try arena.alloc(Path, scanned_config.configuration.generated_files_len), .run_args = run_args, @@ -843,7 +855,7 @@ pub fn main(init: process.Init.Minimal) !void { try select.concurrent(.message, Server.receiveMessage, .{s}); maker.watch = body.flags.watch; - maker.prepare(steps) catch |err| switch (err) { + maker.prepare(steps, &configure_source_files) catch |err| switch (err) { error.DependencyLoopDetected, error.InsufficientMemory => { // TODO handle DependencyLoopDetected as error.FailedButCacheIntact // and handle InsufficientMemory as error.AlreadyReported @@ -859,8 +871,11 @@ pub fn main(init: process.Init.Minimal) !void { if (!Watch.have_impl) unreachable; if (w == null) w = try .init(&maker); - try w.?.update(maker.step_stack.keys()); - try select.concurrent(.fs_event, Watch.wait, .{ &w.?, if (in_debounce) .{ .ms = debounce_interval_ms } else .none }); + try updateWatch(&maker, &w.?); + try select.concurrent(.fs_event, Watch.wait, .{ + &w.?, + if (in_debounce) .{ .ms = debounce_interval_ms } else .none, + }); } continue :loop try select.await(); @@ -870,7 +885,13 @@ pub fn main(init: process.Init.Minimal) !void { }, .fs_event => |payload| { if (!Watch.have_impl) unreachable; - switch (try payload) { + switch (payload catch |err| switch (err) { + error.MustReconfigure => { + try io.sleep(.fromMilliseconds(debounce_interval_ms), .awake); + continue :configure; + }, + else => |e| fatal("file watching failed: {t}", .{e}), + }) { .timeout => { assert(in_debounce); markFailedStepsDirty(&maker); @@ -880,7 +901,10 @@ pub fn main(init: process.Init.Minimal) !void { .dirty => in_debounce = true, .clean => {}, } - try select.concurrent(.fs_event, Watch.wait, .{ &w.?, if (in_debounce) .{ .ms = debounce_interval_ms } else .none }); + try select.concurrent(.fs_event, Watch.wait, .{ + &w.?, + if (in_debounce) .{ .ms = debounce_interval_ms } else .none, + }); continue :loop try select.await(); }, } @@ -889,7 +913,7 @@ pub fn main(init: process.Init.Minimal) !void { const initial_steps = try maker.resolveTopLevelSteps(step_names.items); defer gpa.free(initial_steps); - maker.prepare(initial_steps) catch |err| switch (err) { + maker.prepare(initial_steps, &configure_source_files) catch |err| switch (err) { error.DependencyLoopDetected, error.InsufficientMemory => { // TODO handle DependencyLoopDetected as error.FailedButCacheIntact // and handle InsufficientMemory as error.AlreadyReported @@ -938,7 +962,7 @@ pub fn main(init: process.Init.Minimal) !void { // Comptime-known guard to prevent including the logic below when `!Watch.have_impl`. if (!Watch.have_impl) unreachable; - try w.update(maker.step_stack.keys()); + try updateWatch(&maker, &w); // Wait until a file system notification arrives. Read all such events // until the buffer is empty. Then wait for a debounce interval, resetting @@ -950,21 +974,34 @@ pub fn main(init: process.Init.Minimal) !void { w.dir_count, countSubProcesses(&maker), }) catch &caption_buf; var debouncing_node = main_progress_node.start(caption, 0); + defer debouncing_node.end(); var in_debounce = false; - while (true) switch (try w.wait(if (in_debounce) .{ .ms = debounce_interval_ms } else .none)) { - .timeout => { - assert(in_debounce); - debouncing_node.end(); - markFailedStepsDirty(&maker); - continue :rebuild; - }, - .dirty => if (!in_debounce) { - in_debounce = true; - debouncing_node.end(); - debouncing_node = main_progress_node.start("Debouncing (Change Detected)", 0); - }, - .clean => {}, - }; + while (true) { + const timeout: Watch.Timeout = if (in_debounce) .{ .ms = debounce_interval_ms } else .none; + switch (w.wait(timeout) catch |err| switch (err) { + error.MustReconfigure => { + debouncing_node.end(); + debouncing_node = main_progress_node.start("Debouncing (Change Detected)", 0); + try io.sleep(.fromMilliseconds(debounce_interval_ms), .awake); + continue :configure; + }, + else => |e| fatal("file watching failed: {t}", .{e}), + }) { + .timeout => { + assert(in_debounce); + debouncing_node.end(); + debouncing_node = .none; + markFailedStepsDirty(&maker); + continue :rebuild; + }, + .dirty => if (!in_debounce) { + in_debounce = true; + debouncing_node.end(); + debouncing_node = main_progress_node.start("Debouncing (Change Detected)", 0); + }, + .clean => {}, + } + } } } else |err| { const can_fs_watch = switch (err) { @@ -991,6 +1028,15 @@ pub fn main(init: process.Init.Minimal) !void { } } +/// Temporarily adds the reconfigure pseudostep to step_stack, calls +/// `Watch.update`, and then pops it again. +fn updateWatch(maker: *Maker, watch: *Watch) !void { + const step_stack = &maker.step_stack; + try step_stack.putNoClobber(maker.gpa, @fromBackingInt(@intCast(maker.steps.len - 1)), {}); + defer _ = step_stack.pop().?; + try watch.update(step_stack.keys()); +} + const ConfigureOptions = struct { configure_argv: [][]const u8, conf_argv_index_build_root: usize, @@ -1008,6 +1054,7 @@ const ConfigureOptions = struct { fetch_only: bool, print_configuration: PrintConfiguration, forks: []Fork, + src_files: *Cache.Manifest.Files, }; fn configure(graph: *Graph, options: ConfigureOptions) !ScannedConfig { @@ -1362,14 +1409,14 @@ fn configure(graph: *Graph, options: ConfigureOptions) !ScannedConfig { if (config_man) |man| { if (try man.hit(compile_prog_node)) { + log.debug("configuration cache hit", .{}); const digest = man.final(); - break :cp .{ - .{ - .root_dir = graph.local_cache_root, - .sub_path = try arena.print("c/{s}", .{&digest}), - }, - man.toOwnedLock(), + const path: Path = .{ + .root_dir = graph.local_cache_root, + .sub_path = try arena.print("c/{s}", .{&digest}), }; + options.src_files.* = man.takeFiles(); + break :cp .{ path, man.toOwnedLock() }; } } const configure_exe_path: Path = if (std.zig.buildExeSubprocess(gpa, io, .{ @@ -1505,6 +1552,7 @@ fn configure(graph: *Graph, options: ConfigureOptions) !ScannedConfig { }); }; man.writeManifest() catch |err| log.warn("failed to write cache manifest: {t}", .{err}); + options.src_files.* = man.takeFiles(); break :cp .{ final_path, man.toOwnedLock() }; } }; @@ -2119,7 +2167,13 @@ fn markFailedStepsDirty(maker: *Maker) void { for (all_steps) |step_index| { const step = maker.stepByIndex(step_index); switch (step.state) { - .dependency_failure, .dependency_skipped, .failure, .skipped => _ = maker.invalidateResult(step), + .dependency_failure, + .dependency_skipped, + .failure, + .skipped, + => _ = maker.invalidateResult(step) catch |err| switch (err) { + error.MustReconfigure => unreachable, + }, else => continue, } } @@ -2173,7 +2227,11 @@ fn resolveTopLevelSteps(maker: *Maker, step_names: []const []const u8) ![]const return try gpa.dupe(Configuration.Step.Index, result.keys()); } -fn prepare(maker: *Maker, step_indices: []const Configuration.Step.Index) !void { +fn prepare( + maker: *Maker, + step_indices: []const Configuration.Step.Index, + configure_source_files: *const Cache.Manifest.Files, +) !void { const gpa = maker.gpa; const graph = maker.graph; const arena = graph.arena; @@ -2182,10 +2240,17 @@ fn prepare(maker: *Maker, step_indices: []const Configuration.Step.Index) !void const step_stack = &maker.step_stack; const c = &maker.scanned_config.configuration; - for (maker.steps, 0..) |*step, step_index_usize| { + // The last element is a reserved special pseudostep which contains the + // watch inputs for the configurer executable. + for (maker.steps[0 .. maker.steps.len - 1], 0..) |*step, step_index_usize| { const step_index: Configuration.Step.Index = @fromBackingInt(@intCast(step_index_usize)); step.* = .{ .extended = .init(step_index.ptr(c).flags(c).tag) }; } + { + const last_step = &maker.steps[maker.steps.len - 1]; + last_step.* = .{ .extended = .init(.top_level) }; + try last_step.setWatchInputsFromManifestFiles(maker, configure_source_files, graph.cache.prefixes()); + } try initial_steps.ensureUnusedCapacity(gpa, step_indices.len); try step_stack.ensureUnusedCapacity(gpa, step_indices.len); @@ -3045,14 +3110,15 @@ fn constructGraphAndCheckForDependencyLoop( /// When file watching, prepares the step for being re-evaluated. Returns /// `true` if the step was newly invalidated, `false` if it was already /// invalidated. -pub fn invalidateResult(maker: *Maker, step: *Step) bool { +pub fn invalidateResult(maker: *Maker, step: *Step) error{MustReconfigure}!bool { + if (step == &maker.steps[maker.steps.len - 1]) return error.MustReconfigure; if (step.state == .precheck_done) return false; assert(step.pending_deps == 0); step.state = .precheck_done; step.reset(maker); for (step.dependants.items) |dependant_index| { const dependant = maker.stepByIndex(dependant_index); - _ = invalidateResult(maker, dependant); + _ = try invalidateResult(maker, dependant); dependant.pending_deps += 1; } return true; diff --git a/lib/compiler/Maker/Step.zig b/lib/compiler/Maker/Step.zig index 181cdf2bd75fc678fc45d72cbab16a9e9bbeecd1..2ce90b5a9b7725d452258af167de82b60b973a6b 100644 --- a/lib/compiler/Maker/Step.zig +++ b/lib/compiler/Maker/Step.zig @@ -781,12 +781,20 @@ pub fn writeManifestAndWatch(s: *Step, maker: *Maker, man: *Cache.Manifest) !voi try setWatchInputsFromManifest(s, maker, man); } -fn setWatchInputsFromManifest(s: *Step, maker: *Maker, man: *Cache.Manifest) !void { +pub fn setWatchInputsFromManifest(s: *Step, maker: *Maker, man: *Cache.Manifest) !void { + return setWatchInputsFromManifestFiles(s, maker, &man.files, man.cache.prefixes()); +} + +pub fn setWatchInputsFromManifestFiles( + s: *Step, + maker: *Maker, + files: *const Cache.Manifest.Files, + prefixes: []const Cache.Directory, +) !void { const graph = maker.graph; const arena = graph.arena; // TODO don't leak into process arena - const prefixes = man.cache.prefixes(); clearWatchInputs(s, maker); - for (man.files.keys()) |file| { + for (files.keys()) |file| { // The file path data is freed when the cache manifest is cleaned up at the end of `make`. const sub_path = try arena.dupe(u8, file.prefixed_path.sub_path); try addWatchInputFromPath(s, maker, .{ diff --git a/lib/compiler/Maker/Watch.zig b/lib/compiler/Maker/Watch.zig index 4d40facdfc71076eb18f5958856fa3fce4f56741..6d3fd41ad9baa30b6e26195f1de015e359d3248e 100644 --- a/lib/compiler/Maker/Watch.zig +++ b/lib/compiler/Maker/Watch.zig @@ -49,7 +49,10 @@ const Os = switch (builtin.os.tag) { poll_fds: std.array_hash_map.Auto(MountId, posix.pollfd), const MountId = i32; - const HandleTable = std.array_hash_map.Custom(FileHandle, struct { mount_id: MountId, reaction_set: ReactionSet }, FileHandle.Adapter, false); + const HandleTable = std.array_hash_map.Custom(FileHandle, struct { + mount_id: MountId, + reaction_set: ReactionSet, + }, FileHandle.Adapter, false); const fan_mask: std.os.linux.fanotify.MarkMask = .{ .CLOSE_WRITE = true, @@ -152,10 +155,8 @@ const Os = switch (builtin.os.tag) { }) { assert(meta[0].vers == M.VERSION); if (meta[0].mask.Q_OVERFLOW) { - any_dirty = true; - std.log.warn("file system watch queue overflowed; falling back to fstat", .{}); - markAllFilesDirty(w); - return true; + std.log.warn("file system watch queue overflowed; reconfiguring", .{}); + return error.MustReconfigure; } const fid: *align(1) fanotify.event_info_fid = @ptrCast(meta + 1); switch (fid.hdr.info_type) { @@ -166,9 +167,9 @@ const Os = switch (builtin.os.tag) { const lfh: FileHandle = .{ .handle = file_handle }; if (w.os.handle_table.getPtr(lfh)) |value| { if (value.reaction_set.getPtr(".")) |glob_set| - any_dirty = markStepSetDirty(maker, glob_set, any_dirty); + any_dirty = try markStepSetDirty(maker, glob_set, any_dirty); if (value.reaction_set.getPtr(file_name)) |step_set| - any_dirty = markStepSetDirty(maker, step_set, any_dirty); + any_dirty = try markStepSetDirty(maker, step_set, any_dirty); } }, else => |t| std.log.warn("unexpected fanotify event '{t}'", .{t}), @@ -304,8 +305,11 @@ const Os = switch (builtin.os.tag) { if (events_len == 0) return .timeout; for (w.os.poll_fds.values()) |poll_fd| { - if (poll_fd.revents & std.posix.POLL.IN == std.posix.POLL.IN and try markDirtySteps(w, poll_fd.fd)) + if (poll_fd.revents & std.posix.POLL.IN == std.posix.POLL.IN and + try markDirtySteps(w, poll_fd.fd)) + { return .dirty; + } } return .clean; } @@ -521,10 +525,8 @@ const Os = switch (builtin.os.tag) { var any_dirty = false; const bytes_returned = dir.iosb.Information; if (bytes_returned == 0) { - std.log.warn("file system watch queue overflowed; falling back to fstat", .{}); - markAllFilesDirty(w); - try dir.startListening(w); - return true; + std.log.warn("file system watch queue overflowed; reconfiguring", .{}); + return error.MustReconfigure; } var file_name_buf: [std.fs.max_path_bytes]u8 = undefined; var offset: usize = 0; @@ -532,9 +534,9 @@ const Os = switch (builtin.os.tag) { const notify: *windows.FILE.NOTIFY.INFORMATION = @ptrCast(@alignCast(&dir.buffer[offset])); const file_name = file_name_buf[0..std.unicode.wtf16LeToWtf8(&file_name_buf, notify.fileName())]; if (dir.reaction_set.getPtr(".")) |glob_set| - any_dirty = markStepSetDirty(maker, glob_set, any_dirty); + any_dirty = try markStepSetDirty(maker, glob_set, any_dirty); if (dir.reaction_set.getPtr(file_name)) |step_set| - any_dirty = markStepSetDirty(maker, step_set, any_dirty); + any_dirty = try markStepSetDirty(maker, step_set, any_dirty); if (notify.NextEntryOffset == 0) break; @@ -851,13 +853,13 @@ const Os = switch (builtin.os.tag) { // If we knew the basename of the changed file, here we would // mark only the step set dirty, and possibly the glob set: //if (reaction_set.getPtr(".")) |glob_set| - // any_dirty = markStepSetDirty(maker, glob_set, any_dirty); + // any_dirty = try markStepSetDirty(maker, glob_set, any_dirty); //if (reaction_set.getPtr(file_name)) |step_set| - // any_dirty = markStepSetDirty(maker, step_set, any_dirty); + // any_dirty = try markStepSetDirty(maker, step_set, any_dirty); // However we don't know the file name so just mark all the // sets dirty for this directory. for (reaction_set.values()) |*step_set| { - any_dirty = markStepSetDirty(maker, step_set, any_dirty); + any_dirty = try markStepSetDirty(maker, step_set, any_dirty); } } return any_dirty; @@ -915,31 +917,11 @@ pub const Match = struct { }; }; -fn markAllFilesDirty(w: *Watch) void { - const maker = w.maker; - - for (switch (builtin.os.tag) { - .windows => w.os.handle_table.keys(), - else => w.os.handle_table.values(), - }) |item| { - const reaction_set = switch (builtin.os.tag) { - .linux, .windows => item.reaction_set, - else => item, - }; - for (reaction_set.values()) |step_set| { - for (step_set.keys()) |step_index| { - const step = maker.stepByIndex(step_index); - _ = maker.invalidateResult(step); - } - } - } -} - -fn markStepSetDirty(maker: *Maker, step_set: *StepSet, any_dirty: bool) bool { +fn markStepSetDirty(maker: *Maker, step_set: *StepSet, any_dirty: bool) error{MustReconfigure}!bool { var this_any_dirty = false; for (step_set.keys()) |step_index| { const step = maker.stepByIndex(step_index); - if (maker.invalidateResult(step)) this_any_dirty = true; + if (try maker.invalidateResult(step)) this_any_dirty = true; } return any_dirty or this_any_dirty; } @@ -984,6 +966,7 @@ pub const WaitResult = enum { clean, }; +/// May return `error.MustReconfigure`. pub fn wait(w: *Watch, timeout: Timeout) !WaitResult { return Os.wait(w, timeout); } -- 2.54.0 From 99bfd07854b4e14e01e32e83302707cacde15cb5 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Wed, 12 Aug 2026 22:42:03 -0700 Subject: [PATCH 4/5] Maker: deinit Watch when exiting scope this wasn't really needed before but now it's pretty important since configuration needs to be rerun sometimes --- lib/compiler/Maker.zig | 30 +++++++++-------- lib/compiler/Maker/Watch.zig | 64 +++++++++++++++++++++++++++++++++--- 2 files changed, 76 insertions(+), 18 deletions(-) diff --git a/lib/compiler/Maker.zig b/lib/compiler/Maker.zig index 0b10b8fdacfa220a87b9e078d2067c9fb468a99d..06b4286cb39b58ab82bcc4ea3b6333fbea8f719b 100644 --- a/lib/compiler/Maker.zig +++ b/lib/compiler/Maker.zig @@ -225,7 +225,7 @@ pub fn main(init: process.Init.Minimal) !void { var skip_oom_steps = false; var test_timeout_ns: ?u64 = null; var color: Color = .settingFromEnvironment(&graph.environ_map); - var watch = false; + var watch_flag = false; var fuzz: ?Fuzz.Mode = null; var debounce_interval_ms: u16 = 50; var listen: bool = false; @@ -474,7 +474,7 @@ pub fn main(init: process.Init.Minimal) !void { } else if (mem.eql(u8, arg, "--verbose-llvm-ir")) { graph.verbose_llvm_ir = true; } else if (mem.eql(u8, arg, "--watch")) { - watch = true; + watch_flag = true; } else if (mem.eql(u8, arg, "--time-report")) { graph.time_report = true; if (webui_listen == null) webui_listen = .{ .ip6 = .loopback(0) }; @@ -574,7 +574,7 @@ pub fn main(init: process.Init.Minimal) !void { } const early_exit_mode = fetch_only or help_menu or steps_menu or print_configuration != .none; - const server_mode = !early_exit_mode and (watch or webui_listen != null or fuzz != null or listen); + const server_mode = !early_exit_mode and (watch_flag or webui_listen != null or fuzz != null or listen); process.raiseFileDescriptorLimit(); @@ -701,7 +701,7 @@ pub fn main(init: process.Init.Minimal) !void { var protocol_server_allocation: AvoidableServer = undefined; const protocol_server: ?*AvoidableServer = if (listen) s: { if (builtin.single_threaded) fatal("--listen is not yet supported on single-threaded hosts", .{}); - if (watch) fatal("using '--watch' and '--listen' together is not supported", .{}); + if (watch_flag) fatal("using '--watch' and '--listen' together is not supported", .{}); if (fuzz != null) fatal("using '--fuzz' and '--listen' together is not supported", .{}); if (step_names.items.len > 0) fatal("build steps must be provided over the protocol instead of using CLI arguments", .{}); protocol_server_allocation = .{ @@ -788,7 +788,7 @@ pub fn main(init: process.Init.Minimal) !void { .skip_oom_steps = skip_oom_steps, .unit_test_timeout_ns = test_timeout_ns, - .watch = watch, + .watch = watch_flag, .web_server = web_server, .protocol_server = protocol_server, .protocol_server_mutex = .init, @@ -801,7 +801,7 @@ pub fn main(init: process.Init.Minimal) !void { .multiline_errors = multiline_errors, .summary = summary orelse if (listen) .none - else if (watch or webui_listen != null) + else if (watch_flag or webui_listen != null) .new else .failures, @@ -820,7 +820,8 @@ pub fn main(init: process.Init.Minimal) !void { if (protocol_server) |s| { try s.serveStringMessage(.bsp_configuration, try arena.print("{f}", .{scanned_config.path})); - var w: ?Watch = null; + var watch: ?Watch = null; + defer if (watch) |*w| w.deinit(); const Event = union(enum) { message: Reader.Error!Client.Message.Header, @@ -869,11 +870,11 @@ pub fn main(init: process.Init.Minimal) !void { if (body.flags.watch) { if (!Watch.have_impl) unreachable; - if (w == null) w = try .init(&maker); + if (watch == null) watch = try .init(&maker); - try updateWatch(&maker, &w.?); + try updateWatch(&maker, &watch.?); try select.concurrent(.fs_event, Watch.wait, .{ - &w.?, + &watch.?, if (in_debounce) .{ .ms = debounce_interval_ms } else .none, }); } @@ -902,7 +903,7 @@ pub fn main(init: process.Init.Minimal) !void { .clean => {}, } try select.concurrent(.fs_event, Watch.wait, .{ - &w.?, + &watch.?, if (in_debounce) .{ .ms = debounce_interval_ms } else .none, }); continue :loop try select.await(); @@ -924,10 +925,11 @@ pub fn main(init: process.Init.Minimal) !void { }; var w: Watch = w: { - if (!watch) break :w undefined; + if (!watch_flag) break :w undefined; if (!Watch.have_impl) fatal("--watch not yet implemented for {t}", .{native_os}); break :w try .init(&maker); }; + defer w.deinit(); if (web_server) |ws| try ws.updateConfiguration(&maker); @@ -942,7 +944,7 @@ pub fn main(init: process.Init.Minimal) !void { if (web_server) |ws| { const c = &scanned_config.configuration; - assert(!watch); // fatal error after CLI parsing + assert(!watch_flag); // fatal error after CLI parsing while (true) switch (try ws.wait()) { .rebuild => { for (maker.step_stack.keys()) |step_index| { @@ -1019,7 +1021,7 @@ pub fn main(init: process.Init.Minimal) !void { if (protocol_server != null) { fatal("(zig build system) TODO send error messages to client when build.zig compilation fails", .{}); } - if (watch and can_fs_watch) { + if (watch_flag and can_fs_watch) { fatal("(zig build system) TODO set up fs watching even when build.zig compilation fails", .{}); } else { fatal("(zig build system) TODO stay running and wait for user to request rebuild even when build.zig compilation fails", .{}); diff --git a/lib/compiler/Maker/Watch.zig b/lib/compiler/Maker/Watch.zig index 6d3fd41ad9baa30b6e26195f1de015e359d3248e..c9af991a4bfc7bd6352a8bfeb5f314527de6fb5e 100644 --- a/lib/compiler/Maker/Watch.zig +++ b/lib/compiler/Maker/Watch.zig @@ -84,7 +84,7 @@ const Os = switch (builtin.os.tag) { } fn destroy(lfh: FileHandle, gpa: Allocator) void { - const ptr: [*]u8 = @ptrCast(lfh.handle); + const ptr: [*]align(@alignOf(std.os.linux.file_handle)) u8 = @ptrCast(@alignCast(lfh.handle)); const allocated_slice = ptr[0 .. @sizeOf(std.os.linux.file_handle) + lfh.handle.handle_bytes]; return gpa.free(allocated_slice); } @@ -124,6 +124,24 @@ const Os = switch (builtin.os.tag) { }; } + fn deinit(w: *Watch) void { + const gpa = w.maker.gpa; + + for (w.os.handle_table.keys(), w.os.handle_table.values()) |fh, *reaction| { + fh.destroy(gpa); + reaction.reaction_set.deinit(gpa); + } + w.os.handle_table.deinit(gpa); + + for (w.os.poll_fds.values()) |pollfd| { + Io.Threaded.closeFd(pollfd.fd); + } + w.os.poll_fds.deinit(gpa); + + w.dir_table.deinit(gpa); + w.* = undefined; + } + fn getDirHandle(gpa: Allocator, path: std.Build.Cache.Path, mount_id: *MountId) !FileHandle { var file_handle_buffer: [@sizeOf(std.os.linux.file_handle) + 128]u8 align(@alignOf(std.os.linux.file_handle)) = undefined; var buf: [std.fs.max_path_bytes]u8 = undefined; @@ -365,7 +383,7 @@ const Os = switch (builtin.os.tag) { } } - fn notifyApc(apc_context: ?*anyopaque, iosb: *windows.IO_STATUS_BLOCK, _: windows.ULONG) align(std.Io.Threaded.apc_align) callconv(.winapi) void { + fn notifyApc(apc_context: ?*anyopaque, iosb: *windows.IO_STATUS_BLOCK, _: windows.ULONG) align(Io.Threaded.apc_align) callconv(.winapi) void { const w: *Watch = @ptrCast(@alignCast(apc_context)); const dir: *Directory = @fieldParentPtr("iosb", iosb); assert(iosb.u.Status != .PENDING); @@ -485,6 +503,18 @@ const Os = switch (builtin.os.tag) { }; } + fn deinit(w: *Watch) void { + const gpa = w.maker.gpa; + + for (w.os.handle_table.keys()) |dir| { + dir.deinit(gpa, w); + } + w.os.handle_table.deinit(gpa); + + w.dir_table.deinit(gpa); + w.* = undefined; + } + fn getFileId(handle: windows.HANDLE) !FileId { var file_id: FileId = undefined; var io_status: windows.IO_STATUS_BLOCK = undefined; @@ -695,6 +725,21 @@ const Os = switch (builtin.os.tag) { }; } + fn deinit(w: *Watch) void { + const gpa = w.maker.gpa; + + for (w.os.handles.items(.rs), w.os.handles.items(.dir_fd)) |rs, dir_fd| { + rs.deinit(gpa); + Os.Threaded.closeFd(dir_fd); + } + w.os.handles.deinit(gpa); + + Os.Threaded.closeFd(w.os.kq_fd); + + w.dir_table.deinit(gpa); + w.* = undefined; + } + fn update(w: *Watch, steps: []const Configuration.Step.Index) !void { const maker = w.maker; const gpa = maker.gpa; @@ -713,7 +758,7 @@ const Os = switch (builtin.os.tag) { fatal("failed to open directory {f}: {t}", .{ path, err }); }; // Empirically the dir has to stay open or else no events are triggered. - errdefer if (!skip_open_dir) std.Io.Threaded.closeFd(dir_fd); + errdefer if (!skip_open_dir) Io.Threaded.closeFd(dir_fd); const changes = [1]posix.Kevent{.{ .ident = @bitCast(@as(isize, dir_fd)), .filter = std.c.EVFILT.VNODE, @@ -813,7 +858,7 @@ const Os = switch (builtin.os.tag) { }; const filtered_changes = if (i == handles.len - 1) changes[0..1] else &changes; _ = try Io.Kqueue.kevent(w.os.kq_fd, filtered_changes, &.{}, null); - if (path.sub_path.len != 0) std.Io.Threaded.closeFd(dir_fd); + if (path.sub_path.len != 0) Io.Threaded.closeFd(dir_fd); w.dir_table.swapRemoveAt(i); handles.swapRemove(i); @@ -877,6 +922,13 @@ const Os = switch (builtin.os.tag) { .maker = maker, }; } + fn deinit(w: *Watch) void { + const gpa = w.maker.gpa; + const io = w.maker.io; + w.os.fse.deinit(gpa, io); + w.dir_table.deinit(gpa); + w.* = undefined; + } fn update(w: *Watch, steps: []const Configuration.Step.Index) !void { try w.os.fse.setPaths(w.maker, steps); w.dir_count = w.os.fse.watch_roots.len; @@ -970,3 +1022,7 @@ pub const WaitResult = enum { pub fn wait(w: *Watch, timeout: Timeout) !WaitResult { return Os.wait(w, timeout); } + +pub fn deinit(w: *Watch) void { + Os.deinit(w); +} -- 2.54.0 From bc1f280a77ce118300d3f486932c5768911b0e6f Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Thu, 13 Aug 2026 00:13:37 -0700 Subject: [PATCH 5/5] Maker: fix watching on macos and bsds --- lib/compiler/Maker/Watch.zig | 15 ++++++----- lib/compiler/Maker/Watch/FsEvents.zig | 36 +++++++++++++++++++++------ 2 files changed, 35 insertions(+), 16 deletions(-) diff --git a/lib/compiler/Maker/Watch.zig b/lib/compiler/Maker/Watch.zig index c9af991a4bfc7bd6352a8bfeb5f314527de6fb5e..ba0f260afcf0a3db57eddca9cd59caf2e48de311 100644 --- a/lib/compiler/Maker/Watch.zig +++ b/lib/compiler/Maker/Watch.zig @@ -728,13 +728,13 @@ const Os = switch (builtin.os.tag) { fn deinit(w: *Watch) void { const gpa = w.maker.gpa; - for (w.os.handles.items(.rs), w.os.handles.items(.dir_fd)) |rs, dir_fd| { + for (w.os.handles.items(.rs), w.os.handles.items(.dir_fd)) |*rs, dir_fd| { rs.deinit(gpa); - Os.Threaded.closeFd(dir_fd); + Io.Threaded.closeFd(dir_fd); } w.os.handles.deinit(gpa); - Os.Threaded.closeFd(w.os.kq_fd); + Io.Threaded.closeFd(w.os.kq_fd); w.dir_table.deinit(gpa); w.* = undefined; @@ -875,12 +875,12 @@ const Os = switch (builtin.os.tag) { var n = try Io.Kqueue.kevent(w.os.kq_fd, &.{}, &event_buffer, timeout.toTimespec(×pec_buffer)); if (n == 0) return .timeout; const reaction_sets = w.os.handles.items(.rs); - var any_dirty = markDirtySteps(maker, reaction_sets, event_buffer[0..n], false); + var any_dirty = try markDirtySteps(maker, reaction_sets, event_buffer[0..n], false); timespec_buffer = .{ .sec = 0, .nsec = 0 }; while (n == event_buffer.len) { n = try Io.Kqueue.kevent(w.os.kq_fd, &.{}, &event_buffer, ×pec_buffer); if (n == 0) break; - any_dirty = markDirtySteps(maker, reaction_sets, event_buffer[0..n], any_dirty); + any_dirty = try markDirtySteps(maker, reaction_sets, event_buffer[0..n], any_dirty); } return if (any_dirty) .dirty else .clean; } @@ -890,7 +890,7 @@ const Os = switch (builtin.os.tag) { reaction_sets: []ReactionSet, events: []const std.c.Kevent, start_any_dirty: bool, - ) bool { + ) !bool { var any_dirty = start_any_dirty; for (events) |event| { const index: usize = @intCast(event.udata); @@ -924,9 +924,8 @@ const Os = switch (builtin.os.tag) { } fn deinit(w: *Watch) void { const gpa = w.maker.gpa; - const io = w.maker.io; + const io = w.maker.graph.io; w.os.fse.deinit(gpa, io); - w.dir_table.deinit(gpa); w.* = undefined; } fn update(w: *Watch, steps: []const Configuration.Step.Index) !void { diff --git a/lib/compiler/Maker/Watch/FsEvents.zig b/lib/compiler/Maker/Watch/FsEvents.zig index 6fc0bdbb5769082b8cd4c901cca350e5ff4a31a0..8c42f3ac8491204d65633c1fb4ecc706094d9b36 100644 --- a/lib/compiler/Maker/Watch/FsEvents.zig +++ b/lib/compiler/Maker/Watch/FsEvents.zig @@ -46,6 +46,8 @@ since_event: FSEventStreamEventId, cwd_path: []const u8, +must_reconfigure: bool, + /// All of the symbols we pull from the `dlopen`ed CoreServices framework. If any of these symbols /// is not present, `init` will close the framework and return an error. const ResolvedSymbols = struct { @@ -104,13 +106,15 @@ pub fn init(cwd_path: []const u8) error{ OpenFrameworkFailed, MissingCoreService // to notice any changes which happened during said work. .since_event = resolved_symbols.FSEventsGetCurrentEventId(), .cwd_path = cwd_path, + .must_reconfigure = false, }; } pub fn deinit(fse: *FsEvents, gpa: Allocator, io: Io) void { + _ = io; fse.waiting_semaphore.as_object().release(); fse.dispatch_queue.as_object().release(); - fse.core_services.close(io); + fse.core_services.close(); gpa.free(fse.watch_roots); fse.watch_paths.deinit(gpa); @@ -211,7 +215,7 @@ pub fn setPaths(fse: *FsEvents, maker: *Maker, steps: []const std.Build.Configur } } -pub fn wait(fse: *FsEvents, maker: *Maker, timeout_ns: ?u64) error{ OutOfMemory, StartFailed }!Watch.WaitResult { +pub fn wait(fse: *FsEvents, maker: *Maker, timeout_ns: ?u64) error{ OutOfMemory, StartFailed, MustReconfigure }!Watch.WaitResult { if (fse.watch_roots.len == 0) @panic("nothing to watch"); const gpa = maker.gpa; @@ -285,6 +289,7 @@ pub fn wait(fse: *FsEvents, maker: *Maker, timeout_ns: ?u64) error{ OutOfMemory, const ns = timeout_ns orelse break :timeout .FOREVER; break :timeout .time(.NOW, @intCast(ns)); }); + if (fse.must_reconfigure) return error.MustReconfigure; return switch (result) { 0 => .dirty, else => .timeout, @@ -355,13 +360,23 @@ fn eventCallback( false => { if (fse.watch_paths.get(event_path)) |steps| { assert(steps.len > 0); - if (invalidateSteps(maker, steps)) any_dirty = true; + if (invalidateSteps(maker, steps) catch |err| switch (err) { + error.MustReconfigure => { + fse.must_reconfigure = true; + break; + }, + }) any_dirty = true; } if (std.fs.path.dirname(event_path)) |event_dirname| { // Modifying '/foo/bar' triggers the watch on '/foo'. if (fse.watch_paths.get(event_dirname)) |steps| { assert(steps.len > 0); - if (invalidateSteps(maker, steps)) any_dirty = true; + if (invalidateSteps(maker, steps) catch |err| switch (err) { + error.MustReconfigure => { + fse.must_reconfigure = true; + break; + }, + }) any_dirty = true; } } }, @@ -374,13 +389,18 @@ fn eventCallback( const changed_path = std.fs.path.dirname(event_path) orelse event_path; for (fse.watch_paths.keys(), fse.watch_paths.values()) |watching_path, steps| { if (dirStartsWith(watching_path, changed_path)) { - if (invalidateSteps(maker, steps)) any_dirty = true; + if (invalidateSteps(maker, steps) catch |err| switch (err) { + error.MustReconfigure => { + fse.must_reconfigure = true; + break; + }, + }) any_dirty = true; } } }, } } - if (any_dirty) { + if (any_dirty or fse.must_reconfigure) { fse.since_event = rs.FSEventStreamGetLatestEventId(stream); _ = fse.waiting_semaphore.signal(); } @@ -392,11 +412,11 @@ fn dirStartsWith(path: []const u8, prefix: []const u8) bool { return true; // `path` is `/foo/bar/...`, `prefix` is `/foo/bar` } -fn invalidateSteps(maker: *Maker, steps: []const std.Build.Configuration.Step.Index) bool { +fn invalidateSteps(maker: *Maker, steps: []const std.Build.Configuration.Step.Index) !bool { var any_dirty = false; for (steps) |step_index| { const step = maker.stepByIndex(step_index); - if (maker.invalidateResult(step)) any_dirty = true; + if (try maker.invalidateResult(step)) any_dirty = true; } return any_dirty; } -- 2.54.0