Thread (22 messages) 22 messages, 6 authors, 4h ago

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

From: Phillip Wood <hidden>
Date: 2026-07-28 09:36:36

On 26/07/2026 16:42, Junio C Hamano wrote:
Phillip Wood [off-list ref] writes:
quoted
quoted
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.
quoted
quoted
quoted
+        /*
+         * 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?
We allow HEAD to point to anything below "refs/" - see e9cc02f0e4 
(symbolic-ref: allow refs/<whatever> in HEAD, 2009-02-13). I've not 
tested it but looking at the code I think rebase handles a non-branch 
HEAD correctly.

It would be nice if we didn't have to worry about non-branch HEADs, 
recently Caleb reported on discord being surprised that git allows HEAD 
to point to a tag.

Thanks

Phillip

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help