{"thread":{"id":"64762","subject":"[PATCH 00/17] Fixes and improvements for ref consistency checks","startedAt":"2026-01-09T12:39:36Z","lastAt":"2026-01-16T06:48:13Z","messageCount":61,"participants":["Patrick Steinhardt","shejialuo","Karthik Nayak","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":17},"messages":[{"id":"533345","messageId":"20260109-pks-refs-verify-fixes-v1-0-3587dba18294@pks.im","threadId":"64762","inReplyTo":null,"subject":"[PATCH 00/17] Fixes and improvements for ref consistency checks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-09T12:39:29Z","receivedAt":"2026-01-09T12:39:36Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis patch series contains a bunch of fixes and improvements for ref\nconsistency checks. It is structured as follows:\n\n  - Patches 1 to 4 contain a couple of cleanups for the consistency\n    checks done by the \"files\" backend.\n\n  - Patches 5 to 7 introduce checks for root refs for the \"files\"\n    backend.\n\n  - Patches 9 to 14 introduce infrastructure for shared checks with the\n    \"files\" and \"reftable\" backend.\n\n  - Patches 15 to 17 move some ref consistency checks that were still\n    driven by git-fsck(1) into `git refs verify`.\n\nThanks!\n\nPatrick\n\n---\nPatrick Steinhardt (17):\n      refs/files: simplify iterating through root refs\n      refs/files: move fsck functions into global scope\n      refs/files: remove `refs_check_dir` parameter\n      refs/files: remove useless indirection\n      refs/files: extract function to check single ref\n      refs/files: improve error handling when verifying symrefs\n      refs/files: perform consistency checks for root refs\n      fsck: drop unused fields from `struct fsck_ref_report`\n      refs/files: extract generic symref target checks\n      refs/files: introduce function to perform normal ref checks\n      refs/reftable: adapt includes to become consistent\n      refs/reftable: extract function to retrieve backend for worktree\n      refs/reftable: fix consistency checks with worktrees\n      refs/reftable: introduce generic checks for refs\n      builtin/fsck: move generic object ID checks into `refs_fsck()`\n      builtin/fsck: move generic HEAD check into `refs_fsck()`\n      builtin/fsck: drop `fsck_head_link()`\n\n Documentation/fsck-msgids.adoc |   6 ++\n builtin/fsck.c                 |  46 +--------\n fsck.c                         |   5 -\n fsck.h                         |   4 +-\n refs.c                         |  43 ++++++++\n refs.h                         |  18 ++++\n refs/files-backend.c           | 230 ++++++++++++++++++++++++-----------------\n refs/reftable-backend.c        | 167 ++++++++++++++++++++++--------\n t/t0602-reffiles-fsck.sh       |  30 ++++++\n t/t0614-reftable-fsck.sh       |  44 ++++++++\n t/t1450-fsck.sh                |  10 +-\n 11 files changed, 416 insertions(+), 187 deletions(-)\n\n\n---\nbase-commit: d529f3a197364881746f558e5652f0236131eb86\nchange-id: 20260109-pks-refs-verify-fixes-1e47872317cf\n\n"},{"id":"533346","messageId":"20260109-pks-refs-verify-fixes-v1-1-3587dba18294@pks.im","threadId":"64762","inReplyTo":"20260109-pks-refs-verify-fixes-v1-0-3587dba18294@pks.im","subject":"[PATCH 01/17] refs/files: simplify iterating through root refs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-09T12:39:30Z","receivedAt":"2026-01-09T12:39:38Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When iterating through root refs we first need to determine the\ndirectory in which the refs live. This is done by retrieving the root of\nthe loose refs via `refs->loose->root->name`, and putting it through\n`files_ref_path()` to derive the final path.\n\nThis is somewhat redundant though: the root name of the loose files\ncache is always going to be the empty string. As such, we always end up\npassing that empty string to `files_ref_path()` as the ref hierarchy we\nwant to start. And this actually makes sense: `files_ref_path()` already\ncomputes the location of the root directory, so of course we need to\npass the empty string for the ref hierarchy itself. So going via the\nloose ref cache to figure out that the root of a ref hierarchy is empty\nis only causing confusion.\n\nBut next to the added confusion, it can also lead to a segfault. The\nloose ref cache is populated lazily, so it may not always be set. It\nseems to be sheer luck that this is a condition we do not currently hit.\nThe right thing to do would be to call `get_loose_ref_cache()`, which\nknows to populate the cache if required.\n\nSimplify the code and fix the potential segfault by simply removing the\nindirection via the loose ref cache completely.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n refs/files-backend.c | 11 +++--------\n 1 file changed, 3 insertions(+), 8 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 6f6f76a8d8..297739f203 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -354,13 +354,11 @@ static int for_each_root_ref(struct files_ref_store *refs,\n \t\t\t     void *cb_data)\n {\n \tstruct strbuf path = STRBUF_INIT, refname = STRBUF_INIT;\n-\tconst char *dirname = refs->loose->root->name;\n \tstruct dirent *de;\n-\tsize_t dirnamelen;\n \tint ret;\n \tDIR *d;\n \n-\tfiles_ref_path(refs, &path, dirname);\n+\tfiles_ref_path(refs, &path, \"\");\n \n \td = opendir(path.buf);\n \tif (!d) {\n@@ -368,9 +366,6 @@ static int for_each_root_ref(struct files_ref_store *refs,\n \t\treturn -1;\n \t}\n \n-\tstrbuf_addstr(&refname, dirname);\n-\tdirnamelen = refname.len;\n-\n \twhile ((de = readdir(d)) != NULL) {\n \t\tunsigned char dtype;\n \n@@ -378,6 +373,8 @@ static int for_each_root_ref(struct files_ref_store *refs,\n \t\t\tcontinue;\n \t\tif (ends_with(de->d_name, \".lock\"))\n \t\t\tcontinue;\n+\n+\t\tstrbuf_reset(&refname);\n \t\tstrbuf_addstr(&refname, de->d_name);\n \n \t\tdtype = get_dtype(de, &path, 1);\n@@ -386,8 +383,6 @@ static int for_each_root_ref(struct files_ref_store *refs,\n \t\t\tif (ret)\n \t\t\t\tgoto done;\n \t\t}\n-\n-\t\tstrbuf_setlen(&refname, dirnamelen);\n \t}\n \n \tret = 0;\n\n-- \n2.52.0.542.g9473a8513b.dirty\n\n"},{"id":"533347","messageId":"20260109-pks-refs-verify-fixes-v1-2-3587dba18294@pks.im","threadId":"64762","inReplyTo":"20260109-pks-refs-verify-fixes-v1-0-3587dba18294@pks.im","subject":"[PATCH 02/17] refs/files: move fsck functions into global scope","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-09T12:39:31Z","receivedAt":"2026-01-09T12:39:41Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When performing consistency checks we pass the functions that perform\nthe verification down the calling stack. This is somewhat unnecessary\nthough, as the set of functions doesn't ever change.\n\nSimplify the code by moving the array into global scope and remove the\nparameter.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n refs/files-backend.c | 17 ++++++++---------\n 1 file changed, 8 insertions(+), 9 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 297739f203..feba3ee58b 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -3890,11 +3890,16 @@ static int files_fsck_refs_name(struct ref_store *ref_store UNUSED,\n \treturn ret;\n }\n \n+static const files_fsck_refs_fn fsck_refs_fn[]= {\n+\tfiles_fsck_refs_name,\n+\tfiles_fsck_refs_content,\n+\tNULL,\n+};\n+\n static int files_fsck_refs_dir(struct ref_store *ref_store,\n \t\t\t       struct fsck_options *o,\n \t\t\t       const char *refs_check_dir,\n-\t\t\t       struct worktree *wt,\n-\t\t\t       files_fsck_refs_fn *fsck_refs_fn)\n+\t\t\t       struct worktree *wt)\n {\n \tstruct strbuf refname = STRBUF_INIT;\n \tstruct strbuf sb = STRBUF_INIT;\n@@ -3955,13 +3960,7 @@ static int files_fsck_refs(struct ref_store *ref_store,\n \t\t\t   struct fsck_options *o,\n \t\t\t   struct worktree *wt)\n {\n-\tfiles_fsck_refs_fn fsck_refs_fn[]= {\n-\t\tfiles_fsck_refs_name,\n-\t\tfiles_fsck_refs_content,\n-\t\tNULL,\n-\t};\n-\n-\treturn files_fsck_refs_dir(ref_store, o, \"refs\", wt, fsck_refs_fn);\n+\treturn files_fsck_refs_dir(ref_store, o, \"refs\", wt);\n }\n \n static int files_fsck(struct ref_store *ref_store,\n\n-- \n2.52.0.542.g9473a8513b.dirty\n\n"},{"id":"533348","messageId":"20260109-pks-refs-verify-fixes-v1-3-3587dba18294@pks.im","threadId":"64762","inReplyTo":"20260109-pks-refs-verify-fixes-v1-0-3587dba18294@pks.im","subject":"[PATCH 03/17] refs/files: remove `refs_check_dir` parameter","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-09T12:39:32Z","receivedAt":"2026-01-09T12:39:43Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The parameter `refs_check_dir` determines which directory we want to\ncheck references for. But as we always want to check the complete\nrefs hierarchy, this parameter is always set to \"refs\".\n\nDrop the parameter and hardcode it.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n refs/files-backend.c | 8 +++-----\n 1 file changed, 3 insertions(+), 5 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex feba3ee58b..0a104c7bf6 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -3898,7 +3898,6 @@ static const files_fsck_refs_fn fsck_refs_fn[]= {\n \n static int files_fsck_refs_dir(struct ref_store *ref_store,\n \t\t\t       struct fsck_options *o,\n-\t\t\t       const char *refs_check_dir,\n \t\t\t       struct worktree *wt)\n {\n \tstruct strbuf refname = STRBUF_INIT;\n@@ -3907,7 +3906,7 @@ static int files_fsck_refs_dir(struct ref_store *ref_store,\n \tint iter_status;\n \tint ret = 0;\n \n-\tstrbuf_addf(&sb, \"%s/%s\", ref_store->gitdir, refs_check_dir);\n+\tstrbuf_addf(&sb, \"%s/refs\", ref_store->gitdir);\n \n \titer = dir_iterator_begin(sb.buf, 0);\n \tif (!iter) {\n@@ -3927,8 +3926,7 @@ static int files_fsck_refs_dir(struct ref_store *ref_store,\n \n \t\t\tif (!is_main_worktree(wt))\n \t\t\t\tstrbuf_addf(&refname, \"worktrees/%s/\", wt->id);\n-\t\t\tstrbuf_addf(&refname, \"%s/%s\", refs_check_dir,\n-\t\t\t\t    iter->relative_path);\n+\t\t\tstrbuf_addf(&refname, \"refs/%s\", iter->relative_path);\n \n \t\t\tif (o->verbose)\n \t\t\t\tfprintf_ln(stderr, \"Checking %s\", refname.buf);\n@@ -3960,7 +3958,7 @@ static int files_fsck_refs(struct ref_store *ref_store,\n \t\t\t   struct fsck_options *o,\n \t\t\t   struct worktree *wt)\n {\n-\treturn files_fsck_refs_dir(ref_store, o, \"refs\", wt);\n+\treturn files_fsck_refs_dir(ref_store, o, wt);\n }\n \n static int files_fsck(struct ref_store *ref_store,\n\n-- \n2.52.0.542.g9473a8513b.dirty\n\n"},{"id":"533349","messageId":"20260109-pks-refs-verify-fixes-v1-4-3587dba18294@pks.im","threadId":"64762","inReplyTo":"20260109-pks-refs-verify-fixes-v1-0-3587dba18294@pks.im","subject":"[PATCH 04/17] refs/files: remove useless indirection","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-09T12:39:33Z","receivedAt":"2026-01-09T12:39:47Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The function `files_fsck_refs()` only has a single callsite and forwards\nall of its arguments as-is, so it's basically a useless indirection.\nInline the function call.\n\nWhile at it, also remove the bitwise or that we have for return values.\nWe don't really want to or them at all, but rather just want to return\nan error in case either of the functions has failed.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n refs/files-backend.c | 16 +++++++---------\n 1 file changed, 7 insertions(+), 9 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 0a104c7bf6..4cbee23dad 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -3954,22 +3954,20 @@ static int files_fsck_refs_dir(struct ref_store *ref_store,\n \treturn ret;\n }\n \n-static int files_fsck_refs(struct ref_store *ref_store,\n-\t\t\t   struct fsck_options *o,\n-\t\t\t   struct worktree *wt)\n-{\n-\treturn files_fsck_refs_dir(ref_store, o, wt);\n-}\n-\n static int files_fsck(struct ref_store *ref_store,\n \t\t      struct fsck_options *o,\n \t\t      struct worktree *wt)\n {\n \tstruct files_ref_store *refs =\n \t\tfiles_downcast(ref_store, REF_STORE_READ, \"fsck\");\n+\tint ret = 0;\n \n-\treturn files_fsck_refs(ref_store, o, wt) |\n-\t       refs->packed_ref_store->be->fsck(refs->packed_ref_store, o, wt);\n+\tif (files_fsck_refs_dir(ref_store, o, wt) < 0)\n+\t\tret = -1;\n+\tif (refs->packed_ref_store->be->fsck(refs->packed_ref_store, o, wt) < 0)\n+\t\tret = -1;\n+\n+\treturn ret;\n }\n \n struct ref_storage_be refs_be_files = {\n\n-- \n2.52.0.542.g9473a8513b.dirty\n\n"},{"id":"533350","messageId":"20260109-pks-refs-verify-fixes-v1-5-3587dba18294@pks.im","threadId":"64762","inReplyTo":"20260109-pks-refs-verify-fixes-v1-0-3587dba18294@pks.im","subject":"[PATCH 05/17] refs/files: extract function to check single ref","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-09T12:39:34Z","receivedAt":"2026-01-09T12:39:50Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When checking the consistency of references we create a directory\niterator and then verify each single reference in a loop. The logic to\nperform the actual checks is embedded into that loop, which makes it\nhard to reuse. But In a subsequent commit we're about to introduce a\nsecond path that wants to verify references.\n\nPrepare for this by extracting the logic to check a single reference\ninto a standalone function.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n refs/files-backend.c | 80 +++++++++++++++++++++++++++++++++-------------------\n 1 file changed, 51 insertions(+), 29 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 4cbee23dad..9972221f9f 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -3715,7 +3715,8 @@ static int files_ref_store_remove_on_disk(struct ref_store *ref_store,\n typedef int (*files_fsck_refs_fn)(struct ref_store *ref_store,\n \t\t\t\t  struct fsck_options *o,\n \t\t\t\t  const char *refname,\n-\t\t\t\t  struct dir_iterator *iter);\n+\t\t\t\t  const char *path,\n+\t\t\t\t  int mode);\n \n static int files_fsck_symref_target(struct fsck_options *o,\n \t\t\t\t    struct fsck_ref_report *report,\n@@ -3772,7 +3773,8 @@ static int files_fsck_symref_target(struct fsck_options *o,\n static int files_fsck_refs_content(struct ref_store *ref_store,\n \t\t\t\t   struct fsck_options *o,\n \t\t\t\t   const char *target_name,\n-\t\t\t\t   struct dir_iterator *iter)\n+\t\t\t\t   const char *path,\n+\t\t\t\t   int mode)\n {\n \tstruct strbuf ref_content = STRBUF_INIT;\n \tstruct strbuf abs_gitdir = STRBUF_INIT;\n@@ -3786,7 +3788,7 @@ static int files_fsck_refs_content(struct ref_store *ref_store,\n \n \treport.path = target_name;\n \n-\tif (S_ISLNK(iter->st.st_mode)) {\n+\tif (S_ISLNK(mode)) {\n \t\tconst char *relative_referent_path = NULL;\n \n \t\tret = fsck_report_ref(o, &report,\n@@ -3798,7 +3800,7 @@ static int files_fsck_refs_content(struct ref_store *ref_store,\n \t\tif (!is_dir_sep(abs_gitdir.buf[abs_gitdir.len - 1]))\n \t\t\tstrbuf_addch(&abs_gitdir, '/');\n \n-\t\tstrbuf_add_real_path(&ref_content, iter->path.buf);\n+\t\tstrbuf_add_real_path(&ref_content, path);\n \t\tskip_prefix(ref_content.buf, abs_gitdir.buf,\n \t\t\t    &relative_referent_path);\n \n@@ -3811,7 +3813,7 @@ static int files_fsck_refs_content(struct ref_store *ref_store,\n \t\tgoto cleanup;\n \t}\n \n-\tif (strbuf_read_file(&ref_content, iter->path.buf, 0) < 0) {\n+\tif (strbuf_read_file(&ref_content, path, 0) < 0) {\n \t\t/*\n \t\t * Ref file could be removed by another concurrent process. We should\n \t\t * ignore this error and continue to the next ref.\n@@ -3819,7 +3821,7 @@ static int files_fsck_refs_content(struct ref_store *ref_store,\n \t\tif (errno == ENOENT)\n \t\t\tgoto cleanup;\n \n-\t\tret = error_errno(_(\"cannot read ref file '%s'\"), iter->path.buf);\n+\t\tret = error_errno(_(\"cannot read ref file '%s'\"), path);\n \t\tgoto cleanup;\n \t}\n \n@@ -3861,16 +3863,20 @@ static int files_fsck_refs_content(struct ref_store *ref_store,\n static int files_fsck_refs_name(struct ref_store *ref_store UNUSED,\n \t\t\t\tstruct fsck_options *o,\n \t\t\t\tconst char *refname,\n-\t\t\t\tstruct dir_iterator *iter)\n+\t\t\t\tconst char *path,\n+\t\t\t\tint mode UNUSED)\n {\n \tstruct strbuf sb = STRBUF_INIT;\n+\tconst char *filename;\n \tint ret = 0;\n \n+\tfilename = basename((char *) path);\n+\n \t/*\n \t * Ignore the files ending with \".lock\" as they may be lock files\n \t * However, do not allow bare \".lock\" files.\n \t */\n-\tif (iter->basename[0] != '.' && ends_with(iter->basename, \".lock\"))\n+\tif (filename[0] != '.' && ends_with(filename, \".lock\"))\n \t\tgoto cleanup;\n \n \t/*\n@@ -3896,6 +3902,35 @@ static const files_fsck_refs_fn fsck_refs_fn[]= {\n \tNULL,\n };\n \n+static int files_fsck_ref(struct ref_store *ref_store,\n+\t\t\t  struct fsck_options *o,\n+\t\t\t  const char *refname,\n+\t\t\t  const char *path,\n+\t\t\t  int mode)\n+{\n+\tint ret = 0;\n+\n+\tif (o->verbose)\n+\t\tfprintf_ln(stderr, \"Checking %s\", refname);\n+\n+\tif (!S_ISREG(mode) && !S_ISLNK(mode)) {\n+\t\tstruct fsck_ref_report report = { .path = refname };\n+\n+\t\tif (fsck_report_ref(o, &report,\n+\t\t\t\t    FSCK_MSG_BAD_REF_FILETYPE,\n+\t\t\t\t    \"unexpected file type\"))\n+\t\t\tret = -1;\n+\t\tgoto out;\n+\t}\n+\n+\tfor (size_t i = 0; fsck_refs_fn[i]; i++)\n+\t\tif (fsck_refs_fn[i](ref_store, o, refname, path, mode))\n+\t\t\tret = -1;\n+\n+out:\n+\treturn ret;\n+}\n+\n static int files_fsck_refs_dir(struct ref_store *ref_store,\n \t\t\t       struct fsck_options *o,\n \t\t\t       struct worktree *wt)\n@@ -3918,30 +3953,17 @@ static int files_fsck_refs_dir(struct ref_store *ref_store,\n \t}\n \n \twhile ((iter_status = dir_iterator_advance(iter)) == ITER_OK) {\n-\t\tif (S_ISDIR(iter->st.st_mode)) {\n+\t\tif (S_ISDIR(iter->st.st_mode))\n \t\t\tcontinue;\n-\t\t} else if (S_ISREG(iter->st.st_mode) ||\n-\t\t\t   S_ISLNK(iter->st.st_mode)) {\n-\t\t\tstrbuf_reset(&refname);\n-\n-\t\t\tif (!is_main_worktree(wt))\n-\t\t\t\tstrbuf_addf(&refname, \"worktrees/%s/\", wt->id);\n-\t\t\tstrbuf_addf(&refname, \"refs/%s\", iter->relative_path);\n \n-\t\t\tif (o->verbose)\n-\t\t\t\tfprintf_ln(stderr, \"Checking %s\", refname.buf);\n+\t\tstrbuf_reset(&refname);\n+\t\tif (!is_main_worktree(wt))\n+\t\t\tstrbuf_addf(&refname, \"worktrees/%s/\", wt->id);\n+\t\tstrbuf_addf(&refname, \"refs/%s\", iter->relative_path);\n \n-\t\t\tfor (size_t i = 0; fsck_refs_fn[i]; i++) {\n-\t\t\t\tif (fsck_refs_fn[i](ref_store, o, refname.buf, iter))\n-\t\t\t\t\tret = -1;\n-\t\t\t}\n-\t\t} else {\n-\t\t\tstruct fsck_ref_report report = { .path = iter->basename };\n-\t\t\tif (fsck_report_ref(o, &report,\n-\t\t\t\t\t    FSCK_MSG_BAD_REF_FILETYPE,\n-\t\t\t\t\t    \"unexpected file type\"))\n-\t\t\t\tret = -1;\n-\t\t}\n+\t\tif (files_fsck_ref(ref_store, o, refname.buf,\n+\t\t\t\t   iter->path.buf, iter->st.st_mode) < 0)\n+\t\t\tret = -1;\n \t}\n \n \tif (iter_status != ITER_DONE)\n\n-- \n2.52.0.542.g9473a8513b.dirty\n\n"},{"id":"533351","messageId":"20260109-pks-refs-verify-fixes-v1-6-3587dba18294@pks.im","threadId":"64762","inReplyTo":"20260109-pks-refs-verify-fixes-v1-0-3587dba18294@pks.im","subject":"[PATCH 06/17] refs/files: improve error handling when verifying symrefs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-09T12:39:35Z","receivedAt":"2026-01-09T12:39:52Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The error handling when verifying symbolic refs is a bit on the wild\nside:\n\n  - `fsck_report_ref()` can be told to ignore specific errors. If an\n    error has been ignored and a previous check raised an unignored\n    error, then assigning `ret = fsck_report_ref()` will cause us to\n    swallow the previous error.\n\n  - When the target reference is not valid we bail out early without\n    checking for other errors.\n\nFix both of these issues by consistently or'ing the return value and not\nbailing out early.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n refs/files-backend.c | 28 +++++++++++++---------------\n 1 file changed, 13 insertions(+), 15 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 9972221f9f..abc2165339 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -3737,17 +3737,15 @@ static int files_fsck_symref_target(struct fsck_options *o,\n \tif (!is_referent_root &&\n \t    !starts_with(referent->buf, \"refs/\") &&\n \t    !starts_with(referent->buf, \"worktrees/\")) {\n-\t\tret = fsck_report_ref(o, report,\n-\t\t\t\t      FSCK_MSG_SYMREF_TARGET_IS_NOT_A_REF,\n-\t\t\t\t      \"points to non-ref target '%s'\", referent->buf);\n-\n+\t\tret |= fsck_report_ref(o, report,\n+\t\t\t\t       FSCK_MSG_SYMREF_TARGET_IS_NOT_A_REF,\n+\t\t\t\t       \"points to non-ref target '%s'\", referent->buf);\n \t}\n \n \tif (!is_referent_root && check_refname_format(referent->buf, 0)) {\n-\t\tret = fsck_report_ref(o, report,\n-\t\t\t\t      FSCK_MSG_BAD_REFERENT_NAME,\n-\t\t\t\t      \"points to invalid refname '%s'\", referent->buf);\n-\t\tgoto out;\n+\t\tret |= fsck_report_ref(o, report,\n+\t\t\t\t       FSCK_MSG_BAD_REFERENT_NAME,\n+\t\t\t\t       \"points to invalid refname '%s'\", referent->buf);\n \t}\n \n \tif (symbolic_link)\n@@ -3755,19 +3753,19 @@ static int files_fsck_symref_target(struct fsck_options *o,\n \n \tif (referent->len == orig_len ||\n \t    (referent->len < orig_len && orig_last_byte != '\\n')) {\n-\t\tret = fsck_report_ref(o, report,\n-\t\t\t\t      FSCK_MSG_REF_MISSING_NEWLINE,\n-\t\t\t\t      \"misses LF at the end\");\n+\t\tret |= fsck_report_ref(o, report,\n+\t\t\t\t       FSCK_MSG_REF_MISSING_NEWLINE,\n+\t\t\t\t       \"misses LF at the end\");\n \t}\n \n \tif (referent->len != orig_len && referent->len != orig_len - 1) {\n-\t\tret = fsck_report_ref(o, report,\n-\t\t\t\t      FSCK_MSG_TRAILING_REF_CONTENT,\n-\t\t\t\t      \"has trailing whitespaces or newlines\");\n+\t\tret |= fsck_report_ref(o, report,\n+\t\t\t\t       FSCK_MSG_TRAILING_REF_CONTENT,\n+\t\t\t\t       \"has trailing whitespaces or newlines\");\n \t}\n \n out:\n-\treturn ret;\n+\treturn ret ? -1 : 0;\n }\n \n static int files_fsck_refs_content(struct ref_store *ref_store,\n\n-- \n2.52.0.542.g9473a8513b.dirty\n\n"},{"id":"533352","messageId":"20260109-pks-refs-verify-fixes-v1-7-3587dba18294@pks.im","threadId":"64762","inReplyTo":"20260109-pks-refs-verify-fixes-v1-0-3587dba18294@pks.im","subject":"[PATCH 07/17] refs/files: perform consistency checks for root refs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-09T12:39:36Z","receivedAt":"2026-01-09T12:39:54Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"While the \"files\" backend already knows to perform consistency checks\nfor the \"refs/\" hierarchy, it doesn't verify any of its root refs. Plug\nthis omission.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n refs/files-backend.c     | 52 +++++++++++++++++++++++++++++++++++++++++++++---\n t/t0602-reffiles-fsck.sh | 30 ++++++++++++++++++++++++++++\n 2 files changed, 79 insertions(+), 3 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex abc2165339..0ff047d0df 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -3877,9 +3877,9 @@ static int files_fsck_refs_name(struct ref_store *ref_store UNUSED,\n \tif (filename[0] != '.' && ends_with(filename, \".lock\"))\n \t\tgoto cleanup;\n \n-\t/*\n-\t * This works right now because we never check the root refs.\n-\t */\n+\tif (is_root_ref(refname))\n+\t\tgoto cleanup;\n+\n \tif (check_refname_format(refname, 0)) {\n \t\tstruct fsck_ref_report report = { 0 };\n \n@@ -3974,19 +3974,65 @@ static int files_fsck_refs_dir(struct ref_store *ref_store,\n \treturn ret;\n }\n \n+struct files_fsck_root_ref_data {\n+\tstruct files_ref_store *refs;\n+\tstruct fsck_options *o;\n+\tstruct worktree *wt;\n+\tstruct strbuf refname;\n+\tstruct strbuf path;\n+\tbool errors_found;\n+};\n+\n+static int files_fsck_root_ref(const char *refname, void *cb_data)\n+{\n+\tstruct files_fsck_root_ref_data *data = cb_data;\n+\tstruct stat st;\n+\n+\tstrbuf_reset(&data->refname);\n+\tif (!is_main_worktree(data->wt))\n+\t\tstrbuf_addf(&data->refname, \"worktrees/%s/\", data->wt->id);\n+\tstrbuf_addstr(&data->refname, refname);\n+\n+\tstrbuf_reset(&data->path);\n+\tstrbuf_addf(&data->path, \"%s/%s\", data->refs->gitcommondir, data->refname.buf);\n+\n+\tif (stat(data->path.buf, &st)) {\n+\t\tif (errno == ENOENT)\n+\t\t\treturn 0;\n+\t\treturn error_errno(\"failed to read ref: '%s'\", data->path.buf);\n+\t}\n+\n+\treturn files_fsck_ref(&data->refs->base, data->o, data->refname.buf,\n+\t\t\t      data->path.buf, st.st_mode);\n+}\n+\n static int files_fsck(struct ref_store *ref_store,\n \t\t      struct fsck_options *o,\n \t\t      struct worktree *wt)\n {\n \tstruct files_ref_store *refs =\n \t\tfiles_downcast(ref_store, REF_STORE_READ, \"fsck\");\n+\tstruct files_fsck_root_ref_data data = {\n+\t\t.refs = refs,\n+\t\t.o = o,\n+\t\t.wt = wt,\n+\t\t.refname = STRBUF_INIT,\n+\t\t.path = STRBUF_INIT,\n+\t};\n \tint ret = 0;\n \n \tif (files_fsck_refs_dir(ref_store, o, wt) < 0)\n \t\tret = -1;\n+\n+\tif (for_each_root_ref(refs, files_fsck_root_ref, &data) < 0 ||\n+\t    data.errors_found)\n+\t\tret = -1;\n+\n \tif (refs->packed_ref_store->be->fsck(refs->packed_ref_store, o, wt) < 0)\n \t\tret = -1;\n \n+\tstrbuf_release(&data.refname);\n+\tstrbuf_release(&data.path);\n \treturn ret;\n }\n \ndiff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh\nindex 0ef483659d..479f3d528e 100755\n--- a/t/t0602-reffiles-fsck.sh\n+++ b/t/t0602-reffiles-fsck.sh\n@@ -905,4 +905,34 @@ test_expect_success '--[no-]references option should apply to fsck' '\n \t)\n '\n \n+test_expect_success 'complains about broken root ref' '\n+\ttest_when_finished \"rm -rf repo\" &&\n+\tgit init repo &&\n+\t(\n+\t\tcd repo &&\n+\t\techo \"ref: refs/../HEAD\" >.git/HEAD &&\n+\t\ttest_must_fail git refs verify 2>err &&\n+\t\tcat >expect <<-EOF &&\n+\t\terror: HEAD: badReferentName: points to invalid refname ${SQ}refs/../HEAD${SQ}\n+\t\tEOF\n+\t\ttest_cmp expect err\n+\t)\n+'\n+\n+test_expect_success 'complains about broken root ref in worktree' '\n+\ttest_when_finished \"rm -rf repo worktree\" &&\n+\tgit init repo &&\n+\t(\n+\t\tcd repo &&\n+\t\ttest_commit initial &&\n+\t\tgit worktree add ../worktree &&\n+\t\techo \"ref: refs/../HEAD\" >.git/worktrees/worktree/HEAD &&\n+\t\ttest_must_fail git refs verify 2>err &&\n+\t\tcat >expect <<-EOF &&\n+\t\terror: worktrees/worktree/HEAD: badReferentName: points to invalid refname ${SQ}refs/../HEAD${SQ}\n+\t\tEOF\n+\t\ttest_cmp expect err\n+\t)\n+'\n+\n test_done\n\n-- \n2.52.0.542.g9473a8513b.dirty\n\n"},{"id":"533353","messageId":"20260109-pks-refs-verify-fixes-v1-8-3587dba18294@pks.im","threadId":"64762","inReplyTo":"20260109-pks-refs-verify-fixes-v1-0-3587dba18294@pks.im","subject":"[PATCH 08/17] fsck: drop unused fields from `struct fsck_ref_report`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-09T12:39:37Z","receivedAt":"2026-01-09T12:39:57Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `struct fsck_ref_report` has a couple fields that are intended to\nimprove the error reporting for broken ref reports by showing which\nobject ID or target reference the ref points to. These fields are never\nset though and are thus essentially unused.\n\nRemove them.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n fsck.c | 5 -----\n fsck.h | 2 --\n 2 files changed, 7 deletions(-)\n\ndiff --git a/fsck.c b/fsck.c\nindex fae18d8561..813d927d57 100644\n--- a/fsck.c\n+++ b/fsck.c\n@@ -1310,11 +1310,6 @@ int fsck_refs_error_function(struct fsck_options *options UNUSED,\n \n \tstrbuf_addstr(&sb, report->path);\n \n-\tif (report->oid)\n-\t\tstrbuf_addf(&sb, \" -> (%s)\", oid_to_hex(report->oid));\n-\telse if (report->referent)\n-\t\tstrbuf_addf(&sb, \" -> (%s)\", report->referent);\n-\n \tif (msg_type == FSCK_WARN)\n \t\twarning(\"%s: %s\", sb.buf, message);\n \telse\ndiff --git a/fsck.h b/fsck.h\nindex 336917c045..bfe0d9c6d2 100644\n--- a/fsck.h\n+++ b/fsck.h\n@@ -162,8 +162,6 @@ struct fsck_object_report {\n \n struct fsck_ref_report {\n \tconst char *path;\n-\tconst struct object_id *oid;\n-\tconst char *referent;\n };\n \n struct fsck_options {\n\n-- \n2.52.0.542.g9473a8513b.dirty\n\n"},{"id":"533354","messageId":"20260109-pks-refs-verify-fixes-v1-9-3587dba18294@pks.im","threadId":"64762","inReplyTo":"20260109-pks-refs-verify-fixes-v1-0-3587dba18294@pks.im","subject":"[PATCH 09/17] refs/files: extract generic symref target checks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-09T12:39:38Z","receivedAt":"2026-01-09T12:39:59Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The consistency checks for the \"files\" backend contain a couple of\nverifications for symrefs that verify generic properties of the target\nreference. These properties need to hold for every backend, no matter\nwhether it's using the \"files\" or \"reftable\" backend.\n\nReimplementing these checks for every single backend doesn't really make\nsense. Extract it into a generic `refs_fsck_symref()` function that can\nbe used my other backends, as well. The \"reftable\" backend will be wired\nup in a subsequent commit.\n\nWhile at it, improve the consistency checks so that we don't complain\nabout refs pointing to a non-ref target in case the target refname\nformat does not verify. Otherwise it's very likely that we'll generate\nboth error messages, which feels somewhat redundant in this case.\n\nNote that the function has a couple of `UNUSED` parameters. These will\nbecome referenced in a subsequent commit.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n refs.c               | 21 ++++++++++++++++++++\n refs.h               | 10 ++++++++++\n refs/files-backend.c | 54 ++++++++++++++++++++--------------------------------\n 3 files changed, 52 insertions(+), 33 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex e06e0cb072..739bf9fefc 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -320,6 +320,27 @@ int check_refname_format(const char *refname, int flags)\n \treturn check_or_sanitize_refname(refname, flags, NULL);\n }\n \n+int refs_fsck_symref(struct ref_store *refs UNUSED, struct fsck_options *o,\n+\t\t     struct fsck_ref_report *report,\n+\t\t     const char *refname UNUSED, const char *target)\n+{\n+\tif (is_root_ref(target))\n+\t\treturn 0;\n+\n+\tif (check_refname_format(target, 0) &&\n+\t    fsck_report_ref(o, report, FSCK_MSG_BAD_REFERENT_NAME,\n+\t\t\t    \"points to invalid refname '%s'\", target))\n+\t\treturn -1;\n+\n+\tif (!starts_with(target, \"refs/\") &&\n+\t    !starts_with(target, \"worktrees/\") &&\n+\t    fsck_report_ref(o, report, FSCK_MSG_SYMREF_TARGET_IS_NOT_A_REF,\n+\t\t\t    \"points to non-ref target '%s'\", target))\n+\t\treturn -1;\n+\n+\treturn 0;\n+}\n+\n int refs_fsck(struct ref_store *refs, struct fsck_options *o,\n \t      struct worktree *wt)\n {\ndiff --git a/refs.h b/refs.h\nindex d9051bbb04..d91fcb2d2f 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -653,6 +653,16 @@ int refs_for_each_reflog(struct ref_store *refs, each_reflog_fn fn, void *cb_dat\n  */\n int check_refname_format(const char *refname, int flags);\n \n+struct fsck_ref_report;\n+\n+/*\n+ * Perform generic checks for a specific symref target. This function is\n+ * expected to be called by the ref backends for every symbolic ref.\n+ */\n+int refs_fsck_symref(struct ref_store *refs, struct fsck_options *o,\n+\t\t     struct fsck_ref_report *report,\n+\t\t     const char *refname, const char *target);\n+\n /*\n  * Check the reference database for consistency. Return 0 if refs and\n  * reflogs are consistent, and non-zero otherwise. The errors will be\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 0ff047d0df..72c1db849e 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -3718,53 +3718,39 @@ typedef int (*files_fsck_refs_fn)(struct ref_store *ref_store,\n \t\t\t\t  const char *path,\n \t\t\t\t  int mode);\n \n-static int files_fsck_symref_target(struct fsck_options *o,\n+static int files_fsck_symref_target(struct ref_store *ref_store,\n+\t\t\t\t    struct fsck_options *o,\n \t\t\t\t    struct fsck_ref_report *report,\n+\t\t\t\t    const char *refname,\n \t\t\t\t    struct strbuf *referent,\n \t\t\t\t    unsigned int symbolic_link)\n {\n-\tint is_referent_root;\n \tchar orig_last_byte;\n \tsize_t orig_len;\n \tint ret = 0;\n \n \torig_len = referent->len;\n \torig_last_byte = referent->buf[orig_len - 1];\n-\tif (!symbolic_link)\n-\t\tstrbuf_rtrim(referent);\n-\n-\tis_referent_root = is_root_ref(referent->buf);\n-\tif (!is_referent_root &&\n-\t    !starts_with(referent->buf, \"refs/\") &&\n-\t    !starts_with(referent->buf, \"worktrees/\")) {\n-\t\tret |= fsck_report_ref(o, report,\n-\t\t\t\t       FSCK_MSG_SYMREF_TARGET_IS_NOT_A_REF,\n-\t\t\t\t       \"points to non-ref target '%s'\", referent->buf);\n-\t}\n \n-\tif (!is_referent_root && check_refname_format(referent->buf, 0)) {\n-\t\tret |= fsck_report_ref(o, report,\n-\t\t\t\t       FSCK_MSG_BAD_REFERENT_NAME,\n-\t\t\t\t       \"points to invalid refname '%s'\", referent->buf);\n-\t}\n+\tif (!symbolic_link) {\n+\t\tstrbuf_rtrim(referent);\n \n-\tif (symbolic_link)\n-\t\tgoto out;\n+\t\tif (referent->len == orig_len ||\n+\t\t    (referent->len < orig_len && orig_last_byte != '\\n')) {\n+\t\t\tret |= fsck_report_ref(o, report,\n+\t\t\t\t\t       FSCK_MSG_REF_MISSING_NEWLINE,\n+\t\t\t\t\t       \"misses LF at the end\");\n+\t\t}\n \n-\tif (referent->len == orig_len ||\n-\t    (referent->len < orig_len && orig_last_byte != '\\n')) {\n-\t\tret |= fsck_report_ref(o, report,\n-\t\t\t\t       FSCK_MSG_REF_MISSING_NEWLINE,\n-\t\t\t\t       \"misses LF at the end\");\n+\t\tif (referent->len != orig_len && referent->len != orig_len - 1) {\n+\t\t\tret |= fsck_report_ref(o, report,\n+\t\t\t\t\t       FSCK_MSG_TRAILING_REF_CONTENT,\n+\t\t\t\t\t       \"has trailing whitespaces or newlines\");\n+\t\t}\n \t}\n \n-\tif (referent->len != orig_len && referent->len != orig_len - 1) {\n-\t\tret |= fsck_report_ref(o, report,\n-\t\t\t\t       FSCK_MSG_TRAILING_REF_CONTENT,\n-\t\t\t\t       \"has trailing whitespaces or newlines\");\n-\t}\n+\tret |= refs_fsck_symref(ref_store, o, report, refname, referent->buf);\n \n-out:\n \treturn ret ? -1 : 0;\n }\n \n@@ -3807,7 +3793,8 @@ static int files_fsck_refs_content(struct ref_store *ref_store,\n \t\telse\n \t\t\tstrbuf_addbuf(&referent, &ref_content);\n \n-\t\tret |= files_fsck_symref_target(o, &report, &referent, 1);\n+\t\tret |= files_fsck_symref_target(ref_store, o, &report,\n+\t\t\t\t\t\ttarget_name, &referent, 1);\n \t\tgoto cleanup;\n \t}\n \n@@ -3847,7 +3834,8 @@ static int files_fsck_refs_content(struct ref_store *ref_store,\n \t\t\tgoto cleanup;\n \t\t}\n \t} else {\n-\t\tret = files_fsck_symref_target(o, &report, &referent, 0);\n+\t\tret = files_fsck_symref_target(ref_store, o, &report,\n+\t\t\t\t\t       target_name, &referent, 0);\n \t\tgoto cleanup;\n \t}\n \n\n-- \n2.52.0.542.g9473a8513b.dirty\n\n"},{"id":"533355","messageId":"20260109-pks-refs-verify-fixes-v1-10-3587dba18294@pks.im","threadId":"64762","inReplyTo":"20260109-pks-refs-verify-fixes-v1-0-3587dba18294@pks.im","subject":"[PATCH 10/17] refs/files: introduce function to perform normal ref checks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-09T12:39:39Z","receivedAt":"2026-01-09T12:40:04Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In a subsequent commit we'll introduce new generic checks for direct\nrefs. These checks will be independent of the actual backend.\n\nIntroduce a new function `refs_fsck_ref()` that will be used for this\npurpose. At the current point in time it's still empty, but it will get\npopulated in a subsequent commit.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n refs.c               | 7 +++++++\n refs.h               | 8 ++++++++\n refs/files-backend.c | 2 ++\n 3 files changed, 17 insertions(+)\n\ndiff --git a/refs.c b/refs.c\nindex 739bf9fefc..4fc1317cb3 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -320,6 +320,13 @@ int check_refname_format(const char *refname, int flags)\n \treturn check_or_sanitize_refname(refname, flags, NULL);\n }\n \n+int refs_fsck_ref(struct ref_store *refs UNUSED, struct fsck_options *o UNUSED,\n+\t\t  struct fsck_ref_report *report UNUSED,\n+\t\t  const char *refname UNUSED, const struct object_id *oid UNUSED)\n+{\n+\treturn 0;\n+}\n+\n int refs_fsck_symref(struct ref_store *refs UNUSED, struct fsck_options *o,\n \t\t     struct fsck_ref_report *report,\n \t\t     const char *refname UNUSED, const char *target)\ndiff --git a/refs.h b/refs.h\nindex d91fcb2d2f..61c56cca36 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -655,6 +655,14 @@ int check_refname_format(const char *refname, int flags);\n \n struct fsck_ref_report;\n \n+/*\n+ * Perform generic checks for a specific symref target. This function is\n+ * expected to be called by the ref backends for every symbolic ref.\n+ */\n+int refs_fsck_ref(struct ref_store *refs, struct fsck_options *o,\n+\t\t  struct fsck_ref_report *report,\n+\t\t  const char *refname, const struct object_id *oid);\n+\n /*\n  * Perform generic checks for a specific symref target. This function is\n  * expected to be called by the ref backends for every symbolic ref.\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 72c1db849e..e59794f5da 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -3833,6 +3833,8 @@ static int files_fsck_refs_content(struct ref_store *ref_store,\n \t\t\t\t\t      \"has trailing garbage: '%s'\", trailing);\n \t\t\tgoto cleanup;\n \t\t}\n+\n+\t\tret = refs_fsck_ref(ref_store, o, &report, target_name, &oid);\n \t} else {\n \t\tret = files_fsck_symref_target(ref_store, o, &report,\n \t\t\t\t\t       target_name, &referent, 0);\n\n-- \n2.52.0.542.g9473a8513b.dirty\n\n"},{"id":"533356","messageId":"20260109-pks-refs-verify-fixes-v1-11-3587dba18294@pks.im","threadId":"64762","inReplyTo":"20260109-pks-refs-verify-fixes-v1-0-3587dba18294@pks.im","subject":"[PATCH 11/17] refs/reftable: adapt includes to become consistent","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-09T12:39:40Z","receivedAt":"2026-01-09T12:40:05Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Adapt the includes to be sorted and to use include paths that are\nrelative to the \"refs/\" directory.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n refs/reftable-backend.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/refs/reftable-backend.c b/refs/reftable-backend.c\nindex 4319a4eacb..d61790cf65 100644\n--- a/refs/reftable-backend.c\n+++ b/refs/reftable-backend.c\n@@ -10,9 +10,10 @@\n #include \"../gettext.h\"\n #include \"../hash.h\"\n #include \"../hex.h\"\n-#include \"../iterator.h\"\n #include \"../ident.h\"\n+#include \"../iterator.h\"\n #include \"../object.h\"\n+#include \"../parse.h\"\n #include \"../path.h\"\n #include \"../refs.h\"\n #include \"../reftable/reftable-basics.h\"\n@@ -26,7 +27,6 @@\n #include \"../strmap.h\"\n #include \"../trace2.h\"\n #include \"../write-or-die.h\"\n-#include \"parse.h\"\n #include \"refs-internal.h\"\n \n /*\n\n-- \n2.52.0.542.g9473a8513b.dirty\n\n"},{"id":"533357","messageId":"20260109-pks-refs-verify-fixes-v1-12-3587dba18294@pks.im","threadId":"64762","inReplyTo":"20260109-pks-refs-verify-fixes-v1-0-3587dba18294@pks.im","subject":"[PATCH 12/17] refs/reftable: extract function to retrieve backend for worktree","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-09T12:39:41Z","receivedAt":"2026-01-09T12:40:08Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Pull out the logic to retrieve a backend for a given worktree. This\nfunction will be used in a subsequent commit.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n refs/reftable-backend.c | 70 ++++++++++++++++++++++++++++++-------------------\n 1 file changed, 43 insertions(+), 27 deletions(-)\n\ndiff --git a/refs/reftable-backend.c b/refs/reftable-backend.c\nindex d61790cf65..dda961a32b 100644\n--- a/refs/reftable-backend.c\n+++ b/refs/reftable-backend.c\n@@ -172,6 +172,37 @@ static struct reftable_ref_store *reftable_be_downcast(struct ref_store *ref_sto\n \treturn refs;\n }\n \n+static int backend_for_worktree(struct reftable_backend **out,\n+\t\t\t\tstruct reftable_ref_store *store,\n+\t\t\t\tconst char *worktree_name)\n+{\n+\tstruct strbuf worktree_dir = STRBUF_INIT;\n+\tint ret;\n+\n+\t*out = strmap_get(&store->worktree_backends, worktree_name);\n+\tif (*out) {\n+\t\tret = 0;\n+\t\tgoto out;\n+\t}\n+\n+\tstrbuf_addf(&worktree_dir, \"%s/worktrees/%s/reftable\",\n+\t\t    store->base.repo->commondir, worktree_name);\n+\n+\tCALLOC_ARRAY(*out, 1);\n+\tstore->err = ret = reftable_backend_init(*out, worktree_dir.buf,\n+\t\t\t\t\t\t &store->write_options);\n+\tif (ret < 0) {\n+\t\tfree(*out);\n+\t\tgoto out;\n+\t}\n+\n+\tstrmap_put(&store->worktree_backends, worktree_name, *out);\n+\n+out:\n+\tstrbuf_release(&worktree_dir);\n+\treturn ret;\n+}\n+\n /*\n  * Some refs are global to the repository (refs/heads/{*}), while others are\n  * local to the worktree (eg. HEAD, refs/bisect/{*}). We solve this by having\n@@ -191,19 +222,19 @@ static int backend_for(struct reftable_backend **out,\n \t\t       const char **rewritten_ref,\n \t\t       int reload)\n {\n-\tstruct reftable_backend *be;\n \tconst char *wtname;\n \tint wtname_len;\n+\tint ret;\n \n \tif (!refname) {\n-\t\tbe = &store->main_backend;\n+\t\t*out = &store->main_backend;\n+\t\tret = 0;\n \t\tgoto out;\n \t}\n \n \tswitch (parse_worktree_ref(refname, &wtname, &wtname_len, rewritten_ref)) {\n \tcase REF_WORKTREE_OTHER: {\n \t\tstatic struct strbuf wtname_buf = STRBUF_INIT;\n-\t\tstruct strbuf wt_dir = STRBUF_INIT;\n \n \t\t/*\n \t\t * We're using a static buffer here so that we don't need to\n@@ -223,20 +254,8 @@ static int backend_for(struct reftable_backend **out,\n \t\t * already and error out when trying to write a reference via\n \t\t * both stacks.\n \t\t */\n-\t\tbe = strmap_get(&store->worktree_backends, wtname_buf.buf);\n-\t\tif (!be) {\n-\t\t\tstrbuf_addf(&wt_dir, \"%s/worktrees/%s/reftable\",\n-\t\t\t\t    store->base.repo->commondir, wtname_buf.buf);\n+\t\tret = backend_for_worktree(out, store, wtname_buf.buf);\n \n-\t\t\tCALLOC_ARRAY(be, 1);\n-\t\t\tstore->err = reftable_backend_init(be, wt_dir.buf,\n-\t\t\t\t\t\t\t   &store->write_options);\n-\t\t\tassert(store->err != REFTABLE_API_ERROR);\n-\n-\t\t\tstrmap_put(&store->worktree_backends, wtname_buf.buf, be);\n-\t\t}\n-\n-\t\tstrbuf_release(&wt_dir);\n \t\tgoto out;\n \t}\n \tcase REF_WORKTREE_CURRENT:\n@@ -245,27 +264,24 @@ static int backend_for(struct reftable_backend **out,\n \t\t * main worktree. We thus return the main stack in that case.\n \t\t */\n \t\tif (!store->worktree_backend.stack)\n-\t\t\tbe = &store->main_backend;\n+\t\t\t*out = &store->main_backend;\n \t\telse\n-\t\t\tbe = &store->worktree_backend;\n+\t\t\t*out = &store->worktree_backend;\n+\t\tret = 0;\n \t\tgoto out;\n \tcase REF_WORKTREE_MAIN:\n \tcase REF_WORKTREE_SHARED:\n-\t\tbe = &store->main_backend;\n+\t\t*out = &store->main_backend;\n+\t\tret = 0;\n \t\tgoto out;\n \tdefault:\n \t\tBUG(\"unhandled worktree reference type\");\n \t}\n \n out:\n-\tif (reload) {\n-\t\tint ret = reftable_stack_reload(be->stack);\n-\t\tif (ret)\n-\t\t\treturn ret;\n-\t}\n-\t*out = be;\n-\n-\treturn 0;\n+\tif (reload && !ret)\n+\t\tret = reftable_stack_reload((*out)->stack);\n+\treturn ret;\n }\n \n static int should_write_log(struct reftable_ref_store *refs, const char *refname)\n\n-- \n2.52.0.542.g9473a8513b.dirty\n\n"},{"id":"533358","messageId":"20260109-pks-refs-verify-fixes-v1-13-3587dba18294@pks.im","threadId":"64762","inReplyTo":"20260109-pks-refs-verify-fixes-v1-0-3587dba18294@pks.im","subject":"[PATCH 13/17] refs/reftable: fix consistency checks with worktrees","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-09T12:39:42Z","receivedAt":"2026-01-09T12:40:10Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The ref consistency checks are driven via `cmd_refs_verify()`. That\nfunction loops through all worktrees (including the main worktree) and\nthen checks the ref store for each of them individually. It follows that\nthe backend is expected to only verify refs that belong to the specified\nworktree.\n\nWhile the \"files\" backend handles this correctly, the \"reftable\" backend\ndoesn't. In fact, it completely ignores the passed worktree and instead\nverifies refs of _all_ worktrees. The consequence is that we'll end up\nevery ref store N times, where N is the number of worktrees.\n\nOr rather, that would be the case if we actually iterated through the\nworktree reftable stacks correctly. But we use `strmap_for_each_entry()`\nto iterate through the stacks, but the map is in fact not even properly\npopulated. So instead of checking stacks N^2 times, we actually only end\nup checking the reftable stack of the main worktree.\n\nFix this bug by only verifying the stack of the passed-in worktree and\nconstructing the backends via `backend_for_worktree()`.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n refs/reftable-backend.c  | 29 ++++++++++++++---------------\n t/t0614-reftable-fsck.sh | 32 ++++++++++++++++++++++++++++++++\n 2 files changed, 46 insertions(+), 15 deletions(-)\n\ndiff --git a/refs/reftable-backend.c b/refs/reftable-backend.c\nindex dda961a32b..6361b27015 100644\n--- a/refs/reftable-backend.c\n+++ b/refs/reftable-backend.c\n@@ -26,6 +26,7 @@\n #include \"../setup.h\"\n #include \"../strmap.h\"\n #include \"../trace2.h\"\n+#include \"../worktree.h\"\n #include \"../write-or-die.h\"\n #include \"refs-internal.h\"\n \n@@ -2762,25 +2763,23 @@ static int reftable_fsck_error_handler(struct reftable_fsck_info *info,\n }\n \n static int reftable_be_fsck(struct ref_store *ref_store, struct fsck_options *o,\n-\t\t\t    struct worktree *wt UNUSED)\n+\t\t\t    struct worktree *wt)\n {\n-\tstruct reftable_ref_store *refs;\n-\tstruct strmap_entry *entry;\n-\tstruct hashmap_iter iter;\n-\tint ret = 0;\n-\n-\trefs = reftable_be_downcast(ref_store, REF_STORE_READ, \"fsck\");\n-\n-\tret |= reftable_fsck_check(refs->main_backend.stack, reftable_fsck_error_handler,\n-\t\t\t\t   reftable_fsck_verbose_handler, o);\n+\tstruct reftable_ref_store *refs =\n+\t\treftable_be_downcast(ref_store, REF_STORE_READ, \"fsck\");\n+\tstruct reftable_backend *backend;\n \n-\tstrmap_for_each_entry(&refs->worktree_backends, &iter, entry) {\n-\t\tstruct reftable_backend *b = (struct reftable_backend *)entry->value;\n-\t\tret |= reftable_fsck_check(b->stack, reftable_fsck_error_handler,\n-\t\t\t\t\t   reftable_fsck_verbose_handler, o);\n+\tif (is_main_worktree(wt)) {\n+\t\tbackend = &refs->main_backend;\n+\t} else {\n+\t\tint ret = backend_for_worktree(&backend, refs, wt->id);\n+\t\tif (ret < 0)\n+\t\t\treturn error(_(\"reftable stack for worktree '%s' is broken\"),\n+\t\t\t\t     wt->id);\n \t}\n \n-\treturn ret;\n+\treturn reftable_fsck_check(backend->stack, reftable_fsck_error_handler,\n+\t\t\t\t   reftable_fsck_verbose_handler, o);\n }\n \n struct ref_storage_be refs_be_reftable = {\ndiff --git a/t/t0614-reftable-fsck.sh b/t/t0614-reftable-fsck.sh\nindex 677eb9143c..4757eb5931 100755\n--- a/t/t0614-reftable-fsck.sh\n+++ b/t/t0614-reftable-fsck.sh\n@@ -55,4 +55,36 @@ for TABLE_NAME in \"foo-bar-e4d12d59.ref\" \\\n \t'\n done\n \n+test_expect_success 'worktree stacks can be verified' '\n+\ttest_when_finished \"rm -rf repo worktree\" &&\n+\tgit init repo &&\n+\ttest_commit -C repo initial &&\n+\tgit -C repo worktree add ../worktree &&\n+\n+\tgit -C worktree refs verify 2>err &&\n+\ttest_must_be_empty err &&\n+\n+\tREFTABLE_DIR=$(git -C worktree rev-parse --git-dir)/reftable &&\n+\tEXISTING_TABLE=$(head -n1 \"$REFTABLE_DIR/tables.list\") &&\n+\tmv \"$REFTABLE_DIR/$EXISTING_TABLE\" \"$REFTABLE_DIR/broken.ref\" &&\n+\n+\tfor d in repo worktree\n+\tdo\n+\t\techo \"broken.ref\" >\"$REFTABLE_DIR/tables.list\" &&\n+\t\tgit -C \"$d\" refs verify 2>err &&\n+\t\tcat >expect <<-EOF &&\n+\t\twarning: broken.ref: badReftableTableName: invalid reftable table name\n+\t\tEOF\n+\t\ttest_cmp expect err &&\n+\n+\t\techo garbage >\"$REFTABLE_DIR/tables.list\" &&\n+\t\ttest_must_fail git -C \"$d\" refs verify 2>err &&\n+\t\tcat >expect <<-EOF &&\n+\t\terror: reftable stack for worktree ${SQ}worktree${SQ} is broken\n+\t\tEOF\n+\t\ttest_cmp expect err || return 1\n+\n+\tdone\n+'\n+\n test_done\n\n-- \n2.52.0.542.g9473a8513b.dirty\n\n"},{"id":"533359","messageId":"20260109-pks-refs-verify-fixes-v1-14-3587dba18294@pks.im","threadId":"64762","inReplyTo":"20260109-pks-refs-verify-fixes-v1-0-3587dba18294@pks.im","subject":"[PATCH 14/17] refs/reftable: introduce generic checks for refs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-09T12:39:43Z","receivedAt":"2026-01-09T12:40:13Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In a preceding commit we have extracted generic checks for both direct\nand symbolic refs that apply for all backends. Wire up those checks for\nthe \"reftable\" backend.\n\nNote that this is done by iterating through all refs manually with the\nlow-level reftable ref iterator. We explicitly don't want to use the\nhigher-level iterator that is exposed to users of the reftable backend\nas that iterator may swallow for example broken refs.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n refs/reftable-backend.c  | 82 ++++++++++++++++++++++++++++++++++++++++++++----\n t/t0614-reftable-fsck.sh | 12 +++++++\n 2 files changed, 88 insertions(+), 6 deletions(-)\n\ndiff --git a/refs/reftable-backend.c b/refs/reftable-backend.c\nindex 6361b27015..fe74af73af 100644\n--- a/refs/reftable-backend.c\n+++ b/refs/reftable-backend.c\n@@ -2767,19 +2767,89 @@ static int reftable_be_fsck(struct ref_store *ref_store, struct fsck_options *o,\n {\n \tstruct reftable_ref_store *refs =\n \t\treftable_be_downcast(ref_store, REF_STORE_READ, \"fsck\");\n+\tstruct reftable_ref_iterator *iter = NULL;\n+\tstruct reftable_ref_record ref = { 0 };\n+\tstruct fsck_ref_report report = { 0 };\n+\tstruct strbuf refname = STRBUF_INIT;\n \tstruct reftable_backend *backend;\n+\tint ret, errors = 0;\n \n \tif (is_main_worktree(wt)) {\n \t\tbackend = &refs->main_backend;\n \t} else {\n-\t\tint ret = backend_for_worktree(&backend, refs, wt->id);\n-\t\tif (ret < 0)\n-\t\t\treturn error(_(\"reftable stack for worktree '%s' is broken\"),\n-\t\t\t\t     wt->id);\n+\t\tret = backend_for_worktree(&backend, refs, wt->id);\n+\t\tif (ret < 0) {\n+\t\t\tret = error(_(\"reftable stack for worktree '%s' is broken\"),\n+\t\t\t\t    wt->id);\n+\t\t\tgoto out;\n+\t\t}\n+\t}\n+\n+\terrors |= reftable_fsck_check(backend->stack, reftable_fsck_error_handler,\n+\t\t\t\t      reftable_fsck_verbose_handler, o);\n+\n+\titer = ref_iterator_for_stack(refs, backend->stack, \"\", NULL, 0);\n+\tif (!iter) {\n+\t\tret = error(_(\"could not create iterator for worktree '%s'\"), wt->id);\n+\t\tgoto out;\n+\t}\n+\n+\twhile (1) {\n+\t\tret = reftable_iterator_next_ref(&iter->iter, &ref);\n+\t\tif (ret > 0)\n+\t\t\tbreak;\n+\t\tif (ret < 0) {\n+\t\t\tret = error(_(\"could not read record for worktree '%s'\"), wt->id);\n+\t\t\tgoto out;\n+\t\t}\n+\n+\t\tstrbuf_reset(&refname);\n+\t\tif (!is_main_worktree(wt))\n+\t\t\tstrbuf_addf(&refname, \"worktrees/%s/\", wt->id);\n+\t\tstrbuf_addstr(&refname, ref.refname);\n+\t\treport.path = refname.buf;\n+\n+\t\tswitch (ref.value_type) {\n+\t\tcase REFTABLE_REF_VAL1:\n+\t\tcase REFTABLE_REF_VAL2: {\n+\t\t\tstruct object_id oid;\n+\t\t\tunsigned hash_id;\n+\n+\t\t\tswitch (reftable_stack_hash_id(backend->stack)) {\n+\t\t\tcase REFTABLE_HASH_SHA1:\n+\t\t\t\thash_id = GIT_HASH_SHA1;\n+\t\t\t\tbreak;\n+\t\t\tcase REFTABLE_HASH_SHA256:\n+\t\t\t\thash_id = GIT_HASH_SHA256;\n+\t\t\t\tbreak;\n+\t\t\tdefault:\n+\t\t\t\tBUG(\"unhandled hash ID %d\",\n+\t\t\t\t    reftable_stack_hash_id(backend->stack));\n+\t\t\t}\n+\n+\t\t\toidread(&oid, reftable_ref_record_val1(&ref),\n+\t\t\t\t&hash_algos[hash_id]);\n+\n+\t\t\terrors |= refs_fsck_ref(ref_store, o, &report, ref.refname, &oid);\n+\t\t\tbreak;\n+\t\t}\n+\t\tcase REFTABLE_REF_SYMREF:\n+\t\t\terrors |= refs_fsck_symref(ref_store, o, &report, ref.refname,\n+\t\t\t\t\t\t   ref.value.symref);\n+\t\t\tbreak;\n+\t\tdefault:\n+\t\t\tBUG(\"unhandled reference value type %d\", ref.value_type);\n+\t\t}\n \t}\n \n-\treturn reftable_fsck_check(backend->stack, reftable_fsck_error_handler,\n-\t\t\t\t   reftable_fsck_verbose_handler, o);\n+\tret = errors ? -1 : 0;\n+\n+out:\n+\tif (iter)\n+\t\tref_iterator_free(&iter->base);\n+\treftable_ref_record_release(&ref);\n+\tstrbuf_release(&refname);\n+\treturn ret;\n }\n \n struct ref_storage_be refs_be_reftable = {\ndiff --git a/t/t0614-reftable-fsck.sh b/t/t0614-reftable-fsck.sh\nindex 4757eb5931..d24b87f961 100755\n--- a/t/t0614-reftable-fsck.sh\n+++ b/t/t0614-reftable-fsck.sh\n@@ -87,4 +87,16 @@ test_expect_success 'worktree stacks can be verified' '\n \tdone\n '\n \n+test_expect_success 'invalid symref gets reported' '\n+\ttest_when_finished \"rm -rf repo\" &&\n+\tgit init repo &&\n+\ttest_commit -C repo initial &&\n+\tgit -C repo symbolic-ref refs/heads/symref garbage &&\n+\ttest_must_fail git -C repo refs verify 2>err &&\n+\tcat >expect <<-EOF &&\n+\terror: refs/heads/symref: badReferentName: points to invalid refname ${SQ}garbage${SQ}\n+\tEOF\n+\ttest_cmp expect err\n+'\n+\n test_done\n\n-- \n2.52.0.542.g9473a8513b.dirty\n\n"},{"id":"533360","messageId":"20260109-pks-refs-verify-fixes-v1-15-3587dba18294@pks.im","threadId":"64762","inReplyTo":"20260109-pks-refs-verify-fixes-v1-0-3587dba18294@pks.im","subject":"[PATCH 15/17] builtin/fsck: move generic object ID checks into `refs_fsck()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-09T12:39:44Z","receivedAt":"2026-01-09T12:40:15Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"While most of the logic that verifies the consistency of refs is\ndriven by `refs_fsck()`, we still have a small handful of checks in\n`fsck_head_link()`. These checks don't use the git-fsck(1) reporting\ninfrastructure, and as such it's impossible to for example disable\nsome of those checks.\n\nOne such check detects refs that point to the all-zeroes object ID.\nExtract this check into the generic `refs_fsck_ref()` function that is\nused by both the \"files\" and \"reftable\" backends.\n\nNote that this will cause us to not return an error code from\n`fsck_head_link()` anymore in case this error was detected. This is fine\nthough: the only caller of this function does not check the error code\nanyway. To demonstrate this, adapt the function to drop its return value\naltogether. The function will be removed in a subsequent commit anyway.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Documentation/fsck-msgids.adoc |  3 +++\n builtin/fsck.c                 | 41 +++++++++++++++--------------------------\n fsck.h                         |  1 +\n refs.c                         | 11 ++++++++---\n t/t1450-fsck.sh                |  6 +++---\n 5 files changed, 30 insertions(+), 32 deletions(-)\n\ndiff --git a/Documentation/fsck-msgids.adoc b/Documentation/fsck-msgids.adoc\nindex acac9683af..76609321f6 100644\n--- a/Documentation/fsck-msgids.adoc\n+++ b/Documentation/fsck-msgids.adoc\n@@ -41,6 +41,9 @@\n `badRefName`::\n \t(ERROR) A ref has an invalid format.\n \n+`badRefOid`::\n+\t(ERROR) A ref points to an invalid object ID.\n+\n `badReferentName`::\n \t(ERROR) The referent name of a symref is invalid.\n \ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex 4979bc795e..4dd4d74d1e 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -564,9 +564,9 @@ static int fsck_handle_ref(const struct reference *ref, void *cb_data UNUSED)\n \treturn 0;\n }\n \n-static int fsck_head_link(const char *head_ref_name,\n-\t\t\t  const char **head_points_at,\n-\t\t\t  struct object_id *head_oid);\n+static void fsck_head_link(const char *head_ref_name,\n+\t\t\t   const char **head_points_at,\n+\t\t\t   struct object_id *head_oid);\n \n static void get_default_heads(void)\n {\n@@ -713,12 +713,10 @@ static void fsck_source(struct odb_source *source)\n \tstop_progress(&progress);\n }\n \n-static int fsck_head_link(const char *head_ref_name,\n-\t\t\t  const char **head_points_at,\n-\t\t\t  struct object_id *head_oid)\n+static void fsck_head_link(const char *head_ref_name,\n+\t\t\t   const char **head_points_at,\n+\t\t\t   struct object_id *head_oid)\n {\n-\tint null_is_error = 0;\n-\n \tif (verbose)\n \t\tfprintf_ln(stderr, _(\"Checking %s link\"), head_ref_name);\n \n@@ -727,27 +725,18 @@ static int fsck_head_link(const char *head_ref_name,\n \t\t\t\t\t\t  NULL);\n \tif (!*head_points_at) {\n \t\terrors_found |= ERROR_REFS;\n-\t\treturn error(_(\"invalid %s\"), head_ref_name);\n+\t\terror(_(\"invalid %s\"), head_ref_name);\n+\t\treturn;\n \t}\n-\tif (!strcmp(*head_points_at, head_ref_name))\n-\t\t/* detached HEAD */\n-\t\tnull_is_error = 1;\n-\telse if (!starts_with(*head_points_at, \"refs/heads/\")) {\n+\tif (strcmp(*head_points_at, head_ref_name) &&\n+\t    !starts_with(*head_points_at, \"refs/heads/\")) {\n \t\terrors_found |= ERROR_REFS;\n-\t\treturn error(_(\"%s points to something strange (%s)\"),\n-\t\t\t     head_ref_name, *head_points_at);\n-\t}\n-\tif (is_null_oid(head_oid)) {\n-\t\tif (null_is_error) {\n-\t\t\terrors_found |= ERROR_REFS;\n-\t\t\treturn error(_(\"%s: detached HEAD points at nothing\"),\n-\t\t\t\t     head_ref_name);\n-\t\t}\n-\t\tfprintf_ln(stderr,\n-\t\t\t   _(\"notice: %s points to an unborn branch (%s)\"),\n-\t\t\t   head_ref_name, *head_points_at + 11);\n+\t\terror(_(\"%s points to something strange (%s)\"),\n+\t\t      head_ref_name, *head_points_at);\n+\t\treturn;\n \t}\n-\treturn 0;\n+\n+\treturn;\n }\n \n static int fsck_cache_tree(struct cache_tree *it, const char *index_path)\ndiff --git a/fsck.h b/fsck.h\nindex bfe0d9c6d2..1f472b7daa 100644\n--- a/fsck.h\n+++ b/fsck.h\n@@ -39,6 +39,7 @@ enum fsck_msg_type {\n \tFUNC(BAD_REF_CONTENT, ERROR) \\\n \tFUNC(BAD_REF_FILETYPE, ERROR) \\\n \tFUNC(BAD_REF_NAME, ERROR) \\\n+\tFUNC(BAD_REF_OID, ERROR) \\\n \tFUNC(BAD_TIMEZONE, ERROR) \\\n \tFUNC(BAD_TREE, ERROR) \\\n \tFUNC(BAD_TREE_SHA1, ERROR) \\\ndiff --git a/refs.c b/refs.c\nindex 4fc1317cb3..c3528862c6 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -320,10 +320,15 @@ int check_refname_format(const char *refname, int flags)\n \treturn check_or_sanitize_refname(refname, flags, NULL);\n }\n \n-int refs_fsck_ref(struct ref_store *refs UNUSED, struct fsck_options *o UNUSED,\n-\t\t  struct fsck_ref_report *report UNUSED,\n-\t\t  const char *refname UNUSED, const struct object_id *oid UNUSED)\n+int refs_fsck_ref(struct ref_store *refs UNUSED, struct fsck_options *o,\n+\t\t  struct fsck_ref_report *report,\n+\t\t  const char *refname UNUSED, const struct object_id *oid)\n {\n+\tif (is_null_oid(oid))\n+\t\treturn fsck_report_ref(o, report, FSCK_MSG_BAD_REF_OID,\n+\t\t\t\t       \"points to invalid object ID '%s'\",\n+\t\t\t\t       oid_to_hex(oid));\n+\n \treturn 0;\n }\n \ndiff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh\nindex c4b651c2dc..900c1b2eb2 100755\n--- a/t/t1450-fsck.sh\n+++ b/t/t1450-fsck.sh\n@@ -105,7 +105,7 @@ test_expect_success REFFILES 'HEAD link pointing at a funny object' '\n \techo $ZERO_OID >.git/HEAD &&\n \t# avoid corrupt/broken HEAD from interfering with repo discovery\n \ttest_must_fail env GIT_DIR=.git git fsck 2>out &&\n-\ttest_grep \"detached HEAD points\" out\n+\ttest_grep \"HEAD: badRefOid: points to invalid object ID ${SQ}$ZERO_OID${SQ}\" out\n '\n \n test_expect_success 'HEAD link pointing at a funny place' '\n@@ -123,7 +123,7 @@ test_expect_success REFFILES 'HEAD link pointing at a funny object (from differe\n \techo $ZERO_OID >.git/HEAD &&\n \t# avoid corrupt/broken HEAD from interfering with repo discovery\n \ttest_must_fail git -C wt fsck 2>out &&\n-\ttest_grep \"main-worktree/HEAD: detached HEAD points\" out\n+\ttest_grep \"HEAD: badRefOid: points to invalid object ID ${SQ}$ZERO_OID${SQ}\" out\n '\n \n test_expect_success REFFILES 'other worktree HEAD link pointing at a funny object' '\n@@ -131,7 +131,7 @@ test_expect_success REFFILES 'other worktree HEAD link pointing at a funny objec\n \tgit worktree add other &&\n \techo $ZERO_OID >.git/worktrees/other/HEAD &&\n \ttest_must_fail git fsck 2>out &&\n-\ttest_grep \"worktrees/other/HEAD: detached HEAD points\" out\n+\ttest_grep \"worktrees/other/HEAD: badRefOid: points to invalid object ID ${SQ}$ZERO_OID${SQ}\" out\n '\n \n test_expect_success 'other worktree HEAD link pointing at missing object' '\n\n-- \n2.52.0.542.g9473a8513b.dirty\n\n"},{"id":"533361","messageId":"20260109-pks-refs-verify-fixes-v1-16-3587dba18294@pks.im","threadId":"64762","inReplyTo":"20260109-pks-refs-verify-fixes-v1-0-3587dba18294@pks.im","subject":"[PATCH 16/17] builtin/fsck: move generic HEAD check into `refs_fsck()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-09T12:39:45Z","receivedAt":"2026-01-09T12:40:19Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Move the check that detects \"HEAD\" refs that do not point at a branch\ninto `refs_fsck()`. This follows the same motivation as the preceding\ncommit.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Documentation/fsck-msgids.adoc |  3 +++\n builtin/fsck.c                 |  7 -------\n fsck.h                         |  1 +\n refs.c                         | 12 +++++++++++-\n t/t0602-reffiles-fsck.sh       |  8 ++++----\n t/t1450-fsck.sh                |  4 ++--\n 6 files changed, 21 insertions(+), 14 deletions(-)\n\ndiff --git a/Documentation/fsck-msgids.adoc b/Documentation/fsck-msgids.adoc\nindex 76609321f6..6a4db3a991 100644\n--- a/Documentation/fsck-msgids.adoc\n+++ b/Documentation/fsck-msgids.adoc\n@@ -13,6 +13,9 @@\n `badGpgsig`::\n \t(ERROR) A tag contains a bad (truncated) signature (e.g., `gpgsig`) header.\n \n+`badHeadTarget`::\n+\t(ERROR) The `HEAD` ref is a symref that does not refer to a branch.\n+\n `badHeaderContinuation`::\n \t(ERROR) A continuation header (such as for `gpgsig`) is unexpectedly truncated.\n \ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex 4dd4d74d1e..5dda441f45 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -728,13 +728,6 @@ static void fsck_head_link(const char *head_ref_name,\n \t\terror(_(\"invalid %s\"), head_ref_name);\n \t\treturn;\n \t}\n-\tif (strcmp(*head_points_at, head_ref_name) &&\n-\t    !starts_with(*head_points_at, \"refs/heads/\")) {\n-\t\terrors_found |= ERROR_REFS;\n-\t\terror(_(\"%s points to something strange (%s)\"),\n-\t\t      head_ref_name, *head_points_at);\n-\t\treturn;\n-\t}\n \n \treturn;\n }\ndiff --git a/fsck.h b/fsck.h\nindex 1f472b7daa..65ecbb7fe1 100644\n--- a/fsck.h\n+++ b/fsck.h\n@@ -30,6 +30,7 @@ enum fsck_msg_type {\n \tFUNC(BAD_DATE_OVERFLOW, ERROR) \\\n \tFUNC(BAD_EMAIL, ERROR) \\\n \tFUNC(BAD_GPGSIG, ERROR) \\\n+\tFUNC(BAD_HEAD_TARGET, ERROR) \\\n \tFUNC(BAD_NAME, ERROR) \\\n \tFUNC(BAD_OBJECT_SHA1, ERROR) \\\n \tFUNC(BAD_PACKED_REF_ENTRY, ERROR) \\\ndiff --git a/refs.c b/refs.c\nindex c3528862c6..a772d371cd 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -334,8 +334,18 @@ int refs_fsck_ref(struct ref_store *refs UNUSED, struct fsck_options *o,\n \n int refs_fsck_symref(struct ref_store *refs UNUSED, struct fsck_options *o,\n \t\t     struct fsck_ref_report *report,\n-\t\t     const char *refname UNUSED, const char *target)\n+\t\t     const char *refname, const char *target)\n {\n+\tconst char *stripped_refname;\n+\n+\tparse_worktree_ref(refname, NULL, NULL, &stripped_refname);\n+\n+\tif (!strcmp(stripped_refname, \"HEAD\") &&\n+\t    !starts_with(target, \"refs/heads/\") &&\n+\t    fsck_report_ref(o, report, FSCK_MSG_BAD_HEAD_TARGET,\n+\t\t\t    \"HEAD points to non-branch '%s'\", target))\n+\t\treturn -1;\n+\n \tif (is_root_ref(target))\n \t\treturn 0;\n \ndiff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh\nindex 479f3d528e..3c1f553b81 100755\n--- a/t/t0602-reffiles-fsck.sh\n+++ b/t/t0602-reffiles-fsck.sh\n@@ -910,10 +910,10 @@ test_expect_success 'complains about broken root ref' '\n \tgit init repo &&\n \t(\n \t\tcd repo &&\n-\t\techo \"ref: refs/../HEAD\" >.git/HEAD &&\n+\t\techo \"ref: refs/heads/../HEAD\" >.git/HEAD &&\n \t\ttest_must_fail git refs verify 2>err &&\n \t\tcat >expect <<-EOF &&\n-\t\terror: HEAD: badReferentName: points to invalid refname ${SQ}refs/../HEAD${SQ}\n+\t\terror: HEAD: badReferentName: points to invalid refname ${SQ}refs/heads/../HEAD${SQ}\n \t\tEOF\n \t\ttest_cmp expect err\n \t)\n@@ -926,10 +926,10 @@ test_expect_success 'complains about broken root ref in worktree' '\n \t\tcd repo &&\n \t\ttest_commit initial &&\n \t\tgit worktree add ../worktree &&\n-\t\techo \"ref: refs/../HEAD\" >.git/worktrees/worktree/HEAD &&\n+\t\techo \"ref: refs/heads/../HEAD\" >.git/worktrees/worktree/HEAD &&\n \t\ttest_must_fail git refs verify 2>err &&\n \t\tcat >expect <<-EOF &&\n-\t\terror: worktrees/worktree/HEAD: badReferentName: points to invalid refname ${SQ}refs/../HEAD${SQ}\n+\t\terror: worktrees/worktree/HEAD: badReferentName: points to invalid refname ${SQ}refs/heads/../HEAD${SQ}\n \t\tEOF\n \t\ttest_cmp expect err\n \t)\ndiff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh\nindex 900c1b2eb2..3fae05f9d9 100755\n--- a/t/t1450-fsck.sh\n+++ b/t/t1450-fsck.sh\n@@ -113,7 +113,7 @@ test_expect_success 'HEAD link pointing at a funny place' '\n \ttest-tool ref-store main create-symref HEAD refs/funny/place &&\n \t# avoid corrupt/broken HEAD from interfering with repo discovery\n \ttest_must_fail env GIT_DIR=.git git fsck 2>out &&\n-\ttest_grep \"HEAD points to something strange\" out\n+\ttest_grep \"HEAD: badHeadTarget: HEAD points to non-branch ${SQ}refs/funny/place${SQ}\" out\n '\n \n test_expect_success REFFILES 'HEAD link pointing at a funny object (from different wt)' '\n@@ -148,7 +148,7 @@ test_expect_success 'other worktree HEAD link pointing at a funny place' '\n \tgit worktree add other &&\n \tgit -C other symbolic-ref HEAD refs/funny/place &&\n \ttest_must_fail git fsck 2>out &&\n-\ttest_grep \"worktrees/other/HEAD points to something strange\" out\n+\ttest_grep \"worktrees/other/HEAD: badHeadTarget: HEAD points to non-branch ${SQ}refs/funny/place${SQ}\" out\n '\n \n test_expect_success 'commit with multiple signatures is okay' '\n\n-- \n2.52.0.542.g9473a8513b.dirty\n\n"},{"id":"533362","messageId":"20260109-pks-refs-verify-fixes-v1-17-3587dba18294@pks.im","threadId":"64762","inReplyTo":"20260109-pks-refs-verify-fixes-v1-0-3587dba18294@pks.im","subject":"[PATCH 17/17] builtin/fsck: drop `fsck_head_link()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-09T12:39:46Z","receivedAt":"2026-01-09T12:40:21Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The function `fsck_head_link()` was historically used to perform a\ncouple of consistency checks for refs. (Almost) all of these checks have\nnow been moved into the refs subsystem. There's only a single check\nremaining that verifies whether `refs_resolve_ref_unsafe()` returns a\n`NULL` pointer. This may happen in a couple of cases:\n\n  - When `refs_is_safe()` declares the ref to be unsafe. We already have\n    checks for this as we verify refnames with `check_refname_format()`.\n\n  - When the ref doesn't exist. A repository without \"HEAD\" is\n    completely broken though, and we would notice this error ahead of\n    time already.\n\n  - In case the caller passes `RESOLVE_REF_READING` and the ref is a\n    symref that doesn't resolve. We don't pass this flag though.\n\nAs such, this check doesn't cover anything anymore that isn't already\ncovered by `refs_fsck()`. Drop it, which also allows us to inline the\ncall to `refs_resolve_ref_unsafe()`.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n builtin/fsck.c | 28 ++++------------------------\n 1 file changed, 4 insertions(+), 24 deletions(-)\n\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex 5dda441f45..f104b7af0e 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -564,10 +564,6 @@ static int fsck_handle_ref(const struct reference *ref, void *cb_data UNUSED)\n \treturn 0;\n }\n \n-static void fsck_head_link(const char *head_ref_name,\n-\t\t\t   const char **head_points_at,\n-\t\t\t   struct object_id *head_oid);\n-\n static void get_default_heads(void)\n {\n \tstruct worktree **worktrees, **p;\n@@ -583,7 +579,10 @@ static void get_default_heads(void)\n \t\tstruct strbuf refname = STRBUF_INIT;\n \n \t\tstrbuf_worktree_ref(wt, &refname, \"HEAD\");\n-\t\tfsck_head_link(refname.buf, &head_points_at, &head_oid);\n+\n+\t\thead_points_at = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),\n+\t\t\t\t\t\t\t refname.buf, 0, &head_oid, NULL);\n+\n \t\tif (head_points_at && !is_null_oid(&head_oid)) {\n \t\t\tstruct reference ref = {\n \t\t\t\t.name = refname.buf,\n@@ -713,25 +712,6 @@ static void fsck_source(struct odb_source *source)\n \tstop_progress(&progress);\n }\n \n-static void fsck_head_link(const char *head_ref_name,\n-\t\t\t   const char **head_points_at,\n-\t\t\t   struct object_id *head_oid)\n-{\n-\tif (verbose)\n-\t\tfprintf_ln(stderr, _(\"Checking %s link\"), head_ref_name);\n-\n-\t*head_points_at = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),\n-\t\t\t\t\t\t  head_ref_name, 0, head_oid,\n-\t\t\t\t\t\t  NULL);\n-\tif (!*head_points_at) {\n-\t\terrors_found |= ERROR_REFS;\n-\t\terror(_(\"invalid %s\"), head_ref_name);\n-\t\treturn;\n-\t}\n-\n-\treturn;\n-}\n-\n static int fsck_cache_tree(struct cache_tree *it, const char *index_path)\n {\n \tint i;\n\n-- \n2.52.0.542.g9473a8513b.dirty\n\n"},{"id":"533480","messageId":"aWJF0NNDnuIUXbMo@ArchLinux","threadId":"64762","inReplyTo":"20260109-pks-refs-verify-fixes-v1-1-3587dba18294@pks.im","subject":"Re: [PATCH 01/17] refs/files: simplify iterating through root refs","fromName":"shejialuo","fromEmail":"shejialuo@gmail.com","sentAt":"2026-01-10T12:28:00Z","receivedAt":"2026-01-10T12:28:05Z","isPatch":true,"sender":{"key":"shejialuo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56911263?v=4"},"body":"On Fri, Jan 09, 2026 at 01:39:30PM +0100, Patrick Steinhardt wrote:\n> When iterating through root refs we first need to determine the\n> directory in which the refs live. This is done by retrieving the root of\n> the loose refs via `refs->loose->root->name`, and putting it through\n> `files_ref_path()` to derive the final path.\n> \n> This is somewhat redundant though: the root name of the loose files\n> cache is always going to be the empty string. As such, we always end up\n> passing that empty string to `files_ref_path()` as the ref hierarchy we\n> want to start. And this actually makes sense: `files_ref_path()` already\n> computes the location of the root directory, so of course we need to\n> pass the empty string for the ref hierarchy itself. So going via the\n> loose ref cache to figure out that the root of a ref hierarchy is empty\n> is only causing confusion.\n\nMake sense, in `refs/ref-cache.c` we would call the following to create\nthe root loose cache:\n\n    ret->root = create_dir_entry(ret, \"\", 0)\n\nIt would always be empty.\n\nThanks,\nJialuo\n"},{"id":"533481","messageId":"aWJHX2te19crFKF4@ArchLinux","threadId":"64762","inReplyTo":"20260109-pks-refs-verify-fixes-v1-6-3587dba18294@pks.im","subject":"Re: [PATCH 06/17] refs/files: improve error handling when verifying symrefs","fromName":"shejialuo","fromEmail":"shejialuo@gmail.com","sentAt":"2026-01-10T12:34:39Z","receivedAt":"2026-01-10T12:34:43Z","isPatch":true,"sender":{"key":"shejialuo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56911263?v=4"},"body":"On Fri, Jan 09, 2026 at 01:39:35PM +0100, Patrick Steinhardt wrote:\n> The error handling when verifying symbolic refs is a bit on the wild\n> side:\n> \n>   - `fsck_report_ref()` can be told to ignore specific errors. If an\n>     error has been ignored and a previous check raised an unignored\n>     error, then assigning `ret = fsck_report_ref()` will cause us to\n>     swallow the previous error.\n> \n\nMake sense, I think I haven't thought about this carefully when I wrote\nthe code. I totally ignored the case. Thanks for catching this.\n"},{"id":"533482","messageId":"aWJKYzcY3H_-xy1V@ArchLinux","threadId":"64762","inReplyTo":"20260109-pks-refs-verify-fixes-v1-7-3587dba18294@pks.im","subject":"Re: [PATCH 07/17] refs/files: perform consistency checks for root refs","fromName":"shejialuo","fromEmail":"shejialuo@gmail.com","sentAt":"2026-01-10T12:47:31Z","receivedAt":"2026-01-10T12:47:35Z","isPatch":true,"sender":{"key":"shejialuo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56911263?v=4"},"body":"On Fri, Jan 09, 2026 at 01:39:36PM +0100, Patrick Steinhardt wrote:\n>  static int files_fsck(struct ref_store *ref_store,\n>  \t\t      struct fsck_options *o,\n>  \t\t      struct worktree *wt)\n>  {\n>  \tstruct files_ref_store *refs =\n>  \t\tfiles_downcast(ref_store, REF_STORE_READ, \"fsck\");\n> +\tstruct files_fsck_root_ref_data data = {\n> +\t\t.refs = refs,\n> +\t\t.o = o,\n> +\t\t.wt = wt,\n> +\t\t.refname = STRBUF_INIT,\n> +\t\t.path = STRBUF_INIT,\n> +\t};\n>  \tint ret = 0;\n>  \n>  \tif (files_fsck_refs_dir(ref_store, o, wt) < 0)\n>  \t\tret = -1;\n> +\n> +\tif (for_each_root_ref(refs, files_fsck_root_ref, &data) < 0 ||\n> +\t    data.errors_found)\n\nI am wondering where we update this filed in `files_fsck_root_ref`. It\nseems that we never do this in this commit. I think we should delete\nthis filed in `files_fsck_root_ref_data` and add this field back when we\ndo need this to avoid confusion.\n\nThanks,\nJialuo\n"},{"id":"533483","messageId":"aWJNHgFnimXRHkb6@ArchLinux","threadId":"64762","inReplyTo":"20260109-pks-refs-verify-fixes-v1-9-3587dba18294@pks.im","subject":"Re: [PATCH 09/17] refs/files: extract generic symref target checks","fromName":"shejialuo","fromEmail":"shejialuo@gmail.com","sentAt":"2026-01-10T12:59:10Z","receivedAt":"2026-01-10T12:59:14Z","isPatch":true,"sender":{"key":"shejialuo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56911263?v=4"},"body":"On Fri, Jan 09, 2026 at 01:39:38PM +0100, Patrick Steinhardt wrote:\n> The consistency checks for the \"files\" backend contain a couple of\n> verifications for symrefs that verify generic properties of the target\n> reference. These properties need to hold for every backend, no matter\n> whether it's using the \"files\" or \"reftable\" backend.\n> \n> Reimplementing these checks for every single backend doesn't really make\n> sense. Extract it into a generic `refs_fsck_symref()` function that can\n> be used my other backends, as well. The \"reftable\" backend will be wired\n> up in a subsequent commit.\n> \n\ns/my/by\n\n> While at it, improve the consistency checks so that we don't complain\n> about refs pointing to a non-ref target in case the target refname\n> format does not verify. Otherwise it's very likely that we'll generate\n> both error messages, which feels somewhat redundant in this case.\n> \n\nMake sense, we should fail early in this case.\n\n> Note that the function has a couple of `UNUSED` parameters. These will\n> become referenced in a subsequent commit.\n> \n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  refs.c               | 21 ++++++++++++++++++++\n>  refs.h               | 10 ++++++++++\n>  refs/files-backend.c | 54 ++++++++++++++++++++--------------------------------\n>  3 files changed, 52 insertions(+), 33 deletions(-)\n> \n> diff --git a/refs.c b/refs.c\n> index e06e0cb072..739bf9fefc 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -320,6 +320,27 @@ int check_refname_format(const char *refname, int flags)\n>  \treturn check_or_sanitize_refname(refname, flags, NULL);\n>  }\n>  \n> +int refs_fsck_symref(struct ref_store *refs UNUSED, struct fsck_options *o,\n> +\t\t     struct fsck_ref_report *report,\n> +\t\t     const char *refname UNUSED, const char *target)\n> +{\n> +\tif (is_root_ref(target))\n> +\t\treturn 0;\n> +\n> +\tif (check_refname_format(target, 0) &&\n> +\t    fsck_report_ref(o, report, FSCK_MSG_BAD_REFERENT_NAME,\n> +\t\t\t    \"points to invalid refname '%s'\", target))\n> +\t\treturn -1;\n> +\n> +\tif (!starts_with(target, \"refs/\") &&\n> +\t    !starts_with(target, \"worktrees/\") &&\n> +\t    fsck_report_ref(o, report, FSCK_MSG_SYMREF_TARGET_IS_NOT_A_REF,\n> +\t\t\t    \"points to non-ref target '%s'\", target))\n> +\t\treturn -1;\n> +\n> +\treturn 0;\n> +}\n> +\n>  int refs_fsck(struct ref_store *refs, struct fsck_options *o,\n>  \t      struct worktree *wt)\n>  {\n> diff --git a/refs.h b/refs.h\n> index d9051bbb04..d91fcb2d2f 100644\n> --- a/refs.h\n> +++ b/refs.h\n> @@ -653,6 +653,16 @@ int refs_for_each_reflog(struct ref_store *refs, each_reflog_fn fn, void *cb_dat\n>   */\n>  int check_refname_format(const char *refname, int flags);\n>  \n> +struct fsck_ref_report;\n> +\n> +/*\n> + * Perform generic checks for a specific symref target. This function is\n> + * expected to be called by the ref backends for every symbolic ref.\n> + */\n> +int refs_fsck_symref(struct ref_store *refs, struct fsck_options *o,\n> +\t\t     struct fsck_ref_report *report,\n> +\t\t     const char *refname, const char *target);\n> +\n>  /*\n>   * Check the reference database for consistency. Return 0 if refs and\n>   * reflogs are consistent, and non-zero otherwise. The errors will be\n> diff --git a/refs/files-backend.c b/refs/files-backend.c\n> index 0ff047d0df..72c1db849e 100644\n> --- a/refs/files-backend.c\n> +++ b/refs/files-backend.c\n> @@ -3718,53 +3718,39 @@ typedef int (*files_fsck_refs_fn)(struct ref_store *ref_store,\n>  \t\t\t\t  const char *path,\n>  \t\t\t\t  int mode);\n>  \n> -static int files_fsck_symref_target(struct fsck_options *o,\n> +static int files_fsck_symref_target(struct ref_store *ref_store,\n> +\t\t\t\t    struct fsck_options *o,\n>  \t\t\t\t    struct fsck_ref_report *report,\n> +\t\t\t\t    const char *refname,\n>  \t\t\t\t    struct strbuf *referent,\n>  \t\t\t\t    unsigned int symbolic_link)\n\n\nNit: as we touch this function, maybe we could change `unsigned int\nsymbolic_link` to be `bool symbolic_link`.\n\n>  {\n> -\tint is_referent_root;\n>  \tchar orig_last_byte;\n>  \tsize_t orig_len;\n>  \tint ret = 0;\n>  \n>  \torig_len = referent->len;\n>  \torig_last_byte = referent->buf[orig_len - 1];\n> -\tif (!symbolic_link)\n> -\t\tstrbuf_rtrim(referent);\n> -\n> -\tis_referent_root = is_root_ref(referent->buf);\n> -\tif (!is_referent_root &&\n> -\t    !starts_with(referent->buf, \"refs/\") &&\n> -\t    !starts_with(referent->buf, \"worktrees/\")) {\n> -\t\tret |= fsck_report_ref(o, report,\n> -\t\t\t\t       FSCK_MSG_SYMREF_TARGET_IS_NOT_A_REF,\n> -\t\t\t\t       \"points to non-ref target '%s'\", referent->buf);\n> -\t}\n>  \n> -\tif (!is_referent_root && check_refname_format(referent->buf, 0)) {\n> -\t\tret |= fsck_report_ref(o, report,\n> -\t\t\t\t       FSCK_MSG_BAD_REFERENT_NAME,\n> -\t\t\t\t       \"points to invalid refname '%s'\", referent->buf);\n> -\t}\n> +\tif (!symbolic_link) {\n> +\t\tstrbuf_rtrim(referent);\n>  \n> -\tif (symbolic_link)\n> -\t\tgoto out;\n> +\t\tif (referent->len == orig_len ||\n> +\t\t    (referent->len < orig_len && orig_last_byte != '\\n')) {\n> +\t\t\tret |= fsck_report_ref(o, report,\n> +\t\t\t\t\t       FSCK_MSG_REF_MISSING_NEWLINE,\n> +\t\t\t\t\t       \"misses LF at the end\");\n> +\t\t}\n>  \n> -\tif (referent->len == orig_len ||\n> -\t    (referent->len < orig_len && orig_last_byte != '\\n')) {\n> -\t\tret |= fsck_report_ref(o, report,\n> -\t\t\t\t       FSCK_MSG_REF_MISSING_NEWLINE,\n> -\t\t\t\t       \"misses LF at the end\");\n> +\t\tif (referent->len != orig_len && referent->len != orig_len - 1) {\n> +\t\t\tret |= fsck_report_ref(o, report,\n> +\t\t\t\t\t       FSCK_MSG_TRAILING_REF_CONTENT,\n> +\t\t\t\t\t       \"has trailing whitespaces or newlines\");\n> +\t\t}\n>  \t}\n>  \n> -\tif (referent->len != orig_len && referent->len != orig_len - 1) {\n> -\t\tret |= fsck_report_ref(o, report,\n> -\t\t\t\t       FSCK_MSG_TRAILING_REF_CONTENT,\n> -\t\t\t\t       \"has trailing whitespaces or newlines\");\n> -\t}\n> +\tret |= refs_fsck_symref(ref_store, o, report, refname, referent->buf);\n>  \n> -out:\n>  \treturn ret ? -1 : 0;\n>  }\n>  \n> @@ -3807,7 +3793,8 @@ static int files_fsck_refs_content(struct ref_store *ref_store,\n>  \t\telse\n>  \t\t\tstrbuf_addbuf(&referent, &ref_content);\n>  \n> -\t\tret |= files_fsck_symref_target(o, &report, &referent, 1);\n> +\t\tret |= files_fsck_symref_target(ref_store, o, &report,\n> +\t\t\t\t\t\ttarget_name, &referent, 1);\n\nNit: we might change 1 to be `true`.\n\n>  \t\tgoto cleanup;\n>  \t}\n>  \n> @@ -3847,7 +3834,8 @@ static int files_fsck_refs_content(struct ref_store *ref_store,\n>  \t\t\tgoto cleanup;\n>  \t\t}\n>  \t} else {\n> -\t\tret = files_fsck_symref_target(o, &report, &referent, 0);\n> +\t\tret = files_fsck_symref_target(ref_store, o, &report,\n> +\t\t\t\t\t       target_name, &referent, 0);\n>  \t\tgoto cleanup;\n>  \t}\n>  \n> \n> -- \n> 2.52.0.542.g9473a8513b.dirty\n> \n\nThanks,\nJialuo\n"},{"id":"533484","messageId":"aWJQL3WdZermrAUv@ArchLinux","threadId":"64762","inReplyTo":"20260109-pks-refs-verify-fixes-v1-10-3587dba18294@pks.im","subject":"Re: [PATCH 10/17] refs/files: introduce function to perform normal ref checks","fromName":"shejialuo","fromEmail":"shejialuo@gmail.com","sentAt":"2026-01-10T13:12:15Z","receivedAt":"2026-01-10T13:12:19Z","isPatch":true,"sender":{"key":"shejialuo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56911263?v=4"},"body":"On Fri, Jan 09, 2026 at 01:39:39PM +0100, Patrick Steinhardt wrote:\n> In a subsequent commit we'll introduce new generic checks for direct\n> refs. These checks will be independent of the actual backend.\n> \n> Introduce a new function `refs_fsck_ref()` that will be used for this\n> purpose. At the current point in time it's still empty, but it will get\n> populated in a subsequent commit.\n> \n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  refs.c               | 7 +++++++\n>  refs.h               | 8 ++++++++\n>  refs/files-backend.c | 2 ++\n>  3 files changed, 17 insertions(+)\n> \n> diff --git a/refs.c b/refs.c\n> index 739bf9fefc..4fc1317cb3 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -320,6 +320,13 @@ int check_refname_format(const char *refname, int flags)\n>  \treturn check_or_sanitize_refname(refname, flags, NULL);\n>  }\n>  \n> +int refs_fsck_ref(struct ref_store *refs UNUSED, struct fsck_options *o UNUSED,\n> +\t\t  struct fsck_ref_report *report UNUSED,\n> +\t\t  const char *refname UNUSED, const struct object_id *oid UNUSED)\n> +{\n> +\treturn 0;\n> +}\n> +\n>  int refs_fsck_symref(struct ref_store *refs UNUSED, struct fsck_options *o,\n>  \t\t     struct fsck_ref_report *report,\n>  \t\t     const char *refname UNUSED, const char *target)\n> diff --git a/refs.h b/refs.h\n> index d91fcb2d2f..61c56cca36 100644\n> --- a/refs.h\n> +++ b/refs.h\n> @@ -655,6 +655,14 @@ int check_refname_format(const char *refname, int flags);\n>  \n>  struct fsck_ref_report;\n>  \n> +/*\n> + * Perform generic checks for a specific symref target. This function is\n> + * expected to be called by the ref backends for every symbolic ref.\n> + */\n\nI think above comment is the same as `refs_fsck_symref`, I think we\nshould update to say that we perform generic checks for a ref instead of\na specific symref target.\n\nThanks,\nJialuo\n"},{"id":"533488","messageId":"aWJUm-hrPquegbdf@ArchLinux","threadId":"64762","inReplyTo":"20260109-pks-refs-verify-fixes-v1-16-3587dba18294@pks.im","subject":"Re: [PATCH 16/17] builtin/fsck: move generic HEAD check into `refs_fsck()`","fromName":"shejialuo","fromEmail":"shejialuo@gmail.com","sentAt":"2026-01-10T13:31:07Z","receivedAt":"2026-01-10T13:31:11Z","isPatch":true,"sender":{"key":"shejialuo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56911263?v=4"},"body":"On Fri, Jan 09, 2026 at 01:39:45PM +0100, Patrick Steinhardt wrote:\n> Move the check that detects \"HEAD\" refs that do not point at a branch\n> into `refs_fsck()`. This follows the same motivation as the preceding\n> commit.\n> \n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  Documentation/fsck-msgids.adoc |  3 +++\n>  builtin/fsck.c                 |  7 -------\n>  fsck.h                         |  1 +\n>  refs.c                         | 12 +++++++++++-\n>  t/t0602-reffiles-fsck.sh       |  8 ++++----\n>  t/t1450-fsck.sh                |  4 ++--\n>  6 files changed, 21 insertions(+), 14 deletions(-)\n> \n> diff --git a/Documentation/fsck-msgids.adoc b/Documentation/fsck-msgids.adoc\n> index 76609321f6..6a4db3a991 100644\n> --- a/Documentation/fsck-msgids.adoc\n> +++ b/Documentation/fsck-msgids.adoc\n> @@ -13,6 +13,9 @@\n>  `badGpgsig`::\n>  \t(ERROR) A tag contains a bad (truncated) signature (e.g., `gpgsig`) header.\n>  \n> +`badHeadTarget`::\n> +\t(ERROR) The `HEAD` ref is a symref that does not refer to a branch.\n> +\n>  `badHeaderContinuation`::\n>  \t(ERROR) A continuation header (such as for `gpgsig`) is unexpectedly truncated.\n>  \n> diff --git a/builtin/fsck.c b/builtin/fsck.c\n> index 4dd4d74d1e..5dda441f45 100644\n> --- a/builtin/fsck.c\n> +++ b/builtin/fsck.c\n> @@ -728,13 +728,6 @@ static void fsck_head_link(const char *head_ref_name,\n>  \t\terror(_(\"invalid %s\"), head_ref_name);\n>  \t\treturn;\n>  \t}\n> -\tif (strcmp(*head_points_at, head_ref_name) &&\n> -\t    !starts_with(*head_points_at, \"refs/heads/\")) {\n> -\t\terrors_found |= ERROR_REFS;\n> -\t\terror(_(\"%s points to something strange (%s)\"),\n> -\t\t      head_ref_name, *head_points_at);\n> -\t\treturn;\n> -\t}\n>  \n>  \treturn;\n>  }\n> diff --git a/fsck.h b/fsck.h\n> index 1f472b7daa..65ecbb7fe1 100644\n> --- a/fsck.h\n> +++ b/fsck.h\n> @@ -30,6 +30,7 @@ enum fsck_msg_type {\n>  \tFUNC(BAD_DATE_OVERFLOW, ERROR) \\\n>  \tFUNC(BAD_EMAIL, ERROR) \\\n>  \tFUNC(BAD_GPGSIG, ERROR) \\\n> +\tFUNC(BAD_HEAD_TARGET, ERROR) \\\n>  \tFUNC(BAD_NAME, ERROR) \\\n>  \tFUNC(BAD_OBJECT_SHA1, ERROR) \\\n>  \tFUNC(BAD_PACKED_REF_ENTRY, ERROR) \\\n> diff --git a/refs.c b/refs.c\n> index c3528862c6..a772d371cd 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -334,8 +334,18 @@ int refs_fsck_ref(struct ref_store *refs UNUSED, struct fsck_options *o,\n>  \n>  int refs_fsck_symref(struct ref_store *refs UNUSED, struct fsck_options *o,\n>  \t\t     struct fsck_ref_report *report,\n> -\t\t     const char *refname UNUSED, const char *target)\n> +\t\t     const char *refname, const char *target)\n>  {\n> +\tconst char *stripped_refname;\n> +\n> +\tparse_worktree_ref(refname, NULL, NULL, &stripped_refname);\n> +\n> +\tif (!strcmp(stripped_refname, \"HEAD\") &&\n> +\t    !starts_with(target, \"refs/heads/\") &&\n\nWe would first check whether the current ref is `HEAD`. And I am\nwondering whether we have some common APIs. And I find the similar logic\nin `reglog.c::is_head` like the following shows:\n\n    static int is_head(const char *refname)\n    {\n            const char *stripped_refname;\n            parse_worktree_ref(refname, NULL, NULL, &stripped_refname);\n            return !strcmp(stripped_refname, \"HEAD\");\n    }\n\nI think we might just extract this common logic to avoid introducing\nrepetition.\n\nThanks,\nJialuo\n"},{"id":"533489","messageId":"aWJWCiTFQAZqDb9y@ArchLinux","threadId":"64762","inReplyTo":"20260109-pks-refs-verify-fixes-v1-0-3587dba18294@pks.im","subject":"Re: [PATCH 00/17] Fixes and improvements for ref consistency checks","fromName":"shejialuo","fromEmail":"shejialuo@gmail.com","sentAt":"2026-01-10T13:37:14Z","receivedAt":"2026-01-10T13:37:19Z","isPatch":true,"sender":{"key":"shejialuo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56911263?v=4"},"body":"On Fri, Jan 09, 2026 at 01:39:29PM +0100, Patrick Steinhardt wrote:\n> Hi,\n> \n> this patch series contains a bunch of fixes and improvements for ref\n> consistency checks. It is structured as follows:\n> \n>   - Patches 1 to 4 contain a couple of cleanups for the consistency\n>     checks done by the \"files\" backend.\n> \n>   - Patches 5 to 7 introduce checks for root refs for the \"files\"\n>     backend.\n> \n>   - Patches 9 to 14 introduce infrastructure for shared checks with the\n>     \"files\" and \"reftable\" backend.\n> \n>   - Patches 15 to 17 move some ref consistency checks that were still\n>     driven by git-fsck(1) into `git refs verify`.\n> \n> Thanks!\n> \n> Patrick\n\nI left some comments. In conclusion, I very appreciate the direction to\nshare the common logic for both \"files\" backend and \"reftable\" backend.\nAnd also, we could check the correctness of `HEAD` to make the ref\nsubsystem self-contained.\n\nThanks,\nJialuo\n"},{"id":"533559","messageId":"aWSuH2bjlRqa2WoZ@pks.im","threadId":"64762","inReplyTo":"aWJKYzcY3H_-xy1V@ArchLinux","subject":"Re: [PATCH 07/17] refs/files: perform consistency checks for root refs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-12T08:17:35Z","receivedAt":"2026-01-12T08:17:48Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sat, Jan 10, 2026 at 08:47:31PM +0800, shejialuo wrote:\n> On Fri, Jan 09, 2026 at 01:39:36PM +0100, Patrick Steinhardt wrote:\n> >  static int files_fsck(struct ref_store *ref_store,\n> >  \t\t      struct fsck_options *o,\n> >  \t\t      struct worktree *wt)\n> >  {\n> >  \tstruct files_ref_store *refs =\n> >  \t\tfiles_downcast(ref_store, REF_STORE_READ, \"fsck\");\n> > +\tstruct files_fsck_root_ref_data data = {\n> > +\t\t.refs = refs,\n> > +\t\t.o = o,\n> > +\t\t.wt = wt,\n> > +\t\t.refname = STRBUF_INIT,\n> > +\t\t.path = STRBUF_INIT,\n> > +\t};\n> >  \tint ret = 0;\n> >  \n> >  \tif (files_fsck_refs_dir(ref_store, o, wt) < 0)\n> >  \t\tret = -1;\n> > +\n> > +\tif (for_each_root_ref(refs, files_fsck_root_ref, &data) < 0 ||\n> > +\t    data.errors_found)\n> \n> I am wondering where we update this filed in `files_fsck_root_ref`. It\n> seems that we never do this in this commit. I think we should delete\n> this filed in `files_fsck_root_ref_data` and add this field back when we\n> do need this to avoid confusion.\n\nOh, you're right. I think I did use it in an earlier iteration, but\ndon't seem to do anymore. Will fix.\n\nPatrick\n"},{"id":"533560","messageId":"aWSuLIzHPDSxMg9y@pks.im","threadId":"64762","inReplyTo":"aWJNHgFnimXRHkb6@ArchLinux","subject":"Re: [PATCH 09/17] refs/files: extract generic symref target checks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-12T08:17:48Z","receivedAt":"2026-01-12T08:17:52Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sat, Jan 10, 2026 at 08:59:10PM +0800, shejialuo wrote:\n> On Fri, Jan 09, 2026 at 01:39:38PM +0100, Patrick Steinhardt wrote:\n> > diff --git a/refs/files-backend.c b/refs/files-backend.c\n> > index 0ff047d0df..72c1db849e 100644\n> > --- a/refs/files-backend.c\n> > +++ b/refs/files-backend.c\n> > @@ -3718,53 +3718,39 @@ typedef int (*files_fsck_refs_fn)(struct ref_store *ref_store,\n> >  \t\t\t\t  const char *path,\n> >  \t\t\t\t  int mode);\n> >  \n> > -static int files_fsck_symref_target(struct fsck_options *o,\n> > +static int files_fsck_symref_target(struct ref_store *ref_store,\n> > +\t\t\t\t    struct fsck_options *o,\n> >  \t\t\t\t    struct fsck_ref_report *report,\n> > +\t\t\t\t    const char *refname,\n> >  \t\t\t\t    struct strbuf *referent,\n> >  \t\t\t\t    unsigned int symbolic_link)\n> \n> \n> Nit: as we touch this function, maybe we could change `unsigned int\n> symbolic_link` to be `bool symbolic_link`.\n\nI'd prefer to not have this while-at-it change. The benefit isn't clear\nenough to actually change it, and it would distract from the actual\nchanges a bit.\n\nPatrick\n"},{"id":"533561","messageId":"aWSuNFd6USWqvLQX@pks.im","threadId":"64762","inReplyTo":"aWJQL3WdZermrAUv@ArchLinux","subject":"Re: [PATCH 10/17] refs/files: introduce function to perform normal ref checks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-12T08:17:56Z","receivedAt":"2026-01-12T08:18:02Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sat, Jan 10, 2026 at 09:12:15PM +0800, shejialuo wrote:\n> On Fri, Jan 09, 2026 at 01:39:39PM +0100, Patrick Steinhardt wrote:\n> > diff --git a/refs.h b/refs.h\n> > index d91fcb2d2f..61c56cca36 100644\n> > --- a/refs.h\n> > +++ b/refs.h\n> > @@ -655,6 +655,14 @@ int check_refname_format(const char *refname, int flags);\n> >  \n> >  struct fsck_ref_report;\n> >  \n> > +/*\n> > + * Perform generic checks for a specific symref target. This function is\n> > + * expected to be called by the ref backends for every symbolic ref.\n> > + */\n> \n> I think above comment is the same as `refs_fsck_symref`, I think we\n> should update to say that we perform generic checks for a ref instead of\n> a specific symref target.\n\nIndeed, a classical copy-paste error. Thanks!\n\nPatrick\n"},{"id":"533562","messageId":"aWSuObEsFaxi1NAf@pks.im","threadId":"64762","inReplyTo":"aWJUm-hrPquegbdf@ArchLinux","subject":"Re: [PATCH 16/17] builtin/fsck: move generic HEAD check into `refs_fsck()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-12T08:18:01Z","receivedAt":"2026-01-12T08:18:06Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sat, Jan 10, 2026 at 09:31:07PM +0800, shejialuo wrote:\n> On Fri, Jan 09, 2026 at 01:39:45PM +0100, Patrick Steinhardt wrote:\n> > diff --git a/refs.c b/refs.c\n> > index c3528862c6..a772d371cd 100644\n> > --- a/refs.c\n> > +++ b/refs.c\n> > @@ -334,8 +334,18 @@ int refs_fsck_ref(struct ref_store *refs UNUSED, struct fsck_options *o,\n> >  \n> >  int refs_fsck_symref(struct ref_store *refs UNUSED, struct fsck_options *o,\n> >  \t\t     struct fsck_ref_report *report,\n> > -\t\t     const char *refname UNUSED, const char *target)\n> > +\t\t     const char *refname, const char *target)\n> >  {\n> > +\tconst char *stripped_refname;\n> > +\n> > +\tparse_worktree_ref(refname, NULL, NULL, &stripped_refname);\n> > +\n> > +\tif (!strcmp(stripped_refname, \"HEAD\") &&\n> > +\t    !starts_with(target, \"refs/heads/\") &&\n> \n> We would first check whether the current ref is `HEAD`. And I am\n> wondering whether we have some common APIs. And I find the similar logic\n> in `reglog.c::is_head` like the following shows:\n> \n>     static int is_head(const char *refname)\n>     {\n>             const char *stripped_refname;\n>             parse_worktree_ref(refname, NULL, NULL, &stripped_refname);\n>             return !strcmp(stripped_refname, \"HEAD\");\n>     }\n> \n> I think we might just extract this common logic to avoid introducing\n> repetition.\n\nHm. We could, but I'm a tiny bit worried about just calling it\n`is_head()`. It might be surprising to some callers that there isn't\nonly one \"HEAD\", but that this would also recognize worktree HEADs. If\nsomebody just goes like \"I wanna know whether I've got HEAD\" they might\nnot think about that at all.\n\nSo given that the complexity is comparatively low I'd prefer to keep\nthis as-is for now. On the other hand, if you've got some proposal for\nhow to make this interface not confusing I'm very open to that :)\n\nThanks!\n\nPatrick\n"},{"id":"533563","messageId":"aWSuPkzH4RsG472A@pks.im","threadId":"64762","inReplyTo":"aWJWCiTFQAZqDb9y@ArchLinux","subject":"Re: [PATCH 00/17] Fixes and improvements for ref consistency checks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-12T08:18:06Z","receivedAt":"2026-01-12T08:18:11Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sat, Jan 10, 2026 at 09:37:14PM +0800, shejialuo wrote:\n> On Fri, Jan 09, 2026 at 01:39:29PM +0100, Patrick Steinhardt wrote:\n> > Hi,\n> > \n> > this patch series contains a bunch of fixes and improvements for ref\n> > consistency checks. It is structured as follows:\n> > \n> >   - Patches 1 to 4 contain a couple of cleanups for the consistency\n> >     checks done by the \"files\" backend.\n> > \n> >   - Patches 5 to 7 introduce checks for root refs for the \"files\"\n> >     backend.\n> > \n> >   - Patches 9 to 14 introduce infrastructure for shared checks with the\n> >     \"files\" and \"reftable\" backend.\n> > \n> >   - Patches 15 to 17 move some ref consistency checks that were still\n> >     driven by git-fsck(1) into `git refs verify`.\n> > \n> > Thanks!\n> > \n> > Patrick\n> \n> I left some comments. In conclusion, I very appreciate the direction to\n> share the common logic for both \"files\" backend and \"reftable\" backend.\n> And also, we could check the correctness of `HEAD` to make the ref\n> subsystem self-contained.\n\nThanks for your review!\n\nPatrick\n"},{"id":"533577","messageId":"20260112-pks-refs-verify-fixes-v2-0-2e9e453bd6c3@pks.im","threadId":"64762","inReplyTo":"20260109-pks-refs-verify-fixes-v1-0-3587dba18294@pks.im","subject":"[PATCH v2 00/17] Fixes and improvements for ref consistency checks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-12T09:02:49Z","receivedAt":"2026-01-12T09:02:59Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis patch series contains a bunch of fixes and improvements for ref\nconsistency checks. It is structured as follows:\n\n  - Patches 1 to 4 contain a couple of cleanups for the consistency\n    checks done by the \"files\" backend.\n\n  - Patches 5 to 7 introduce checks for root refs for the \"files\"\n    backend.\n\n  - Patches 9 to 14 introduce infrastructure for shared checks with the\n    \"files\" and \"reftable\" backend.\n\n  - Patches 15 to 17 move some ref consistency checks that were still\n    driven by git-fsck(1) into `git refs verify`.\n\nChanges in v2:\n  - Remove unused `errors_found` field.\n  - Fix a commit message typo.\n  - Fix a copy-paste error in a function comment.\n  - Link to v1: https://lore.kernel.org/r/20260109-pks-refs-verify-fixes-v1-0-3587dba18294@pks.im\n\nThanks!\n\nPatrick\n\n---\nPatrick Steinhardt (17):\n      refs/files: simplify iterating through root refs\n      refs/files: move fsck functions into global scope\n      refs/files: remove `refs_check_dir` parameter\n      refs/files: remove useless indirection\n      refs/files: extract function to check single ref\n      refs/files: improve error handling when verifying symrefs\n      refs/files: perform consistency checks for root refs\n      fsck: drop unused fields from `struct fsck_ref_report`\n      refs/files: extract generic symref target checks\n      refs/files: introduce function to perform normal ref checks\n      refs/reftable: adapt includes to become consistent\n      refs/reftable: extract function to retrieve backend for worktree\n      refs/reftable: fix consistency checks with worktrees\n      refs/reftable: introduce generic checks for refs\n      builtin/fsck: move generic object ID checks into `refs_fsck()`\n      builtin/fsck: move generic HEAD check into `refs_fsck()`\n      builtin/fsck: drop `fsck_head_link()`\n\n Documentation/fsck-msgids.adoc |   6 ++\n builtin/fsck.c                 |  46 +--------\n fsck.c                         |   5 -\n fsck.h                         |   4 +-\n refs.c                         |  43 ++++++++\n refs.h                         |  18 ++++\n refs/files-backend.c           | 228 ++++++++++++++++++++++++-----------------\n refs/reftable-backend.c        | 167 ++++++++++++++++++++++--------\n t/t0602-reffiles-fsck.sh       |  30 ++++++\n t/t0614-reftable-fsck.sh       |  44 ++++++++\n t/t1450-fsck.sh                |  10 +-\n 11 files changed, 414 insertions(+), 187 deletions(-)\n\nRange-diff versus v1:\n\n 1:  201451626d =  1:  21531efb05 refs/files: simplify iterating through root refs\n 2:  88252f2b99 =  2:  861bd57d6e refs/files: move fsck functions into global scope\n 3:  56d8ce2c85 =  3:  e06b8bdd23 refs/files: remove `refs_check_dir` parameter\n 4:  ddf450134c =  4:  92992a522e refs/files: remove useless indirection\n 5:  2d3ebf80fd =  5:  904fecf80e refs/files: extract function to check single ref\n 6:  316dafeff8 =  6:  b5f5e86f1f refs/files: improve error handling when verifying symrefs\n 7:  94a9b3d58b !  7:  d1abff98f8 refs/files: perform consistency checks for root refs\n    @@ refs/files-backend.c: static int files_fsck_refs_dir(struct ref_store *ref_store\n     +\tstruct worktree *wt;\n     +\tstruct strbuf refname;\n     +\tstruct strbuf path;\n    -+\tbool errors_found;\n     +};\n     +\n     +static int files_fsck_root_ref(const char *refname, void *cb_data)\n    @@ refs/files-backend.c: static int files_fsck_refs_dir(struct ref_store *ref_store\n      \tif (files_fsck_refs_dir(ref_store, o, wt) < 0)\n      \t\tret = -1;\n     +\n    -+\tif (for_each_root_ref(refs, files_fsck_root_ref, &data) < 0 ||\n    -+\t    data.errors_found)\n    ++\tif (for_each_root_ref(refs, files_fsck_root_ref, &data) < 0)\n     +\t\tret = -1;\n     +\n      \tif (refs->packed_ref_store->be->fsck(refs->packed_ref_store, o, wt) < 0)\n 8:  a773fd65e9 =  8:  0b0e0e0033 fsck: drop unused fields from `struct fsck_ref_report`\n 9:  12cf39ce8c !  9:  4bf249e530 refs/files: extract generic symref target checks\n    @@ Commit message\n     \n         Reimplementing these checks for every single backend doesn't really make\n         sense. Extract it into a generic `refs_fsck_symref()` function that can\n    -    be used my other backends, as well. The \"reftable\" backend will be wired\n    +    be used by other backends, as well. The \"reftable\" backend will be wired\n         up in a subsequent commit.\n     \n         While at it, improve the consistency checks so that we don't complain\n10:  9ae96c0acb ! 10:  5bd34fb53c refs/files: introduce function to perform normal ref checks\n    @@ refs.h: int check_refname_format(const char *refname, int flags);\n      struct fsck_ref_report;\n      \n     +/*\n    -+ * Perform generic checks for a specific symref target. This function is\n    ++ * Perform generic checks for a specific direct ref. This function is\n     + * expected to be called by the ref backends for every symbolic ref.\n     + */\n     +int refs_fsck_ref(struct ref_store *refs, struct fsck_options *o,\n11:  c2b0a1f517 = 11:  ca62b50abc refs/reftable: adapt includes to become consistent\n12:  608b689d9e = 12:  66b5d6c981 refs/reftable: extract function to retrieve backend for worktree\n13:  d39733206f = 13:  d31b7fb348 refs/reftable: fix consistency checks with worktrees\n14:  37b8d22941 = 14:  eb960e66f2 refs/reftable: introduce generic checks for refs\n15:  72b81062d2 = 15:  d0e2e3fe33 builtin/fsck: move generic object ID checks into `refs_fsck()`\n16:  07a2403bc7 = 16:  029d02dd8a builtin/fsck: move generic HEAD check into `refs_fsck()`\n17:  e944a0e430 = 17:  99eb06f153 builtin/fsck: drop `fsck_head_link()`\n\n---\nbase-commit: d529f3a197364881746f558e5652f0236131eb86\nchange-id: 20260109-pks-refs-verify-fixes-1e47872317cf\n\n"},{"id":"533580","messageId":"20260112-pks-refs-verify-fixes-v2-1-2e9e453bd6c3@pks.im","threadId":"64762","inReplyTo":"20260112-pks-refs-verify-fixes-v2-0-2e9e453bd6c3@pks.im","subject":"[PATCH v2 01/17] refs/files: simplify iterating through root refs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-12T09:02:50Z","receivedAt":"2026-01-12T09:02:59Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When iterating through root refs we first need to determine the\ndirectory in which the refs live. This is done by retrieving the root of\nthe loose refs via `refs->loose->root->name`, and putting it through\n`files_ref_path()` to derive the final path.\n\nThis is somewhat redundant though: the root name of the loose files\ncache is always going to be the empty string. As such, we always end up\npassing that empty string to `files_ref_path()` as the ref hierarchy we\nwant to start. And this actually makes sense: `files_ref_path()` already\ncomputes the location of the root directory, so of course we need to\npass the empty string for the ref hierarchy itself. So going via the\nloose ref cache to figure out that the root of a ref hierarchy is empty\nis only causing confusion.\n\nBut next to the added confusion, it can also lead to a segfault. The\nloose ref cache is populated lazily, so it may not always be set. It\nseems to be sheer luck that this is a condition we do not currently hit.\nThe right thing to do would be to call `get_loose_ref_cache()`, which\nknows to populate the cache if required.\n\nSimplify the code and fix the potential segfault by simply removing the\nindirection via the loose ref cache completely.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n refs/files-backend.c | 11 +++--------\n 1 file changed, 3 insertions(+), 8 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 6f6f76a8d8..297739f203 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -354,13 +354,11 @@ static int for_each_root_ref(struct files_ref_store *refs,\n \t\t\t     void *cb_data)\n {\n \tstruct strbuf path = STRBUF_INIT, refname = STRBUF_INIT;\n-\tconst char *dirname = refs->loose->root->name;\n \tstruct dirent *de;\n-\tsize_t dirnamelen;\n \tint ret;\n \tDIR *d;\n \n-\tfiles_ref_path(refs, &path, dirname);\n+\tfiles_ref_path(refs, &path, \"\");\n \n \td = opendir(path.buf);\n \tif (!d) {\n@@ -368,9 +366,6 @@ static int for_each_root_ref(struct files_ref_store *refs,\n \t\treturn -1;\n \t}\n \n-\tstrbuf_addstr(&refname, dirname);\n-\tdirnamelen = refname.len;\n-\n \twhile ((de = readdir(d)) != NULL) {\n \t\tunsigned char dtype;\n \n@@ -378,6 +373,8 @@ static int for_each_root_ref(struct files_ref_store *refs,\n \t\t\tcontinue;\n \t\tif (ends_with(de->d_name, \".lock\"))\n \t\t\tcontinue;\n+\n+\t\tstrbuf_reset(&refname);\n \t\tstrbuf_addstr(&refname, de->d_name);\n \n \t\tdtype = get_dtype(de, &path, 1);\n@@ -386,8 +383,6 @@ static int for_each_root_ref(struct files_ref_store *refs,\n \t\t\tif (ret)\n \t\t\t\tgoto done;\n \t\t}\n-\n-\t\tstrbuf_setlen(&refname, dirnamelen);\n \t}\n \n \tret = 0;\n\n-- \n2.52.0.590.g1f87b77810.dirty\n\n"},{"id":"533581","messageId":"20260112-pks-refs-verify-fixes-v2-2-2e9e453bd6c3@pks.im","threadId":"64762","inReplyTo":"20260112-pks-refs-verify-fixes-v2-0-2e9e453bd6c3@pks.im","subject":"[PATCH v2 02/17] refs/files: move fsck functions into global scope","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-12T09:02:51Z","receivedAt":"2026-01-12T09:03:01Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When performing consistency checks we pass the functions that perform\nthe verification down the calling stack. This is somewhat unnecessary\nthough, as the set of functions doesn't ever change.\n\nSimplify the code by moving the array into global scope and remove the\nparameter.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n refs/files-backend.c | 17 ++++++++---------\n 1 file changed, 8 insertions(+), 9 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 297739f203..feba3ee58b 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -3890,11 +3890,16 @@ static int files_fsck_refs_name(struct ref_store *ref_store UNUSED,\n \treturn ret;\n }\n \n+static const files_fsck_refs_fn fsck_refs_fn[]= {\n+\tfiles_fsck_refs_name,\n+\tfiles_fsck_refs_content,\n+\tNULL,\n+};\n+\n static int files_fsck_refs_dir(struct ref_store *ref_store,\n \t\t\t       struct fsck_options *o,\n \t\t\t       const char *refs_check_dir,\n-\t\t\t       struct worktree *wt,\n-\t\t\t       files_fsck_refs_fn *fsck_refs_fn)\n+\t\t\t       struct worktree *wt)\n {\n \tstruct strbuf refname = STRBUF_INIT;\n \tstruct strbuf sb = STRBUF_INIT;\n@@ -3955,13 +3960,7 @@ static int files_fsck_refs(struct ref_store *ref_store,\n \t\t\t   struct fsck_options *o,\n \t\t\t   struct worktree *wt)\n {\n-\tfiles_fsck_refs_fn fsck_refs_fn[]= {\n-\t\tfiles_fsck_refs_name,\n-\t\tfiles_fsck_refs_content,\n-\t\tNULL,\n-\t};\n-\n-\treturn files_fsck_refs_dir(ref_store, o, \"refs\", wt, fsck_refs_fn);\n+\treturn files_fsck_refs_dir(ref_store, o, \"refs\", wt);\n }\n \n static int files_fsck(struct ref_store *ref_store,\n\n-- \n2.52.0.590.g1f87b77810.dirty\n\n"},{"id":"533578","messageId":"20260112-pks-refs-verify-fixes-v2-3-2e9e453bd6c3@pks.im","threadId":"64762","inReplyTo":"20260112-pks-refs-verify-fixes-v2-0-2e9e453bd6c3@pks.im","subject":"[PATCH v2 03/17] refs/files: remove `refs_check_dir` parameter","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-12T09:02:52Z","receivedAt":"2026-01-12T09:03:04Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The parameter `refs_check_dir` determines which directory we want to\ncheck references for. But as we always want to check the complete\nrefs hierarchy, this parameter is always set to \"refs\".\n\nDrop the parameter and hardcode it.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n refs/files-backend.c | 8 +++-----\n 1 file changed, 3 insertions(+), 5 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex feba3ee58b..0a104c7bf6 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -3898,7 +3898,6 @@ static const files_fsck_refs_fn fsck_refs_fn[]= {\n \n static int files_fsck_refs_dir(struct ref_store *ref_store,\n \t\t\t       struct fsck_options *o,\n-\t\t\t       const char *refs_check_dir,\n \t\t\t       struct worktree *wt)\n {\n \tstruct strbuf refname = STRBUF_INIT;\n@@ -3907,7 +3906,7 @@ static int files_fsck_refs_dir(struct ref_store *ref_store,\n \tint iter_status;\n \tint ret = 0;\n \n-\tstrbuf_addf(&sb, \"%s/%s\", ref_store->gitdir, refs_check_dir);\n+\tstrbuf_addf(&sb, \"%s/refs\", ref_store->gitdir);\n \n \titer = dir_iterator_begin(sb.buf, 0);\n \tif (!iter) {\n@@ -3927,8 +3926,7 @@ static int files_fsck_refs_dir(struct ref_store *ref_store,\n \n \t\t\tif (!is_main_worktree(wt))\n \t\t\t\tstrbuf_addf(&refname, \"worktrees/%s/\", wt->id);\n-\t\t\tstrbuf_addf(&refname, \"%s/%s\", refs_check_dir,\n-\t\t\t\t    iter->relative_path);\n+\t\t\tstrbuf_addf(&refname, \"refs/%s\", iter->relative_path);\n \n \t\t\tif (o->verbose)\n \t\t\t\tfprintf_ln(stderr, \"Checking %s\", refname.buf);\n@@ -3960,7 +3958,7 @@ static int files_fsck_refs(struct ref_store *ref_store,\n \t\t\t   struct fsck_options *o,\n \t\t\t   struct worktree *wt)\n {\n-\treturn files_fsck_refs_dir(ref_store, o, \"refs\", wt);\n+\treturn files_fsck_refs_dir(ref_store, o, wt);\n }\n \n static int files_fsck(struct ref_store *ref_store,\n\n-- \n2.52.0.590.g1f87b77810.dirty\n\n"},{"id":"533579","messageId":"20260112-pks-refs-verify-fixes-v2-4-2e9e453bd6c3@pks.im","threadId":"64762","inReplyTo":"20260112-pks-refs-verify-fixes-v2-0-2e9e453bd6c3@pks.im","subject":"[PATCH v2 04/17] refs/files: remove useless indirection","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-12T09:02:53Z","receivedAt":"2026-01-12T09:03:06Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The function `files_fsck_refs()` only has a single callsite and forwards\nall of its arguments as-is, so it's basically a useless indirection.\nInline the function call.\n\nWhile at it, also remove the bitwise or that we have for return values.\nWe don't really want to or them at all, but rather just want to return\nan error in case either of the functions has failed.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n refs/files-backend.c | 16 +++++++---------\n 1 file changed, 7 insertions(+), 9 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 0a104c7bf6..4cbee23dad 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -3954,22 +3954,20 @@ static int files_fsck_refs_dir(struct ref_store *ref_store,\n \treturn ret;\n }\n \n-static int files_fsck_refs(struct ref_store *ref_store,\n-\t\t\t   struct fsck_options *o,\n-\t\t\t   struct worktree *wt)\n-{\n-\treturn files_fsck_refs_dir(ref_store, o, wt);\n-}\n-\n static int files_fsck(struct ref_store *ref_store,\n \t\t      struct fsck_options *o,\n \t\t      struct worktree *wt)\n {\n \tstruct files_ref_store *refs =\n \t\tfiles_downcast(ref_store, REF_STORE_READ, \"fsck\");\n+\tint ret = 0;\n \n-\treturn files_fsck_refs(ref_store, o, wt) |\n-\t       refs->packed_ref_store->be->fsck(refs->packed_ref_store, o, wt);\n+\tif (files_fsck_refs_dir(ref_store, o, wt) < 0)\n+\t\tret = -1;\n+\tif (refs->packed_ref_store->be->fsck(refs->packed_ref_store, o, wt) < 0)\n+\t\tret = -1;\n+\n+\treturn ret;\n }\n \n struct ref_storage_be refs_be_files = {\n\n-- \n2.52.0.590.g1f87b77810.dirty\n\n"},{"id":"533582","messageId":"20260112-pks-refs-verify-fixes-v2-5-2e9e453bd6c3@pks.im","threadId":"64762","inReplyTo":"20260112-pks-refs-verify-fixes-v2-0-2e9e453bd6c3@pks.im","subject":"[PATCH v2 05/17] refs/files: extract function to check single ref","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-12T09:02:54Z","receivedAt":"2026-01-12T09:03:10Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When checking the consistency of references we create a directory\niterator and then verify each single reference in a loop. The logic to\nperform the actual checks is embedded into that loop, which makes it\nhard to reuse. But In a subsequent commit we're about to introduce a\nsecond path that wants to verify references.\n\nPrepare for this by extracting the logic to check a single reference\ninto a standalone function.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n refs/files-backend.c | 80 +++++++++++++++++++++++++++++++++-------------------\n 1 file changed, 51 insertions(+), 29 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 4cbee23dad..9972221f9f 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -3715,7 +3715,8 @@ static int files_ref_store_remove_on_disk(struct ref_store *ref_store,\n typedef int (*files_fsck_refs_fn)(struct ref_store *ref_store,\n \t\t\t\t  struct fsck_options *o,\n \t\t\t\t  const char *refname,\n-\t\t\t\t  struct dir_iterator *iter);\n+\t\t\t\t  const char *path,\n+\t\t\t\t  int mode);\n \n static int files_fsck_symref_target(struct fsck_options *o,\n \t\t\t\t    struct fsck_ref_report *report,\n@@ -3772,7 +3773,8 @@ static int files_fsck_symref_target(struct fsck_options *o,\n static int files_fsck_refs_content(struct ref_store *ref_store,\n \t\t\t\t   struct fsck_options *o,\n \t\t\t\t   const char *target_name,\n-\t\t\t\t   struct dir_iterator *iter)\n+\t\t\t\t   const char *path,\n+\t\t\t\t   int mode)\n {\n \tstruct strbuf ref_content = STRBUF_INIT;\n \tstruct strbuf abs_gitdir = STRBUF_INIT;\n@@ -3786,7 +3788,7 @@ static int files_fsck_refs_content(struct ref_store *ref_store,\n \n \treport.path = target_name;\n \n-\tif (S_ISLNK(iter->st.st_mode)) {\n+\tif (S_ISLNK(mode)) {\n \t\tconst char *relative_referent_path = NULL;\n \n \t\tret = fsck_report_ref(o, &report,\n@@ -3798,7 +3800,7 @@ static int files_fsck_refs_content(struct ref_store *ref_store,\n \t\tif (!is_dir_sep(abs_gitdir.buf[abs_gitdir.len - 1]))\n \t\t\tstrbuf_addch(&abs_gitdir, '/');\n \n-\t\tstrbuf_add_real_path(&ref_content, iter->path.buf);\n+\t\tstrbuf_add_real_path(&ref_content, path);\n \t\tskip_prefix(ref_content.buf, abs_gitdir.buf,\n \t\t\t    &relative_referent_path);\n \n@@ -3811,7 +3813,7 @@ static int files_fsck_refs_content(struct ref_store *ref_store,\n \t\tgoto cleanup;\n \t}\n \n-\tif (strbuf_read_file(&ref_content, iter->path.buf, 0) < 0) {\n+\tif (strbuf_read_file(&ref_content, path, 0) < 0) {\n \t\t/*\n \t\t * Ref file could be removed by another concurrent process. We should\n \t\t * ignore this error and continue to the next ref.\n@@ -3819,7 +3821,7 @@ static int files_fsck_refs_content(struct ref_store *ref_store,\n \t\tif (errno == ENOENT)\n \t\t\tgoto cleanup;\n \n-\t\tret = error_errno(_(\"cannot read ref file '%s'\"), iter->path.buf);\n+\t\tret = error_errno(_(\"cannot read ref file '%s'\"), path);\n \t\tgoto cleanup;\n \t}\n \n@@ -3861,16 +3863,20 @@ static int files_fsck_refs_content(struct ref_store *ref_store,\n static int files_fsck_refs_name(struct ref_store *ref_store UNUSED,\n \t\t\t\tstruct fsck_options *o,\n \t\t\t\tconst char *refname,\n-\t\t\t\tstruct dir_iterator *iter)\n+\t\t\t\tconst char *path,\n+\t\t\t\tint mode UNUSED)\n {\n \tstruct strbuf sb = STRBUF_INIT;\n+\tconst char *filename;\n \tint ret = 0;\n \n+\tfilename = basename((char *) path);\n+\n \t/*\n \t * Ignore the files ending with \".lock\" as they may be lock files\n \t * However, do not allow bare \".lock\" files.\n \t */\n-\tif (iter->basename[0] != '.' && ends_with(iter->basename, \".lock\"))\n+\tif (filename[0] != '.' && ends_with(filename, \".lock\"))\n \t\tgoto cleanup;\n \n \t/*\n@@ -3896,6 +3902,35 @@ static const files_fsck_refs_fn fsck_refs_fn[]= {\n \tNULL,\n };\n \n+static int files_fsck_ref(struct ref_store *ref_store,\n+\t\t\t  struct fsck_options *o,\n+\t\t\t  const char *refname,\n+\t\t\t  const char *path,\n+\t\t\t  int mode)\n+{\n+\tint ret = 0;\n+\n+\tif (o->verbose)\n+\t\tfprintf_ln(stderr, \"Checking %s\", refname);\n+\n+\tif (!S_ISREG(mode) && !S_ISLNK(mode)) {\n+\t\tstruct fsck_ref_report report = { .path = refname };\n+\n+\t\tif (fsck_report_ref(o, &report,\n+\t\t\t\t    FSCK_MSG_BAD_REF_FILETYPE,\n+\t\t\t\t    \"unexpected file type\"))\n+\t\t\tret = -1;\n+\t\tgoto out;\n+\t}\n+\n+\tfor (size_t i = 0; fsck_refs_fn[i]; i++)\n+\t\tif (fsck_refs_fn[i](ref_store, o, refname, path, mode))\n+\t\t\tret = -1;\n+\n+out:\n+\treturn ret;\n+}\n+\n static int files_fsck_refs_dir(struct ref_store *ref_store,\n \t\t\t       struct fsck_options *o,\n \t\t\t       struct worktree *wt)\n@@ -3918,30 +3953,17 @@ static int files_fsck_refs_dir(struct ref_store *ref_store,\n \t}\n \n \twhile ((iter_status = dir_iterator_advance(iter)) == ITER_OK) {\n-\t\tif (S_ISDIR(iter->st.st_mode)) {\n+\t\tif (S_ISDIR(iter->st.st_mode))\n \t\t\tcontinue;\n-\t\t} else if (S_ISREG(iter->st.st_mode) ||\n-\t\t\t   S_ISLNK(iter->st.st_mode)) {\n-\t\t\tstrbuf_reset(&refname);\n-\n-\t\t\tif (!is_main_worktree(wt))\n-\t\t\t\tstrbuf_addf(&refname, \"worktrees/%s/\", wt->id);\n-\t\t\tstrbuf_addf(&refname, \"refs/%s\", iter->relative_path);\n \n-\t\t\tif (o->verbose)\n-\t\t\t\tfprintf_ln(stderr, \"Checking %s\", refname.buf);\n+\t\tstrbuf_reset(&refname);\n+\t\tif (!is_main_worktree(wt))\n+\t\t\tstrbuf_addf(&refname, \"worktrees/%s/\", wt->id);\n+\t\tstrbuf_addf(&refname, \"refs/%s\", iter->relative_path);\n \n-\t\t\tfor (size_t i = 0; fsck_refs_fn[i]; i++) {\n-\t\t\t\tif (fsck_refs_fn[i](ref_store, o, refname.buf, iter))\n-\t\t\t\t\tret = -1;\n-\t\t\t}\n-\t\t} else {\n-\t\t\tstruct fsck_ref_report report = { .path = iter->basename };\n-\t\t\tif (fsck_report_ref(o, &report,\n-\t\t\t\t\t    FSCK_MSG_BAD_REF_FILETYPE,\n-\t\t\t\t\t    \"unexpected file type\"))\n-\t\t\t\tret = -1;\n-\t\t}\n+\t\tif (files_fsck_ref(ref_store, o, refname.buf,\n+\t\t\t\t   iter->path.buf, iter->st.st_mode) < 0)\n+\t\t\tret = -1;\n \t}\n \n \tif (iter_status != ITER_DONE)\n\n-- \n2.52.0.590.g1f87b77810.dirty\n\n"},{"id":"533583","messageId":"20260112-pks-refs-verify-fixes-v2-6-2e9e453bd6c3@pks.im","threadId":"64762","inReplyTo":"20260112-pks-refs-verify-fixes-v2-0-2e9e453bd6c3@pks.im","subject":"[PATCH v2 06/17] refs/files: improve error handling when verifying symrefs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-12T09:02:55Z","receivedAt":"2026-01-12T09:03:12Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The error handling when verifying symbolic refs is a bit on the wild\nside:\n\n  - `fsck_report_ref()` can be told to ignore specific errors. If an\n    error has been ignored and a previous check raised an unignored\n    error, then assigning `ret = fsck_report_ref()` will cause us to\n    swallow the previous error.\n\n  - When the target reference is not valid we bail out early without\n    checking for other errors.\n\nFix both of these issues by consistently or'ing the return value and not\nbailing out early.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n refs/files-backend.c | 28 +++++++++++++---------------\n 1 file changed, 13 insertions(+), 15 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 9972221f9f..abc2165339 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -3737,17 +3737,15 @@ static int files_fsck_symref_target(struct fsck_options *o,\n \tif (!is_referent_root &&\n \t    !starts_with(referent->buf, \"refs/\") &&\n \t    !starts_with(referent->buf, \"worktrees/\")) {\n-\t\tret = fsck_report_ref(o, report,\n-\t\t\t\t      FSCK_MSG_SYMREF_TARGET_IS_NOT_A_REF,\n-\t\t\t\t      \"points to non-ref target '%s'\", referent->buf);\n-\n+\t\tret |= fsck_report_ref(o, report,\n+\t\t\t\t       FSCK_MSG_SYMREF_TARGET_IS_NOT_A_REF,\n+\t\t\t\t       \"points to non-ref target '%s'\", referent->buf);\n \t}\n \n \tif (!is_referent_root && check_refname_format(referent->buf, 0)) {\n-\t\tret = fsck_report_ref(o, report,\n-\t\t\t\t      FSCK_MSG_BAD_REFERENT_NAME,\n-\t\t\t\t      \"points to invalid refname '%s'\", referent->buf);\n-\t\tgoto out;\n+\t\tret |= fsck_report_ref(o, report,\n+\t\t\t\t       FSCK_MSG_BAD_REFERENT_NAME,\n+\t\t\t\t       \"points to invalid refname '%s'\", referent->buf);\n \t}\n \n \tif (symbolic_link)\n@@ -3755,19 +3753,19 @@ static int files_fsck_symref_target(struct fsck_options *o,\n \n \tif (referent->len == orig_len ||\n \t    (referent->len < orig_len && orig_last_byte != '\\n')) {\n-\t\tret = fsck_report_ref(o, report,\n-\t\t\t\t      FSCK_MSG_REF_MISSING_NEWLINE,\n-\t\t\t\t      \"misses LF at the end\");\n+\t\tret |= fsck_report_ref(o, report,\n+\t\t\t\t       FSCK_MSG_REF_MISSING_NEWLINE,\n+\t\t\t\t       \"misses LF at the end\");\n \t}\n \n \tif (referent->len != orig_len && referent->len != orig_len - 1) {\n-\t\tret = fsck_report_ref(o, report,\n-\t\t\t\t      FSCK_MSG_TRAILING_REF_CONTENT,\n-\t\t\t\t      \"has trailing whitespaces or newlines\");\n+\t\tret |= fsck_report_ref(o, report,\n+\t\t\t\t       FSCK_MSG_TRAILING_REF_CONTENT,\n+\t\t\t\t       \"has trailing whitespaces or newlines\");\n \t}\n \n out:\n-\treturn ret;\n+\treturn ret ? -1 : 0;\n }\n \n static int files_fsck_refs_content(struct ref_store *ref_store,\n\n-- \n2.52.0.590.g1f87b77810.dirty\n\n"},{"id":"533584","messageId":"20260112-pks-refs-verify-fixes-v2-7-2e9e453bd6c3@pks.im","threadId":"64762","inReplyTo":"20260112-pks-refs-verify-fixes-v2-0-2e9e453bd6c3@pks.im","subject":"[PATCH v2 07/17] refs/files: perform consistency checks for root refs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-12T09:02:56Z","receivedAt":"2026-01-12T09:03:16Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"While the \"files\" backend already knows to perform consistency checks\nfor the \"refs/\" hierarchy, it doesn't verify any of its root refs. Plug\nthis omission.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n refs/files-backend.c     | 50 +++++++++++++++++++++++++++++++++++++++++++++---\n t/t0602-reffiles-fsck.sh | 30 +++++++++++++++++++++++++++++\n 2 files changed, 77 insertions(+), 3 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex abc2165339..9ae80b700a 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -3877,9 +3877,9 @@ static int files_fsck_refs_name(struct ref_store *ref_store UNUSED,\n \tif (filename[0] != '.' && ends_with(filename, \".lock\"))\n \t\tgoto cleanup;\n \n-\t/*\n-\t * This works right now because we never check the root refs.\n-\t */\n+\tif (is_root_ref(refname))\n+\t\tgoto cleanup;\n+\n \tif (check_refname_format(refname, 0)) {\n \t\tstruct fsck_ref_report report = { 0 };\n \n@@ -3974,19 +3974,63 @@ static int files_fsck_refs_dir(struct ref_store *ref_store,\n \treturn ret;\n }\n \n+struct files_fsck_root_ref_data {\n+\tstruct files_ref_store *refs;\n+\tstruct fsck_options *o;\n+\tstruct worktree *wt;\n+\tstruct strbuf refname;\n+\tstruct strbuf path;\n+};\n+\n+static int files_fsck_root_ref(const char *refname, void *cb_data)\n+{\n+\tstruct files_fsck_root_ref_data *data = cb_data;\n+\tstruct stat st;\n+\n+\tstrbuf_reset(&data->refname);\n+\tif (!is_main_worktree(data->wt))\n+\t\tstrbuf_addf(&data->refname, \"worktrees/%s/\", data->wt->id);\n+\tstrbuf_addstr(&data->refname, refname);\n+\n+\tstrbuf_reset(&data->path);\n+\tstrbuf_addf(&data->path, \"%s/%s\", data->refs->gitcommondir, data->refname.buf);\n+\n+\tif (stat(data->path.buf, &st)) {\n+\t\tif (errno == ENOENT)\n+\t\t\treturn 0;\n+\t\treturn error_errno(\"failed to read ref: '%s'\", data->path.buf);\n+\t}\n+\n+\treturn files_fsck_ref(&data->refs->base, data->o, data->refname.buf,\n+\t\t\t      data->path.buf, st.st_mode);\n+}\n+\n static int files_fsck(struct ref_store *ref_store,\n \t\t      struct fsck_options *o,\n \t\t      struct worktree *wt)\n {\n \tstruct files_ref_store *refs =\n \t\tfiles_downcast(ref_store, REF_STORE_READ, \"fsck\");\n+\tstruct files_fsck_root_ref_data data = {\n+\t\t.refs = refs,\n+\t\t.o = o,\n+\t\t.wt = wt,\n+\t\t.refname = STRBUF_INIT,\n+\t\t.path = STRBUF_INIT,\n+\t};\n \tint ret = 0;\n \n \tif (files_fsck_refs_dir(ref_store, o, wt) < 0)\n \t\tret = -1;\n+\n+\tif (for_each_root_ref(refs, files_fsck_root_ref, &data) < 0)\n+\t\tret = -1;\n+\n \tif (refs->packed_ref_store->be->fsck(refs->packed_ref_store, o, wt) < 0)\n \t\tret = -1;\n \n+\tstrbuf_release(&data.refname);\n+\tstrbuf_release(&data.path);\n \treturn ret;\n }\n \ndiff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh\nindex 0ef483659d..479f3d528e 100755\n--- a/t/t0602-reffiles-fsck.sh\n+++ b/t/t0602-reffiles-fsck.sh\n@@ -905,4 +905,34 @@ test_expect_success '--[no-]references option should apply to fsck' '\n \t)\n '\n \n+test_expect_success 'complains about broken root ref' '\n+\ttest_when_finished \"rm -rf repo\" &&\n+\tgit init repo &&\n+\t(\n+\t\tcd repo &&\n+\t\techo \"ref: refs/../HEAD\" >.git/HEAD &&\n+\t\ttest_must_fail git refs verify 2>err &&\n+\t\tcat >expect <<-EOF &&\n+\t\terror: HEAD: badReferentName: points to invalid refname ${SQ}refs/../HEAD${SQ}\n+\t\tEOF\n+\t\ttest_cmp expect err\n+\t)\n+'\n+\n+test_expect_success 'complains about broken root ref in worktree' '\n+\ttest_when_finished \"rm -rf repo worktree\" &&\n+\tgit init repo &&\n+\t(\n+\t\tcd repo &&\n+\t\ttest_commit initial &&\n+\t\tgit worktree add ../worktree &&\n+\t\techo \"ref: refs/../HEAD\" >.git/worktrees/worktree/HEAD &&\n+\t\ttest_must_fail git refs verify 2>err &&\n+\t\tcat >expect <<-EOF &&\n+\t\terror: worktrees/worktree/HEAD: badReferentName: points to invalid refname ${SQ}refs/../HEAD${SQ}\n+\t\tEOF\n+\t\ttest_cmp expect err\n+\t)\n+'\n+\n test_done\n\n-- \n2.52.0.590.g1f87b77810.dirty\n\n"},{"id":"533585","messageId":"20260112-pks-refs-verify-fixes-v2-8-2e9e453bd6c3@pks.im","threadId":"64762","inReplyTo":"20260112-pks-refs-verify-fixes-v2-0-2e9e453bd6c3@pks.im","subject":"[PATCH v2 08/17] fsck: drop unused fields from `struct fsck_ref_report`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-12T09:02:57Z","receivedAt":"2026-01-12T09:03:20Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `struct fsck_ref_report` has a couple fields that are intended to\nimprove the error reporting for broken ref reports by showing which\nobject ID or target reference the ref points to. These fields are never\nset though and are thus essentially unused.\n\nRemove them.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n fsck.c | 5 -----\n fsck.h | 2 --\n 2 files changed, 7 deletions(-)\n\ndiff --git a/fsck.c b/fsck.c\nindex fae18d8561..813d927d57 100644\n--- a/fsck.c\n+++ b/fsck.c\n@@ -1310,11 +1310,6 @@ int fsck_refs_error_function(struct fsck_options *options UNUSED,\n \n \tstrbuf_addstr(&sb, report->path);\n \n-\tif (report->oid)\n-\t\tstrbuf_addf(&sb, \" -> (%s)\", oid_to_hex(report->oid));\n-\telse if (report->referent)\n-\t\tstrbuf_addf(&sb, \" -> (%s)\", report->referent);\n-\n \tif (msg_type == FSCK_WARN)\n \t\twarning(\"%s: %s\", sb.buf, message);\n \telse\ndiff --git a/fsck.h b/fsck.h\nindex 336917c045..bfe0d9c6d2 100644\n--- a/fsck.h\n+++ b/fsck.h\n@@ -162,8 +162,6 @@ struct fsck_object_report {\n \n struct fsck_ref_report {\n \tconst char *path;\n-\tconst struct object_id *oid;\n-\tconst char *referent;\n };\n \n struct fsck_options {\n\n-- \n2.52.0.590.g1f87b77810.dirty\n\n"},{"id":"533586","messageId":"20260112-pks-refs-verify-fixes-v2-9-2e9e453bd6c3@pks.im","threadId":"64762","inReplyTo":"20260112-pks-refs-verify-fixes-v2-0-2e9e453bd6c3@pks.im","subject":"[PATCH v2 09/17] refs/files: extract generic symref target checks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-12T09:02:58Z","receivedAt":"2026-01-12T09:03:21Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The consistency checks for the \"files\" backend contain a couple of\nverifications for symrefs that verify generic properties of the target\nreference. These properties need to hold for every backend, no matter\nwhether it's using the \"files\" or \"reftable\" backend.\n\nReimplementing these checks for every single backend doesn't really make\nsense. Extract it into a generic `refs_fsck_symref()` function that can\nbe used by other backends, as well. The \"reftable\" backend will be wired\nup in a subsequent commit.\n\nWhile at it, improve the consistency checks so that we don't complain\nabout refs pointing to a non-ref target in case the target refname\nformat does not verify. Otherwise it's very likely that we'll generate\nboth error messages, which feels somewhat redundant in this case.\n\nNote that the function has a couple of `UNUSED` parameters. These will\nbecome referenced in a subsequent commit.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n refs.c               | 21 ++++++++++++++++++++\n refs.h               | 10 ++++++++++\n refs/files-backend.c | 54 ++++++++++++++++++++--------------------------------\n 3 files changed, 52 insertions(+), 33 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex e06e0cb072..739bf9fefc 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -320,6 +320,27 @@ int check_refname_format(const char *refname, int flags)\n \treturn check_or_sanitize_refname(refname, flags, NULL);\n }\n \n+int refs_fsck_symref(struct ref_store *refs UNUSED, struct fsck_options *o,\n+\t\t     struct fsck_ref_report *report,\n+\t\t     const char *refname UNUSED, const char *target)\n+{\n+\tif (is_root_ref(target))\n+\t\treturn 0;\n+\n+\tif (check_refname_format(target, 0) &&\n+\t    fsck_report_ref(o, report, FSCK_MSG_BAD_REFERENT_NAME,\n+\t\t\t    \"points to invalid refname '%s'\", target))\n+\t\treturn -1;\n+\n+\tif (!starts_with(target, \"refs/\") &&\n+\t    !starts_with(target, \"worktrees/\") &&\n+\t    fsck_report_ref(o, report, FSCK_MSG_SYMREF_TARGET_IS_NOT_A_REF,\n+\t\t\t    \"points to non-ref target '%s'\", target))\n+\t\treturn -1;\n+\n+\treturn 0;\n+}\n+\n int refs_fsck(struct ref_store *refs, struct fsck_options *o,\n \t      struct worktree *wt)\n {\ndiff --git a/refs.h b/refs.h\nindex d9051bbb04..d91fcb2d2f 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -653,6 +653,16 @@ int refs_for_each_reflog(struct ref_store *refs, each_reflog_fn fn, void *cb_dat\n  */\n int check_refname_format(const char *refname, int flags);\n \n+struct fsck_ref_report;\n+\n+/*\n+ * Perform generic checks for a specific symref target. This function is\n+ * expected to be called by the ref backends for every symbolic ref.\n+ */\n+int refs_fsck_symref(struct ref_store *refs, struct fsck_options *o,\n+\t\t     struct fsck_ref_report *report,\n+\t\t     const char *refname, const char *target);\n+\n /*\n  * Check the reference database for consistency. Return 0 if refs and\n  * reflogs are consistent, and non-zero otherwise. The errors will be\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 9ae80b700a..687c26ddcb 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -3718,53 +3718,39 @@ typedef int (*files_fsck_refs_fn)(struct ref_store *ref_store,\n \t\t\t\t  const char *path,\n \t\t\t\t  int mode);\n \n-static int files_fsck_symref_target(struct fsck_options *o,\n+static int files_fsck_symref_target(struct ref_store *ref_store,\n+\t\t\t\t    struct fsck_options *o,\n \t\t\t\t    struct fsck_ref_report *report,\n+\t\t\t\t    const char *refname,\n \t\t\t\t    struct strbuf *referent,\n \t\t\t\t    unsigned int symbolic_link)\n {\n-\tint is_referent_root;\n \tchar orig_last_byte;\n \tsize_t orig_len;\n \tint ret = 0;\n \n \torig_len = referent->len;\n \torig_last_byte = referent->buf[orig_len - 1];\n-\tif (!symbolic_link)\n-\t\tstrbuf_rtrim(referent);\n-\n-\tis_referent_root = is_root_ref(referent->buf);\n-\tif (!is_referent_root &&\n-\t    !starts_with(referent->buf, \"refs/\") &&\n-\t    !starts_with(referent->buf, \"worktrees/\")) {\n-\t\tret |= fsck_report_ref(o, report,\n-\t\t\t\t       FSCK_MSG_SYMREF_TARGET_IS_NOT_A_REF,\n-\t\t\t\t       \"points to non-ref target '%s'\", referent->buf);\n-\t}\n \n-\tif (!is_referent_root && check_refname_format(referent->buf, 0)) {\n-\t\tret |= fsck_report_ref(o, report,\n-\t\t\t\t       FSCK_MSG_BAD_REFERENT_NAME,\n-\t\t\t\t       \"points to invalid refname '%s'\", referent->buf);\n-\t}\n+\tif (!symbolic_link) {\n+\t\tstrbuf_rtrim(referent);\n \n-\tif (symbolic_link)\n-\t\tgoto out;\n+\t\tif (referent->len == orig_len ||\n+\t\t    (referent->len < orig_len && orig_last_byte != '\\n')) {\n+\t\t\tret |= fsck_report_ref(o, report,\n+\t\t\t\t\t       FSCK_MSG_REF_MISSING_NEWLINE,\n+\t\t\t\t\t       \"misses LF at the end\");\n+\t\t}\n \n-\tif (referent->len == orig_len ||\n-\t    (referent->len < orig_len && orig_last_byte != '\\n')) {\n-\t\tret |= fsck_report_ref(o, report,\n-\t\t\t\t       FSCK_MSG_REF_MISSING_NEWLINE,\n-\t\t\t\t       \"misses LF at the end\");\n+\t\tif (referent->len != orig_len && referent->len != orig_len - 1) {\n+\t\t\tret |= fsck_report_ref(o, report,\n+\t\t\t\t\t       FSCK_MSG_TRAILING_REF_CONTENT,\n+\t\t\t\t\t       \"has trailing whitespaces or newlines\");\n+\t\t}\n \t}\n \n-\tif (referent->len != orig_len && referent->len != orig_len - 1) {\n-\t\tret |= fsck_report_ref(o, report,\n-\t\t\t\t       FSCK_MSG_TRAILING_REF_CONTENT,\n-\t\t\t\t       \"has trailing whitespaces or newlines\");\n-\t}\n+\tret |= refs_fsck_symref(ref_store, o, report, refname, referent->buf);\n \n-out:\n \treturn ret ? -1 : 0;\n }\n \n@@ -3807,7 +3793,8 @@ static int files_fsck_refs_content(struct ref_store *ref_store,\n \t\telse\n \t\t\tstrbuf_addbuf(&referent, &ref_content);\n \n-\t\tret |= files_fsck_symref_target(o, &report, &referent, 1);\n+\t\tret |= files_fsck_symref_target(ref_store, o, &report,\n+\t\t\t\t\t\ttarget_name, &referent, 1);\n \t\tgoto cleanup;\n \t}\n \n@@ -3847,7 +3834,8 @@ static int files_fsck_refs_content(struct ref_store *ref_store,\n \t\t\tgoto cleanup;\n \t\t}\n \t} else {\n-\t\tret = files_fsck_symref_target(o, &report, &referent, 0);\n+\t\tret = files_fsck_symref_target(ref_store, o, &report,\n+\t\t\t\t\t       target_name, &referent, 0);\n \t\tgoto cleanup;\n \t}\n \n\n-- \n2.52.0.590.g1f87b77810.dirty\n\n"},{"id":"533587","messageId":"20260112-pks-refs-verify-fixes-v2-10-2e9e453bd6c3@pks.im","threadId":"64762","inReplyTo":"20260112-pks-refs-verify-fixes-v2-0-2e9e453bd6c3@pks.im","subject":"[PATCH v2 10/17] refs/files: introduce function to perform normal ref checks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-12T09:02:59Z","receivedAt":"2026-01-12T09:03:23Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In a subsequent commit we'll introduce new generic checks for direct\nrefs. These checks will be independent of the actual backend.\n\nIntroduce a new function `refs_fsck_ref()` that will be used for this\npurpose. At the current point in time it's still empty, but it will get\npopulated in a subsequent commit.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n refs.c               | 7 +++++++\n refs.h               | 8 ++++++++\n refs/files-backend.c | 2 ++\n 3 files changed, 17 insertions(+)\n\ndiff --git a/refs.c b/refs.c\nindex 739bf9fefc..4fc1317cb3 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -320,6 +320,13 @@ int check_refname_format(const char *refname, int flags)\n \treturn check_or_sanitize_refname(refname, flags, NULL);\n }\n \n+int refs_fsck_ref(struct ref_store *refs UNUSED, struct fsck_options *o UNUSED,\n+\t\t  struct fsck_ref_report *report UNUSED,\n+\t\t  const char *refname UNUSED, const struct object_id *oid UNUSED)\n+{\n+\treturn 0;\n+}\n+\n int refs_fsck_symref(struct ref_store *refs UNUSED, struct fsck_options *o,\n \t\t     struct fsck_ref_report *report,\n \t\t     const char *refname UNUSED, const char *target)\ndiff --git a/refs.h b/refs.h\nindex d91fcb2d2f..f0abfa1d93 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -655,6 +655,14 @@ int check_refname_format(const char *refname, int flags);\n \n struct fsck_ref_report;\n \n+/*\n+ * Perform generic checks for a specific direct ref. This function is\n+ * expected to be called by the ref backends for every symbolic ref.\n+ */\n+int refs_fsck_ref(struct ref_store *refs, struct fsck_options *o,\n+\t\t  struct fsck_ref_report *report,\n+\t\t  const char *refname, const struct object_id *oid);\n+\n /*\n  * Perform generic checks for a specific symref target. This function is\n  * expected to be called by the ref backends for every symbolic ref.\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 687c26ddcb..240d3c3b26 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -3833,6 +3833,8 @@ static int files_fsck_refs_content(struct ref_store *ref_store,\n \t\t\t\t\t      \"has trailing garbage: '%s'\", trailing);\n \t\t\tgoto cleanup;\n \t\t}\n+\n+\t\tret = refs_fsck_ref(ref_store, o, &report, target_name, &oid);\n \t} else {\n \t\tret = files_fsck_symref_target(ref_store, o, &report,\n \t\t\t\t\t       target_name, &referent, 0);\n\n-- \n2.52.0.590.g1f87b77810.dirty\n\n"},{"id":"533588","messageId":"20260112-pks-refs-verify-fixes-v2-11-2e9e453bd6c3@pks.im","threadId":"64762","inReplyTo":"20260112-pks-refs-verify-fixes-v2-0-2e9e453bd6c3@pks.im","subject":"[PATCH v2 11/17] refs/reftable: adapt includes to become consistent","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-12T09:03:00Z","receivedAt":"2026-01-12T09:03:25Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Adapt the includes to be sorted and to use include paths that are\nrelative to the \"refs/\" directory.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n refs/reftable-backend.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/refs/reftable-backend.c b/refs/reftable-backend.c\nindex 4319a4eacb..d61790cf65 100644\n--- a/refs/reftable-backend.c\n+++ b/refs/reftable-backend.c\n@@ -10,9 +10,10 @@\n #include \"../gettext.h\"\n #include \"../hash.h\"\n #include \"../hex.h\"\n-#include \"../iterator.h\"\n #include \"../ident.h\"\n+#include \"../iterator.h\"\n #include \"../object.h\"\n+#include \"../parse.h\"\n #include \"../path.h\"\n #include \"../refs.h\"\n #include \"../reftable/reftable-basics.h\"\n@@ -26,7 +27,6 @@\n #include \"../strmap.h\"\n #include \"../trace2.h\"\n #include \"../write-or-die.h\"\n-#include \"parse.h\"\n #include \"refs-internal.h\"\n \n /*\n\n-- \n2.52.0.590.g1f87b77810.dirty\n\n"},{"id":"533589","messageId":"20260112-pks-refs-verify-fixes-v2-12-2e9e453bd6c3@pks.im","threadId":"64762","inReplyTo":"20260112-pks-refs-verify-fixes-v2-0-2e9e453bd6c3@pks.im","subject":"[PATCH v2 12/17] refs/reftable: extract function to retrieve backend for worktree","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-12T09:03:01Z","receivedAt":"2026-01-12T09:03:29Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Pull out the logic to retrieve a backend for a given worktree. This\nfunction will be used in a subsequent commit.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n refs/reftable-backend.c | 70 ++++++++++++++++++++++++++++++-------------------\n 1 file changed, 43 insertions(+), 27 deletions(-)\n\ndiff --git a/refs/reftable-backend.c b/refs/reftable-backend.c\nindex d61790cf65..dda961a32b 100644\n--- a/refs/reftable-backend.c\n+++ b/refs/reftable-backend.c\n@@ -172,6 +172,37 @@ static struct reftable_ref_store *reftable_be_downcast(struct ref_store *ref_sto\n \treturn refs;\n }\n \n+static int backend_for_worktree(struct reftable_backend **out,\n+\t\t\t\tstruct reftable_ref_store *store,\n+\t\t\t\tconst char *worktree_name)\n+{\n+\tstruct strbuf worktree_dir = STRBUF_INIT;\n+\tint ret;\n+\n+\t*out = strmap_get(&store->worktree_backends, worktree_name);\n+\tif (*out) {\n+\t\tret = 0;\n+\t\tgoto out;\n+\t}\n+\n+\tstrbuf_addf(&worktree_dir, \"%s/worktrees/%s/reftable\",\n+\t\t    store->base.repo->commondir, worktree_name);\n+\n+\tCALLOC_ARRAY(*out, 1);\n+\tstore->err = ret = reftable_backend_init(*out, worktree_dir.buf,\n+\t\t\t\t\t\t &store->write_options);\n+\tif (ret < 0) {\n+\t\tfree(*out);\n+\t\tgoto out;\n+\t}\n+\n+\tstrmap_put(&store->worktree_backends, worktree_name, *out);\n+\n+out:\n+\tstrbuf_release(&worktree_dir);\n+\treturn ret;\n+}\n+\n /*\n  * Some refs are global to the repository (refs/heads/{*}), while others are\n  * local to the worktree (eg. HEAD, refs/bisect/{*}). We solve this by having\n@@ -191,19 +222,19 @@ static int backend_for(struct reftable_backend **out,\n \t\t       const char **rewritten_ref,\n \t\t       int reload)\n {\n-\tstruct reftable_backend *be;\n \tconst char *wtname;\n \tint wtname_len;\n+\tint ret;\n \n \tif (!refname) {\n-\t\tbe = &store->main_backend;\n+\t\t*out = &store->main_backend;\n+\t\tret = 0;\n \t\tgoto out;\n \t}\n \n \tswitch (parse_worktree_ref(refname, &wtname, &wtname_len, rewritten_ref)) {\n \tcase REF_WORKTREE_OTHER: {\n \t\tstatic struct strbuf wtname_buf = STRBUF_INIT;\n-\t\tstruct strbuf wt_dir = STRBUF_INIT;\n \n \t\t/*\n \t\t * We're using a static buffer here so that we don't need to\n@@ -223,20 +254,8 @@ static int backend_for(struct reftable_backend **out,\n \t\t * already and error out when trying to write a reference via\n \t\t * both stacks.\n \t\t */\n-\t\tbe = strmap_get(&store->worktree_backends, wtname_buf.buf);\n-\t\tif (!be) {\n-\t\t\tstrbuf_addf(&wt_dir, \"%s/worktrees/%s/reftable\",\n-\t\t\t\t    store->base.repo->commondir, wtname_buf.buf);\n+\t\tret = backend_for_worktree(out, store, wtname_buf.buf);\n \n-\t\t\tCALLOC_ARRAY(be, 1);\n-\t\t\tstore->err = reftable_backend_init(be, wt_dir.buf,\n-\t\t\t\t\t\t\t   &store->write_options);\n-\t\t\tassert(store->err != REFTABLE_API_ERROR);\n-\n-\t\t\tstrmap_put(&store->worktree_backends, wtname_buf.buf, be);\n-\t\t}\n-\n-\t\tstrbuf_release(&wt_dir);\n \t\tgoto out;\n \t}\n \tcase REF_WORKTREE_CURRENT:\n@@ -245,27 +264,24 @@ static int backend_for(struct reftable_backend **out,\n \t\t * main worktree. We thus return the main stack in that case.\n \t\t */\n \t\tif (!store->worktree_backend.stack)\n-\t\t\tbe = &store->main_backend;\n+\t\t\t*out = &store->main_backend;\n \t\telse\n-\t\t\tbe = &store->worktree_backend;\n+\t\t\t*out = &store->worktree_backend;\n+\t\tret = 0;\n \t\tgoto out;\n \tcase REF_WORKTREE_MAIN:\n \tcase REF_WORKTREE_SHARED:\n-\t\tbe = &store->main_backend;\n+\t\t*out = &store->main_backend;\n+\t\tret = 0;\n \t\tgoto out;\n \tdefault:\n \t\tBUG(\"unhandled worktree reference type\");\n \t}\n \n out:\n-\tif (reload) {\n-\t\tint ret = reftable_stack_reload(be->stack);\n-\t\tif (ret)\n-\t\t\treturn ret;\n-\t}\n-\t*out = be;\n-\n-\treturn 0;\n+\tif (reload && !ret)\n+\t\tret = reftable_stack_reload((*out)->stack);\n+\treturn ret;\n }\n \n static int should_write_log(struct reftable_ref_store *refs, const char *refname)\n\n-- \n2.52.0.590.g1f87b77810.dirty\n\n"},{"id":"533590","messageId":"20260112-pks-refs-verify-fixes-v2-13-2e9e453bd6c3@pks.im","threadId":"64762","inReplyTo":"20260112-pks-refs-verify-fixes-v2-0-2e9e453bd6c3@pks.im","subject":"[PATCH v2 13/17] refs/reftable: fix consistency checks with worktrees","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-12T09:03:02Z","receivedAt":"2026-01-12T09:03:31Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The ref consistency checks are driven via `cmd_refs_verify()`. That\nfunction loops through all worktrees (including the main worktree) and\nthen checks the ref store for each of them individually. It follows that\nthe backend is expected to only verify refs that belong to the specified\nworktree.\n\nWhile the \"files\" backend handles this correctly, the \"reftable\" backend\ndoesn't. In fact, it completely ignores the passed worktree and instead\nverifies refs of _all_ worktrees. The consequence is that we'll end up\nevery ref store N times, where N is the number of worktrees.\n\nOr rather, that would be the case if we actually iterated through the\nworktree reftable stacks correctly. But we use `strmap_for_each_entry()`\nto iterate through the stacks, but the map is in fact not even properly\npopulated. So instead of checking stacks N^2 times, we actually only end\nup checking the reftable stack of the main worktree.\n\nFix this bug by only verifying the stack of the passed-in worktree and\nconstructing the backends via `backend_for_worktree()`.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n refs/reftable-backend.c  | 29 ++++++++++++++---------------\n t/t0614-reftable-fsck.sh | 32 ++++++++++++++++++++++++++++++++\n 2 files changed, 46 insertions(+), 15 deletions(-)\n\ndiff --git a/refs/reftable-backend.c b/refs/reftable-backend.c\nindex dda961a32b..6361b27015 100644\n--- a/refs/reftable-backend.c\n+++ b/refs/reftable-backend.c\n@@ -26,6 +26,7 @@\n #include \"../setup.h\"\n #include \"../strmap.h\"\n #include \"../trace2.h\"\n+#include \"../worktree.h\"\n #include \"../write-or-die.h\"\n #include \"refs-internal.h\"\n \n@@ -2762,25 +2763,23 @@ static int reftable_fsck_error_handler(struct reftable_fsck_info *info,\n }\n \n static int reftable_be_fsck(struct ref_store *ref_store, struct fsck_options *o,\n-\t\t\t    struct worktree *wt UNUSED)\n+\t\t\t    struct worktree *wt)\n {\n-\tstruct reftable_ref_store *refs;\n-\tstruct strmap_entry *entry;\n-\tstruct hashmap_iter iter;\n-\tint ret = 0;\n-\n-\trefs = reftable_be_downcast(ref_store, REF_STORE_READ, \"fsck\");\n-\n-\tret |= reftable_fsck_check(refs->main_backend.stack, reftable_fsck_error_handler,\n-\t\t\t\t   reftable_fsck_verbose_handler, o);\n+\tstruct reftable_ref_store *refs =\n+\t\treftable_be_downcast(ref_store, REF_STORE_READ, \"fsck\");\n+\tstruct reftable_backend *backend;\n \n-\tstrmap_for_each_entry(&refs->worktree_backends, &iter, entry) {\n-\t\tstruct reftable_backend *b = (struct reftable_backend *)entry->value;\n-\t\tret |= reftable_fsck_check(b->stack, reftable_fsck_error_handler,\n-\t\t\t\t\t   reftable_fsck_verbose_handler, o);\n+\tif (is_main_worktree(wt)) {\n+\t\tbackend = &refs->main_backend;\n+\t} else {\n+\t\tint ret = backend_for_worktree(&backend, refs, wt->id);\n+\t\tif (ret < 0)\n+\t\t\treturn error(_(\"reftable stack for worktree '%s' is broken\"),\n+\t\t\t\t     wt->id);\n \t}\n \n-\treturn ret;\n+\treturn reftable_fsck_check(backend->stack, reftable_fsck_error_handler,\n+\t\t\t\t   reftable_fsck_verbose_handler, o);\n }\n \n struct ref_storage_be refs_be_reftable = {\ndiff --git a/t/t0614-reftable-fsck.sh b/t/t0614-reftable-fsck.sh\nindex 677eb9143c..4757eb5931 100755\n--- a/t/t0614-reftable-fsck.sh\n+++ b/t/t0614-reftable-fsck.sh\n@@ -55,4 +55,36 @@ for TABLE_NAME in \"foo-bar-e4d12d59.ref\" \\\n \t'\n done\n \n+test_expect_success 'worktree stacks can be verified' '\n+\ttest_when_finished \"rm -rf repo worktree\" &&\n+\tgit init repo &&\n+\ttest_commit -C repo initial &&\n+\tgit -C repo worktree add ../worktree &&\n+\n+\tgit -C worktree refs verify 2>err &&\n+\ttest_must_be_empty err &&\n+\n+\tREFTABLE_DIR=$(git -C worktree rev-parse --git-dir)/reftable &&\n+\tEXISTING_TABLE=$(head -n1 \"$REFTABLE_DIR/tables.list\") &&\n+\tmv \"$REFTABLE_DIR/$EXISTING_TABLE\" \"$REFTABLE_DIR/broken.ref\" &&\n+\n+\tfor d in repo worktree\n+\tdo\n+\t\techo \"broken.ref\" >\"$REFTABLE_DIR/tables.list\" &&\n+\t\tgit -C \"$d\" refs verify 2>err &&\n+\t\tcat >expect <<-EOF &&\n+\t\twarning: broken.ref: badReftableTableName: invalid reftable table name\n+\t\tEOF\n+\t\ttest_cmp expect err &&\n+\n+\t\techo garbage >\"$REFTABLE_DIR/tables.list\" &&\n+\t\ttest_must_fail git -C \"$d\" refs verify 2>err &&\n+\t\tcat >expect <<-EOF &&\n+\t\terror: reftable stack for worktree ${SQ}worktree${SQ} is broken\n+\t\tEOF\n+\t\ttest_cmp expect err || return 1\n+\n+\tdone\n+'\n+\n test_done\n\n-- \n2.52.0.590.g1f87b77810.dirty\n\n"},{"id":"533591","messageId":"20260112-pks-refs-verify-fixes-v2-14-2e9e453bd6c3@pks.im","threadId":"64762","inReplyTo":"20260112-pks-refs-verify-fixes-v2-0-2e9e453bd6c3@pks.im","subject":"[PATCH v2 14/17] refs/reftable: introduce generic checks for refs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-12T09:03:03Z","receivedAt":"2026-01-12T09:03:34Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In a preceding commit we have extracted generic checks for both direct\nand symbolic refs that apply for all backends. Wire up those checks for\nthe \"reftable\" backend.\n\nNote that this is done by iterating through all refs manually with the\nlow-level reftable ref iterator. We explicitly don't want to use the\nhigher-level iterator that is exposed to users of the reftable backend\nas that iterator may swallow for example broken refs.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n refs/reftable-backend.c  | 82 ++++++++++++++++++++++++++++++++++++++++++++----\n t/t0614-reftable-fsck.sh | 12 +++++++\n 2 files changed, 88 insertions(+), 6 deletions(-)\n\ndiff --git a/refs/reftable-backend.c b/refs/reftable-backend.c\nindex 6361b27015..fe74af73af 100644\n--- a/refs/reftable-backend.c\n+++ b/refs/reftable-backend.c\n@@ -2767,19 +2767,89 @@ static int reftable_be_fsck(struct ref_store *ref_store, struct fsck_options *o,\n {\n \tstruct reftable_ref_store *refs =\n \t\treftable_be_downcast(ref_store, REF_STORE_READ, \"fsck\");\n+\tstruct reftable_ref_iterator *iter = NULL;\n+\tstruct reftable_ref_record ref = { 0 };\n+\tstruct fsck_ref_report report = { 0 };\n+\tstruct strbuf refname = STRBUF_INIT;\n \tstruct reftable_backend *backend;\n+\tint ret, errors = 0;\n \n \tif (is_main_worktree(wt)) {\n \t\tbackend = &refs->main_backend;\n \t} else {\n-\t\tint ret = backend_for_worktree(&backend, refs, wt->id);\n-\t\tif (ret < 0)\n-\t\t\treturn error(_(\"reftable stack for worktree '%s' is broken\"),\n-\t\t\t\t     wt->id);\n+\t\tret = backend_for_worktree(&backend, refs, wt->id);\n+\t\tif (ret < 0) {\n+\t\t\tret = error(_(\"reftable stack for worktree '%s' is broken\"),\n+\t\t\t\t    wt->id);\n+\t\t\tgoto out;\n+\t\t}\n+\t}\n+\n+\terrors |= reftable_fsck_check(backend->stack, reftable_fsck_error_handler,\n+\t\t\t\t      reftable_fsck_verbose_handler, o);\n+\n+\titer = ref_iterator_for_stack(refs, backend->stack, \"\", NULL, 0);\n+\tif (!iter) {\n+\t\tret = error(_(\"could not create iterator for worktree '%s'\"), wt->id);\n+\t\tgoto out;\n+\t}\n+\n+\twhile (1) {\n+\t\tret = reftable_iterator_next_ref(&iter->iter, &ref);\n+\t\tif (ret > 0)\n+\t\t\tbreak;\n+\t\tif (ret < 0) {\n+\t\t\tret = error(_(\"could not read record for worktree '%s'\"), wt->id);\n+\t\t\tgoto out;\n+\t\t}\n+\n+\t\tstrbuf_reset(&refname);\n+\t\tif (!is_main_worktree(wt))\n+\t\t\tstrbuf_addf(&refname, \"worktrees/%s/\", wt->id);\n+\t\tstrbuf_addstr(&refname, ref.refname);\n+\t\treport.path = refname.buf;\n+\n+\t\tswitch (ref.value_type) {\n+\t\tcase REFTABLE_REF_VAL1:\n+\t\tcase REFTABLE_REF_VAL2: {\n+\t\t\tstruct object_id oid;\n+\t\t\tunsigned hash_id;\n+\n+\t\t\tswitch (reftable_stack_hash_id(backend->stack)) {\n+\t\t\tcase REFTABLE_HASH_SHA1:\n+\t\t\t\thash_id = GIT_HASH_SHA1;\n+\t\t\t\tbreak;\n+\t\t\tcase REFTABLE_HASH_SHA256:\n+\t\t\t\thash_id = GIT_HASH_SHA256;\n+\t\t\t\tbreak;\n+\t\t\tdefault:\n+\t\t\t\tBUG(\"unhandled hash ID %d\",\n+\t\t\t\t    reftable_stack_hash_id(backend->stack));\n+\t\t\t}\n+\n+\t\t\toidread(&oid, reftable_ref_record_val1(&ref),\n+\t\t\t\t&hash_algos[hash_id]);\n+\n+\t\t\terrors |= refs_fsck_ref(ref_store, o, &report, ref.refname, &oid);\n+\t\t\tbreak;\n+\t\t}\n+\t\tcase REFTABLE_REF_SYMREF:\n+\t\t\terrors |= refs_fsck_symref(ref_store, o, &report, ref.refname,\n+\t\t\t\t\t\t   ref.value.symref);\n+\t\t\tbreak;\n+\t\tdefault:\n+\t\t\tBUG(\"unhandled reference value type %d\", ref.value_type);\n+\t\t}\n \t}\n \n-\treturn reftable_fsck_check(backend->stack, reftable_fsck_error_handler,\n-\t\t\t\t   reftable_fsck_verbose_handler, o);\n+\tret = errors ? -1 : 0;\n+\n+out:\n+\tif (iter)\n+\t\tref_iterator_free(&iter->base);\n+\treftable_ref_record_release(&ref);\n+\tstrbuf_release(&refname);\n+\treturn ret;\n }\n \n struct ref_storage_be refs_be_reftable = {\ndiff --git a/t/t0614-reftable-fsck.sh b/t/t0614-reftable-fsck.sh\nindex 4757eb5931..d24b87f961 100755\n--- a/t/t0614-reftable-fsck.sh\n+++ b/t/t0614-reftable-fsck.sh\n@@ -87,4 +87,16 @@ test_expect_success 'worktree stacks can be verified' '\n \tdone\n '\n \n+test_expect_success 'invalid symref gets reported' '\n+\ttest_when_finished \"rm -rf repo\" &&\n+\tgit init repo &&\n+\ttest_commit -C repo initial &&\n+\tgit -C repo symbolic-ref refs/heads/symref garbage &&\n+\ttest_must_fail git -C repo refs verify 2>err &&\n+\tcat >expect <<-EOF &&\n+\terror: refs/heads/symref: badReferentName: points to invalid refname ${SQ}garbage${SQ}\n+\tEOF\n+\ttest_cmp expect err\n+'\n+\n test_done\n\n-- \n2.52.0.590.g1f87b77810.dirty\n\n"},{"id":"533592","messageId":"20260112-pks-refs-verify-fixes-v2-15-2e9e453bd6c3@pks.im","threadId":"64762","inReplyTo":"20260112-pks-refs-verify-fixes-v2-0-2e9e453bd6c3@pks.im","subject":"[PATCH v2 15/17] builtin/fsck: move generic object ID checks into `refs_fsck()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-12T09:03:04Z","receivedAt":"2026-01-12T09:03:36Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"While most of the logic that verifies the consistency of refs is\ndriven by `refs_fsck()`, we still have a small handful of checks in\n`fsck_head_link()`. These checks don't use the git-fsck(1) reporting\ninfrastructure, and as such it's impossible to for example disable\nsome of those checks.\n\nOne such check detects refs that point to the all-zeroes object ID.\nExtract this check into the generic `refs_fsck_ref()` function that is\nused by both the \"files\" and \"reftable\" backends.\n\nNote that this will cause us to not return an error code from\n`fsck_head_link()` anymore in case this error was detected. This is fine\nthough: the only caller of this function does not check the error code\nanyway. To demonstrate this, adapt the function to drop its return value\naltogether. The function will be removed in a subsequent commit anyway.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Documentation/fsck-msgids.adoc |  3 +++\n builtin/fsck.c                 | 41 +++++++++++++++--------------------------\n fsck.h                         |  1 +\n refs.c                         | 11 ++++++++---\n t/t1450-fsck.sh                |  6 +++---\n 5 files changed, 30 insertions(+), 32 deletions(-)\n\ndiff --git a/Documentation/fsck-msgids.adoc b/Documentation/fsck-msgids.adoc\nindex acac9683af..76609321f6 100644\n--- a/Documentation/fsck-msgids.adoc\n+++ b/Documentation/fsck-msgids.adoc\n@@ -41,6 +41,9 @@\n `badRefName`::\n \t(ERROR) A ref has an invalid format.\n \n+`badRefOid`::\n+\t(ERROR) A ref points to an invalid object ID.\n+\n `badReferentName`::\n \t(ERROR) The referent name of a symref is invalid.\n \ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex 4979bc795e..4dd4d74d1e 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -564,9 +564,9 @@ static int fsck_handle_ref(const struct reference *ref, void *cb_data UNUSED)\n \treturn 0;\n }\n \n-static int fsck_head_link(const char *head_ref_name,\n-\t\t\t  const char **head_points_at,\n-\t\t\t  struct object_id *head_oid);\n+static void fsck_head_link(const char *head_ref_name,\n+\t\t\t   const char **head_points_at,\n+\t\t\t   struct object_id *head_oid);\n \n static void get_default_heads(void)\n {\n@@ -713,12 +713,10 @@ static void fsck_source(struct odb_source *source)\n \tstop_progress(&progress);\n }\n \n-static int fsck_head_link(const char *head_ref_name,\n-\t\t\t  const char **head_points_at,\n-\t\t\t  struct object_id *head_oid)\n+static void fsck_head_link(const char *head_ref_name,\n+\t\t\t   const char **head_points_at,\n+\t\t\t   struct object_id *head_oid)\n {\n-\tint null_is_error = 0;\n-\n \tif (verbose)\n \t\tfprintf_ln(stderr, _(\"Checking %s link\"), head_ref_name);\n \n@@ -727,27 +725,18 @@ static int fsck_head_link(const char *head_ref_name,\n \t\t\t\t\t\t  NULL);\n \tif (!*head_points_at) {\n \t\terrors_found |= ERROR_REFS;\n-\t\treturn error(_(\"invalid %s\"), head_ref_name);\n+\t\terror(_(\"invalid %s\"), head_ref_name);\n+\t\treturn;\n \t}\n-\tif (!strcmp(*head_points_at, head_ref_name))\n-\t\t/* detached HEAD */\n-\t\tnull_is_error = 1;\n-\telse if (!starts_with(*head_points_at, \"refs/heads/\")) {\n+\tif (strcmp(*head_points_at, head_ref_name) &&\n+\t    !starts_with(*head_points_at, \"refs/heads/\")) {\n \t\terrors_found |= ERROR_REFS;\n-\t\treturn error(_(\"%s points to something strange (%s)\"),\n-\t\t\t     head_ref_name, *head_points_at);\n-\t}\n-\tif (is_null_oid(head_oid)) {\n-\t\tif (null_is_error) {\n-\t\t\terrors_found |= ERROR_REFS;\n-\t\t\treturn error(_(\"%s: detached HEAD points at nothing\"),\n-\t\t\t\t     head_ref_name);\n-\t\t}\n-\t\tfprintf_ln(stderr,\n-\t\t\t   _(\"notice: %s points to an unborn branch (%s)\"),\n-\t\t\t   head_ref_name, *head_points_at + 11);\n+\t\terror(_(\"%s points to something strange (%s)\"),\n+\t\t      head_ref_name, *head_points_at);\n+\t\treturn;\n \t}\n-\treturn 0;\n+\n+\treturn;\n }\n \n static int fsck_cache_tree(struct cache_tree *it, const char *index_path)\ndiff --git a/fsck.h b/fsck.h\nindex bfe0d9c6d2..1f472b7daa 100644\n--- a/fsck.h\n+++ b/fsck.h\n@@ -39,6 +39,7 @@ enum fsck_msg_type {\n \tFUNC(BAD_REF_CONTENT, ERROR) \\\n \tFUNC(BAD_REF_FILETYPE, ERROR) \\\n \tFUNC(BAD_REF_NAME, ERROR) \\\n+\tFUNC(BAD_REF_OID, ERROR) \\\n \tFUNC(BAD_TIMEZONE, ERROR) \\\n \tFUNC(BAD_TREE, ERROR) \\\n \tFUNC(BAD_TREE_SHA1, ERROR) \\\ndiff --git a/refs.c b/refs.c\nindex 4fc1317cb3..c3528862c6 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -320,10 +320,15 @@ int check_refname_format(const char *refname, int flags)\n \treturn check_or_sanitize_refname(refname, flags, NULL);\n }\n \n-int refs_fsck_ref(struct ref_store *refs UNUSED, struct fsck_options *o UNUSED,\n-\t\t  struct fsck_ref_report *report UNUSED,\n-\t\t  const char *refname UNUSED, const struct object_id *oid UNUSED)\n+int refs_fsck_ref(struct ref_store *refs UNUSED, struct fsck_options *o,\n+\t\t  struct fsck_ref_report *report,\n+\t\t  const char *refname UNUSED, const struct object_id *oid)\n {\n+\tif (is_null_oid(oid))\n+\t\treturn fsck_report_ref(o, report, FSCK_MSG_BAD_REF_OID,\n+\t\t\t\t       \"points to invalid object ID '%s'\",\n+\t\t\t\t       oid_to_hex(oid));\n+\n \treturn 0;\n }\n \ndiff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh\nindex c4b651c2dc..900c1b2eb2 100755\n--- a/t/t1450-fsck.sh\n+++ b/t/t1450-fsck.sh\n@@ -105,7 +105,7 @@ test_expect_success REFFILES 'HEAD link pointing at a funny object' '\n \techo $ZERO_OID >.git/HEAD &&\n \t# avoid corrupt/broken HEAD from interfering with repo discovery\n \ttest_must_fail env GIT_DIR=.git git fsck 2>out &&\n-\ttest_grep \"detached HEAD points\" out\n+\ttest_grep \"HEAD: badRefOid: points to invalid object ID ${SQ}$ZERO_OID${SQ}\" out\n '\n \n test_expect_success 'HEAD link pointing at a funny place' '\n@@ -123,7 +123,7 @@ test_expect_success REFFILES 'HEAD link pointing at a funny object (from differe\n \techo $ZERO_OID >.git/HEAD &&\n \t# avoid corrupt/broken HEAD from interfering with repo discovery\n \ttest_must_fail git -C wt fsck 2>out &&\n-\ttest_grep \"main-worktree/HEAD: detached HEAD points\" out\n+\ttest_grep \"HEAD: badRefOid: points to invalid object ID ${SQ}$ZERO_OID${SQ}\" out\n '\n \n test_expect_success REFFILES 'other worktree HEAD link pointing at a funny object' '\n@@ -131,7 +131,7 @@ test_expect_success REFFILES 'other worktree HEAD link pointing at a funny objec\n \tgit worktree add other &&\n \techo $ZERO_OID >.git/worktrees/other/HEAD &&\n \ttest_must_fail git fsck 2>out &&\n-\ttest_grep \"worktrees/other/HEAD: detached HEAD points\" out\n+\ttest_grep \"worktrees/other/HEAD: badRefOid: points to invalid object ID ${SQ}$ZERO_OID${SQ}\" out\n '\n \n test_expect_success 'other worktree HEAD link pointing at missing object' '\n\n-- \n2.52.0.590.g1f87b77810.dirty\n\n"},{"id":"533593","messageId":"20260112-pks-refs-verify-fixes-v2-16-2e9e453bd6c3@pks.im","threadId":"64762","inReplyTo":"20260112-pks-refs-verify-fixes-v2-0-2e9e453bd6c3@pks.im","subject":"[PATCH v2 16/17] builtin/fsck: move generic HEAD check into `refs_fsck()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-12T09:03:05Z","receivedAt":"2026-01-12T09:03:39Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Move the check that detects \"HEAD\" refs that do not point at a branch\ninto `refs_fsck()`. This follows the same motivation as the preceding\ncommit.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Documentation/fsck-msgids.adoc |  3 +++\n builtin/fsck.c                 |  7 -------\n fsck.h                         |  1 +\n refs.c                         | 12 +++++++++++-\n t/t0602-reffiles-fsck.sh       |  8 ++++----\n t/t1450-fsck.sh                |  4 ++--\n 6 files changed, 21 insertions(+), 14 deletions(-)\n\ndiff --git a/Documentation/fsck-msgids.adoc b/Documentation/fsck-msgids.adoc\nindex 76609321f6..6a4db3a991 100644\n--- a/Documentation/fsck-msgids.adoc\n+++ b/Documentation/fsck-msgids.adoc\n@@ -13,6 +13,9 @@\n `badGpgsig`::\n \t(ERROR) A tag contains a bad (truncated) signature (e.g., `gpgsig`) header.\n \n+`badHeadTarget`::\n+\t(ERROR) The `HEAD` ref is a symref that does not refer to a branch.\n+\n `badHeaderContinuation`::\n \t(ERROR) A continuation header (such as for `gpgsig`) is unexpectedly truncated.\n \ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex 4dd4d74d1e..5dda441f45 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -728,13 +728,6 @@ static void fsck_head_link(const char *head_ref_name,\n \t\terror(_(\"invalid %s\"), head_ref_name);\n \t\treturn;\n \t}\n-\tif (strcmp(*head_points_at, head_ref_name) &&\n-\t    !starts_with(*head_points_at, \"refs/heads/\")) {\n-\t\terrors_found |= ERROR_REFS;\n-\t\terror(_(\"%s points to something strange (%s)\"),\n-\t\t      head_ref_name, *head_points_at);\n-\t\treturn;\n-\t}\n \n \treturn;\n }\ndiff --git a/fsck.h b/fsck.h\nindex 1f472b7daa..65ecbb7fe1 100644\n--- a/fsck.h\n+++ b/fsck.h\n@@ -30,6 +30,7 @@ enum fsck_msg_type {\n \tFUNC(BAD_DATE_OVERFLOW, ERROR) \\\n \tFUNC(BAD_EMAIL, ERROR) \\\n \tFUNC(BAD_GPGSIG, ERROR) \\\n+\tFUNC(BAD_HEAD_TARGET, ERROR) \\\n \tFUNC(BAD_NAME, ERROR) \\\n \tFUNC(BAD_OBJECT_SHA1, ERROR) \\\n \tFUNC(BAD_PACKED_REF_ENTRY, ERROR) \\\ndiff --git a/refs.c b/refs.c\nindex c3528862c6..a772d371cd 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -334,8 +334,18 @@ int refs_fsck_ref(struct ref_store *refs UNUSED, struct fsck_options *o,\n \n int refs_fsck_symref(struct ref_store *refs UNUSED, struct fsck_options *o,\n \t\t     struct fsck_ref_report *report,\n-\t\t     const char *refname UNUSED, const char *target)\n+\t\t     const char *refname, const char *target)\n {\n+\tconst char *stripped_refname;\n+\n+\tparse_worktree_ref(refname, NULL, NULL, &stripped_refname);\n+\n+\tif (!strcmp(stripped_refname, \"HEAD\") &&\n+\t    !starts_with(target, \"refs/heads/\") &&\n+\t    fsck_report_ref(o, report, FSCK_MSG_BAD_HEAD_TARGET,\n+\t\t\t    \"HEAD points to non-branch '%s'\", target))\n+\t\treturn -1;\n+\n \tif (is_root_ref(target))\n \t\treturn 0;\n \ndiff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh\nindex 479f3d528e..3c1f553b81 100755\n--- a/t/t0602-reffiles-fsck.sh\n+++ b/t/t0602-reffiles-fsck.sh\n@@ -910,10 +910,10 @@ test_expect_success 'complains about broken root ref' '\n \tgit init repo &&\n \t(\n \t\tcd repo &&\n-\t\techo \"ref: refs/../HEAD\" >.git/HEAD &&\n+\t\techo \"ref: refs/heads/../HEAD\" >.git/HEAD &&\n \t\ttest_must_fail git refs verify 2>err &&\n \t\tcat >expect <<-EOF &&\n-\t\terror: HEAD: badReferentName: points to invalid refname ${SQ}refs/../HEAD${SQ}\n+\t\terror: HEAD: badReferentName: points to invalid refname ${SQ}refs/heads/../HEAD${SQ}\n \t\tEOF\n \t\ttest_cmp expect err\n \t)\n@@ -926,10 +926,10 @@ test_expect_success 'complains about broken root ref in worktree' '\n \t\tcd repo &&\n \t\ttest_commit initial &&\n \t\tgit worktree add ../worktree &&\n-\t\techo \"ref: refs/../HEAD\" >.git/worktrees/worktree/HEAD &&\n+\t\techo \"ref: refs/heads/../HEAD\" >.git/worktrees/worktree/HEAD &&\n \t\ttest_must_fail git refs verify 2>err &&\n \t\tcat >expect <<-EOF &&\n-\t\terror: worktrees/worktree/HEAD: badReferentName: points to invalid refname ${SQ}refs/../HEAD${SQ}\n+\t\terror: worktrees/worktree/HEAD: badReferentName: points to invalid refname ${SQ}refs/heads/../HEAD${SQ}\n \t\tEOF\n \t\ttest_cmp expect err\n \t)\ndiff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh\nindex 900c1b2eb2..3fae05f9d9 100755\n--- a/t/t1450-fsck.sh\n+++ b/t/t1450-fsck.sh\n@@ -113,7 +113,7 @@ test_expect_success 'HEAD link pointing at a funny place' '\n \ttest-tool ref-store main create-symref HEAD refs/funny/place &&\n \t# avoid corrupt/broken HEAD from interfering with repo discovery\n \ttest_must_fail env GIT_DIR=.git git fsck 2>out &&\n-\ttest_grep \"HEAD points to something strange\" out\n+\ttest_grep \"HEAD: badHeadTarget: HEAD points to non-branch ${SQ}refs/funny/place${SQ}\" out\n '\n \n test_expect_success REFFILES 'HEAD link pointing at a funny object (from different wt)' '\n@@ -148,7 +148,7 @@ test_expect_success 'other worktree HEAD link pointing at a funny place' '\n \tgit worktree add other &&\n \tgit -C other symbolic-ref HEAD refs/funny/place &&\n \ttest_must_fail git fsck 2>out &&\n-\ttest_grep \"worktrees/other/HEAD points to something strange\" out\n+\ttest_grep \"worktrees/other/HEAD: badHeadTarget: HEAD points to non-branch ${SQ}refs/funny/place${SQ}\" out\n '\n \n test_expect_success 'commit with multiple signatures is okay' '\n\n-- \n2.52.0.590.g1f87b77810.dirty\n\n"},{"id":"533594","messageId":"20260112-pks-refs-verify-fixes-v2-17-2e9e453bd6c3@pks.im","threadId":"64762","inReplyTo":"20260112-pks-refs-verify-fixes-v2-0-2e9e453bd6c3@pks.im","subject":"[PATCH v2 17/17] builtin/fsck: drop `fsck_head_link()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-12T09:03:06Z","receivedAt":"2026-01-12T09:03:42Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The function `fsck_head_link()` was historically used to perform a\ncouple of consistency checks for refs. (Almost) all of these checks have\nnow been moved into the refs subsystem. There's only a single check\nremaining that verifies whether `refs_resolve_ref_unsafe()` returns a\n`NULL` pointer. This may happen in a couple of cases:\n\n  - When `refs_is_safe()` declares the ref to be unsafe. We already have\n    checks for this as we verify refnames with `check_refname_format()`.\n\n  - When the ref doesn't exist. A repository without \"HEAD\" is\n    completely broken though, and we would notice this error ahead of\n    time already.\n\n  - In case the caller passes `RESOLVE_REF_READING` and the ref is a\n    symref that doesn't resolve. We don't pass this flag though.\n\nAs such, this check doesn't cover anything anymore that isn't already\ncovered by `refs_fsck()`. Drop it, which also allows us to inline the\ncall to `refs_resolve_ref_unsafe()`.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n builtin/fsck.c | 28 ++++------------------------\n 1 file changed, 4 insertions(+), 24 deletions(-)\n\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex 5dda441f45..f104b7af0e 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -564,10 +564,6 @@ static int fsck_handle_ref(const struct reference *ref, void *cb_data UNUSED)\n \treturn 0;\n }\n \n-static void fsck_head_link(const char *head_ref_name,\n-\t\t\t   const char **head_points_at,\n-\t\t\t   struct object_id *head_oid);\n-\n static void get_default_heads(void)\n {\n \tstruct worktree **worktrees, **p;\n@@ -583,7 +579,10 @@ static void get_default_heads(void)\n \t\tstruct strbuf refname = STRBUF_INIT;\n \n \t\tstrbuf_worktree_ref(wt, &refname, \"HEAD\");\n-\t\tfsck_head_link(refname.buf, &head_points_at, &head_oid);\n+\n+\t\thead_points_at = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),\n+\t\t\t\t\t\t\t refname.buf, 0, &head_oid, NULL);\n+\n \t\tif (head_points_at && !is_null_oid(&head_oid)) {\n \t\t\tstruct reference ref = {\n \t\t\t\t.name = refname.buf,\n@@ -713,25 +712,6 @@ static void fsck_source(struct odb_source *source)\n \tstop_progress(&progress);\n }\n \n-static void fsck_head_link(const char *head_ref_name,\n-\t\t\t   const char **head_points_at,\n-\t\t\t   struct object_id *head_oid)\n-{\n-\tif (verbose)\n-\t\tfprintf_ln(stderr, _(\"Checking %s link\"), head_ref_name);\n-\n-\t*head_points_at = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),\n-\t\t\t\t\t\t  head_ref_name, 0, head_oid,\n-\t\t\t\t\t\t  NULL);\n-\tif (!*head_points_at) {\n-\t\terrors_found |= ERROR_REFS;\n-\t\terror(_(\"invalid %s\"), head_ref_name);\n-\t\treturn;\n-\t}\n-\n-\treturn;\n-}\n-\n static int fsck_cache_tree(struct cache_tree *it, const char *index_path)\n {\n \tint i;\n\n-- \n2.52.0.590.g1f87b77810.dirty\n\n"},{"id":"533601","messageId":"CAOLa=ZSu3MGejqN9n4NaytANNbU4bY1GEAK84-W0B94jcCddpA@mail.gmail.com","threadId":"64762","inReplyTo":"20260112-pks-refs-verify-fixes-v2-1-2e9e453bd6c3@pks.im","subject":"Re: [PATCH v2 01/17] refs/files: simplify iterating through root refs","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-12T09:56:03Z","receivedAt":"2026-01-12T09:56:05Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> When iterating through root refs we first need to determine the\n> directory in which the refs live. This is done by retrieving the root of\n> the loose refs via `refs->loose->root->name`, and putting it through\n> `files_ref_path()` to derive the final path.\n>\n> This is somewhat redundant though: the root name of the loose files\n> cache is always going to be the empty string. As such, we always end up\n> passing that empty string to `files_ref_path()` as the ref hierarchy we\n> want to start. And this actually makes sense: `files_ref_path()` already\n> computes the location of the root directory, so of course we need to\n> pass the empty string for the ref hierarchy itself. So going via the\n> loose ref cache to figure out that the root of a ref hierarchy is empty\n> is only causing confusion.\n>\n> But next to the added confusion, it can also lead to a segfault. The\n> loose ref cache is populated lazily, so it may not always be set. It\n> seems to be sheer luck that this is a condition we do not currently hit.\n> The right thing to do would be to call `get_loose_ref_cache()`, which\n> knows to populate the cache if required.\n>\n> Simplify the code and fix the potential segfault by simply removing the\n> indirection via the loose ref cache completely.\n>\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  refs/files-backend.c | 11 +++--------\n>  1 file changed, 3 insertions(+), 8 deletions(-)\n>\n> diff --git a/refs/files-backend.c b/refs/files-backend.c\n> index 6f6f76a8d8..297739f203 100644\n> --- a/refs/files-backend.c\n> +++ b/refs/files-backend.c\n> @@ -354,13 +354,11 @@ static int for_each_root_ref(struct files_ref_store *refs,\n>  \t\t\t     void *cb_data)\n>  {\n>  \tstruct strbuf path = STRBUF_INIT, refname = STRBUF_INIT;\n> -\tconst char *dirname = refs->loose->root->name;\n>  \tstruct dirent *de;\n> -\tsize_t dirnamelen;\n>  \tint ret;\n>  \tDIR *d;\n>\n> -\tfiles_ref_path(refs, &path, dirname);\n> +\tfiles_ref_path(refs, &path, \"\");\n>\n\nSince refs->loose->root->name is always `\"\"`, we directly pass that\ninstead. Makes sense.\n\n>  \td = opendir(path.buf);\n>  \tif (!d) {\n> @@ -368,9 +366,6 @@ static int for_each_root_ref(struct files_ref_store *refs,\n>  \t\treturn -1;\n>  \t}\n>\n> -\tstrbuf_addstr(&refname, dirname);\n> -\tdirnamelen = refname.len;\n> -\n\nThis too is unnecessary since the len here is 0.\n\n>  \twhile ((de = readdir(d)) != NULL) {\n>  \t\tunsigned char dtype;\n>\n> @@ -378,6 +373,8 @@ static int for_each_root_ref(struct files_ref_store *refs,\n>  \t\t\tcontinue;\n>  \t\tif (ends_with(de->d_name, \".lock\"))\n>  \t\t\tcontinue;\n> +\n> +\t\tstrbuf_reset(&refname);\n>  \t\tstrbuf_addstr(&refname, de->d_name);\n>\n>  \t\tdtype = get_dtype(de, &path, 1);\n> @@ -386,8 +383,6 @@ static int for_each_root_ref(struct files_ref_store *refs,\n>  \t\t\tif (ret)\n>  \t\t\t\tgoto done;\n>  \t\t}\n> -\n> -\t\tstrbuf_setlen(&refname, dirnamelen);\n\nEarlier we were setting the length to 0, but thats the same as\nstrbuf_reset(), so we do that now. This gets rid of the `dirnamelen`\nvarible. Looks good.\n\n>  \t}\n>\n>  \tret = 0;\n>\n> --\n> 2.52.0.590.g1f87b77810.dirty\n"},{"id":"533602","messageId":"CAOLa=ZSGd76M=Dj0E512w23rtGM2eEvYeMMCzFffh-oNJ4br-Q@mail.gmail.com","threadId":"64762","inReplyTo":"20260112-pks-refs-verify-fixes-v2-4-2e9e453bd6c3@pks.im","subject":"Re: [PATCH v2 04/17] refs/files: remove useless indirection","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-12T10:01:01Z","receivedAt":"2026-01-12T10:01:02Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> The function `files_fsck_refs()` only has a single callsite and forwards\n> all of its arguments as-is, so it's basically a useless indirection.\n> Inline the function call.\n>\n> While at it, also remove the bitwise or that we have for return values.\n> We don't really want to or them at all, but rather just want to return\n> an error in case either of the functions has failed.\n>\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  refs/files-backend.c | 16 +++++++---------\n>  1 file changed, 7 insertions(+), 9 deletions(-)\n>\n> diff --git a/refs/files-backend.c b/refs/files-backend.c\n> index 0a104c7bf6..4cbee23dad 100644\n> --- a/refs/files-backend.c\n> +++ b/refs/files-backend.c\n> @@ -3954,22 +3954,20 @@ static int files_fsck_refs_dir(struct ref_store *ref_store,\n>  \treturn ret;\n>  }\n>\n> -static int files_fsck_refs(struct ref_store *ref_store,\n> -\t\t\t   struct fsck_options *o,\n> -\t\t\t   struct worktree *wt)\n> -{\n> -\treturn files_fsck_refs_dir(ref_store, o, wt);\n> -}\n> -\n>  static int files_fsck(struct ref_store *ref_store,\n>  \t\t      struct fsck_options *o,\n>  \t\t      struct worktree *wt)\n>  {\n>  \tstruct files_ref_store *refs =\n>  \t\tfiles_downcast(ref_store, REF_STORE_READ, \"fsck\");\n> +\tint ret = 0;\n>\n> -\treturn files_fsck_refs(ref_store, o, wt) |\n> -\t       refs->packed_ref_store->be->fsck(refs->packed_ref_store, o, wt);\n> +\tif (files_fsck_refs_dir(ref_store, o, wt) < 0)\n> +\t\tret = -1;\n> +\tif (refs->packed_ref_store->be->fsck(refs->packed_ref_store, o, wt) < 0)\n> +\t\tret = -1;\n> +\n\nI wonder if this should have been a logical or instead of the bitwise\nor, but then we directly return so even that wouldn't work. This looks\ngood! Thanks\n"},{"id":"533604","messageId":"CAOLa=ZRMvbRT64+XdKobM5RZhgiPd=2k5_Yf=rgKyjWnbpMg1A@mail.gmail.com","threadId":"64762","inReplyTo":"20260112-pks-refs-verify-fixes-v2-10-2e9e453bd6c3@pks.im","subject":"Re: [PATCH v2 10/17] refs/files: introduce function to perform normal ref checks","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-12T11:42:04Z","receivedAt":"2026-01-12T11:42:06Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> In a subsequent commit we'll introduce new generic checks for direct\n> refs. These checks will be independent of the actual backend.\n\nI don't think we've used the terminology 'direct refs' before. Took\nme a second to understand. We generally use 'regular refs', but that\nincludes symrefs, so I think this does make sense.\n\n[snip]\n"},{"id":"533605","messageId":"CAOLa=ZTDTqpWgKW7=X70ofFJEK66mfOEOgQy-PpMHo_n6kyS=Q@mail.gmail.com","threadId":"64762","inReplyTo":"20260112-pks-refs-verify-fixes-v2-13-2e9e453bd6c3@pks.im","subject":"Re: [PATCH v2 13/17] refs/reftable: fix consistency checks with worktrees","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-12T11:45:19Z","receivedAt":"2026-01-12T11:45:21Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> The ref consistency checks are driven via `cmd_refs_verify()`. That\n> function loops through all worktrees (including the main worktree) and\n> then checks the ref store for each of them individually. It follows that\n> the backend is expected to only verify refs that belong to the specified\n> worktree.\n>\n> While the \"files\" backend handles this correctly, the \"reftable\" backend\n> doesn't. In fact, it completely ignores the passed worktree and instead\n> verifies refs of _all_ worktrees. The consequence is that we'll end up\n> every ref store N times, where N is the number of worktrees.\n>\n> Or rather, that would be the case if we actually iterated through the\n> worktree reftable stacks correctly. But we use `strmap_for_each_entry()`\n> to iterate through the stacks, but the map is in fact not even properly\n> populated. So instead of checking stacks N^2 times, we actually only end\n> up checking the reftable stack of the main worktree.\n>\n> Fix this bug by only verifying the stack of the passed-in worktree and\n> constructing the backends via `backend_for_worktree()`.\n>\n\nAh, I was the author of this, I didn't know that the fsck function gets\ncalled per worktree, so thanks for fixing it!\n\n[snip]\n"},{"id":"533606","messageId":"CAOLa=ZShPP3BPXa=YnC-vuX4zF=pUTFdUidZwOdna8bfVTNM9w@mail.gmail.com","threadId":"64762","inReplyTo":"20260112-pks-refs-verify-fixes-v2-0-2e9e453bd6c3@pks.im","subject":"Re: [PATCH v2 00/17] Fixes and improvements for ref consistency checks","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-12T11:50:17Z","receivedAt":"2026-01-12T11:50:19Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> Hi,\n>\n> this patch series contains a bunch of fixes and improvements for ref\n> consistency checks. It is structured as follows:\n>\n>   - Patches 1 to 4 contain a couple of cleanups for the consistency\n>     checks done by the \"files\" backend.\n>\n>   - Patches 5 to 7 introduce checks for root refs for the \"files\"\n>     backend.\n>\n>   - Patches 9 to 14 introduce infrastructure for shared checks with the\n>     \"files\" and \"reftable\" backend.\n>\n>   - Patches 15 to 17 move some ref consistency checks that were still\n>     driven by git-fsck(1) into `git refs verify`.\n>\n\nI reviewed the series and it already looks good, thanks for fixing some\nof the broken parts and cleaning up.\n\n[snip]\n"},{"id":"533619","messageId":"aWTyXufNdKckmBTC@pks.im","threadId":"64762","inReplyTo":"CAOLa=ZRMvbRT64+XdKobM5RZhgiPd=2k5_Yf=rgKyjWnbpMg1A@mail.gmail.com","subject":"Re: [PATCH v2 10/17] refs/files: introduce function to perform normal ref checks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-12T13:08:46Z","receivedAt":"2026-01-12T13:08:51Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Jan 12, 2026 at 06:42:04AM -0500, Karthik Nayak wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> \n> > In a subsequent commit we'll introduce new generic checks for direct\n> > refs. These checks will be independent of the actual backend.\n> \n> I don't think we've used the terminology 'direct refs' before. Took\n> me a second to understand. We generally use 'regular refs', but that\n> includes symrefs, so I think this does make sense.\n\nYeah, I didn't really know what to call these other than \"direct refs\".\nWe could instead say \"non-symbolic refs\", but that also feels kind of\nawkward. So I guess this is good enough...?\n\nPatrick\n"},{"id":"533620","messageId":"aWTybZHqZC_H3dGS@pks.im","threadId":"64762","inReplyTo":"CAOLa=ZShPP3BPXa=YnC-vuX4zF=pUTFdUidZwOdna8bfVTNM9w@mail.gmail.com","subject":"Re: [PATCH v2 00/17] Fixes and improvements for ref consistency checks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-12T13:09:01Z","receivedAt":"2026-01-12T13:09:05Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Jan 12, 2026 at 06:50:17AM -0500, Karthik Nayak wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> \n> > Hi,\n> >\n> > this patch series contains a bunch of fixes and improvements for ref\n> > consistency checks. It is structured as follows:\n> >\n> >   - Patches 1 to 4 contain a couple of cleanups for the consistency\n> >     checks done by the \"files\" backend.\n> >\n> >   - Patches 5 to 7 introduce checks for root refs for the \"files\"\n> >     backend.\n> >\n> >   - Patches 9 to 14 introduce infrastructure for shared checks with the\n> >     \"files\" and \"reftable\" backend.\n> >\n> >   - Patches 15 to 17 move some ref consistency checks that were still\n> >     driven by git-fsck(1) into `git refs verify`.\n> >\n> \n> I reviewed the series and it already looks good, thanks for fixing some\n> of the broken parts and cleaning up.\n\nThanks for your review!\n\nPatrick\n"},{"id":"533631","messageId":"xmqqldi2oqve.fsf@gitster.g","threadId":"64762","inReplyTo":"aWTyXufNdKckmBTC@pks.im","subject":"Re: [PATCH v2 10/17] refs/files: introduce function to perform normal ref checks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-12T14:19:33Z","receivedAt":"2026-01-12T14:19:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> On Mon, Jan 12, 2026 at 06:42:04AM -0500, Karthik Nayak wrote:\n>> Patrick Steinhardt <ps@pks.im> writes:\n>> \n>> > In a subsequent commit we'll introduce new generic checks for direct\n>> > refs. These checks will be independent of the actual backend.\n>> \n>> I don't think we've used the terminology 'direct refs' before. Took\n>> me a second to understand. We generally use 'regular refs', but that\n>> includes symrefs, so I think this does make sense.\n>\n> Yeah, I didn't really know what to call these other than \"direct refs\".\n> We could instead say \"non-symbolic refs\", but that also feels kind of\n> awkward. So I guess this is good enough...?\n\nThe latter is understandable, if awkward.  The former is not.\n\nThanks.\n"},{"id":"533635","messageId":"xmqq8qe2oq26.fsf@gitster.g","threadId":"64762","inReplyTo":"xmqqldi2oqve.fsf@gitster.g","subject":"Re: [PATCH v2 10/17] refs/files: introduce function to perform normal ref checks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-12T14:37:05Z","receivedAt":"2026-01-12T14:37:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Patrick Steinhardt <ps@pks.im> writes:\n>\n>> On Mon, Jan 12, 2026 at 06:42:04AM -0500, Karthik Nayak wrote:\n>>> Patrick Steinhardt <ps@pks.im> writes:\n>>> \n>>> > In a subsequent commit we'll introduce new generic checks for direct\n>>> > refs. These checks will be independent of the actual backend.\n>>> \n>>> I don't think we've used the terminology 'direct refs' before. Took\n>>> me a second to understand. We generally use 'regular refs', but that\n>>> includes symrefs, so I think this does make sense.\n>>\n>> Yeah, I didn't really know what to call these other than \"direct refs\".\n>> We could instead say \"non-symbolic refs\", but that also feels kind of\n>> awkward. So I guess this is good enough...?\n>\n> The latter is understandable, if awkward.  The former is not.\n\nWell, I failed to elaborate why I think \"the former is not\".\n\nThe former would have been, if we were calling HEAD as \"indirect\nref\", instead of \"symbolic ref\".  But we use the latter, hence\n\"direct ref\" is much less understandable than \"non-symbolic ref\".\n\nThanks.\n"},{"id":"533641","messageId":"aWUM9pc1S07uIgJp@pks.im","threadId":"64762","inReplyTo":"xmqq8qe2oq26.fsf@gitster.g","subject":"Re: [PATCH v2 10/17] refs/files: introduce function to perform normal ref checks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-12T15:02:14Z","receivedAt":"2026-01-12T15:02:26Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Jan 12, 2026 at 06:37:05AM -0800, Junio C Hamano wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n> \n> > Patrick Steinhardt <ps@pks.im> writes:\n> >\n> >> On Mon, Jan 12, 2026 at 06:42:04AM -0500, Karthik Nayak wrote:\n> >>> Patrick Steinhardt <ps@pks.im> writes:\n> >>> \n> >>> > In a subsequent commit we'll introduce new generic checks for direct\n> >>> > refs. These checks will be independent of the actual backend.\n> >>> \n> >>> I don't think we've used the terminology 'direct refs' before. Took\n> >>> me a second to understand. We generally use 'regular refs', but that\n> >>> includes symrefs, so I think this does make sense.\n> >>\n> >> Yeah, I didn't really know what to call these other than \"direct refs\".\n> >> We could instead say \"non-symbolic refs\", but that also feels kind of\n> >> awkward. So I guess this is good enough...?\n> >\n> > The latter is understandable, if awkward.  The former is not.\n> \n> Well, I failed to elaborate why I think \"the former is not\".\n> \n> The former would have been, if we were calling HEAD as \"indirect\n> ref\", instead of \"symbolic ref\".  But we use the latter, hence\n> \"direct ref\" is much less understandable than \"non-symbolic ref\".\n\nFair enough. I've queued this change locally and will send it out with\nthe next iteration. Thanks!\n\nPatrick\n"},{"id":"533948","messageId":"aWjjK8fmf8L7vlNi@ArchLinux","threadId":"64762","inReplyTo":"aWSuObEsFaxi1NAf@pks.im","subject":"Re: [PATCH 16/17] builtin/fsck: move generic HEAD check into `refs_fsck()`","fromName":"shejialuo","fromEmail":"shejialuo@gmail.com","sentAt":"2026-01-15T12:52:59Z","receivedAt":"2026-01-15T12:53:04Z","isPatch":true,"sender":{"key":"shejialuo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56911263?v=4"},"body":"On Mon, Jan 12, 2026 at 09:18:01AM +0100, Patrick Steinhardt wrote:\n> On Sat, Jan 10, 2026 at 09:31:07PM +0800, shejialuo wrote:\n> > On Fri, Jan 09, 2026 at 01:39:45PM +0100, Patrick Steinhardt wrote:\n> > > diff --git a/refs.c b/refs.c\n> > > index c3528862c6..a772d371cd 100644\n> > > --- a/refs.c\n> > > +++ b/refs.c\n> > > @@ -334,8 +334,18 @@ int refs_fsck_ref(struct ref_store *refs UNUSED, struct fsck_options *o,\n> > >  \n> > >  int refs_fsck_symref(struct ref_store *refs UNUSED, struct fsck_options *o,\n> > >  \t\t     struct fsck_ref_report *report,\n> > > -\t\t     const char *refname UNUSED, const char *target)\n> > > +\t\t     const char *refname, const char *target)\n> > >  {\n> > > +\tconst char *stripped_refname;\n> > > +\n> > > +\tparse_worktree_ref(refname, NULL, NULL, &stripped_refname);\n> > > +\n> > > +\tif (!strcmp(stripped_refname, \"HEAD\") &&\n> > > +\t    !starts_with(target, \"refs/heads/\") &&\n> > \n> > We would first check whether the current ref is `HEAD`. And I am\n> > wondering whether we have some common APIs. And I find the similar logic\n> > in `reglog.c::is_head` like the following shows:\n> > \n> >     static int is_head(const char *refname)\n> >     {\n> >             const char *stripped_refname;\n> >             parse_worktree_ref(refname, NULL, NULL, &stripped_refname);\n> >             return !strcmp(stripped_refname, \"HEAD\");\n> >     }\n> > \n> > I think we might just extract this common logic to avoid introducing\n> > repetition.\n> \n> Hm. We could, but I'm a tiny bit worried about just calling it\n> `is_head()`. It might be surprising to some callers that there isn't\n> only one \"HEAD\", but that this would also recognize worktree HEADs. If\n> somebody just goes like \"I wanna know whether I've got HEAD\" they might\n> not think about that at all.\n> \n\nMake sense.\n\n> So given that the complexity is comparatively low I'd prefer to keep\n> this as-is for now. On the other hand, if you've got some proposal for\n> how to make this interface not confusing I'm very open to that :)\n> \n\nYeah, I cannot give some better idea, either. Let's keep this as-is :)\n\n> Thanks!\n> \n> Patrick\n"},{"id":"533949","messageId":"aWjj5wBi71KZy0dd@ArchLinux","threadId":"64762","inReplyTo":"20260112-pks-refs-verify-fixes-v2-0-2e9e453bd6c3@pks.im","subject":"Re: [PATCH v2 00/17] Fixes and improvements for ref consistency checks","fromName":"shejialuo","fromEmail":"shejialuo@gmail.com","sentAt":"2026-01-15T12:56:07Z","receivedAt":"2026-01-15T12:56:12Z","isPatch":true,"sender":{"key":"shejialuo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56911263?v=4"},"body":"On Mon, Jan 12, 2026 at 10:02:49AM +0100, Patrick Steinhardt wrote:\n> Hi,\n> \n> this patch series contains a bunch of fixes and improvements for ref\n> consistency checks. It is structured as follows:\n> \n>   - Patches 1 to 4 contain a couple of cleanups for the consistency\n>     checks done by the \"files\" backend.\n> \n>   - Patches 5 to 7 introduce checks for root refs for the \"files\"\n>     backend.\n> \n>   - Patches 9 to 14 introduce infrastructure for shared checks with the\n>     \"files\" and \"reftable\" backend.\n> \n>   - Patches 15 to 17 move some ref consistency checks that were still\n>     driven by git-fsck(1) into `git refs verify`.\n> \n> Changes in v2:\n>   - Remove unused `errors_found` field.\n>   - Fix a commit message typo.\n>   - Fix a copy-paste error in a function comment.\n>   - Link to v1: https://lore.kernel.org/r/20260109-pks-refs-verify-fixes-v1-0-3587dba18294@pks.im\n> \n> Thanks!\n> \n> Patrick\n> \n\nThe range-diff looks good to me.\n\nThanks,\nJialuo\n"},{"id":"534013","messageId":"aWnfJ1KKgP_otIMm@pks.im","threadId":"64762","inReplyTo":"aWjj5wBi71KZy0dd@ArchLinux","subject":"Re: [PATCH v2 00/17] Fixes and improvements for ref consistency checks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-16T06:48:07Z","receivedAt":"2026-01-16T06:48:13Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Jan 15, 2026 at 08:56:07PM +0800, shejialuo wrote:\n> On Mon, Jan 12, 2026 at 10:02:49AM +0100, Patrick Steinhardt wrote:\n> > Hi,\n> > \n> > this patch series contains a bunch of fixes and improvements for ref\n> > consistency checks. It is structured as follows:\n> > \n> >   - Patches 1 to 4 contain a couple of cleanups for the consistency\n> >     checks done by the \"files\" backend.\n> > \n> >   - Patches 5 to 7 introduce checks for root refs for the \"files\"\n> >     backend.\n> > \n> >   - Patches 9 to 14 introduce infrastructure for shared checks with the\n> >     \"files\" and \"reftable\" backend.\n> > \n> >   - Patches 15 to 17 move some ref consistency checks that were still\n> >     driven by git-fsck(1) into `git refs verify`.\n> > \n> > Changes in v2:\n> >   - Remove unused `errors_found` field.\n> >   - Fix a commit message typo.\n> >   - Fix a copy-paste error in a function comment.\n> >   - Link to v1: https://lore.kernel.org/r/20260109-pks-refs-verify-fixes-v1-0-3587dba18294@pks.im\n> > \n> > Thanks!\n> > \n> > Patrick\n> > \n> \n> The range-diff looks good to me.\n\nThanks!\n\nPatrick\n"}]}