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

[Junio C Hamano <gitster@pobox.com>]

ADMINISTRIVIA.  Check the address you place on the CC: line.  What
we can see for this message at

https://lore.kernel.org/git/011c10f488610b0a795a843bff66723477783761.1717694801.git.gitgitgadget@gmail.com/raw

looks like this.

    Cc: "Phillip Wood [ ]" <phillip.wood123@gmail.com>,
        Kristoffer Haugsbakk <[code@khaugsbakk.name]>,
        "Jeff King [ ]" <peff@peff.net>,
        "Patrick Steinhardt [ ]" <ps@pks.im>,
        "=?UTF-8?Q?Jean-No=C3=ABl?= Avila [ ]" <avila.jn@gmail.com>,
        John Cai <johncai86@gmail.com>,
        John Cai <johncai86@gmail.com>

I fixed them manually, but it wasn't pleasant.  I think we saw a
similar breakage earlier coming via GGG, but I do not recall the
details of how to cause such breakages (iow, what to avoid repeating
this).

Anyway.

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

I am assuming that this will benefit only a selected few and the
callers that would want to take advantage of the new feature will
remain low.  Have you considered renaming refs_resolve_ref_unsafe()
to a new name (say, refs_resolve_ref_unsafe_with_referent()) and
implement the new feature (which is only triggered when the new
parameter gets a non NULL value), make refs_resolve_ref_unsafe() a
very thin wrapper that passes NULL to the new thing?

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.

Or will they also benefit from the new feature in the future?

Offhand, I do not know how a caller like this ...

> diff --git a/add-interactive.c b/add-interactive.c
> index b5d6cd689a1..041d30cf2b3 100644
> --- a/add-interactive.c
> +++ b/add-interactive.c
> @@ -533,7 +533,7 @@ static int get_modified_files(struct repository *r,
>  {
>  	struct object_id head_oid;
>  	int is_initial = !refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
> -						  "HEAD", RESOLVE_REF_READING,
> +						  "HEAD", NULL, RESOLVE_REF_READING,
>  						  &head_oid, NULL);
>  	struct collection_status s = { 0 };
>  	int i;

... would be helped.

Thanks.