Re: [PATCH 1/6] refs: properly apply exclude patterns to namespaced refs

> Reference namespaces allow commands like git-upload-pack(1) to serve
> different sets of references to the client depending on which namespace
> is enabled, which is for example useful in fork networks. Namespaced
> refs are stored with a `refs/namespaces/$namespace` prefix, but all the
> user will ultimately see is a stripped version where that prefix is
> removed.
>
> The way that this interacts with "transfer.hideRefs" is not immediately
> obvious: the hidden refs can either apply to the stripped references, or
> to the non-stripped ones that still have the namespace prefix. In fact,
> the "transfer.hideRefs" machinery does the former and applies to the
> stripped reference by default, but rules can have "^" prefixed to switch
> this behaviour to iinstead match against the rull reference name.

> Namespaces are exclusively handled at the generic "refs" layer, the
> respective backends have no clue that such a thing even exists. This
> also has the consequence that they cannot handle hiding references as
> soon as reference namespaces come into play because they neither know
> whether a namespace is active, nor do they know how to strip references
> if they are active.
>
> Handling such exclude patterns in `refs_for_each_namespaced_ref()` and
> `refs_for_each_fullref_in_prefixes()` is broken though, as both support
> that the user passes both namespaces and exclude patterns. In the case
> where both are set we will exclude references with unstripped names,
> even though we really wanted to exclude references based on their
> stripped names.
>
> This only surfaces when:
>
>   - A repository uses reference namespaces.
>
>   - "transfer.hideRefs" is active.
>
>   - The namespaced references are packed into the "packed-refs" file.
>

> Fix this bug by prefixing exclude patterns with the namespace in the
> generic layer.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  refs.c                           | 35 ++++++++++++++++++++++++++++----
>  refs.h                           |  9 ++++++++
>  t/t5509-fetch-push-namespaces.sh |  1 +
>  3 files changed, 41 insertions(+), 4 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index ceb72d4bd74..b3a367ea12c 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1517,6 +1517,19 @@ const char **hidden_refs_to_excludes(const struct strvec *hide_refs)
>  	return hide_refs->v;
>  }
>
> +const char **get_namespaced_exclude_patterns(const char **exclude_patterns,
> +					     const char *namespace,
> +					     struct strvec *out)
> +{
> +	if (!namespace || !*namespace || !exclude_patterns || !*exclude_patterns)

> +		return exclude_patterns;
> +
> +	for (size_t i = 0; exclude_patterns[i]; i++)
> +		strvec_pushf(out, "%s%s", namespace, exclude_patterns[i]);
> +
> +	return out->v;
> +}
> +
>  const char *find_descendant_ref(const char *dirname,
>  				const struct string_list *extras,
>  				const struct string_list *skip)
> @@ -1634,11 +1647,19 @@ int refs_for_each_namespaced_ref(struct ref_store *refs,
>  				 const char **exclude_patterns,
>  				 each_ref_fn fn, void *cb_data)
>  {
> -	struct strbuf buf = STRBUF_INIT;
> +	struct strvec namespaced_exclude_patterns = STRVEC_INIT;
> +	struct strbuf prefix = STRBUF_INIT;
>  	int ret;
> -	strbuf_addf(&buf, "%srefs/", get_git_namespace());
> -	ret = do_for_each_ref(refs, buf.buf, exclude_patterns, fn, 0, 0, cb_data);
> -	strbuf_release(&buf);
> +
> +	exclude_patterns = get_namespaced_exclude_patterns(exclude_patterns,
> +							   get_git_namespace(),
> +							   &namespaced_exclude_patterns);
> +
> +	strbuf_addf(&prefix, "%srefs/", get_git_namespace());
> +	ret = do_for_each_ref(refs, prefix.buf, exclude_patterns, fn, 0, 0, cb_data);
> +
> +	strvec_clear(&namespaced_exclude_patterns);
> +	strbuf_release(&prefix);
>  	return ret;
>  }
>
> @@ -1719,6 +1740,7 @@ int refs_for_each_fullref_in_prefixes(struct ref_store *ref_store,
>  				      const char **exclude_patterns,
>  				      each_ref_fn fn, void *cb_data)
>  {
> +	struct strvec namespaced_exclude_patterns = STRVEC_INIT;
>  	struct string_list prefixes = STRING_LIST_INIT_DUP;
>  	struct string_list_item *prefix;
>  	struct strbuf buf = STRBUF_INIT;
> @@ -1730,6 +1752,10 @@ int refs_for_each_fullref_in_prefixes(struct ref_store *ref_store,
>  		strbuf_addstr(&buf, namespace);
>  	namespace_len = buf.len;
>
> +	exclude_patterns = get_namespaced_exclude_patterns(exclude_patterns,
> +							   namespace,
> +							   &namespaced_exclude_patterns);
> +
>  	for_each_string_list_item(prefix, &prefixes) {
>  		strbuf_addstr(&buf, prefix->string);
>  		ret = refs_for_each_fullref_in(ref_store, buf.buf,
> @@ -1739,6 +1765,7 @@ int refs_for_each_fullref_in_prefixes(struct ref_store *ref_store,
>  		strbuf_setlen(&buf, namespace_len);
>  	}
>
> +	strvec_clear(&namespaced_exclude_patterns);
>  	string_list_clear(&prefixes, 0);
>  	strbuf_release(&buf);
>  	return ret;
> diff --git a/refs.h b/refs.h
> index f8b919a1388..3f774e96d18 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -859,6 +859,15 @@ int ref_is_hidden(const char *, const char *, const struct strvec *);
>   */
>  const char **hidden_refs_to_excludes(const struct strvec *hide_refs);
>
> +/*
> + * Prefix all exclude patterns with the namespace, if any. This is required
> + * because exclude patterns apply to the stripped reference name, not the full
> + * reference name with the namespace.
> + */
> +const char **get_namespaced_exclude_patterns(const char **exclude_patterns,
> +					     const char *namespace,
> +					     struct strvec *out);
> +