All of lore.kernel.org
 help / color / mirror / Atom feed
From: Phillip Wood <phillip.wood123@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Son Luong Ngoc via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org,
	Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com>,
	Son Luong Ngoc <sluongng@gmail.com>
Subject: Re: [PATCH v3 1/2] rebase: skip branch symref aliases
Date: Tue, 28 Jul 2026 10:36:33 +0100	[thread overview]
Message-ID: <8631114b-aa6f-446e-9710-92c400320eac@gmail.com> (raw)
In-Reply-To: <xmqq7bmhycxq.fsf@gitster.g>

On 26/07/2026 16:42, Junio C Hamano wrote:
> Phillip Wood <phillip.wood123@gmail.com> 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?

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



  reply	other threads:[~2026-07-28  9:36 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-28  5:41 [PATCH 0/2] rebase: handle --update-refs branch symrefs Son Luong Ngoc via GitGitGadget
2026-05-28  5:42 ` [PATCH 1/2] t3404: add failing branch symref test Son Luong Ngoc via GitGitGadget
2026-06-01 13:52   ` Phillip Wood
2026-05-28  5:42 ` [PATCH 2/2] rebase: skip branch symref aliases Son Luong Ngoc via GitGitGadget
2026-05-28  7:08   ` Kristoffer Haugsbakk
2026-06-01 14:10   ` Phillip Wood
2026-05-28 20:42 ` [PATCH 0/2] rebase: handle --update-refs branch symrefs Junio C Hamano
2026-06-03 10:27 ` [PATCH v2] rebase: skip branch symref aliases Son Luong Ngoc via GitGitGadget
2026-06-04 15:37   ` Phillip Wood
2026-07-22  8:16     ` Son Luong Ngoc
2026-07-22  8:15   ` [PATCH v3 0/2] rebase: handle --update-refs branch symrefs Son Luong Ngoc via GitGitGadget
2026-07-22  8:15     ` [PATCH v3 1/2] rebase: skip branch symref aliases Son Luong Ngoc via GitGitGadget
2026-07-23 18:58       ` Phillip Wood
2026-07-24  9:55         ` Phillip Wood
2026-07-25 11:21           ` Erik Cervin-Edin
2026-07-26 15:42           ` Junio C Hamano
2026-07-28  9:36             ` Phillip Wood [this message]
2026-07-28 14:23               ` Junio C Hamano
2026-07-29  9:31                 ` Phillip Wood
2026-07-22  8:15     ` [PATCH v3 2/2] rebase: guard non-branch symref targets Son Luong Ngoc via GitGitGadget

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8631114b-aa6f-446e-9710-92c400320eac@gmail.com \
    --to=phillip.wood123@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=kristofferhaugsbakk@fastmail.com \
    --cc=phillip.wood@dunelm.org.uk \
    --cc=sluongng@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.