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); }