Re: [PATCH 2/6] reftable/stack: don't call fsync(3p) unless provided

[Patrick Steinhardt <ps@pks.im>]

On Tue, Mar 31, 2026 at 11:09:31AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > Users of the reftable library are expected to provide their own function
> > callback in cases they want to sync(3p) data to disk via the reftable
> > write options. But if no such function was provided we end up calling
> > fsync(3p) directly, which may not even be available on some systems.
> >
> > Drop the call to fsync(3p) and rely on the callback function
> > exclusively.
> 
> Hmph, reftable-backend.c seems to do
> 
> 	refs->write_options.fsync = reftable_be_fsync;
> 
> in its _be_init(), so this change is a no-op in the context of our
> system, so this may be _safe_, but for a caller that wanted a fsync
> to happen, returning to it without doing anything may be a bit
> unexpected.  I am wondering if it should be more like
> 
> 	if (!opts->fsync)
> 		/* BUG("whoa where is your fsync callback???") */
> 		reutrn -1;
> 	return opts->fsync(fd);
> 
> instead.

Hm, I guess that's a fair concern. Anyone who doesn't want to fsync can
just provide a no-op function, so it's easy enough for others to stub
out while being safe by default.

I guess the only question in that case is whether it even makes sense to
specify this as an option, or whether we'd rather want to make this part
of the "system.{c,h}" interface, as well.

Patrick