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

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 7, 2025, 17:31 UTC
Message-ID: <xmqq7c50g32y.fsf@gitster.g>
In-Reply-To: <Z8q99U9u6HQvHXFT@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> I think you've swapped things around a bit by accident. The problem is
> that the patterns were being matched too loosely by the underlying
> backends, which had the consequence that the backends marked too many
> refs as excluded.

OK, I agree it is confusing. As a selection mechanism for refs to be shown or processed, exclusion should be "we omit it because we clearly know this one should not be in the final result, but we may pass questionable ones, relying on our caller to have the final say". As a selection mechanism for refs to be excluded, the logic should be the other way around, so false positive and false negative are going to be swapped. We want the exclusion at the lower layer to only say "this ref clearly matches with given exclusion pattern", but we used to claim matches for refs that shouldn't match.

OK.  Thanks for straightening me out.

Show 5 quoted lines

> What makes me feel a bit uneasy is that for the "files" backend the
> optimization depends on the packed state, which is quite awkward overall
> as our tests may not uncover issues only because we didn't pack refs. I
> don't really see a way to address this potential test gap generically
> though.

True. An obvious optimization for "files" _might_ be to lazily walk the directory hierarchy and skip recursive readdir when a directory clearly matches the given exclusion pattern, but the result of such an optimization (in other words, what would seep through the sieve) to be filtered out at the upper layer would be different from what the "packed-refs" backend does for its optimization, and they would be different for reftable or any other future backends.

But I think that is the nature of lower-level optimization---each backend takes advantage of intimately knowing how it organizes the underlying data, and how they can omit without looking into a bulk of the section of data deeply would be different.

Thanks.