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

[Patrick Steinhardt <ps@pks.im>]

On Mon, May 12, 2025 at 08:25:06PM +0800, shejialuo wrote:
> On Mon, May 12, 2025 at 10:36:50AM +0200, Patrick Steinhardt wrote:
> > 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.
> > 
> 
> I somehow think we would always write header in the Modern Git. But
> "create_snapshot" accept an empty existing "packed-refs" file at
> runtime.
> 
> And header is introduced in 694b7a1999 (repack_without_ref(): write peeled
> refs in the rewritten file, 2013-04-22). At this commit, we would always
> write the header into the "packed-refs" file.

So what happened before this commit? Would we end up writing a
completely empty file?

> But in runtime, we accept empty file or no header of the file content as we
> want to keep compatible. In my humble word, I think we should allow
> empty file at now. Then, In Git 3.0, we tighten all the rules (there
> must always be a header etc) and also update the runtime behavior.

I think it depends. If we can prove that Git (and other, third-party
implementations thereof) wouldn't have ever written such "packed-refs"
files then we should start warning now already, even if we accept such
files at runtime. Otherwise, if there were versions of Git that could
have ended up writing such files I agree that we have to accept this for
now.

But if we were to deprecate the ability to read an empty file I wouldn't
adapt both paths at the Git 3.0 boundary. If we decide that an empty
file is broken we should introduce the check for `git refs verify` way
before we tighten the runtime behaviour. Otherwise there isn't really an
opportunity for people to learn that we are about to remove this.

Patrick