Artifact c43465f253553b13e4b68f1c4e5eee3ec7638e2cd7ed8b4ce8dab4445bfb89d3:
- File
src/tidy.zig
— part of check-in
[de218fa6f6]
at
2026-07-25 00:21:57
on branch trunk
— serve: open the listener before priming; move the post-push rebuild off the request path
zit serve built its whole in-memory snapshot of the repository before it opened the listener socket. The build is O(store), so past a few thousand artifacts it outlasted the client's readiness probe and the client reported the server never came up — the store could be served, just never reached.
Bring-up now binds first: listen, announce, then prime, then accept. A client that connects during the prime queues in the kernel accept backlog instead of being refused, so the readiness window starts at the bind rather than after the scans. The listener open moves ahead of BOTH O(store) reads on the path — the config read and the prime. The connection deadline is armed only on accept, so priming never falls inside it. On a 254k-artifact store a connect now completes in ~2 ms against a ~5 s prime.
A push made the snapshot stale, and the old code rebuilt it inside the pushing round's own reply deadline — an O(store) rebuild charged to the request that caused it. The rebuild moves to a maintain hook the accept loop runs between connections, with no connection open to charge it to; the append-only stat gate makes the stale snapshot self-correcting until then.
The reorder is inherently a large-corpus latency property: reverting it leaves every unit test green, because the failure only shows when prime latency exceeds the readiness window. It is pinned in the milestone tier instead, where smoke already lives: zig build bench -- serve synthesizes a store, spawns the real server, and asserts time-to-first-connect is a small fraction of the prime (fixed order ~0.1; reverted order jumps past 1.0). The de-race and the accept-loop hook are pinned by ordinary unit tests. (user: dmestas, size: 12967) [annotate] [blame] [check-ins using]
//! tidy: a dead-file guard over the src/ test surface. //! //! Zig compiles and runs only the tests reachable from a root the build wires. //! A `.zig` file that no root imports still COMPILES — its tests simply stop //! running, and the coverage evaporates with no error. This guard closes that //! hazard: it walks `src/**/*.zig`, builds the `@import` graph, and asserts every //! file is reachable from one of the three roots `zig build test` runs — //! `src/root.zig` (the exported module), `main.zig`'s test block (the CLI //! surface), and `src/dst.zig` (the sibling harnesses). //! //! The mechanism is borrowed from TigerBeetle's `src/tidy.zig` DeadFilesDetector //! (github.com/tigerbeetle/tigerbeetle), which walks its own source tree and //! fails the build when a file falls out of the import graph. This is a smaller, //! single-purpose port of that idea onto zit's three test roots. //! //! It exists BEFORE the `main.zig` -> `src/cmd/` split precisely so that split //! cannot silently orphan a file's tests: move a file, forget to re-root it, and //! this test names the file and the one-line fix. //! //! Imports are extracted with the Zig tokenizer, not a byte scan, so an //! `@import("...")` written inside a string literal or comment — as this very //! file does in the remediation message below — is correctly NOT read as an //! import edge. const std = @import("std"); /// The repository root, wired in build.zig's `build_unit_test_module` (the same /// handle src/packaging_test.zig uses). A test binary's working directory is not /// the checkout, so the src tree is opened by this absolute path rather than a /// cwd-relative one. const repo_options = @import("repo_options"); /// The three roots `zig build test` executes, each of which aggregates other /// files through `_ = @import("...")` lines that can be dropped by hand. Kept in /// lockstep with build.zig: `mod_tests` (root.zig), `cli.tests` (main.zig), and /// `dst_tests` (dst.zig). Paths are relative to `src/`. const roots = [_][]const u8{ "root.zig", "main.zig", "dst.zig" }; /// Files legitimately unreachable from the three test roots because build.zig /// wires them as their OWN roots through dedicated steps. Each was verified /// against build.zig — this is the reviewed exception set, not a convenience to /// silence a real orphan. If one of these ever becomes reachable from a test /// root, the hygiene check below fails and demands the entry be removed. Paths /// are relative to `src/`. const allowlist = [_][]const u8{ // Root of `zig build e2e`. `zig build test` compiles it as a type-check gate // but never runs it (it spawns the real `zit` binary ~150x). Its own tests // run under the e2e step, so it needs no test-root import. "e2e.zig", // Root of `zig build smoke`. Compiled-but-not-run by `test`, run only by the // milestone-manual smoke step that drives the real `fossil` binary. "smoke.zig", // Root of `zig build bench`. build.zig ALSO compiles it into a test binary // (`bench_tests`) that joins `zig build test`, so its inline tests do run — // just not through one of the three roots. It is its own root. "logstore_benchmark.zig", // Reached only from logstore_benchmark.zig (the bench root above); their // tests ride the same `bench_tests` binary into `zig build test`. "storage_benchmark.zig", "sync_benchmark.zig", "warmread_benchmark.zig", // The `serve` readiness probe (`zig build bench -- serve`), dispatched from // logstore_benchmark.zig; its pure tests ride the same `bench_tests` binary. "serve_benchmark.zig", }; /// Vacuity floor: the walk must find at least this many `.zig` files or the tree /// was not actually scanned (wrong path, empty handle) and every check below /// would pass by finding nothing. The tree holds ~60 files today; 40 is a floor /// with headroom, not a running count to keep in sync. const min_expected_files = 40; /// Read cap per source file. The largest src file is well under 1 MiB; 8 MiB is /// a tripwire against a runaway read, not an expected size. const max_source_bytes = 8 << 20; test "every src/**/*.zig is reachable from a test root" { const io = std.testing.io; // One arena for every path string and graph structure: keys stay pointer // stable for the hash maps, and a single deinit reclaims all of it. var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); defer arena_state.deinit(); const arena = arena_state.allocator(); const src_abs = try std.fmt.allocPrint(arena, "{s}/src", .{repo_options.root}); var src_dir = std.Io.Dir.cwd().openDir(io, src_abs, .{ .iterate = true }) catch |err| switch (err) { // The only legitimate skip: the wired root is not a directory here, so // the machine cannot answer the question — the same stance the // version-control pin in packaging_test.zig takes. Anything else is a // real fault and must fail loudly rather than skip into silence. error.FileNotFound, error.NotDir => return error.SkipZigTest, else => { std.debug.print("\ntidy: cannot open {s}: {s}\n", .{ src_abs, @errorName(err) }); return error.TidyWalkFailed; }, }; defer src_dir.close(io); // Pass 1 — every src/**/*.zig, path relative to src/. On POSIX the walker's // separator is '/', matching both the import strings and the roots/allowlist // above; a '\\'-separated platform would make every lookup miss and fail this // guard loudly, never pass it wrongly. var files: std.ArrayList([]const u8) = .empty; var file_set: std.StringHashMapUnmanaged(void) = .empty; { var walker = try src_dir.walk(arena); defer walker.deinit(); while (try walker.next(io)) |entry| { if (entry.kind != .file) continue; if (!std.mem.endsWith(u8, entry.path, ".zig")) continue; const rel = try arena.dupe(u8, entry.path); try files.append(arena, rel); try file_set.put(arena, rel, {}); } } // Vacuity guard: a silent zero (wrong cwd, empty handle) would let every // assertion below pass by finding nothing to check. if (files.items.len < min_expected_files) { std.debug.print("\ntidy: walked {s} and found only {d} .zig files; expected at least {d}. " ++ "The walk is not seeing the source tree — this guard would pass vacuously.\n", .{ src_abs, files.items.len, min_expected_files }); return error.TidyWalkFailed; } // Each root must be a real file, or reachability starts from nothing and // waves everything through. A renamed or moved root has to fail here. for (roots) |root| { if (!file_set.contains(root)) { std.debug.print("\ntidy: test root \"{s}\" does not exist under src/. The reachability " ++ "guard starts from the three roots wired in build.zig; update this list when a root moves.\n", .{root}); return error.TidyRootMissing; } } // Pass 2 — breadth-first over the @import graph from the three roots. var reached: std.StringHashMapUnmanaged(void) = .empty; var queue: std.ArrayList([]const u8) = .empty; for (roots) |root| { const key = file_set.getKey(root).?; // just asserted present if ((try reached.getOrPut(arena, key)).found_existing) continue; try queue.append(arena, key); } var head: usize = 0; while (head < queue.items.len) : (head += 1) { const file = queue.items[head]; const source = try src_dir.readFileAllocOptions(io, file, arena, .limited(max_source_bytes), .of(u8), 0); var imports: std.ArrayList([]const u8) = .empty; try collectImports(arena, source, parentDir(file), &imports); for (imports.items) |target| { // Imports of `std`, `zit`, `build_options`, `repo_options`, etc. carry // no `.zig` and are already filtered; a resolved path not in the file // set is a cross-module or generated import and is not our concern. const key = file_set.getKey(target) orelse continue; if ((try reached.getOrPut(arena, key)).found_existing) continue; try queue.append(arena, key); } } // Allowlist hygiene: every entry must be a real file that is genuinely // unreachable. A stale entry (file gone) or a dead one (now reachable) means // the list papers over nothing — fail so it cannot rot into cover for a // future real orphan. for (allowlist) |allowed| { if (!file_set.contains(allowed)) { std.debug.print("\ntidy: allowlist names \"{s}\", which no longer exists under src/. " ++ "Remove the stale entry.\n", .{allowed}); return error.TidyAllowlistStale; } if (reached.contains(allowed)) { std.debug.print("\ntidy: allowlist names \"{s}\", but it is now reachable from a test root. " ++ "Remove it from the allowlist — the exemption is dead.\n", .{allowed}); return error.TidyAllowlistDead; } } // The assertion: every src file is reached or explicitly allowlisted. var orphans: usize = 0; for (files.items) |file| { if (reached.contains(file)) continue; if (isAllowlisted(file)) continue; orphans += 1; std.debug.print( \\ \\tidy: src/{s} is not reachable from any test root. \\ Add `_ = @import("{s}");` to one of: \\ src/root.zig (exported API) · main.zig test block (CLI) · src/dst.zig (harness) \\ A file no root imports still compiles — its tests just never run. \\ , .{ file, file }); } if (orphans != 0) return error.TidyDeadFile; } fn isAllowlisted(file: []const u8) bool { for (allowlist) |a| { if (std.mem.eql(u8, a, file)) return true; } return false; } /// The directory portion of a src-relative path: `sim/net.zig` -> `sim`, and a /// file directly under src/ -> `""`. fn parentDir(path: []const u8) []const u8 { const slash = std.mem.lastIndexOfScalar(u8, path, '/') orelse return ""; return path[0..slash]; } /// Extract every `@import("...zig")` from `source`, resolve each against the /// importing file's directory, and append the src-relative results to `out`. /// /// The Zig tokenizer drives this so imports inside comments or string literals /// are not mistaken for edges. Import path literals carry no escapes, so the slice /// between the string_literal token's quotes is the literal path. fn collectImports( arena: std.mem.Allocator, source: [:0]const u8, importer_dir: []const u8, out: *std.ArrayList([]const u8), ) !void { var tokenizer = std.zig.Tokenizer.init(source); while (true) { const builtin = tokenizer.next(); if (builtin.tag == .eof) break; if (builtin.tag != .builtin) continue; if (!std.mem.eql(u8, source[builtin.loc.start..builtin.loc.end], "@import")) continue; const lparen = tokenizer.next(); if (lparen.tag != .l_paren) continue; const literal = tokenizer.next(); if (literal.tag != .string_literal) continue; std.debug.assert(literal.loc.end - literal.loc.start >= 2); const raw = source[literal.loc.start + 1 .. literal.loc.end - 1]; if (!std.mem.endsWith(u8, raw, ".zig")) continue; try out.append(arena, try resolveImport(arena, importer_dir, raw)); } } /// Join an import target onto the importing file's directory and collapse `.` /// and `..` components, yielding an arena-owned src-relative path. No real import /// escapes src/ today; the `..` handling keeps the resolver correct if one ever /// does. fn resolveImport(arena: std.mem.Allocator, dir: []const u8, target: []const u8) ![]const u8 { var components: std.ArrayList([]const u8) = .empty; defer components.deinit(arena); var dir_it = std.mem.splitScalar(u8, dir, '/'); while (dir_it.next()) |component| try pushComponent(&components, arena, component); var target_it = std.mem.splitScalar(u8, target, '/'); while (target_it.next()) |component| try pushComponent(&components, arena, component); var out: std.ArrayList(u8) = .empty; defer out.deinit(arena); for (components.items, 0..) |component, i| { if (i != 0) try out.append(arena, '/'); try out.appendSlice(arena, component); } return arena.dupe(u8, out.items); } fn pushComponent(components: *std.ArrayList([]const u8), arena: std.mem.Allocator, component: []const u8) !void { if (component.len == 0 or std.mem.eql(u8, component, ".")) return; if (std.mem.eql(u8, component, "..")) { if (components.items.len != 0) _ = components.pop(); return; } try components.append(arena, component); }