* [PATCH 2/2] rebase: skip branch symref aliases
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-05-28 5:42 ` 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
3 siblings, 2 replies; 8+ messages in thread
From: Son Luong Ngoc via GitGitGadget @ 2026-05-28 5:42 UTC (permalink / raw)
To: git; +Cc: Son Luong Ngoc, Son Luong Ngoc
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
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v2] rebase: skip branch symref aliases
2026-05-28 5:41 [PATCH 0/2] rebase: handle --update-refs branch symrefs Son Luong Ngoc via GitGitGadget
` (2 preceding siblings ...)
2026-05-28 20:42 ` [PATCH 0/2] rebase: handle --update-refs branch symrefs Junio C Hamano
@ 2026-06-03 10:27 ` Son Luong Ngoc via GitGitGadget
3 siblings, 0 replies; 8+ messages in thread
From: Son Luong Ngoc via GitGitGadget @ 2026-06-03 10:27 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Phillip Wood, Son Luong Ngoc,
Son Luong Ngoc
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.
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.
Signed-off-by: Son Luong Ngoc <sluongng@gmail.com>
---
rebase: handle --update-refs branch symrefs
Changes since v1:
* Squashed the regression test and fix into a single patch.
* Reworked the implementation per Phillip's review: skip local branch
symrefs that are not checked out when their targets resolve to
refs/heads/*, while preserving the existing behavior for symrefs to
non-branch targets.
* Preserved checked-out branch comments before applying the new symref
skip, so worktrees still get the existing todo-list warning instead
of an update-ref command.
* Folded the symref coverage into the existing "--update-refs updates
refs correctly" test, covering both an alias of HEAD and an alias of
another branch.
* Kept the relationship to 106b6885c7 in the commit message: non-local
decorations are already filtered, and this patch handles the
remaining local branch decorations that are symref aliases.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2126%2Fsluongng%2Fsl%2Frebase-update-refs-symrefs-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2126/sluongng/sl/rebase-update-refs-symrefs-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2126
Range-diff vs v1:
1: a550923440 < -: ---------- t3404: add failing branch symref test
2: 0ab0a71744 ! 1: 68f698225c rebase: skip branch symref aliases
@@ Metadata
## Commit message ##
rebase: skip branch symref aliases
- 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.
+ 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.
- 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.
+ 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.
+
+ 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.
Signed-off-by: Son Luong Ngoc <sluongng@gmail.com>
@@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,
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,
+- 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;
++ struct ref_store *refs = get_main_ref_store(the_repository);
++ char *head_ref = refs_resolve_refdup(refs, "HEAD",
++ RESOLVE_REF_READING,
++ NULL, NULL);
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;
/*
-@@ sequencer.c: 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.
-+ *
+- * 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.
@@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,
+ continue;
+ }
+
-+ if (head_ref && !strcmp(head_ref, ref)) {
++ path = branch_checked_out(decoration->name);
++
++ /*
++ * 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, unless it is checked out and
++ * needs a comment in the todo list.
++ */
++ if (!path && head_ref && !strcmp(head_ref, decoration->name)) {
+ decoration = decoration->next;
+ continue;
+ }
+
-+ strbuf_reset(&update_ref);
-+ resolved_ref = refs_resolve_ref_unsafe(refs, ref,
-+ RESOLVE_REF_READING |
-+ RESOLVE_REF_NO_RECURSE,
++ resolved_ref = refs_resolve_ref_unsafe(refs, decoration->name,
++ RESOLVE_REF_READING,
+ 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)) {
++ if (!path && resolved_ref && (flags & REF_ISSYMREF) &&
++ starts_with(resolved_ref, "refs/heads/")) {
decoration = decoration->next;
continue;
}
@@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,
/* If the branch is checked out, then leave a comment instead. */
- if ((path = branch_checked_out(decoration->name))) {
-+ if ((path = branch_checked_out(ref))) {
++ if (path) {
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;
@@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,
decoration = decoration->next;
}
-+ strbuf_release(&update_ref);
-+ free(resolved_head_ref);
++ free(head_ref);
return 0;
}
## t/t3404-rebase-interactive.sh ##
@@ t/t3404-rebase-interactive.sh: 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 &&
+ test_expect_success '--update-refs updates refs correctly' '
++ test_when_finished "
++ test_might_fail git symbolic-ref -d refs/heads/no-conflict-branch-alias &&
++ test_might_fail git symbolic-ref -d refs/heads/second-alias
++ " &&
+ git checkout -B update-refs no-conflict-branch &&
+ git branch -f base HEAD~4 &&
+ git branch -f first HEAD~3 &&
+ git branch -f second HEAD~3 &&
+ git branch -f third HEAD~1 &&
++ git symbolic-ref refs/heads/no-conflict-branch-alias \
++ refs/heads/no-conflict-branch &&
++ git symbolic-ref refs/heads/second-alias refs/heads/second &&
+ test_commit extra2 fileX &&
+ git commit --amend --fixup=L &&
+
+@@ t/t3404-rebase-interactive.sh: test_expect_success '--update-refs updates refs correctly' '
+
+ test_cmp_rev HEAD~3 refs/heads/first &&
+ test_cmp_rev HEAD~3 refs/heads/second &&
++ test_cmp_rev HEAD~3 refs/heads/second-alias &&
+ test_cmp_rev HEAD~1 refs/heads/third &&
+ test_cmp_rev HEAD refs/heads/no-conflict-branch &&
++ test_cmp_rev HEAD refs/heads/no-conflict-branch-alias &&
++ test_write_lines refs/heads/no-conflict-branch >expect &&
++ git symbolic-ref refs/heads/no-conflict-branch-alias >actual &&
++ test_cmp expect actual &&
++ test_write_lines refs/heads/second >expect &&
++ git symbolic-ref refs/heads/second-alias >actual &&
++ test_cmp expect actual &&
+
+ q_to_tab >expect <<-\EOF &&
+ Successfully rebased and updated refs/heads/update-refs.
sequencer.c | 43 +++++++++++++++++++++++++----------
t/t3404-rebase-interactive.sh | 15 ++++++++++++
2 files changed, 46 insertions(+), 12 deletions(-)
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);
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;
+ }
+
+ path = branch_checked_out(decoration->name);
+
+ /*
+ * 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, unless it is checked out and
+ * needs a comment in the todo list.
+ */
+ if (!path && head_ref && !strcmp(head_ref, decoration->name)) {
+ decoration = decoration->next;
+ continue;
+ }
+
+ resolved_ref = refs_resolve_ref_unsafe(refs, decoration->name,
+ RESOLVE_REF_READING,
+ NULL, &flags);
+ if (!path && resolved_ref && (flags & REF_ISSYMREF) &&
+ starts_with(resolved_ref, "refs/heads/")) {
decoration = decoration->next;
continue;
}
@@ -6478,7 +6496,7 @@ 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) {
item->command = TODO_COMMENT;
strbuf_commented_addf(ctx->buf, comment_line_str,
"Ref %s checked out at '%s'\n",
@@ -6501,6 +6519,7 @@ static int add_decorations_to_list(const struct commit *commit,
decoration = decoration->next;
}
+ free(head_ref);
return 0;
}
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 58b3bb0c27..bc0b6a31f7 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1979,11 +1979,18 @@ test_expect_success '--update-refs ignores non-branch decorations' '
'
test_expect_success '--update-refs updates refs correctly' '
+ test_when_finished "
+ test_might_fail git symbolic-ref -d refs/heads/no-conflict-branch-alias &&
+ test_might_fail git symbolic-ref -d refs/heads/second-alias
+ " &&
git checkout -B update-refs no-conflict-branch &&
git branch -f base HEAD~4 &&
git branch -f first HEAD~3 &&
git branch -f second HEAD~3 &&
git branch -f third HEAD~1 &&
+ git symbolic-ref refs/heads/no-conflict-branch-alias \
+ refs/heads/no-conflict-branch &&
+ git symbolic-ref refs/heads/second-alias refs/heads/second &&
test_commit extra2 fileX &&
git commit --amend --fixup=L &&
@@ -1991,8 +1998,16 @@ test_expect_success '--update-refs updates refs correctly' '
test_cmp_rev HEAD~3 refs/heads/first &&
test_cmp_rev HEAD~3 refs/heads/second &&
+ test_cmp_rev HEAD~3 refs/heads/second-alias &&
test_cmp_rev HEAD~1 refs/heads/third &&
test_cmp_rev HEAD refs/heads/no-conflict-branch &&
+ test_cmp_rev HEAD refs/heads/no-conflict-branch-alias &&
+ test_write_lines refs/heads/no-conflict-branch >expect &&
+ git symbolic-ref refs/heads/no-conflict-branch-alias >actual &&
+ test_cmp expect actual &&
+ test_write_lines refs/heads/second >expect &&
+ git symbolic-ref refs/heads/second-alias >actual &&
+ test_cmp expect actual &&
q_to_tab >expect <<-\EOF &&
Successfully rebased and updated refs/heads/update-refs.
base-commit: c69baaf57ba26cf117c2b6793802877f19738b0d
--
gitgitgadget
^ permalink raw reply related [flat|nested] 8+ messages in thread