Subject: Re: [PATCH 8/8] core.fsync: new option to harden packed references

From: Patrick Steinhardt <[email-masked]>
Date: Fri, 11 Mar 2022 10:10:15 +0100

On Thu, Mar 10, 2022 at 10:28:57AM -0800, Junio C Hamano wrote:
> Patrick Steinhardt <[email-masked]> writes:
> 
> > diff --git a/refs/packed-backend.c b/refs/packed-backend.c
> > index 27dd8c3922..32d6635969 100644
> > --- a/refs/packed-backend.c
> > +++ b/refs/packed-backend.c
> > @@ -1262,7 +1262,8 @@ static int write_with_updates(struct packed_ref_store *refs,
> >  		goto error;
> >  	}
> >  
> > -	if (close_tempfile_gently(refs->tempfile)) {
> > +	if (fsync_component(FSYNC_COMPONENT_PACKED_REFS, get_tempfile_fd(refs->tempfile)) ||
> > +	    close_tempfile_gently(refs->tempfile)) {
> >  		strbuf_addf(err, "error closing file %s: %s",
> >  			    get_tempfile_path(refs->tempfile),
> >  			    strerror(errno));
> 
> I do not necessarily agree with the organization to have it as a
> component that is separate from other ref backends, but it is
> very pleasing to see that there is only one fsync necessary for the
> packed backend.
> 
> Nice.

I was mostly adapting to the precedent set by Neeraj, where we also
distinguish loose objects and packed objects. Personally I don't mind
much whether we want to discern those two cases, and I'd be happy to
just merge them into a single "refs" knob. We can still split these up
at a later point if the need ever arises.

Patrick