Subject: Re: [PATCH 1/4] packed-backend: skip checking consistency of empty packed-refs file

From: shejialuo <[email-masked]>
Date: Wed, 7 May 2025 20:10:28 +0800

On Tue, May 06, 2025 at 12:14:59PM -0700, Junio C Hamano wrote:
> shejialuo <[email-masked]> writes:
> 
> > In "load_contents", when the "packed-refs" is empty, we will just return
> > the snapshot. However, we would report an error to the user when
> > checking the consistency of the empty "packed-refs".
> 
> Neither the commit title nor the above paragraph hints that this is
> talking about "fsck" part of the packed-refs subsystem.  That leaves
> the readers confused when they read "with the runtime behavior"
> below.
> 

That's right. My message is vague.

> > We should align with the runtime behavior. As what "load_contents" does,
> > let's check whether the file size is zero and if so, we will skip
> > checking the consistency and simply return.
> 
> How about
> 
> 	During fsck, an empty "packed-refs" file gives an error;
> 	this is unwarranted.  We should instead just return an empty
> 	"snapshot" and let the caller happily declare success, just
> 	like the code paths that implement the runtime use of the
> 	file do.
> 
> or something?
> 
> As to the title
> 
> 	packed-backend: fsck should allow an empty packed-refs file
> 
> is shorter and clearer, I would think.
> 

Thanks, I will improve this in the next version.

> The code change is trivially correct, I think.  Nicely found.