Re: [PATCH 1/4] refs: add referent parameter to refs_resolve_ref_unsafe

From: Patrick Steinhardt <ps@pks.im>
Date: Jun 10, 2024, 07:29 UTC
Message-ID: <ZmarVcF5JjsZx0dl@tanuki>
In-Reply-To: <xmqqv82mkk15.fsf@gitster.g>

On Thu, Jun 06, 2024 at 11:23:18AM -0700, Junio C Hamano wrote:

Show 18 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > "John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:
> >
> >>  29 files changed, 64 insertions(+), 52 deletions(-)
> >
> > Wow, the blast radius of this thing is rather big.  Among these
> > existing callers of refs_resolve_ref_unsafe(), how many of them
> > will benefit from being able to pass a non NULL parameter at the end
> > of the series, and more importantly, in the future to take advantage
> > of the new feature possibly with a separate series?
> > ...
> > That way, you do not have to touch those existing callers that will
> > never benefit from the new capability in the future.  You won't risk
> > conflicting with in flight topics semantically, either.
> 
> The same comment applies to [3/4], but I do not want to fix the Cc: header
> again, so I am replying to this message.

I wonder whether we can future-proof the code a bit more by introducing a struct that contains everything we want to pass to the callback function. That wouldn't fix the blast radius of this patch, but would mean that we can easily add additional fielids to that struct in the future without having to adapt any callers at all anymore.

Something like:

    struct ref_data {
        const char *refname;
        const struct object_id *oid;
        const char *referent;
        int flags;
    };

This would also allow us to get rid of this awful `peel_iterated_oid()` function that relies on global state in many places, as we can put the peeled OID into that structure, as well.

Patrick