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

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Fri, Jul 23 2021, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> 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.

It won't be using the lockfile API, but as an implementation detail the
non-const old_oid is where the "struct object id *" you get passed comes
from in the file API.

Is that what you mean by the behavior being file-backend specific?