From: Junio C Hamano Date: Sun, 26 Jul 2026 15:42:57 GMT Subject: Re: [PATCH v3 1/2] rebase: skip branch symref aliases Message-ID: In-Reply-To: <00e529b6-7ae7-463f-a4b3-0991e9411aba@gmail.com> Phillip Wood writes: >> 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. >>> +        /* >>> +         * 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?