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

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 7, 2024, 21:06 UTC
Message-ID: <xmqqwmqdkb0d.fsf@gitster.g>
In-Reply-To: <Zeopa57o1fMxPoZg@framework>

Patrick Steinhardt <ps@pks.im> writes:

Show 14 quoted lines

> On Thu, Mar 07, 2024 at 09:59:50AM -0800, Junio C Hamano wrote:
>> Patrick Steinhardt <ps@pks.im> 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.