Re: [PATCH v2 1/6] refs: properly apply exclude patterns to namespaced refs
- From
Taylor Blau <me@ttaylorr.com>
- Date
- Sep 17, 2024, 09:12 UTC
- Message-ID
- <ZulIB7k18+4CzwZb@nand.local>
- In-Reply-To
- <7497166422ea702aabdf4159b0d7780f1422ba13.1726476401.git.ps@pks.im>
On Mon, Sep 16, 2024 at 10:50:03AM +0200, Patrick Steinhardt wrote:
Show 16 quoted lines
> This only surfaces when: > > - A repository uses reference namespaces. > > - "transfer.hideRefs" is active. > > - The namespaced references are packed into the "packed-refs" file. > > None of our tests exercise this scenario, and thus we haven't ever hit > it. While t5509 exercises both (1) and (2), it does not happen to hit > (3). It is trivial to demonstrate the bug though by explicitly packing > refs in the tests, and then we indeed surface the breakage. > > Fix this bug by prefixing exclude patterns with the namespace in the > generic layer. The newly introduced function will be used outside of > "refs.c" in the next patch, so we add a declaration to "refs.h".
Thanks for finding and fixing this bug!
Show 21 quoted lines
> 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;
> +}
> +Is it safe to concatenate each exclude pattern with the specified namespace? If I'm reading this correctly, I think we silently do the wrong thing for exclude patterns that start with '^'.
I guess we reject such patterns in the hidden_refs_to_excludes() function, but perhaps we wouldn't have to if this function stripped those prefixes for us when the caller does or doesn't specify exclude patterns with a '^'?
Thanks, Taylor