Re: [PATCH v3 1/2] rebase: skip branch symref aliases
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 26, 2026, 15:42 UTC
- Message-ID
- <xmqq7bmhycxq.fsf@gitster.g>
- In-Reply-To
- <00e529b6-7ae7-463f-a4b3-0991e9411aba@gmail.com>
Phillip Wood <phillip.wood123@gmail.com> writes:
Show 10 quoted lines
>> Thanks for re-rolling I'm pretty sure the logic is sound now but I'm a >> bit confused by a couple of things - see my comments below. >> ... >> It would be nice to have a comment here explaining what we're doing. >> Also I don't think we need to copy the refname so it would be more >> efficient to use refs_resolve_ref_unsafe(). > > Looking at this again we cannot use refs_resolve_ref_unsafe() because > the result would be overwritten by the call to refs_resolve_refdup() in > branch_checked_out().
Makes sense. Thanks for raising a possible alternative and then clarifying that it is not quite workable.
Show 18 quoted lines
>>> + /*
>>> + * 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;
>>> }
>>
>> Then we check to see if the decoration matches HEAD which we used to do
>> above - I'm not clear why we have moved this check.
>
> 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.Yeah, decoration is very much end-user facing and if we can make behavioural decision based on a more stable resolved_ref that would make it easier to reason about.
But stepping back a bit, is having a HEAD that is a symref and points outside "refs/heads/" an invalid state? Why are we catering to such a configuration to begin with?