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

From: Patrick Steinhardt <ps@pks.im>
Date: Mar 11, 2022, 09:10 UTC
Message-ID: <YisR92nI81SHlOYb@ncase>
In-Reply-To: <xmqqzglxek5y.fsf@gitster.g>

On Thu, Mar 10, 2022 at 10:28:57AM -0800, Junio C Hamano wrote:

Show 23 quoted lines

> Patrick Steinhardt <ps@pks.im> 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