Re: [PATCH v3 3/4] refs: selectively set prefix in the seek functions

From: Karthik Nayak <karthik.188@gmail.com>
Date: Jul 17, 2025, 09:01 UTC
Message-ID: <CAOLa=ZSKsdGDERNS_1hJrBMn0muSOJ46SZz2bn8OHZndJ2Hu3g@mail.gmail.com>
In-Reply-To: <xmqqfrevc3b6.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

Show 23 quoted lines

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> Let's say a user is iterating with a prefix set to 'refs/heads/', this
>> would iterate over all the refs with that prefix. But mid-way the user
>> realizes that they only care about 'refs/heads/feature/' prefix and they
>> ask the iterator to set that as the prefix.
>>
>> In such a situation, the iterator seeks to 'refs/heads/feature/' and
>> will only yield references with that prefix. In short, the previous
>> prefix state was reset.
>
> Yes, even though I wouldn't call such an operation "seek", "Ah, I do
> not need the entire refs/heads/ walked, only refs/heads/feature/ is
> enough" is an operation mode that makes sense.
>
> But not for paging, though.
>
> If your web application is showing all branches, one pageful at a
> time, and the first page ended at refs/heads/feature/something and
> you ended up "seeking" to refs/heads/feature/ to start the second
> page, you do not want your second page to end when the iteration
> goes out of refs/heads/feature/ hierarchy, no?
>

Yup and this (we show all references beyond the seek) is the current implementation. I was talking about the internal implementation of 'refs_iteration_seek()' which is the function used for seek and setting the prefix.

To clarify, this is the current implementation:

$ git for-each-ref 6f4b58c4968eb82277cf5b1cf8775117e5b83de0 commit refs/heads/bar 6f4b58c4968eb82277cf5b1cf8775117e5b83de0 commit refs/heads/feature/x 6f4b58c4968eb82277cf5b1cf8775117e5b83de0 commit refs/heads/feature/y 6f4b58c4968eb82277cf5b1cf8775117e5b83de0 commit refs/heads/foo 6f4b58c4968eb82277cf5b1cf8775117e5b83de0 commit refs/heads/goo/x 6f4b58c4968eb82277cf5b1cf8775117e5b83de0 commit refs/heads/goo/y 6f4b58c4968eb82277cf5b1cf8775117e5b83de0 commit refs/heads/master 6f4b58c4968eb82277cf5b1cf8775117e5b83de0 commit refs/tags/tagged/2 6f4b58c4968eb82277cf5b1cf8775117e5b83de0 commit refs/tags/tagged/3 6f4b58c4968eb82277cf5b1cf8775117e5b83de0 commit refs/tags/v1 6f4b58c4968eb82277cf5b1cf8775117e5b83de0 commit refs/tags/v2

$ git for-each-ref --format="%(refname)" --start-after=refs/heads/goo refs/heads/goo/x refs/heads/goo/y refs/heads/master refs/tags/tagged/2 refs/tags/tagged/3 refs/tags/v1 refs/tags/v2

$ git for-each-ref --format="%(refname)" --start-after=refs/heads/master refs/tags/tagged/2 refs/tags/tagged/3 refs/tags/v1 refs/tags/v2

$ git for-each-ref --format="%(refname)" --start-after=refs/heads/goo/x refs/heads/goo/y refs/heads/master refs/tags/tagged/2 refs/tags/tagged/3 refs/tags/v1 refs/tags/v2

$ git for-each-ref --format="%(refname)" refs/heads/feature refs/heads/feature/x refs/heads/feature/y

You can see we list all references beyond the seek.

Show 10 quoted lines

> It seems to me that the root cause of the confusion is because
> prefix, which is to let iteration finish way before the data runs
> out (instead finish when the iteration steps out of a given
> subhierarchy denoted by the prefix), is somehow abused as the
> current position of the cursor.  Shouldn't they be two separate
> concepts?  The cursor needs to fall within the prefix while the
> iterator is active, so they are not two totally independent things,
> but prefix is pretty much static while the cursor position is very
> dynamic.
>

The prefix setup in 'ref_iteration_seek' does two things, let's consider
prefix: 'refs/heads/feature'
1. It sets the cursor to seek to 'refs/heads/feature'
2. It also sets the internal prefix matching to 'refs/heads/feature'

In Contrast seeking via 'ref_iteration_seek' only sets the cursor to 'refs/heads/feature'.

To make this simpler, we've changed 'ref_iteration_seek' to do:
1. seek the cursor to the requested reference
2. Set prefix if the REF_ITERATOR_SEEK_SET_PREFIX is set, and unset the
prefix otherwise.

The state reset I was talking about in my previous emails refers to step #2 here, where when no 'REF_ITERATOR_SEEK_SET_PREFIX' is set, we remove any previous prefix set.

Show 15 quoted lines

>> This series did start out that way around, so ease of implementation
>> isn't it. It was more of a side-effect of not clearing state.
>
> I am even more worried about usability and correctness aspect of
> what was described here now.  After seeking to refs/heads/feature/,
> do we continue to iterate and step out of refs/heads/feature/
> hierarchy or can we cut off a particular page that started with a
> ref within refs/heads/feature/ subhierarchy when we exhaust refs in
> refs/heads/feature/ and have to wait for getting asked for the next
> page before we show refs/heads/gsomething that is outside
> refs/heads/feature/ and sorts after?  The "I reset to iterate over
> refs/heads/feature/ because the entire refs/heads/ is not what I
> care about" example makes me worried about this.
>
> Thanks.

I think we're crossing paths and talking different things. I hope the examples above clarify things. The current implementation doesn't support '--start-after' and prefix setting at the same time:

$ git for-each-ref --format="%(refname)" --start-after=refs/heads/master refs/heads fatal: cannot use --start-after with patterns

Happy to clarify if this doesn't make sense.

Thanks