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

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 26, 2021, 17:47 UTC
Message-ID: <xmqqpmv5j7b4.fsf@gitster.g>
In-Reply-To: <87fsw1skcb.fsf@evledraar.gmail.com>

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

Show 14 quoted lines

>> 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?

It wasn't that deep.  It was just

 - The constness of the oids _expire() method of backends and
   _prepare() callback take used to be the same;

 - The recent files backend update made them different, because the
   callback made to _prepare() by the files backend wants to lose
   constness because files backend wants to pass the oid embedded in
   the lock object now.

But do we expect other backends, like reftable, to also use lockfile API so that they will have a writable oid handy to pass their calls to _prepare()? I somehow doubted it. If they want to obtain a writable place because _prepare() callback now wants to take a writable oid, they can of course create an on-stack copy and pass it, but that feels to be working around this "files-backend centric" change.

In any case, we seem to be focusing on an irrelevant tangent in this discussion; what made me feel that this change was files-backend centric does not matter much in the larger picture. What matters more now is that this change does not work well with Han-Wen's latest round, and we need to know what the final shape of these APIs are.

Thanks.