Skip to content
zit Artifact [ac9d12e40a]
Login

Artifact [ac9d12e40a]

Check-ins Using Download Hex

Artifact ac9d12e40af03b24e35947c6aa86ce228f34581f9aa7f0b8e4c6099061692a64:

//! Structural pins over which repository representation each read verb opens.
//!
//! `Repository.OpenOptions.mode` defaults to `.full` — a whole-store derive.
//! That default is correct for a write verb and wrong for nearly every reader,
//! and the difference is INVISIBLE: a verb on the wrong side produces exactly
//! the right answer, merely proportional to the store instead of to the work.
//! Nothing fails. No test goes red. #141 lived that way for as long as `diff`
//! has taken `--from`/`--to`: 0.35s against 0.04s for a one-line diff on a
//! 302MB store, and it surfaced only when someone timed a page built on it.
//!
//! A timing assertion would be the direct test and would also be a flaky one —
//! the thing being pinned is a code-shape fact, so it is pinned as one, the way
//! src/ci_test.zig pins the workflow: embed the source, assert on its text.
//!
//! These are text assertions, not a Zig parse. A pinned call sitting inside a
//! comment would satisfy them. That trade is taken knowingly and is the same
//! one ci_test.zig takes; what the pins buy is the negative space — the
//! reviewed list of verbs that are DELIBERATELY `.full`, which reading alone
//! does not catch.
//!
//! Coverage is the two verb sources embedded below, and is deliberately not
//! claimed to be exhaustive. `bisect` lives in cmd/checkout.zig and is not
//! pinned here. `grep` IS in range and is NOT pinned: it opens `.full` with no
//! recorded reason and reads `checkinsByMtime`, `files` and `logStore` -- the
//! image's own surface -- so it looks like #141 again, but it also walks every
//! check-in's file set, which likely dwarfs a derive, and it wires
//! `--no-cache`, which has to move with the mode (see `crosslink`, which
//! passes `!bypass`). Pinning a value nobody has reasoned about would read as
//! coverage; leaving it named here reads as the open question it is.

const std = @import("std");

const history_source = @embedFile("cmd_history_source");
const tracking_source = @embedFile("cmd_tracking_source");

/// One verb's `openRepo` call site, as it appears in the source.
///
/// The `fast_read` argument is positional (the 8th), so this matches the
/// prefix through `repo_override` and reads the two booleans that follow. A
/// call written differently — a named struct, a wrapper — does not match, and
/// `expectMode` fails rather than silently passing, which is the behavior
/// wanted: an unrecognized call site is an unreviewed one.
fn modeOf(source: []const u8, verb: []const u8, buf: []u8) ?bool {
    const needle = std.fmt.bufPrint(buf, "openRepo(gpa, io, arena, streams, \"{s}\", ", .{verb}) catch return null;
    const at = std.mem.indexOf(u8, source, needle) orelse return null;
    const rest = source[at + needle.len ..];
    const end = std.mem.indexOfScalar(u8, rest, ')') orelse return null;
    var it = std.mem.splitSequence(u8, rest[0..end], ", ");
    _ = it.next() orelse return null; // repo_override
    _ = it.next() orelse return null; // bypass_cache
    const fast_read = it.next() orelse return null;
    if (std.mem.eql(u8, fast_read, "true")) return true;
    if (std.mem.eql(u8, fast_read, "false")) return false;
    return null; // a computed argument is not a pinned one
}

fn expectMode(source: []const u8, verb: []const u8, want_fast_read: bool) !void {
    var buf: [128]u8 = undefined;
    const got = modeOf(source, verb, &buf) orelse {
        std.debug.print(
            "\nreadpath pin: no recognizable openRepo call site for '{s}'\n",
            .{verb},
        );
        return error.ReadPathCallSiteMissing;
    };
    if (got == want_fast_read) return;
    std.debug.print(
        "\nreadpath pin: '{s}' opens fast_read={}, expected {}\n" ++
            "A read verb on `.full` re-derives the whole store per invocation (#141).\n" ++
            "If this change is deliberate, say WHY here and in the call site.\n",
        .{ verb, got, want_fast_read },
    );
    return error.ReadPathModeChanged;
}

test "readpath #141: the check-in read verbs take the zero-parse image path" {
    // Each of these resolves names and reads trees or content, and needs
    // nothing the image does not carry.
    try expectMode(history_source, "ls", true);
    try expectMode(history_source, "cat", true);
    try expectMode(history_source, "info", true);
    try expectMode(history_source, "leaves", true);
    try expectMode(history_source, "branch", true);
    try expectMode(history_source, "frames", true);

    // #141 itself. `diff --from/--to` needs two resolved names and two file
    // lists -- exactly `ls`'s surface. It reached past `main.openRepo` to
    // `Repository.open` directly and inherited `.full` by omission.
    try expectMode(tracking_source, "diff", true);

    // Same class, found with it: `artifact` reads only the content log
    // (`resolveArtifactPrefix` and `blob` both go through `logStore`), so the
    // derived view it paid to build was never read.
    try expectMode(history_source, "artifact", true);
}

test "readpath #141: the deliberate full-derive readers keep their recorded reason" {
    // The reviewed exceptions. Both are `.full` because they read what the
    // image does not carry -- NOT by the omission #141 was about -- and both
    // say so at the call site. Pinned together with the comment that justifies
    // them, because an unexplained `.full` is indistinguishable from the bug:
    // flipping one for speed should have to be an argued change, and keeping
    // one should have to stay explained.
    try expectMode(history_source, "timeline", false); // §3.2 merge mark, off adjacency
    try expectMode(history_source, "annotate", false); // walks §3.3 ancestry
    for ([_][]const u8{
        "A cold `.full` verb", // timeline
        "A cold read: it walks ancestry", // annotate
    }) |reason| {
        if (std.mem.indexOf(u8, history_source, reason) != null) continue;
        std.debug.print(
            "\nreadpath pin: a `.full` open lost the comment saying why (\"{s}\").\n" ++
                "An unexplained `.full` is indistinguishable from #141's omission.\n",
            .{reason},
        );
        return error.ReadPathReasonMissing;
    }
}