Re: [PATCH v3 1/3] packed-backend: fsck should allow an empty "packed-refs" file

From: Patrick Steinhardt <ps@pks.im>
Date: May 12, 2025, 08:36 UTC
Message-ID: <aCGzIlLH_ESNg6-v@pks.im>
In-Reply-To: <aCCtx2mqihlc0M7H@ArchLinux>

On Sun, May 11, 2025 at 10:01:43PM +0800, shejialuo wrote:

> During fsck, an empty "packed-refs" gives an error; this is unwarranted.
> We should just skip checking the content of "packed-refs" just like the
> runtime code paths such as "create_snapshot" which simply returns the
> "snapshot" without checking the content of "packed-refs".

I think this doesn't quite answer the question whether this is a _good_ idea though. The question that we need to answer is whether there are any writing code paths that may end up writing a "packed-refs" file that is completely empty. Modern Git would at least write the packed-refs header, wouldn't it?

The reason why I'm a little sceptical is that there is a common problem with ext4 caused by its delayed allocation [1]. If you:

  1. Write data to a temporary file.
  2. Rename the file into place.
  3. The host system crashes.

Then it may happen that the renamed file is now completely empty.

The root cause is a bug in the application: before renaming the file into place it _must_ fsync the file to disk. Git does that by default, but it is extremely easy to get wrong and we had bugs around this until ~2 years ago, if I remember correctly. We hit the problem several times in our production systems.

So I wonder whether ignoring empty files would cause us to miss such a common error. But I guess if there are valid cases where we may end up with an empty "packed-refs" file we cannot do anything about it.

Patrick

[1]: https://thunk.org/tytso/blog/2009/03/12/delayed-allocation-and-the-zero-length-file-problem/