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, 14:39 UTC
Message-ID: <aCIIL6IWiiWiGbFd@pks.im>
In-Reply-To: <aCHoovrKiSUemBCL@ArchLinux>

On Mon, May 12, 2025 at 08:25:06PM +0800, shejialuo wrote:

Show 13 quoted lines

> 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