Re: [PATCH v3 06/12] refs API: pass the "lock OID" to reflog "prepare"

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 23, 2021, 20:49 UTC
Message-ID: <xmqqzguclpr2.fsf@gitster.g>
In-Reply-To: <87lf5wstlv.fsf@evledraar.gmail.com>

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

Show 32 quoted lines

> On Wed, Jul 21 2021, Han-Wen Nienhuys wrote:
>
>> On Wed, Jul 21, 2021 at 7:48 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>>  Junio C Hamano <gitster@pobox.com> writes:
>>
>>  > This obviously breaks the latest round of reftable topic, as it
>>  > still wants this type to take const oid, and I do not think using
>>  > on-filesystem lock as if we are using the files backend is not a
>>  > good solution.
>>
>>  Sorry for redundant negation.  "I do not think it is a good solution
>>  to have everybody pretend as if they are files backend when they
>>  lock refs." was what I meant.
>>
>> Reftable could easily read the current OID for the reference, if necessary. 
>
> (I'm replying to a mail of Han-Wen's that didn't make it on-list due to
> inline HTML, quoted here in its entirety sans signature, see
> https://lore.kernel.org/git/87eebptr7i.fsf@evledraar.gmail.com/)
>
> Junio: I can change the const around if desired. I thought we weren't
> particularly concerned about it in general except to avoid the verbosity
> of frequent casting, and in this case the lock API doesn't have "const".
>
> But as for the reftable incompatibility it seems to me irrespective of
> backend that a reflog API that supports expiry is going to want to have
> a callback for "give me a lock to expire this branch" and give you a
> reply of "OK, you have the lock, you can expire the log, and it's at
> this OID".
>
> Why would it be file-backend specific?

If you feed const oid to *_reflog_expire() method of any backend (including the ones that that are *not* files backend), and if you expect they all will use lockfile API to copy it into lock->old_oid so that it can be fed safely to prepare_fn(), then the arrangement for constness you have set up in your series would work out OK for everybody. But for any backend that does not use one-file-per-ref filesystem mapping, it is rather strange to use lockfile API for locking refs, no? THat is what I meant by files-backend specific.