Re: [PATCH v2 2/6] builtin/receive-pack: fix exclude patterns when announcing refs

[Taylor Blau <me@ttaylorr.com>]

On Mon, Sep 16, 2024 at 10:50:05AM +0200, Patrick Steinhardt wrote:
> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> index 3f35140e489..478c62ca836 100644
> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -339,12 +339,26 @@ static void show_one_alternate_ref(const struct object_id *oid,
>  static void write_head_info(void)
>  {
>  	static struct oidset seen = OIDSET_INIT;
> +	struct strvec excludes_vector = STRVEC_INIT;
> +	const char **exclude_patterns;
> +
> +	/*
> +	 * We need access to the reference names both with and without their
> +	 * namespace and thus cannot use `refs_for_each_namespaced_ref()`. We
> +	 * thus have to adapt exclude patterns to carry the namespace prefix
> +	 * ourselves.
> +	 */
> +	exclude_patterns = get_namespaced_exclude_patterns(
> +		hidden_refs_to_excludes(&hidden_refs),
> +		get_git_namespace(), &excludes_vector);

OK, so here we use the result of calling hidden_refs_to_excludes() as
the first argument to your new get_namespaced_exclude_patterns().

But I think that in this case when the caller specifies a pattern with
'^', we still do not exclude any references in the backend, since
hidden_refs_to_excludes() will return NULL when there is >1 pattern with
'^' as the first character.

I don't think that this results in broken behavior, since the callback
to the refs API will still be expected to filter out references that it
doesn't want.

>  	refs_for_each_fullref_in(get_main_ref_store(the_repository), "",
> -				 hidden_refs_to_excludes(&hidden_refs),
> -				 show_ref_cb, &seen);
> +				 exclude_patterns, show_ref_cb, &seen);
>  	for_each_alternate_ref(show_one_alternate_ref, &seen);

Thanks,
Taylor