Re: [PATCH v3 1/2] rebase: skip branch symref aliases

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Aug 7, 2026, 15:28 UTC
Message-ID: <1eba5fb2-ab76-41e9-955d-e283256ad25d@gmail.com>
In-Reply-To: <xmqqh5l7huuc.fsf@gitster.g>

On 06/08/2026 21:12, Junio C Hamano wrote:

Show 33 quoted lines

> Phillip Wood <phillip.wood123@gmail.com> writes:
> 
>> On 29/07/2026 15:26, Junio C Hamano wrote:
>>> Phillip Wood <phillip.wood123@gmail.com> writes:
>>>
>>>>> But that was about a low level mechanism that must be more lenient
>>>>> to be usable as repair tools to recover from such a broken state,
>>>>> no?
>>>>
>>>> It checks the new value of HEAD, not the old one so I don't think so.
>>>> The commit message talks about topgit using "git symbolic-ref" to set
>>>> head outside "refs/heads/" - peff had previously tried to tighten it to
>>>> reject non-branch refs but that broke topgit. I've just had a quick look
>>>> at the topgit code and still sets HEAD to point to "refs/top-bases/..."
>>>> by default[1], although there are plans to start using
>>>> "refs/heads/{top-bases}/..." instead.
>>>
>>> Ah, that name vaguely rings a bell.  Is it still in use, and now
>>> they prevent us from forbidding funny characters like {} in the
>>> refname?  Sigh...
>>
>> Yes, it still seems to be maintained, I guess they chose the funny
>> characters to try and avoid name collisions because no-one would want
>> them in a "normal" branch name.
>>
>> Thanks
> 
> I guess I dropped the ball here.  So given that we are OK to see
> HEAD pointing outside refs/heads/, this subthread should be closed.
> 
> My understanding is that we still have an issue reported by Erik in
> <amSSYagL0jTgzElD@mbp> that needs to be addressed before this topic
> can move further?

Maybe I misunderstood but I thought Erik was pointing out a bug in v2 that was fixed in v3. I do think we want a re-roll though for

 >> +        /*
 >> +         * If the branch is the current HEAD, then it will be
 >> +         * updated by the default rebase behavior.
 >> +         */
 >> +        if (head_ref && !strcmp(head_ref, decoration->name)) {
 >> +            free(resolved_ref);
 >>               decoration = decoration->next;
 >>               continue;
 >>           }
 > [...]
 > Should we be using "resolved_ref" instead of "decoration->name"? That
 > would explain why this was moved and would makes sense as we resolve
 > symrefs when reading HEAD. When HEAD points outside "refs/heads/" 
we'd > then skip updating any symrefs under "refs/heads/" that pointed 
to the > same ref as HEAD.

in [1]. I've also just left some comments on the second patch

Thanks

Phillip

[1] https://lore.kernel.org/git/00e529b6-7ae7-463f-a4b3-0991e9411aba@gmail.com