[PATCH 0/2] rebase: handle --update-refs branch symrefs

[PATCH 0/2] rebase: handle --update-refs branch symrefs

[Son Luong Ngoc via GitGitGadget]

git rebase --update-refs can fail after the normal rebase path has
successfully updated the current branch when another local branch is a
symbolic ref to it.

One practical way to arrive at that setup is a default branch rename from
master to main. While the migration is in progress, a user may keep
refs/heads/main as a symbolic ref to refs/heads/master so that both names
continue to work locally.

If pull.rebase is enabled, a plain git pull can then finish the rebase of
master and still fail while trying to update the main alias. The reported
failure looked like this, with line breaks adjusted for the cover letter:

Successfully rebased and updated refs/heads/master.
error: update_ref failed for ref 'refs/heads/main':
cannot lock ref 'refs/heads/main':
is at fc2c7bd5f17abec7861ef759edcd33a1e16662a1
but expected 531cabdfb49098d6ffa502ed4bf91d1b35edfcfa
Updated the following refs with --update-refs:
Failed to update the following refs with --update-refs:
        refs/heads/main


The sequencer builds its update-ref todo commands from local branch
decorations. It excludes the current branch by comparing the literal
decoration name with the resolved HEAD ref, so refs/heads/main is not
recognized as the current branch alias. The final update then dereferences
the alias back to master, but master has already moved, so the old-OID check
fails.

This series keeps that failure mode explicit by adding a known-breakage test
first, so that the behavioral expectation is clear before the fix. The fix
then resolves local branch symref decorations before queuing update-ref
commands, skips aliases of the current branch, and skips duplicate
referents. That keeps the existing old-OID protection on the single real
branch update. It also avoids teaching the final ref update step to treat
this race-looking mismatch as success.

Patch overview:

 * Patch 1 records the branch-symref failure in t3404.
 * Patch 2 flips the test and canonicalizes sequencer behavior.

Son Luong Ngoc (2):
  t3404: add failing branch symref test
  rebase: skip branch symref aliases

 sequencer.c                   | 63 +++++++++++++++++++++++++++++------
 t/t3404-rebase-interactive.sh | 25 ++++++++++++++
 2 files changed, 77 insertions(+), 11 deletions(-)


base-commit: c69baaf57ba26cf117c2b6793802877f19738b0d
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2126%2Fsluongng%2Fsl%2Frebase-update-refs-symrefs-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2126/sluongng/sl/rebase-update-refs-symrefs-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2126
-- 
gitgitgadget

[PATCH 1/2] t3404: add failing branch symref test

[Son Luong Ngoc via GitGitGadget]

From: Son Luong Ngoc <sluongng@gmail.com>

rebase --update-refs queues local branch decorations by their literal
refnames. When a branch such as refs/heads/main is a symbolic ref to
the current branch, the normal rebase path first updates the current
branch and the queued symref update later tries to update the same
referent with the old value it recorded before the rebase.

Add a known-breakage test that exercises this case so that the fix can
flip it to test_expect_success. The expected behavior is that the branch
symref keeps pointing at the rebased current branch.

Signed-off-by: Son Luong Ngoc <sluongng@gmail.com>
---
 t/t3404-rebase-interactive.sh | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 58b3bb0c27..42ba8cc313 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1978,6 +1978,31 @@ test_expect_success '--update-refs ignores non-branch decorations' '
 	test_cmp expect actual
 '
 
+test_expect_failure '--update-refs skips branch symrefs to current branch' '
+	test_when_finished "
+		test_might_fail git rebase --abort &&
+		git checkout primary &&
+		test_might_fail git symbolic-ref -d refs/heads/update-refs-symref-alias &&
+		test_might_fail git branch -D update-refs-symref update-refs-symref-base
+	" &&
+	git checkout -B update-refs-symref-base primary &&
+	test_commit --no-tag update-refs-symref-base symref-base.t &&
+	git checkout -B update-refs-symref &&
+	test_commit --no-tag update-refs-symref-topic symref-topic.t &&
+	git checkout update-refs-symref-base &&
+	test_commit --no-tag update-refs-symref-newbase symref-newbase.t &&
+	git checkout update-refs-symref &&
+	git symbolic-ref refs/heads/update-refs-symref-alias refs/heads/update-refs-symref &&
+
+	git rebase --update-refs update-refs-symref-base 2>err &&
+
+	test_cmp_rev update-refs-symref-base update-refs-symref^ &&
+	test_cmp_rev refs/heads/update-refs-symref refs/heads/update-refs-symref-alias &&
+	test_write_lines refs/heads/update-refs-symref >expect &&
+	git symbolic-ref refs/heads/update-refs-symref-alias >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success '--update-refs updates refs correctly' '
 	git checkout -B update-refs no-conflict-branch &&
 	git branch -f base HEAD~4 &&
-- 
gitgitgadget

[PATCH 2/2] rebase: skip branch symref aliases

[Son Luong Ngoc via GitGitGadget]

From: Son Luong Ngoc <sluongng@gmail.com>

rebase --update-refs records local branch decorations before replaying
commits. If a decoration is a symbolic branch such as refs/heads/main
pointing at refs/heads/master, updating it later dereferences back to
master and can fail because the normal rebase path already moved that
branch.

Resolve local branch symref decorations to their referents before
queuing update-ref commands, and skip duplicates. This keeps branch
aliases from scheduling a second update for the same underlying branch
while still using the existing old-OID check for the single queued
update.

Signed-off-by: Son Luong Ngoc <sluongng@gmail.com>
---
 sequencer.c                   | 63 +++++++++++++++++++++++++++++------
 t/t3404-rebase-interactive.sh |  2 +-
 2 files changed, 53 insertions(+), 12 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 1ee4b2875b..4a83d1337c 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -6445,15 +6445,22 @@ 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",
+	struct ref_store *refs = get_main_ref_store(the_repository);
+	const char *head_ref = refs_resolve_ref_unsafe(refs, "HEAD",
 						       RESOLVE_REF_READING,
-						       NULL,
-						       NULL);
+						       NULL, NULL);
+	char *resolved_head_ref = refs_resolve_refdup(refs, "HEAD",
+						       RESOLVE_REF_READING,
+						       NULL, NULL);
+	struct strbuf update_ref = STRBUF_INIT;
 
 	while (decoration) {
 		struct todo_item *item;
 		const char *path;
+		const char *ref = decoration->name;
+		const char *resolved_ref;
+		int is_symref = 0;
+		int flags = 0;
 		size_t base_offset = ctx->buf->len;
 
 		/*
@@ -6461,12 +6468,44 @@ static int add_decorations_to_list(const struct commit *commit,
 		 * updated by the default rebase behavior.
 		 * Exclude it from the list of refs to update,
 		 * as well as any non-branch decorations.
+		 *
+		 * Resolve branch symrefs after checking for the current HEAD so
+		 * that aliases do not schedule duplicate updates for their
+		 * referents.
+		 *
 		 * 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 (head_ref && !strcmp(head_ref, ref)) {
+			decoration = decoration->next;
+			continue;
+		}
+
+		strbuf_reset(&update_ref);
+		resolved_ref = refs_resolve_ref_unsafe(refs, ref,
+						       RESOLVE_REF_READING |
+						       RESOLVE_REF_NO_RECURSE,
+						       NULL, &flags);
+		if ((flags & REF_ISSYMREF) && resolved_ref) {
+			if (!starts_with(resolved_ref, "refs/heads/")) {
+				decoration = decoration->next;
+				continue;
+			}
+
+			strbuf_addstr(&update_ref, resolved_ref);
+			ref = update_ref.buf;
+			is_symref = 1;
+		}
+
+		if ((is_symref && resolved_head_ref &&
+		     !strcmp(resolved_head_ref, ref)) ||
+		    string_list_has_string(&ctx->refs_to_oids, ref)) {
 			decoration = decoration->next;
 			continue;
 		}
@@ -6478,19 +6517,19 @@ static int add_decorations_to_list(const struct commit *commit,
 		memset(item, 0, sizeof(*item));
 
 		/* If the branch is checked out, then leave a comment instead. */
-		if ((path = branch_checked_out(decoration->name))) {
+		if ((path = branch_checked_out(ref))) {
 			item->command = TODO_COMMENT;
 			strbuf_commented_addf(ctx->buf, comment_line_str,
 					      "Ref %s checked out at '%s'\n",
-					      decoration->name, path);
+					      ref, path);
 		} else {
 			struct string_list_item *sti;
 			item->command = TODO_UPDATE_REF;
-			strbuf_addf(ctx->buf, "%s\n", decoration->name);
+			strbuf_addf(ctx->buf, "%s\n", ref);
 
 			sti = string_list_insert(&ctx->refs_to_oids,
-						 decoration->name);
-			sti->util = init_update_ref_record(decoration->name);
+						 ref);
+			sti->util = init_update_ref_record(ref);
 		}
 
 		item->offset_in_buf = base_offset;
@@ -6501,6 +6540,8 @@ static int add_decorations_to_list(const struct commit *commit,
 		decoration = decoration->next;
 	}
 
+	strbuf_release(&update_ref);
+	free(resolved_head_ref);
 	return 0;
 }
 
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 42ba8cc313..29447c0fc3 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1978,7 +1978,7 @@ test_expect_success '--update-refs ignores non-branch decorations' '
 	test_cmp expect actual
 '
 
-test_expect_failure '--update-refs skips branch symrefs to current branch' '
+test_expect_success '--update-refs skips branch symrefs to current branch' '
 	test_when_finished "
 		test_might_fail git rebase --abort &&
 		git checkout primary &&
-- 
gitgitgadget

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

[Kristoffer Haugsbakk]

On Thu, May 28, 2026, at 07:42, Son Luong Ngoc via GitGitGadget wrote:
>[snip]
> -test_expect_failure '--update-refs skips branch symrefs to current branch' '
> +test_expect_success '--update-refs skips branch symrefs to current branch' '
>  	test_when_finished "
>  		test_might_fail git rebase --abort &&
>  		git checkout primary &&

This style of fixing a bug by:

• Add failing test `test_expect_failure` in the first commit
• Fix the bug in the next commit and flip to `test_expect_success`

Is legitimate and makes it easier to verify that the test really
exercises the regression. But in this project it is preferred to
just do the bug fix + regression test in one patch.

See https://lore.kernel.org/git/xmqqfrdk3aqy.fsf@gitster.g/

> --
> gitgitgadget

Re: [PATCH 0/2] rebase: handle --update-refs branch symrefs

[Junio C Hamano]

"Son Luong Ngoc via GitGitGadget" <gitgitgadget@gmail.com> writes:

> git rebase --update-refs can fail after the normal rebase path has
> successfully updated the current branch when another local branch is a
> symbolic ref to it.
>
> One practical way to arrive at that setup is a default branch rename from
> master to main. While the migration is in progress, a user may keep
> refs/heads/main as a symbolic ref to refs/heads/master so that both names
> continue to work locally.
>
> If pull.rebase is enabled, a plain git pull can then finish the rebase of
> master and still fail while trying to update the main alias. The reported
> failure looked like this, with line breaks adjusted for the cover letter:
>
> Successfully rebased and updated refs/heads/master.
> error: update_ref failed for ref 'refs/heads/main':
> cannot lock ref 'refs/heads/main':
> is at fc2c7bd5f17abec7861ef759edcd33a1e16662a1
> but expected 531cabdfb49098d6ffa502ed4bf91d1b35edfcfa
> Updated the following refs with --update-refs:
> Failed to update the following refs with --update-refs:
>         refs/heads/main

I vaguely recall we saw a different topic that dealt with a
situation somewhat similar to this topic (I think it was about
'describe' giving a name that is not a branch).  How would this mesh
with what the other topic wanted to do?  Instead of filtering out
non-branch names (which the other topic did), here we want to filter
out names that are not concrete branches but pointers to something
else.  Would it mean that the logic here is more broad (i.e., both
wants to filter out names of non-branches), making the other topic
unnecessary?