Re: [PATCH v2 2/2] refs.c: stop matching non-directory prefixes in exclude patterns

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

Taylor Blau <me@ttaylorr.com> writes:

> So there is a subtle bug with '--exclude' which is that in the
> packed-refs backend we will consider "refs/heads/bar" to be a pattern
> match against "refs/heads/ba" when we shouldn't. Likewise, the reftable
> backend (which in this case is bug-compatible with the packed backend)
> exhibits the same broken behavior.
> ...
> There is some minor test fallout in the "overlapping excluded regions"
> test, which happens to use 'refs/ba' as an exclude pattern, and expects
> references under the "refs/heads/bar/*" and "refs/heads/baz/*"
> hierarchies to be excluded from the results.
>
> ... test (since the range is no longer
> overlapping under the stricter interpretation of --exclude patterns
> presented here).

The code change, reasoning, and the tests look all good.  It just
leaves a bit awkward aftertaste.

In general, I think our "we have a tree-like structure with patterns
to match paths" code paths, like pathspec matching, are structured
in such a way that the low level is expected to merely cull
candidates early as a performance optimization measure (in other
words, they are allowed false positives and say something matches
when they do not, but not allowed false negatives) and leave the
upper level to further reject the ones that do not match the
pattern.  If packed-refs backend was too loose in its matching and
erroneously considered that refs/heads/bar matched refs/heads/ba
pattern, I would naïvely expect that the upper layer would catch and
reject that refs/heads/bar as not matching.

Apparently that was not happening and that is why we need this fix?

Is the excluded region optimization expected to be powerful enough
to cover all our needs so that we do not need to post-process what
it passes?

Thanks.