Re: [PATCH v2] rebase: skip branch symref aliases

[Phillip Wood <phillip.wood123@gmail.com>]

On 03/06/2026 11:27, Son Luong Ngoc via GitGitGadget wrote:
> From: Son Luong Ngoc <sluongng@gmail.com>
> 
> git rebase --update-refs can fail after the normal rebase path has
> updated the current branch when another local branch is a symref to it.
> This can happen during a default-branch rename where refs/heads/main
> points at refs/heads/master while users migrate.
> 
> The sequencer queues update-ref commands from local branch decorations.
> Commit 106b6885c7 (rebase: ignore non-branch update-refs) filters out
> decorations that are not local branches, such as HEAD and tags. A branch
> symref is different: it is still a local branch decoration, but if it
> resolves to another branch then that target branch is itself present in
> the decoration list and will be updated as a concrete branch.
> 
> Skip branch decorations whose symrefs resolve to refs/heads/*, because
> those targets are already represented by concrete branch decorations.
> This prevents aliases from scheduling a second update for the same
> branch. Keep symrefs to non-branch targets on the existing path.

Makes sense

> Preserve the existing checked-out branch handling before applying these
> skips. Such refs still need a todo-list comment instead of an update-ref
> command, even when the checked-out ref is the branch being rebased or a
> branch symref alias. Use a copy of the resolved HEAD ref so later ref
> resolution does not overwrite it.

I don't quite understand this. A symref that points to another branch 
should always be skipped. When we look up which branches are checked out 
(see worktree.c:add_head_info()) we use

  	refs_resolve_ref_unsafe(get_worktree_ref_store(wt),
				 "HEAD",
				 0,
				 &wt->head_oid, &flags);

so it will never report a symref as being checked out - it always 
resolves any symrefs first.

If we have a symref pointing somewhere outside of "refs/heads" then we 
need to check whether the target is checked out, not the symref itself. 
I'm not sure how likely that is to happen in practice.

> diff --git a/sequencer.c b/sequencer.c
> index 1ee4b2875b..6ab8b47108 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -6445,28 +6445,46 @@ static int add_decorations_to_list(const struct commit *commit,
>   				   struct todo_add_branch_context *ctx)
>   {
>   	const struct name_decoration *decoration = get_name_decoration(&commit->object);
> -	const char *head_ref = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
> -						       "HEAD",
> -						       RESOLVE_REF_READING,
> -						       NULL,
> -						       NULL);
> +	struct ref_store *refs = get_main_ref_store(the_repository);
> +	char *head_ref = refs_resolve_refdup(refs, "HEAD",
> +					     RESOLVE_REF_READING,
> +					     NULL, NULL);

This part and the test look good now
>   	while (decoration) {
>   		struct todo_item *item;
>   		const char *path;
> +		const char *resolved_ref;
> +		int flags = 0;
>   		size_t base_offset = ctx->buf->len;
>   
>   		/*
> -		 * If the branch is the current HEAD, then it will be
> -		 * updated by the default rebase behavior.
> -		 * Exclude it from the list of refs to update,
> -		 * as well as any non-branch decorations.
>   		 * Non-branch decorations may be present if the pretty format
>   		 * includes "%d", which would have loaded all refs
>   		 * into the global decoration table.
>   		 */
> -		if ((head_ref && !strcmp(head_ref, decoration->name)) ||
> -		    (decoration->type != DECORATION_REF_LOCAL)) {
> +		if (decoration->type != DECORATION_REF_LOCAL) {
> +			decoration = decoration->next;
> +			continue;
> +		}

If a decoration matches the current branch why don't we just skip it 
like we used to? (As an aside the existing code in wrong because if the 
user runs "git rebase --update-refs <upstream> <branch>" HEAD does not 
point to "<branch>" but lets not worry about that now)

> +		path = branch_checked_out(decoration->name);

As I said above if the symref target is anther branch we should skip it 
and if the target is not a branch then we need to check if the target is 
checked out so we need to resolve the ref before calling 
branch_checked_out().

Thanks

Phillip