[PATCH v2] refs/files: skip lock files during consistency checks

[Karthik Nayak <karthik.188@gmail.com>]

Consistency checks in the files reference backend involve two steps:

1. Iterate over all entries within the 'refs/' directory and call
`files_fsck_ref()` on each.
2. Iterate over all root refs via `for_each_root_ref()` and call
`files_fsck_ref()` on each.

`files_fsck_ref()` then runs all fsck checks defined in
`fsck_refs_fn[]`. Step 2 goes through the refs API and only sees valid
refs, but step 1 iterates the directory directly and will also encounter
intermediate '*.lock' files.

Currently, `files_fsck_refs_name()`, one of the functions in
`fsck_refs_fn[]`, filters out lock files itself. The other function,
`files_fsck_refs_content()`, has no such check and would parse the lock
file. Any new function added to `fsck_refs_fn[]` would have the same
problem.

Move the filter up into `files_fsck_refs_dir()`, where the directory
iteration happens. Since step 2 cannot produce lock files, this is the
only site where the filter is needed, and individual checks no longer
have to re-implement it.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
Changes in v2:
- Modified the commit message to clarify the changes made and reasoning.
- Modify the comment in the code to be more accurate.
- Add another additional test for bare lock files.
- Link to v1: https://patch.msgid.link/20260420-refs-fsck-skip-lock-files-v1-1-c2595e206a76@gmail.com
---
 refs/files-backend.c     | 22 +++++++++++-----------
 t/t0602-reffiles-fsck.sh | 41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 11 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index b3b0c25f84..1504a1e2f3 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3864,22 +3864,12 @@ static int files_fsck_refs_content(struct ref_store *ref_store,
 static int files_fsck_refs_name(struct ref_store *ref_store UNUSED,
 				struct fsck_options *o,
 				const char *refname,
-				const char *path,
+				const char *path UNUSED,
 				int mode UNUSED)
 {
 	struct strbuf sb = STRBUF_INIT;
-	const char *filename;
 	int ret = 0;
 
-	filename = basename((char *) path);
-
-	/*
-	 * Ignore the files ending with ".lock" as they may be lock files
-	 * However, do not allow bare ".lock" files.
-	 */
-	if (filename[0] != '.' && ends_with(filename, ".lock"))
-		goto cleanup;
-
 	if (is_root_ref(refname))
 		goto cleanup;
 
@@ -3939,6 +3929,7 @@ static int files_fsck_refs_dir(struct ref_store *ref_store,
 	struct strbuf refname = STRBUF_INIT;
 	struct strbuf sb = STRBUF_INIT;
 	struct dir_iterator *iter;
+	const char *filename;
 	int iter_status;
 	int ret = 0;
 
@@ -3962,6 +3953,15 @@ static int files_fsck_refs_dir(struct ref_store *ref_store,
 			strbuf_addf(&refname, "worktrees/%s/", wt->id);
 		strbuf_addf(&refname, "refs/%s", iter->relative_path);
 
+		filename = basename((char *) iter->path.buf);
+
+		/*
+		 * Ignore the files ending with ".lock" as they may be lock files.
+		 * However, do not skip invalid refnames with '.lock' suffix.
+		 */
+		if (filename[0] != '.' && ends_with(filename, ".lock"))
+			continue;
+
 		if (files_fsck_ref(ref_store, o, refname.buf,
 				   iter->path.buf, iter->st.st_mode) < 0)
 			ret = -1;
diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh
index 3c1f553b81..13259821a0 100755
--- a/t/t0602-reffiles-fsck.sh
+++ b/t/t0602-reffiles-fsck.sh
@@ -87,6 +87,47 @@ test_expect_success 'ref name should be checked' '
 	)
 '
 
+test_expect_success 'lock files should be ignored' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		git commit --allow-empty -m initial &&
+		git checkout -b branch-1 &&
+
+		touch .git/refs/heads/branch-1.lock &&
+		git refs verify 2>err &&
+		test_must_be_empty err &&
+
+		echo "foobar" >.git/refs/heads/branch-2 &&
+		test_must_fail git refs verify 2>err &&
+		cat >expect <<-EOF &&
+		error: refs/heads/branch-2: badRefContent: foobar
+		EOF
+		test_cmp expect err
+	)
+'
+
+test_expect_success 'bare lock files should not be ignored' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		git commit --allow-empty -m initial &&
+		git checkout -b branch-1 &&
+
+		# invalid refname should be reported
+		cp .git/refs/heads/branch-1 .git/refs/heads/.branch-1.lock &&
+		# invalid refname and content should be reported
+		touch .git/refs/heads/.lock &&
+
+		test_must_fail git refs verify 2>err &&
+		test_grep "error: refs/heads/.branch-1.lock: badRefName: invalid refname format" err &&
+		test_grep "error: refs/heads/.lock: badRefName: invalid refname format" err &&
+		test_grep "error: refs/heads/.lock: badRefContent: " err
+	)
+'
+
 test_expect_success 'ref name check should be adapted into fsck messages' '
 	test_when_finished "rm -rf repo" &&
 	git init repo &&