On Tue, Sep 17, 2024 at 05:12:39AM -0400, Taylor Blau wrote:
> On Mon, Sep 16, 2024 at 10:50:03AM +0200, Patrick Steinhardt wrote:
> > 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 '^'?
Yeah, as you mention, `hidden_refs_to_excludes()` drops excludes
completely in case there's any pattern starting with '^' or '!'. So the
current assumption should be safe because we don't use excludes in this
scenario at all.
I also agree that we should eventually think about allowing '^'-scoped
references. We can handle them correctly, but it requires a bit more
thought wire that up. So I'd prefer to keep that out of this series, as
it is basically a new feature.
Patrick
