Subject: Re: [PATCH 2/4] reftable/stack: register new tables as tempfiles

From: Junio C Hamano <[email-masked]>
Date: Thu, 07 Mar 2024 13:06:10 -0800

Patrick Steinhardt <[email-masked]> writes:

> On Thu, Mar 07, 2024 at 09:59:50AM -0800, Junio C Hamano wrote:
>> Patrick Steinhardt <[email-masked]> writes:
> [snip]
>> I sense there might be some clean-up opportunities around here.
>> After all, lockfile is (or at least pretends to be) built on top of
>> tempfile, and it is for more permanent (as opposed to temporary)
>> files, but it somehow wasn't a good fit to wrap new tables in this
>> series?
> ...
> As lockfiles to me are rather about mutually exclusive locking I think
> that using tempfiles directly is preferable. As far as I can see there
> is also no real benefit with lockfiles in our context, except for the
> mode handling. But given that we have "default_permissions" I'd say it
> is preferable to consistently use these for now via chmod(3P).

I wasn't talking about the use of temp or lock API in _this_ series,
but was talking about the different permission bit handling between
the two API.  The loose object codepath that uses the tempfile API
is closer to what you are doing, which suffers from the same "at
creation tempfile API does not bother with permission bits because
it may be removed at the end".  The index codepath that uses the
hold_lock_file_for_update() does not have to care, as it gets the
permission right from the start.

Because of these differences, the loose object codepath has to do
the adjust_perm_bits() itself, and similarly, you have to fix the
permission the same.

These callers may become simpler if we give an option to the
git_mkstemps_mode() to return a file whose permission bits are
already correct from the start.