Re: [PATCH v2 2/4] pack-bitmap: fix bug with exact ref match in "pack.preferBitmapTips"

From: Patrick Steinhardt <ps@pks.im>
Date: Feb 6, 2026, 07:13 UTC
Message-ID: <aYWUnM7aA5SJBdwS@pks.im>
In-Reply-To: <aYAIVw5UMQeP3Ilr@nand.local>

On Sun, Feb 01, 2026 at 09:13:43PM -0500, Taylor Blau wrote:

Show 20 quoted lines

> On Fri, Jan 30, 2026 at 02:27:43PM +0100, Patrick Steinhardt wrote:
> > [...] 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.

I think my point is mostly that this somewhat surprising behaviour already exists right now.

Show 31 quoted lines

> 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.

I wouldn't quite frame it in the way of ambiguity, but rather in the way of it being very niche. I would argue that almost nobody out there will use this configuration outside of hosting providers. GitHub will probably use it correctly, GitLab doesn't use it at all.

So I guess it's fine overall if we introduce a breaking change here.

> 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.

I disagree with (a) as you can do strictly less, but being constistent with (b) might be a good thing.

At the end I'm not entirely convinced by the arguments, but as I said I don't have too much skin in the game, either. So let's take your approach.

Thanks!

Patrick