authorgravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2026-08-12 21:59:34-07:00
committergravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2026-08-13 11:32:46+02:00
log470f77600d79a5c17b3734f97eaf2462c452718e
tree821eed96b9957d69def837fabb95043b15563f86
parenta68b5ee372365034289308967ff488b194aa9fa9

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

3 files changed, 133 insertions(+), 76 deletions(-)

lib/compiler/Maker.zig+100-34
......@@ -44,6 +44,10 @@ gpa: Allocator,
4444graph: *Graph,
4545install_paths: InstallPaths,
4646scanned_config: *const ScannedConfig,
47/// Includes an extra auto-generated placeholder Step at the end that indicates
48/// configure must be rerun. It is done this way so that the hot path of file
49/// system watching does not need to make any special cases, and to avoid more
50/// OS-specific logic in file system watching implementation.
4751steps: []Step,
4852generated_files: []Path,
4953run_args: ?[]const []const u8,
......@@ -708,7 +712,12 @@ pub fn main(init: process.Init.Minimal) !void {
708712 break :s &protocol_server_allocation;
709713 } else null;
710714
711 while (true) {
715 configure: while (true) {
716 // Set of files that, if modified, imply that recompiling and rerunning
717 // configurer is needed.
718 var configure_source_files: Cache.Manifest.Files = .empty;
719 defer Cache.Manifest.freeFiles(gpa, &configure_source_files);
720
712721 // If this fails, we can still start the server and wait for user
713722 // to request a rebuild. If it returns error.FailedButCacheIntact
714723 // we can even still do file system watching and automatically
......@@ -730,6 +739,7 @@ pub fn main(init: process.Init.Minimal) !void {
730739 .fetch_only = fetch_only,
731740 .print_configuration = print_configuration,
732741 .forks = forks.items,
742 .src_files = &configure_source_files,
733743 })) |scanned_config| {
734744 if (help_menu) {
735745 scanned_config.printUsage(&graph, initStdoutWriter(io)) catch |err| switch (err) {
......@@ -766,7 +776,9 @@ pub fn main(init: process.Init.Minimal) !void {
766776 .include = install_include_path,
767777 },
768778
769 .steps = try arena.alloc(Step, scanned_config.configuration.steps.len),
779 // Extra step at the end which is the autogenerated placeholder
780 // step which indicates that we need to reconfigure.
781 .steps = try arena.alloc(Step, scanned_config.configuration.steps.len + 1),
770782 .generated_files = try arena.alloc(Path, scanned_config.configuration.generated_files_len),
771783 .run_args = run_args,
772784
......@@ -843,7 +855,7 @@ pub fn main(init: process.Init.Minimal) !void {
843855 try select.concurrent(.message, Server.receiveMessage, .{s});
844856
845857 maker.watch = body.flags.watch;
846 maker.prepare(steps) catch |err| switch (err) {
858 maker.prepare(steps, &configure_source_files) catch |err| switch (err) {
847859 error.DependencyLoopDetected, error.InsufficientMemory => {
848860 // TODO handle DependencyLoopDetected as error.FailedButCacheIntact
849861 // and handle InsufficientMemory as error.AlreadyReported
......@@ -859,8 +871,11 @@ pub fn main(init: process.Init.Minimal) !void {
859871 if (!Watch.have_impl) unreachable;
860872 if (w == null) w = try .init(&maker);
861873
862 try w.?.update(maker.step_stack.keys());
863 try select.concurrent(.fs_event, Watch.wait, .{ &w.?, if (in_debounce) .{ .ms = debounce_interval_ms } else .none });
874 try updateWatch(&maker, &w.?);
875 try select.concurrent(.fs_event, Watch.wait, .{
876 &w.?,
877 if (in_debounce) .{ .ms = debounce_interval_ms } else .none,
878 });
864879 }
865880
866881 continue :loop try select.await();
......@@ -870,7 +885,13 @@ pub fn main(init: process.Init.Minimal) !void {
870885 },
871886 .fs_event => |payload| {
872887 if (!Watch.have_impl) unreachable;
873 switch (try payload) {
888 switch (payload catch |err| switch (err) {
889 error.MustReconfigure => {
890 try io.sleep(.fromMilliseconds(debounce_interval_ms), .awake);
891 continue :configure;
892 },
893 else => |e| fatal("file watching failed: {t}", .{e}),
894 }) {
874895 .timeout => {
875896 assert(in_debounce);
876897 markFailedStepsDirty(&maker);
......@@ -880,7 +901,10 @@ pub fn main(init: process.Init.Minimal) !void {
880901 .dirty => in_debounce = true,
881902 .clean => {},
882903 }
883 try select.concurrent(.fs_event, Watch.wait, .{ &w.?, if (in_debounce) .{ .ms = debounce_interval_ms } else .none });
904 try select.concurrent(.fs_event, Watch.wait, .{
905 &w.?,
906 if (in_debounce) .{ .ms = debounce_interval_ms } else .none,
907 });
884908 continue :loop try select.await();
885909 },
886910 }
......@@ -889,7 +913,7 @@ pub fn main(init: process.Init.Minimal) !void {
889913 const initial_steps = try maker.resolveTopLevelSteps(step_names.items);
890914 defer gpa.free(initial_steps);
891915
892 maker.prepare(initial_steps) catch |err| switch (err) {
916 maker.prepare(initial_steps, &configure_source_files) catch |err| switch (err) {
893917 error.DependencyLoopDetected, error.InsufficientMemory => {
894918 // TODO handle DependencyLoopDetected as error.FailedButCacheIntact
895919 // and handle InsufficientMemory as error.AlreadyReported
......@@ -938,7 +962,7 @@ pub fn main(init: process.Init.Minimal) !void {
938962 // Comptime-known guard to prevent including the logic below when `!Watch.have_impl`.
939963 if (!Watch.have_impl) unreachable;
940964
941 try w.update(maker.step_stack.keys());
965 try updateWatch(&maker, &w);
942966
943967 // Wait until a file system notification arrives. Read all such events
944968 // until the buffer is empty. Then wait for a debounce interval, resetting
......@@ -950,21 +974,34 @@ pub fn main(init: process.Init.Minimal) !void {
950974 w.dir_count, countSubProcesses(&maker),
951975 }) catch &caption_buf;
952976 var debouncing_node = main_progress_node.start(caption, 0);
977 defer debouncing_node.end();
953978 var in_debounce = false;
954 while (true) switch (try w.wait(if (in_debounce) .{ .ms = debounce_interval_ms } else .none)) {
955 .timeout => {
956 assert(in_debounce);
957 debouncing_node.end();
958 markFailedStepsDirty(&maker);
959 continue :rebuild;
960 },
961 .dirty => if (!in_debounce) {
962 in_debounce = true;
963 debouncing_node.end();
964 debouncing_node = main_progress_node.start("Debouncing (Change Detected)", 0);
965 },
966 .clean => {},
967 };
979 while (true) {
980 const timeout: Watch.Timeout = if (in_debounce) .{ .ms = debounce_interval_ms } else .none;
981 switch (w.wait(timeout) catch |err| switch (err) {
982 error.MustReconfigure => {
983 debouncing_node.end();
984 debouncing_node = main_progress_node.start("Debouncing (Change Detected)", 0);
985 try io.sleep(.fromMilliseconds(debounce_interval_ms), .awake);
986 continue :configure;
987 },
988 else => |e| fatal("file watching failed: {t}", .{e}),
989 }) {
990 .timeout => {
991 assert(in_debounce);
992 debouncing_node.end();
993 debouncing_node = .none;
994 markFailedStepsDirty(&maker);
995 continue :rebuild;
996 },
997 .dirty => if (!in_debounce) {
998 in_debounce = true;
999 debouncing_node.end();
1000 debouncing_node = main_progress_node.start("Debouncing (Change Detected)", 0);
1001 },
1002 .clean => {},
1003 }
1004 }
9681005 }
9691006 } else |err| {
9701007 const can_fs_watch = switch (err) {
......@@ -991,6 +1028,15 @@ pub fn main(init: process.Init.Minimal) !void {
9911028 }
9921029}
9931030
1031/// Temporarily adds the reconfigure pseudostep to step_stack, calls
1032/// `Watch.update`, and then pops it again.
1033fn updateWatch(maker: *Maker, watch: *Watch) !void {
1034 const step_stack = &maker.step_stack;
1035 try step_stack.putNoClobber(maker.gpa, @fromBackingInt(@intCast(maker.steps.len - 1)), {});
1036 defer _ = step_stack.pop().?;
1037 try watch.update(step_stack.keys());
1038}
1039
9941040const ConfigureOptions = struct {
9951041 configure_argv: [][]const u8,
9961042 conf_argv_index_build_root: usize,
......@@ -1008,6 +1054,7 @@ const ConfigureOptions = struct {
10081054 fetch_only: bool,
10091055 print_configuration: PrintConfiguration,
10101056 forks: []Fork,
1057 src_files: *Cache.Manifest.Files,
10111058};
10121059
10131060fn configure(graph: *Graph, options: ConfigureOptions) !ScannedConfig {
......@@ -1362,14 +1409,14 @@ fn configure(graph: *Graph, options: ConfigureOptions) !ScannedConfig {
13621409
13631410 if (config_man) |man| {
13641411 if (try man.hit(compile_prog_node)) {
1412 log.debug("configuration cache hit", .{});
13651413 const digest = man.final();
1366 break :cp .{
1367 .{
1368 .root_dir = graph.local_cache_root,
1369 .sub_path = try arena.print("c/{s}", .{&digest}),
1370 },
1371 man.toOwnedLock(),
1414 const path: Path = .{
1415 .root_dir = graph.local_cache_root,
1416 .sub_path = try arena.print("c/{s}", .{&digest}),
13721417 };
1418 options.src_files.* = man.takeFiles();
1419 break :cp .{ path, man.toOwnedLock() };
13731420 }
13741421 }
13751422 const configure_exe_path: Path = if (std.zig.buildExeSubprocess(gpa, io, .{
......@@ -1505,6 +1552,7 @@ fn configure(graph: *Graph, options: ConfigureOptions) !ScannedConfig {
15051552 });
15061553 };
15071554 man.writeManifest() catch |err| log.warn("failed to write cache manifest: {t}", .{err});
1555 options.src_files.* = man.takeFiles();
15081556 break :cp .{ final_path, man.toOwnedLock() };
15091557 }
15101558 };
......@@ -2119,7 +2167,13 @@ fn markFailedStepsDirty(maker: *Maker) void {
21192167 for (all_steps) |step_index| {
21202168 const step = maker.stepByIndex(step_index);
21212169 switch (step.state) {
2122 .dependency_failure, .dependency_skipped, .failure, .skipped => _ = maker.invalidateResult(step),
2170 .dependency_failure,
2171 .dependency_skipped,
2172 .failure,
2173 .skipped,
2174 => _ = maker.invalidateResult(step) catch |err| switch (err) {
2175 error.MustReconfigure => unreachable,
2176 },
21232177 else => continue,
21242178 }
21252179 }
......@@ -2173,7 +2227,11 @@ fn resolveTopLevelSteps(maker: *Maker, step_names: []const []const u8) ![]const
21732227 return try gpa.dupe(Configuration.Step.Index, result.keys());
21742228}
21752229
2176fn prepare(maker: *Maker, step_indices: []const Configuration.Step.Index) !void {
2230fn prepare(
2231 maker: *Maker,
2232 step_indices: []const Configuration.Step.Index,
2233 configure_source_files: *const Cache.Manifest.Files,
2234) !void {
21772235 const gpa = maker.gpa;
21782236 const graph = maker.graph;
21792237 const arena = graph.arena;
......@@ -2182,10 +2240,17 @@ fn prepare(maker: *Maker, step_indices: []const Configuration.Step.Index) !void
21822240 const step_stack = &maker.step_stack;
21832241 const c = &maker.scanned_config.configuration;
21842242
2185 for (maker.steps, 0..) |*step, step_index_usize| {
2243 // The last element is a reserved special pseudostep which contains the
2244 // watch inputs for the configurer executable.
2245 for (maker.steps[0 .. maker.steps.len - 1], 0..) |*step, step_index_usize| {
21862246 const step_index: Configuration.Step.Index = @fromBackingInt(@intCast(step_index_usize));
21872247 step.* = .{ .extended = .init(step_index.ptr(c).flags(c).tag) };
21882248 }
2249 {
2250 const last_step = &maker.steps[maker.steps.len - 1];
2251 last_step.* = .{ .extended = .init(.top_level) };
2252 try last_step.setWatchInputsFromManifestFiles(maker, configure_source_files, graph.cache.prefixes());
2253 }
21892254
21902255 try initial_steps.ensureUnusedCapacity(gpa, step_indices.len);
21912256 try step_stack.ensureUnusedCapacity(gpa, step_indices.len);
......@@ -3045,14 +3110,15 @@ fn constructGraphAndCheckForDependencyLoop(
30453110/// When file watching, prepares the step for being re-evaluated. Returns
30463111/// `true` if the step was newly invalidated, `false` if it was already
30473112/// invalidated.
3048pub fn invalidateResult(maker: *Maker, step: *Step) bool {
3113pub fn invalidateResult(maker: *Maker, step: *Step) error{MustReconfigure}!bool {
3114 if (step == &maker.steps[maker.steps.len - 1]) return error.MustReconfigure;
30493115 if (step.state == .precheck_done) return false;
30503116 assert(step.pending_deps == 0);
30513117 step.state = .precheck_done;
30523118 step.reset(maker);
30533119 for (step.dependants.items) |dependant_index| {
30543120 const dependant = maker.stepByIndex(dependant_index);
3055 _ = invalidateResult(maker, dependant);
3121 _ = try invalidateResult(maker, dependant);
30563122 dependant.pending_deps += 1;
30573123 }
30583124 return true;
lib/compiler/Maker/Step.zig+11-3
......@@ -781,12 +781,20 @@ pub fn writeManifestAndWatch(s: *Step, maker: *Maker, man: *Cache.Manifest) !voi
781781 try setWatchInputsFromManifest(s, maker, man);
782782}
783783
784fn setWatchInputsFromManifest(s: *Step, maker: *Maker, man: *Cache.Manifest) !void {
784pub fn setWatchInputsFromManifest(s: *Step, maker: *Maker, man: *Cache.Manifest) !void {
785 return setWatchInputsFromManifestFiles(s, maker, &man.files, man.cache.prefixes());
786}
787
788pub fn setWatchInputsFromManifestFiles(
789 s: *Step,
790 maker: *Maker,
791 files: *const Cache.Manifest.Files,
792 prefixes: []const Cache.Directory,
793) !void {
785794 const graph = maker.graph;
786795 const arena = graph.arena; // TODO don't leak into process arena
787 const prefixes = man.cache.prefixes();
788796 clearWatchInputs(s, maker);
789 for (man.files.keys()) |file| {
797 for (files.keys()) |file| {
790798 // The file path data is freed when the cache manifest is cleaned up at the end of `make`.
791799 const sub_path = try arena.dupe(u8, file.prefixed_path.sub_path);
792800 try addWatchInputFromPath(s, maker, .{
lib/compiler/Maker/Watch.zig+22-39
......@@ -49,7 +49,10 @@ const Os = switch (builtin.os.tag) {
4949 poll_fds: std.array_hash_map.Auto(MountId, posix.pollfd),
5050
5151 const MountId = i32;
52 const HandleTable = std.array_hash_map.Custom(FileHandle, struct { mount_id: MountId, reaction_set: ReactionSet }, FileHandle.Adapter, false);
52 const HandleTable = std.array_hash_map.Custom(FileHandle, struct {
53 mount_id: MountId,
54 reaction_set: ReactionSet,
55 }, FileHandle.Adapter, false);
5356
5457 const fan_mask: std.os.linux.fanotify.MarkMask = .{
5558 .CLOSE_WRITE = true,
......@@ -152,10 +155,8 @@ const Os = switch (builtin.os.tag) {
152155 }) {
153156 assert(meta[0].vers == M.VERSION);
154157 if (meta[0].mask.Q_OVERFLOW) {
155 any_dirty = true;
156 std.log.warn("file system watch queue overflowed; falling back to fstat", .{});
157 markAllFilesDirty(w);
158 return true;
158 std.log.warn("file system watch queue overflowed; reconfiguring", .{});
159 return error.MustReconfigure;
159160 }
160161 const fid: *align(1) fanotify.event_info_fid = @ptrCast(meta + 1);
161162 switch (fid.hdr.info_type) {
......@@ -166,9 +167,9 @@ const Os = switch (builtin.os.tag) {
166167 const lfh: FileHandle = .{ .handle = file_handle };
167168 if (w.os.handle_table.getPtr(lfh)) |value| {
168169 if (value.reaction_set.getPtr(".")) |glob_set|
169 any_dirty = markStepSetDirty(maker, glob_set, any_dirty);
170 any_dirty = try markStepSetDirty(maker, glob_set, any_dirty);
170171 if (value.reaction_set.getPtr(file_name)) |step_set|
171 any_dirty = markStepSetDirty(maker, step_set, any_dirty);
172 any_dirty = try markStepSetDirty(maker, step_set, any_dirty);
172173 }
173174 },
174175 else => |t| std.log.warn("unexpected fanotify event '{t}'", .{t}),
......@@ -304,8 +305,11 @@ const Os = switch (builtin.os.tag) {
304305 if (events_len == 0)
305306 return .timeout;
306307 for (w.os.poll_fds.values()) |poll_fd| {
307 if (poll_fd.revents & std.posix.POLL.IN == std.posix.POLL.IN and try markDirtySteps(w, poll_fd.fd))
308 if (poll_fd.revents & std.posix.POLL.IN == std.posix.POLL.IN and
309 try markDirtySteps(w, poll_fd.fd))
310 {
308311 return .dirty;
312 }
309313 }
310314 return .clean;
311315 }
......@@ -521,10 +525,8 @@ const Os = switch (builtin.os.tag) {
521525 var any_dirty = false;
522526 const bytes_returned = dir.iosb.Information;
523527 if (bytes_returned == 0) {
524 std.log.warn("file system watch queue overflowed; falling back to fstat", .{});
525 markAllFilesDirty(w);
526 try dir.startListening(w);
527 return true;
528 std.log.warn("file system watch queue overflowed; reconfiguring", .{});
529 return error.MustReconfigure;
528530 }
529531 var file_name_buf: [std.fs.max_path_bytes]u8 = undefined;
530532 var offset: usize = 0;
......@@ -532,9 +534,9 @@ const Os = switch (builtin.os.tag) {
532534 const notify: *windows.FILE.NOTIFY.INFORMATION = @ptrCast(@alignCast(&dir.buffer[offset]));
533535 const file_name = file_name_buf[0..std.unicode.wtf16LeToWtf8(&file_name_buf, notify.fileName())];
534536 if (dir.reaction_set.getPtr(".")) |glob_set|
535 any_dirty = markStepSetDirty(maker, glob_set, any_dirty);
537 any_dirty = try markStepSetDirty(maker, glob_set, any_dirty);
536538 if (dir.reaction_set.getPtr(file_name)) |step_set|
537 any_dirty = markStepSetDirty(maker, step_set, any_dirty);
539 any_dirty = try markStepSetDirty(maker, step_set, any_dirty);
538540 if (notify.NextEntryOffset == 0)
539541 break;
540542
......@@ -851,13 +853,13 @@ const Os = switch (builtin.os.tag) {
851853 // If we knew the basename of the changed file, here we would
852854 // mark only the step set dirty, and possibly the glob set:
853855 //if (reaction_set.getPtr(".")) |glob_set|
854 // any_dirty = markStepSetDirty(maker, glob_set, any_dirty);
856 // any_dirty = try markStepSetDirty(maker, glob_set, any_dirty);
855857 //if (reaction_set.getPtr(file_name)) |step_set|
856 // any_dirty = markStepSetDirty(maker, step_set, any_dirty);
858 // any_dirty = try markStepSetDirty(maker, step_set, any_dirty);
857859 // However we don't know the file name so just mark all the
858860 // sets dirty for this directory.
859861 for (reaction_set.values()) |*step_set| {
860 any_dirty = markStepSetDirty(maker, step_set, any_dirty);
862 any_dirty = try markStepSetDirty(maker, step_set, any_dirty);
861863 }
862864 }
863865 return any_dirty;
......@@ -915,31 +917,11 @@ pub const Match = struct {
915917 };
916918};
917919
918fn markAllFilesDirty(w: *Watch) void {
919 const maker = w.maker;
920
921 for (switch (builtin.os.tag) {
922 .windows => w.os.handle_table.keys(),
923 else => w.os.handle_table.values(),
924 }) |item| {
925 const reaction_set = switch (builtin.os.tag) {
926 .linux, .windows => item.reaction_set,
927 else => item,
928 };
929 for (reaction_set.values()) |step_set| {
930 for (step_set.keys()) |step_index| {
931 const step = maker.stepByIndex(step_index);
932 _ = maker.invalidateResult(step);
933 }
934 }
935 }
936}
937
938fn markStepSetDirty(maker: *Maker, step_set: *StepSet, any_dirty: bool) bool {
920fn markStepSetDirty(maker: *Maker, step_set: *StepSet, any_dirty: bool) error{MustReconfigure}!bool {
939921 var this_any_dirty = false;
940922 for (step_set.keys()) |step_index| {
941923 const step = maker.stepByIndex(step_index);
942 if (maker.invalidateResult(step)) this_any_dirty = true;
924 if (try maker.invalidateResult(step)) this_any_dirty = true;
943925 }
944926 return any_dirty or this_any_dirty;
945927}
......@@ -984,6 +966,7 @@ pub const WaitResult = enum {
984966 clean,
985967};
986968
969/// May return `error.MustReconfigure`.
987970pub fn wait(w: *Watch, timeout: Timeout) !WaitResult {
988971 return Os.wait(w, timeout);
989972}