git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: shejialuo <shejialuo@gmail.com>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>, Jeff King <peff@peff.net>,
	Patrick Steinhardt <ps@pks.im>
Subject: [PATCH v5 1/3] packed-backend: fsck should warn when "packed-refs" file is empty
Date: Wed, 14 May 2025 23:50:26 +0800	[thread overview]
Message-ID: <aCS7wk-ZCBRiHMqF@ArchLinux> (raw)
In-Reply-To: <aCS7O8tNekg_u9Wp@ArchLinux>

We assume the "packed-refs" won't be empty and instead has at least one
line in it (even when there are no refs packed, there is the file header
line). Because there is no terminating LF in the empty file, we will
report "packedRefEntryNotTerminated(ERROR)" to the user.

However, the runtime code paths would accept an empty "packed-refs"
file, for example, "create_snapshot" would simply return the "snapshot"
without checking the content of "packed-refs". So, we should skip
checking the content of "packed-refs" when it is empty during fsck.

After 694b7a1999 (repack_without_ref(): write peeled refs in the
rewritten file, 2013-04-22), we would always write a header into the
"packed-refs" file. So, versions of Git that are not too ancient never
write such an empty "packed-refs" file.

As an empty file often indicates a sign of a filesystem-level issue, the
way we want to resolve this inconsistency is not make everybody totally
silent but notice and report the anomaly.

Let's create a "FSCK_INFO" message id "EMPTY_PACKED_REFS_FILE" to report
to the users that "packed-refs" is empty.

Signed-off-by: shejialuo <shejialuo@gmail.com>
---
 Documentation/fsck-msgids.adoc |  6 ++++++
 fsck.h                         |  1 +
 refs/packed-backend.c          |  9 +++++++++
 t/t0602-reffiles-fsck.sh       | 17 +++++++++++++++++
 4 files changed, 33 insertions(+)

diff --git a/Documentation/fsck-msgids.adoc b/Documentation/fsck-msgids.adoc
index 9601fff228..0ba4f9a27e 100644
--- a/Documentation/fsck-msgids.adoc
+++ b/Documentation/fsck-msgids.adoc
@@ -59,6 +59,12 @@
 `emptyName`::
 	(WARN) A path contains an empty name.
 
+`emptyPackedRefsFile`::
+	(INFO) "packed-refs" file is empty. Report to the
+	git@vger.kernel.org mailing list if you see this error. As only
+	very early versions of Git would create such an empty
+	"packed_refs" file, we might tighten this rule in the future.
+
 `extraHeaderEntry`::
 	(IGNORE) Extra headers found after `tagger`.
 
diff --git a/fsck.h b/fsck.h
index b1deae61ee..0c5869ac34 100644
--- a/fsck.h
+++ b/fsck.h
@@ -84,6 +84,7 @@ enum fsck_msg_type {
 	FUNC(LARGE_PATHNAME, WARN) \
 	/* infos (reported as warnings, but ignored by default) */ \
 	FUNC(BAD_FILEMODE, INFO) \
+	FUNC(EMPTY_PACKED_REFS_FILE, INFO) \
 	FUNC(GITMODULES_PARSE, INFO) \
 	FUNC(GITIGNORE_SYMLINK, INFO) \
 	FUNC(GITATTRIBUTES_SYMLINK, INFO) \
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 3ad1ed0787..fb91833e76 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -2103,6 +2103,15 @@ static int packed_fsck(struct ref_store *ref_store,
 		goto cleanup;
 	}
 
+	if (!st.st_size) {
+		struct fsck_ref_report report = { 0 };
+		report.path = "packed-refs";
+		ret = fsck_report_ref(o, &report,
+				      FSCK_MSG_EMPTY_PACKED_REFS_FILE,
+				      "file is empty");
+		goto cleanup;
+	}
+
 	if (strbuf_read(&packed_ref_content, fd, 0) < 0) {
 		ret = error_errno(_("unable to read '%s'"), refs->path);
 		goto cleanup;
diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh
index 9d1dc2144c..f671ac4d3a 100755
--- a/t/t0602-reffiles-fsck.sh
+++ b/t/t0602-reffiles-fsck.sh
@@ -647,6 +647,23 @@ test_expect_success SYMLINKS 'the filetype of packed-refs should be checked' '
 	)
 '
 
+test_expect_success 'empty packed-refs should be reported' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		test_commit default &&
+
+		>.git/packed-refs &&
+		git refs verify 2>err &&
+		cat >expect <<-EOF &&
+		warning: packed-refs: emptyPackedRefsFile: file is empty
+		EOF
+		rm .git/packed-refs &&
+		test_cmp expect err
+	)
+'
+
 test_expect_success 'packed-refs header should be checked' '
 	test_when_finished "rm -rf repo" &&
 	git init repo &&
-- 
2.49.0


  reply	other threads:[~2025-05-14 15:50 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-06 16:39 [PATCH 0/4] align the behavior when opening "packed-refs" shejialuo
2025-05-06 16:41 ` [PATCH 1/4] packed-backend: skip checking consistency of empty packed-refs file shejialuo
2025-05-06 18:42   ` Junio C Hamano
2025-05-07 12:09     ` shejialuo
2025-05-06 19:14   ` Junio C Hamano
2025-05-07 12:10     ` shejialuo
2025-05-06 16:41 ` [PATCH 2/4] packed-backend: extract snapshot allocation in `load_contents` shejialuo
2025-05-06 19:16   ` Junio C Hamano
2025-05-06 16:41 ` [PATCH 3/4] packed-backend: extract munmap operation for `MMAP_TEMPORARY` shejialuo
2025-05-06 18:52   ` Junio C Hamano
2025-05-06 22:17     ` Junio C Hamano
2025-05-07 12:21     ` shejialuo
2025-05-06 16:41 ` [PATCH 4/4] packed-backend: use mmap when opening large "packed-refs" file shejialuo
2025-05-06 19:00   ` Junio C Hamano
2025-05-06 22:18     ` Junio C Hamano
2025-05-07 12:34     ` shejialuo
2025-05-07 14:52 ` [PATCH v2 0/4] align the behavior when opening "packed-refs" shejialuo
2025-05-07 14:53   ` [PATCH v2 1/4] packed-backend: fsck should allow an empty "packed-refs" file shejialuo
2025-05-07 14:53   ` [PATCH v2 2/4] packed-backend: extract snapshot allocation in `load_contents` shejialuo
2025-05-07 14:53   ` [PATCH v2 3/4] packed-backend: extract munmap operation for `MMAP_TEMPORARY` shejialuo
2025-05-08 19:57     ` Jeff King
2025-05-08 20:05       ` Junio C Hamano
2025-05-09 15:03         ` shejialuo
2025-05-07 14:54   ` [PATCH v2 4/4] packed-backend: mmap large "packed-refs" file during fsck shejialuo
2025-05-08 20:07     ` Jeff King
2025-05-09 15:21       ` shejialuo
2025-05-09 15:59         ` Jeff King
2025-05-09 16:40           ` shejialuo
2025-05-07 22:51   ` [PATCH v2 0/4] align the behavior when opening "packed-refs" Junio C Hamano
2025-05-08 20:08     ` Jeff King
2025-05-08 20:20       ` Junio C Hamano
2025-05-08 20:33         ` Jeff King
2025-05-09 15:26           ` shejialuo
2025-05-11 13:59   ` [PATCH v3 0/3] " shejialuo
2025-05-11 14:01     ` [PATCH v3 1/3] packed-backend: fsck should allow an empty "packed-refs" file shejialuo
2025-05-12  8:36       ` Patrick Steinhardt
2025-05-12 12:25         ` shejialuo
2025-05-12 14:39           ` Patrick Steinhardt
2025-05-12 15:56             ` Jeff King
2025-05-12 17:18               ` Junio C Hamano
2025-05-13  5:08                 ` Patrick Steinhardt
2025-05-13  7:06                   ` shejialuo
2025-05-11 14:01     ` [PATCH v3 2/3] packed-backend: extract snapshot allocation in `load_contents` shejialuo
2025-05-12  8:37       ` Patrick Steinhardt
2025-05-12 10:35         ` shejialuo
2025-05-12 14:41           ` Patrick Steinhardt
2025-05-12 13:06       ` Jeff King
2025-05-13  6:55         ` shejialuo
2025-05-11 14:01     ` [PATCH v3 3/3] packed-backend: mmap large "packed-refs" file during fsck shejialuo
2025-05-12 13:08       ` Jeff King
2025-05-13 11:06     ` [PATCH v4 0/3] align the behavior when opening "packed-refs" shejialuo
2025-05-13 11:07       ` [PATCH v4 1/3] packed-backend: fsck should warn when "packed-refs" file is empty shejialuo
2025-05-13 16:30         ` Junio C Hamano
2025-05-14 12:51           ` shejialuo
2025-05-13 11:07       ` [PATCH v4 2/3] packed-backend: extract snapshot allocation in `load_contents` shejialuo
2025-05-13 11:07       ` [PATCH v4 3/3] packed-backend: mmap large "packed-refs" file during fsck shejialuo
2025-05-13 16:51         ` Junio C Hamano
2025-05-14 13:05           ` shejialuo
2025-05-14 15:48       ` [PATCH v5 0/3] align the behavior when opening "packed-refs" shejialuo
2025-05-14 15:50         ` shejialuo [this message]
2025-05-14 15:50         ` [PATCH v5 2/3] packed-backend: extract snapshot allocation in `load_contents` shejialuo
2025-05-14 15:50         ` [PATCH v5 3/3] packed-backend: mmap large "packed-refs" file during fsck shejialuo
2025-05-15 12:57         ` [PATCH v5 0/3] align the behavior when opening "packed-refs" Junio C Hamano
2025-05-21 16:31         ` Junio C Hamano
2025-05-22  5:50           ` Jeff King
2025-05-23  9:40             ` Patrick Steinhardt
2025-05-23 15:58               ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aCS7wk-ZCBRiHMqF@ArchLinux \
    --to=shejialuo@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=ps@pks.im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).