Re: [PATCH v2 2/4] pack-bitmap: fix bug with exact ref match in "pack.preferBitmapTips"
- From
Taylor Blau <me@ttaylorr.com>
- Date
- Feb 2, 2026, 02:13 UTC
- Message-ID
- <aYAIVw5UMQeP3Ilr@nand.local>
- In-Reply-To
- <20260130-b4-pks-fix-for-each-ref-in-misuse-v2-2-0449b198a681@pks.im>
On Fri, Jan 30, 2026 at 02:27:43PM +0100, Patrick Steinhardt wrote:
Show 15 quoted lines
> [...] There are two possible ways to fix this issue: > > - We can fix the bug by using `refs_for_each_fullref_in()` instead, > which does not strip the prefix at all. Consequently, we would now > start to accept all references that start with the configured > prefix, including exact matches. So if we had "refs/heads/main", we > would both match "refs/heads/main" and "refs/heads/main-branch". > > - Or we can fix the bug by appending a slash to the prefix if it > doesn't already have one. This would mean that we only match > ref hierarchies that start with this prefix. > > The first fix leaves the user with strictly _more_ configuration > options: they can have prefix matches by not appending a slash to the > configuration, and they can have ref hierarchy matches by appending one.
I would definitely like to err on the side of more flexible configuration options, but I am still concerned that this change would lead to somewhat surprising behavior.
A couple of thoughts:
- Like I mentioned in the earlier round, 10e8a9352bc (refs.c: stop matching non-directory prefixes in exclude patterns, 2025-03-06) takes the opposite approach as what is being proposed here. I worry that users will find the difference in behavior between pack.preferBitmapTips and for-each-ref's --exclude patterns to be confusing.
- If a user wants to list all references that start with "refs/heads/ma" in the string prefix sense (that is, matching "refs/heads/ma", "refs/heads/main", "refs/heads/master" and so on), then they would do
$ git for-each-ref 'refs/heads/ma*'
, not 'refs/heads/ma'. In fact, enumerating 'refs/heads/ma' when there exist references "refs/heads/ma/foo", "refs/heads/ma/bar", etc., for-each-ref will output those three references (but only "refs/heads/ma" itself if it exists).
I suppose there is an argument to be made that we are dealing with "patterns" vs. "prefixes" here, but TBH I am not sure that is a distinction that is well-understood by users (nor should we expect it to be).
The original intent of this configuration was that "suffix" in this context meant directory suffix or exact match, not string suffix. The implementation does not match that intent, but I think there is enough ambiguity here that I wouldn't consider the change I'm suggesting to be a breaking one.
Overall, I think interpreting the pack.preferBitmapTips configuration as a reference pattern gives the user both (a) more flexibility in which references to match, and (b) does so in a way that is consistent with 10e8a9352bc and the existing behavior of for-each-ref.
Thanks, Taylor