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

From: Junio C Hamano <[email-masked]>
Date: Fri, 23 Jul 2021 13:49:37 -0700

Ævar Arnfjörð Bjarmason <[email-masked]> writes:

> On Wed, Jul 21 2021, Han-Wen Nienhuys wrote:
>
>> On Wed, Jul 21, 2021 at 7:48 PM Junio C Hamano <[email-masked]> wrote:
>>
>>  Junio C Hamano <[email-masked]> 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/[email-masked]/)
>
> 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.