Re: [PATCH] fsck: snapshot default refs before object walk
From: Elijah Newren <hidden>
Date: 2026-01-06 23:19:34
On Mon, Dec 29, 2025 at 4:46 PM Junio C Hamano [off-list ref] wrote:
"Elijah Newren via GitGitGadget" [off-list ref] writes:quoted
This problem doesn't occur when refs are specified on the command line for us to check, since we use those specified refs for both walking and checking. Using the same refs for walking and checking seems to just make sense, so modify the existing code to do the same when refs aren't specified.Excellent analysis and good approach.quoted
Snapshot the refs at the beginning, and also ignore all reflog entries since the time of our snapshot (while this technically means we could ignore a reflog entry created before the fsck process if the local clock is weird, since reflogs are local-only there are not concerns about differences between clocks on different machines).Repository on a network filesystem being accessed by hosts with broken clock?
Oh, indeed.
I do not think our reflog API has (1) give me some token to mark your current state (2) here is the token you gave me earlier, now iterate and yield entries but ignore entries added after you gave me that token, so going by the reflog timestamp is probably the best we could do. Any approach may get confused when the user tries to be cute and issues "reflog delete" or "reflog expire" in the middle anyway, I suspect ;-)quoted
While worries about live updates while running fsck is likely of most interest for forge operators, it will likely also benefit those with automated jobs (such as git maintenance) or even casual users who want to do other work in their clone while fsck is running.Great. Will queue. Thanks.quoted
@@ -509,6 +510,9 @@ static int fsck_handle_reflog_ent(const char *refname, timestamp_t timestamp, int tz UNUSED, const char *message UNUSED, void *cb_data UNUSED) { + if (now && timestamp > now) + return 0; + if (verbose) fprintf_ln(stderr, _("Checking reflog %s->%s"), oid_to_hex(ooid), oid_to_hex(noid));@@ -567,14 +571,53 @@ static int fsck_head_link(const char *head_ref_name, const char **head_points_at, struct object_id *head_oid); -static void get_default_heads(void) +struct ref_snapshot { + size_t nr; + size_t name_alloc; + size_t oid_alloc; + char **refname; + struct object_id *oid; +};This data structure is somewhat unexpected. Instead of a struct that holds two arrays, I would have rather expected an array of "struct { refname, oid }", with the possiblity to add a "token to mark the latest reflog entry" to the mix I alluded to earlier when such an API function materializes.
Yeah, that makes sense. It'll mean that there won't be anything left of Matthew's original patch that I was trying to upstream (especially with the further changes Peff highlighted elsewhere in this thread), but I can just take the authorship and note Matthew's contribution in a trailer.
[Footnote] We could call refs_for_each_reflog_ent_reverse(), grab the parameters that each_reflog_ent_fn receives as that "token" for the latest reflog entry and stop. That way, we will learn the value of <old,new,committer,timestamp,tz,msg>, which should be a robust enough unique key. After that when iterating over the reflog, we know we should stop after processing the reflog entry that holds the recorded value.
Interesting. The global timestamp for reflogs seems good enough for me (network filesystems with a broken clock feel niche to me), but I can leave a TODO in the code for those that want to pursue improving the reflog handling further.