{"thread":{"id":"65707","subject":"[PATCH 0/2] rebase: handle --update-refs branch symrefs","startedAt":"2026-05-28T05:42:06Z","lastAt":"2026-08-07T15:28:21Z","messageCount":25,"participants":["Son Luong Ngoc via GitGitGadget","Kristoffer Haugsbakk","Junio C Hamano","Phillip Wood","Son Luong Ngoc","Erik Cervin-Edin"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"544211","messageId":"pull.2126.git.1779946921.gitgitgadget@gmail.com","threadId":"65707","inReplyTo":null,"subject":"[PATCH 0/2] rebase: handle --update-refs branch symrefs","fromName":"Son Luong Ngoc via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-05-28T05:41:59Z","receivedAt":"2026-05-28T05:42:06Z","isPatch":true,"body":"git rebase --update-refs can fail after the normal rebase path has\nsuccessfully updated the current branch when another local branch is a\nsymbolic ref to it.\n\nOne practical way to arrive at that setup is a default branch rename from\nmaster to main. While the migration is in progress, a user may keep\nrefs/heads/main as a symbolic ref to refs/heads/master so that both names\ncontinue to work locally.\n\nIf pull.rebase is enabled, a plain git pull can then finish the rebase of\nmaster and still fail while trying to update the main alias. The reported\nfailure looked like this, with line breaks adjusted for the cover letter:\n\nSuccessfully rebased and updated refs/heads/master.\nerror: update_ref failed for ref 'refs/heads/main':\ncannot lock ref 'refs/heads/main':\nis at fc2c7bd5f17abec7861ef759edcd33a1e16662a1\nbut expected 531cabdfb49098d6ffa502ed4bf91d1b35edfcfa\nUpdated the following refs with --update-refs:\nFailed to update the following refs with --update-refs:\n refs/heads/main\n\n\nThe sequencer builds its update-ref todo commands from local branch\ndecorations. It excludes the current branch by comparing the literal\ndecoration name with the resolved HEAD ref, so refs/heads/main is not\nrecognized as the current branch alias. The final update then dereferences\nthe alias back to master, but master has already moved, so the old-OID check\nfails.\n\nThis series keeps that failure mode explicit by adding a known-breakage test\nfirst, so that the behavioral expectation is clear before the fix. The fix\nthen resolves local branch symref decorations before queuing update-ref\ncommands, skips aliases of the current branch, and skips duplicate\nreferents. That keeps the existing old-OID protection on the single real\nbranch update. It also avoids teaching the final ref update step to treat\nthis race-looking mismatch as success.\n\nPatch overview:\n\n * Patch 1 records the branch-symref failure in t3404.\n * Patch 2 flips the test and canonicalizes sequencer behavior.\n\nSon Luong Ngoc (2):\n t3404: add failing branch symref test\n rebase: skip branch symref aliases\n\n sequencer.c | 63 +++++++++++++++++++++++++++++------\n t/t3404-rebase-interactive.sh | 25 ++++++++++++++\n 2 files changed, 77 insertions(+), 11 deletions(-)\n\n\nbase-commit: c69baaf57ba26cf117c2b6793802877f19738b0d\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2126%2Fsluongng%2Fsl%2Frebase-update-refs-symrefs-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2126/sluongng/sl/rebase-update-refs-symrefs-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/2126\n-- \ngitgitgadget\n"},{"id":"544212","messageId":"a550923440a233daea0b9819e05d6c380de00d09.1779946921.git.gitgitgadget@gmail.com","threadId":"65707","inReplyTo":"pull.2126.git.1779946921.gitgitgadget@gmail.com","subject":"[PATCH 1/2] t3404: add failing branch symref test","fromName":"Son Luong Ngoc via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-05-28T05:42:00Z","receivedAt":"2026-05-28T05:42:08Z","isPatch":true,"body":"From: Son Luong Ngoc \n\nrebase --update-refs queues local branch decorations by their literal\nrefnames. When a branch such as refs/heads/main is a symbolic ref to\nthe current branch, the normal rebase path first updates the current\nbranch and the queued symref update later tries to update the same\nreferent with the old value it recorded before the rebase.\n\nAdd a known-breakage test that exercises this case so that the fix can\nflip it to test_expect_success. The expected behavior is that the branch\nsymref keeps pointing at the rebased current branch.\n\nSigned-off-by: Son Luong Ngoc \n---\n t/t3404-rebase-interactive.sh | 25 +++++++++++++++++++++++++\n 1 file changed, 25 insertions(+)\n\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 58b3bb0c27..42ba8cc313 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -1978,6 +1978,31 @@ test_expect_success '--update-refs ignores non-branch decorations' '\n \ttest_cmp expect actual\n '\n \n+test_expect_failure '--update-refs skips branch symrefs to current branch' '\n+\ttest_when_finished \"\n+\t\ttest_might_fail git rebase --abort &&\n+\t\tgit checkout primary &&\n+\t\ttest_might_fail git symbolic-ref -d refs/heads/update-refs-symref-alias &&\n+\t\ttest_might_fail git branch -D update-refs-symref update-refs-symref-base\n+\t\" &&\n+\tgit checkout -B update-refs-symref-base primary &&\n+\ttest_commit --no-tag update-refs-symref-base symref-base.t &&\n+\tgit checkout -B update-refs-symref &&\n+\ttest_commit --no-tag update-refs-symref-topic symref-topic.t &&\n+\tgit checkout update-refs-symref-base &&\n+\ttest_commit --no-tag update-refs-symref-newbase symref-newbase.t &&\n+\tgit checkout update-refs-symref &&\n+\tgit symbolic-ref refs/heads/update-refs-symref-alias refs/heads/update-refs-symref &&\n+\n+\tgit rebase --update-refs update-refs-symref-base 2>err &&\n+\n+\ttest_cmp_rev update-refs-symref-base update-refs-symref^ &&\n+\ttest_cmp_rev refs/heads/update-refs-symref refs/heads/update-refs-symref-alias &&\n+\ttest_write_lines refs/heads/update-refs-symref >expect &&\n+\tgit symbolic-ref refs/heads/update-refs-symref-alias >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success '--update-refs updates refs correctly' '\n \tgit checkout -B update-refs no-conflict-branch &&\n \tgit branch -f base HEAD~4 &&\n-- \ngitgitgadget\n\n"},{"id":"544213","messageId":"0ab0a717441e9fc7c494da194065a948a35a7f01.1779946921.git.gitgitgadget@gmail.com","threadId":"65707","inReplyTo":"pull.2126.git.1779946921.gitgitgadget@gmail.com","subject":"[PATCH 2/2] rebase: skip branch symref aliases","fromName":"Son Luong Ngoc via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-05-28T05:42:01Z","receivedAt":"2026-05-28T05:42:11Z","isPatch":true,"body":"From: Son Luong Ngoc \n\nrebase --update-refs records local branch decorations before replaying\ncommits. If a decoration is a symbolic branch such as refs/heads/main\npointing at refs/heads/master, updating it later dereferences back to\nmaster and can fail because the normal rebase path already moved that\nbranch.\n\nResolve local branch symref decorations to their referents before\nqueuing update-ref commands, and skip duplicates. This keeps branch\naliases from scheduling a second update for the same underlying branch\nwhile still using the existing old-OID check for the single queued\nupdate.\n\nSigned-off-by: Son Luong Ngoc \n---\n sequencer.c | 63 +++++++++++++++++++++++++++++------\n t/t3404-rebase-interactive.sh | 2 +-\n 2 files changed, 53 insertions(+), 12 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 1ee4b2875b..4a83d1337c 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -6445,15 +6445,22 @@ static int add_decorations_to_list(const struct commit *commit,\n \t\t\t\t struct todo_add_branch_context *ctx)\n {\n \tconst struct name_decoration *decoration = get_name_decoration(&commit->object);\n-\tconst char *head_ref = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),\n-\t\t\t\t\t\t \"HEAD\",\n+\tstruct ref_store *refs = get_main_ref_store(the_repository);\n+\tconst char *head_ref = refs_resolve_ref_unsafe(refs, \"HEAD\",\n \t\t\t\t\t\t RESOLVE_REF_READING,\n-\t\t\t\t\t\t NULL,\n-\t\t\t\t\t\t NULL);\n+\t\t\t\t\t\t NULL, NULL);\n+\tchar *resolved_head_ref = refs_resolve_refdup(refs, \"HEAD\",\n+\t\t\t\t\t\t RESOLVE_REF_READING,\n+\t\t\t\t\t\t NULL, NULL);\n+\tstruct strbuf update_ref = STRBUF_INIT;\n \n \twhile (decoration) {\n \t\tstruct todo_item *item;\n \t\tconst char *path;\n+\t\tconst char *ref = decoration->name;\n+\t\tconst char *resolved_ref;\n+\t\tint is_symref = 0;\n+\t\tint flags = 0;\n \t\tsize_t base_offset = ctx->buf->len;\n \n \t\t/*\n@@ -6461,12 +6468,44 @@ static int add_decorations_to_list(const struct commit *commit,\n \t\t * updated by the default rebase behavior.\n \t\t * Exclude it from the list of refs to update,\n \t\t * as well as any non-branch decorations.\n+\t\t *\n+\t\t * Resolve branch symrefs after checking for the current HEAD so\n+\t\t * that aliases do not schedule duplicate updates for their\n+\t\t * referents.\n+\t\t *\n \t\t * Non-branch decorations may be present if the pretty format\n \t\t * includes \"%d\", which would have loaded all refs\n \t\t * into the global decoration table.\n \t\t */\n-\t\tif ((head_ref && !strcmp(head_ref, decoration->name)) ||\n-\t\t (decoration->type != DECORATION_REF_LOCAL)) {\n+\t\tif (decoration->type != DECORATION_REF_LOCAL) {\n+\t\t\tdecoration = decoration->next;\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tif (head_ref && !strcmp(head_ref, ref)) {\n+\t\t\tdecoration = decoration->next;\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tstrbuf_reset(&update_ref);\n+\t\tresolved_ref = refs_resolve_ref_unsafe(refs, ref,\n+\t\t\t\t\t\t RESOLVE_REF_READING |\n+\t\t\t\t\t\t RESOLVE_REF_NO_RECURSE,\n+\t\t\t\t\t\t NULL, &flags);\n+\t\tif ((flags & REF_ISSYMREF) && resolved_ref) {\n+\t\t\tif (!starts_with(resolved_ref, \"refs/heads/\")) {\n+\t\t\t\tdecoration = decoration->next;\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\n+\t\t\tstrbuf_addstr(&update_ref, resolved_ref);\n+\t\t\tref = update_ref.buf;\n+\t\t\tis_symref = 1;\n+\t\t}\n+\n+\t\tif ((is_symref && resolved_head_ref &&\n+\t\t !strcmp(resolved_head_ref, ref)) ||\n+\t\t string_list_has_string(&ctx->refs_to_oids, ref)) {\n \t\t\tdecoration = decoration->next;\n \t\t\tcontinue;\n \t\t}\n@@ -6478,19 +6517,19 @@ static int add_decorations_to_list(const struct commit *commit,\n \t\tmemset(item, 0, sizeof(*item));\n \n \t\t/* If the branch is checked out, then leave a comment instead. */\n-\t\tif ((path = branch_checked_out(decoration->name))) {\n+\t\tif ((path = branch_checked_out(ref))) {\n \t\t\titem->command = TODO_COMMENT;\n \t\t\tstrbuf_commented_addf(ctx->buf, comment_line_str,\n \t\t\t\t\t \"Ref %s checked out at '%s'\\n\",\n-\t\t\t\t\t decoration->name, path);\n+\t\t\t\t\t ref, path);\n \t\t} else {\n \t\t\tstruct string_list_item *sti;\n \t\t\titem->command = TODO_UPDATE_REF;\n-\t\t\tstrbuf_addf(ctx->buf, \"%s\\n\", decoration->name);\n+\t\t\tstrbuf_addf(ctx->buf, \"%s\\n\", ref);\n \n \t\t\tsti = string_list_insert(&ctx->refs_to_oids,\n-\t\t\t\t\t\t decoration->name);\n-\t\t\tsti->util = init_update_ref_record(decoration->name);\n+\t\t\t\t\t\t ref);\n+\t\t\tsti->util = init_update_ref_record(ref);\n \t\t}\n \n \t\titem->offset_in_buf = base_offset;\n@@ -6501,6 +6540,8 @@ static int add_decorations_to_list(const struct commit *commit,\n \t\tdecoration = decoration->next;\n \t}\n \n+\tstrbuf_release(&update_ref);\n+\tfree(resolved_head_ref);\n \treturn 0;\n }\n \ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 42ba8cc313..29447c0fc3 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -1978,7 +1978,7 @@ test_expect_success '--update-refs ignores non-branch decorations' '\n \ttest_cmp expect actual\n '\n \n-test_expect_failure '--update-refs skips branch symrefs to current branch' '\n+test_expect_success '--update-refs skips branch symrefs to current branch' '\n \ttest_when_finished \"\n \t\ttest_might_fail git rebase --abort &&\n \t\tgit checkout primary &&\n-- \ngitgitgadget\n"},{"id":"544219","messageId":"ec2be3bc-caea-4295-829c-db5fd9e121d7@app.fastmail.com","threadId":"65707","inReplyTo":"0ab0a717441e9fc7c494da194065a948a35a7f01.1779946921.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] rebase: skip branch symref aliases","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2026-05-28T07:08:20Z","receivedAt":"2026-05-28T07:08:41Z","isPatch":true,"body":"On Thu, May 28, 2026, at 07:42, Son Luong Ngoc via GitGitGadget wrote:\n>[snip]\n> -test_expect_failure '--update-refs skips branch symrefs to current branch' '\n> +test_expect_success '--update-refs skips branch symrefs to current branch' '\n> \ttest_when_finished \"\n> \t\ttest_might_fail git rebase --abort &&\n> \t\tgit checkout primary &&\n\nThis style of fixing a bug by:\n\n• Add failing test `test_expect_failure` in the first commit\n• Fix the bug in the next commit and flip to `test_expect_success`\n\nIs legitimate and makes it easier to verify that the test really\nexercises the regression. But in this project it is preferred to\njust do the bug fix + regression test in one patch.\n\nSee https://lore.kernel.org/git/xmqqfrdk3aqy.fsf@gitster.g/\n\n> --\n> gitgitgadget\n"},{"id":"544240","messageId":"xmqqwlwni7vk.fsf@gitster.g","threadId":"65707","inReplyTo":"pull.2126.git.1779946921.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/2] rebase: handle --update-refs branch symrefs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-05-28T20:42:07Z","receivedAt":"2026-05-28T20:42:09Z","isPatch":true,"body":"\"Son Luong Ngoc via GitGitGadget\" writes:\n\n> git rebase --update-refs can fail after the normal rebase path has\n> successfully updated the current branch when another local branch is a\n> symbolic ref to it.\n>\n> One practical way to arrive at that setup is a default branch rename from\n> master to main. While the migration is in progress, a user may keep\n> refs/heads/main as a symbolic ref to refs/heads/master so that both names\n> continue to work locally.\n>\n> If pull.rebase is enabled, a plain git pull can then finish the rebase of\n> master and still fail while trying to update the main alias. The reported\n> failure looked like this, with line breaks adjusted for the cover letter:\n>\n> Successfully rebased and updated refs/heads/master.\n> error: update_ref failed for ref 'refs/heads/main':\n> cannot lock ref 'refs/heads/main':\n> is at fc2c7bd5f17abec7861ef759edcd33a1e16662a1\n> but expected 531cabdfb49098d6ffa502ed4bf91d1b35edfcfa\n> Updated the following refs with --update-refs:\n> Failed to update the following refs with --update-refs:\n> refs/heads/main\n\nI vaguely recall we saw a different topic that dealt with a\nsituation somewhat similar to this topic (I think it was about\n'describe' giving a name that is not a branch). How would this mesh\nwith what the other topic wanted to do? Instead of filtering out\nnon-branch names (which the other topic did), here we want to filter\nout names that are not concrete branches but pointers to something\nelse. Would it mean that the logic here is more broad (i.e., both\nwants to filter out names of non-branches), making the other topic\nunnecessary?\n\n"},{"id":"544389","messageId":"67e1a735-4f7b-476a-8841-09649290dd51@gmail.com","threadId":"65707","inReplyTo":"a550923440a233daea0b9819e05d6c380de00d09.1779946921.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] t3404: add failing branch symref test","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-06-01T13:52:52Z","receivedAt":"2026-06-01T13:52:55Z","isPatch":true,"body":"On 28/05/2026 06:42, Son Luong Ngoc via GitGitGadget wrote:\n> From: Son Luong Ngoc \n> \n> rebase --update-refs queues local branch decorations by their literal\n> refnames. When a branch such as refs/heads/main is a symbolic ref to\n> the current branch, the normal rebase path first updates the current\n> branch and the queued symref update later tries to update the same\n> referent with the old value it recorded before the rebase.\n> \n> Add a known-breakage test that exercises this case so that the fix can\n> flip it to test_expect_success. The expected behavior is that the branch\n> symref keeps pointing at the rebased current branch.\n\nThanks for adding a test, I'd find it easier to review this series if \nthe test was added in the same patch as the fix which is our usual practice.\n\n> +test_expect_failure '--update-refs skips branch symrefs to current branch' '\n> +\ttest_when_finished \"\n> +\t\ttest_might_fail git rebase --abort &&\n> +\t\tgit checkout primary &&\n> +\t\ttest_might_fail git symbolic-ref -d refs/heads/update-refs-symref-alias &&\n> +\t\ttest_might_fail git branch -D update-refs-symref update-refs-symref-base\n> +\t\" &&\n> +\tgit checkout -B update-refs-symref-base primary &&\n> +\ttest_commit --no-tag update-refs-symref-base symref-base.t &&\n> +\tgit checkout -B update-refs-symref &&\n> +\ttest_commit --no-tag update-refs-symref-topic symref-topic.t &&\n> +\tgit checkout update-refs-symref-base &&\n> +\ttest_commit --no-tag update-refs-symref-newbase symref-newbase.t &&\n> +\tgit checkout update-refs-symref &&\n> +\tgit symbolic-ref refs/heads/update-refs-symref-alias refs/heads/update-refs-symref &&\n\nI think we want to test a symref that does not match HEAD as well. \nRather than adding a new test, can we instead add a couple of symref \nbranches to the test \"--update-refs updates refs correctly\"?\n\nThanks\n\nPhillip\n\n> +\n> +\tgit rebase --update-refs update-refs-symref-base 2>err &&\n> +\n> +\ttest_cmp_rev update-refs-symref-base update-refs-symref^ &&\n> +\ttest_cmp_rev refs/heads/update-refs-symref refs/heads/update-refs-symref-alias &&\n> +\ttest_write_lines refs/heads/update-refs-symref >expect &&\n> +\tgit symbolic-ref refs/heads/update-refs-symref-alias >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> test_expect_success '--update-refs updates refs correctly' '\n> \tgit checkout -B update-refs no-conflict-branch &&\n> \tgit branch -f base HEAD~4 &&\n\n"},{"id":"544392","messageId":"f1b662d5-f9bb-4274-ad42-3a2227d2a060@gmail.com","threadId":"65707","inReplyTo":"0ab0a717441e9fc7c494da194065a948a35a7f01.1779946921.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] rebase: skip branch symref aliases","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-06-01T14:10:58Z","receivedAt":"2026-06-01T14:11:01Z","isPatch":true,"body":"On 28/05/2026 06:42, Son Luong Ngoc via GitGitGadget wrote:\n> From: Son Luong Ngoc \n> \n> rebase --update-refs records local branch decorations before replaying\n> commits. If a decoration is a symbolic branch such as refs/heads/main\n> pointing at refs/heads/master, updating it later dereferences back to\n> master and can fail because the normal rebase path already moved that\n> branch.\n\nGood explanation, thanks for working on this.\n\n> Resolve local branch symref decorations to their referents before\n\ns/referents/targets/ ?\n\n> queuing update-ref commands, and skip duplicates. This keeps branch\n> aliases from scheduling a second update for the same underlying branch\n> while still using the existing old-OID check for the single queued\n> update.\n\nThat's not quite what the patch does though - it only checks that the \ntarget of the symref differs from the target of HEAD. If a symref points \nto another branch we still try to update it when we should skip it.\n\n> Signed-off-by: Son Luong Ngoc \n> ---\n> sequencer.c | 63 +++++++++++++++++++++++++++++------\n> t/t3404-rebase-interactive.sh | 2 +-\n> 2 files changed, 53 insertions(+), 12 deletions(-)\n> \n> diff --git a/sequencer.c b/sequencer.c\n> index 1ee4b2875b..4a83d1337c 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -6445,15 +6445,22 @@ static int add_decorations_to_list(const struct commit *commit,\n> \t\t\t\t struct todo_add_branch_context *ctx)\n> {\n> \tconst struct name_decoration *decoration = get_name_decoration(&commit->object);\n> -\tconst char *head_ref = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),\n> -\t\t\t\t\t\t \"HEAD\",\n> +\tstruct ref_store *refs = get_main_ref_store(the_repository);\n> +\tconst char *head_ref = refs_resolve_ref_unsafe(refs, \"HEAD\",\n> \t\t\t\t\t\t RESOLVE_REF_READING,\n> -\t\t\t\t\t\t NULL,\n> -\t\t\t\t\t\t NULL);\n> +\t\t\t\t\t\t NULL, NULL);\n> +\tchar *resolved_head_ref = refs_resolve_refdup(refs, \"HEAD\",\n> +\t\t\t\t\t\t RESOLVE_REF_READING,\n> +\t\t\t\t\t\t NULL, NULL);\n\nWe need to use refs_resolve_refdup() instead of \nrefs_resolve_ref_unsafe() so that the return value is not overwritten by \nthe later calls to refs_resolve_ref_unsafe() that are added below. But \nthat is the only change that is needed - we do not need to add a new \nvariable, we just replace refs_resolve_ref_unsafe() with \nrefs_resole_refdup() and free \"head_ref\" before we return.\n\n> +\tstruct strbuf update_ref = STRBUF_INIT;\n> \n> \twhile (decoration) {\n> \t\tstruct todo_item *item;\n> \t\tconst char *path;\n> +\t\tconst char *ref = decoration->name;\n> +\t\tconst char *resolved_ref;\n> +\t\tint is_symref = 0;\n> +\t\tint flags = 0;\n> \t\tsize_t base_offset = ctx->buf->len;\n> \n> \t\t/*\n> @@ -6461,12 +6468,44 @@ static int add_decorations_to_list(const struct commit *commit,\n> \t\t * updated by the default rebase behavior.\n> \t\t * Exclude it from the list of refs to update,\n> \t\t * as well as any non-branch decorations.\n> +\t\t *\n> +\t\t * Resolve branch symrefs after checking for the current HEAD so\n> +\t\t * that aliases do not schedule duplicate updates for their\n> +\t\t * referents.\n> +\t\t *\n> \t\t * Non-branch decorations may be present if the pretty format\n> \t\t * includes \"%d\", which would have loaded all refs\n> \t\t * into the global decoration table.\n> \t\t */\n> -\t\tif ((head_ref && !strcmp(head_ref, decoration->name)) ||\n> -\t\t (decoration->type != DECORATION_REF_LOCAL)) {\n> +\t\tif (decoration->type != DECORATION_REF_LOCAL) {\n> +\t\t\tdecoration = decoration->next;\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> +\t\tif (head_ref && !strcmp(head_ref, ref)) {\n> +\t\t\tdecoration = decoration->next;\n> +\t\t\tcontinue;\n> +\t\t}\n\nThis is just rewriting the existing if statement which has nothing to do \nwith the stated aim of this patch - lets leave it as it was.\n\n> +\n> +\t\tstrbuf_reset(&update_ref);\n> +\t\tresolved_ref = refs_resolve_ref_unsafe(refs, ref,\n> +\t\t\t\t\t\t RESOLVE_REF_READING |\n> +\t\t\t\t\t\t RESOLVE_REF_NO_RECURSE,\n\nWhy are we passing RESOLVE_REF_NO_RECURSE here? I'd have thought we want \nto resolve the whole chain of symbolic refs to find out which ref is \nactually going to be updated.\n\n> +\t\t\t\t\t\t NULL, &flags);\n> +\t\tif ((flags & REF_ISSYMREF) && resolved_ref) {\n\nI think it is generally safer to check the return value before using any \nof the \"out\" parameters from a function call. In this case the function \nunconditionally clears flags at the beginning so it is safe.\n\n> +\t\t\tif (!starts_with(resolved_ref, \"refs/heads/\")) {\n> +\t\t\t\tdecoration = decoration->next;\n> +\t\t\t\tcontinue;\n\nThis is the opposite of what I was expecting - if the decoration is a \nsymref that resolves to a branch then that branch will also be in the \nlist of decorations and so will be updated. If the decoration is a \nsymref that resolves outside \"refs/heads/\" then we want to add the \ndecoration to the list of refs to update to keep the current behavior.\n\nIf we do that then we skip all symbolic refs that point to another \nbranch, instead of just skipping those that match HEAD and we don't need \nany of the changes below here.\n\nThanks\n\nPhillip\n\n> +\t\t\t}\n> +\n> +\t\t\tstrbuf_addstr(&update_ref, resolved_ref);\n> +\t\t\tref = update_ref.buf;\n> +\t\t\tis_symref = 1;\n> +\t\t}\n> +\n> +\t\tif ((is_symref && resolved_head_ref &&\n> +\t\t !strcmp(resolved_head_ref, ref)) ||\n> +\t\t string_list_has_string(&ctx->refs_to_oids, ref)) {\n> \t\t\tdecoration = decoration->next;\n> \t\t\tcontinue;\n> \t\t}\n> @@ -6478,19 +6517,19 @@ static int add_decorations_to_list(const struct commit *commit,\n> \t\tmemset(item, 0, sizeof(*item));\n> \n> \t\t/* If the branch is checked out, then leave a comment instead. */\n> -\t\tif ((path = branch_checked_out(decoration->name))) {\n> +\t\tif ((path = branch_checked_out(ref))) {\n> \t\t\titem->command = TODO_COMMENT;\n> \t\t\tstrbuf_commented_addf(ctx->buf, comment_line_str,\n> \t\t\t\t\t \"Ref %s checked out at '%s'\\n\",\n> -\t\t\t\t\t decoration->name, path);\n> +\t\t\t\t\t ref, path);\n> \t\t} else {\n> \t\t\tstruct string_list_item *sti;\n> \t\t\titem->command = TODO_UPDATE_REF;\n> -\t\t\tstrbuf_addf(ctx->buf, \"%s\\n\", decoration->name);\n> +\t\t\tstrbuf_addf(ctx->buf, \"%s\\n\", ref);\n> \n> \t\t\tsti = string_list_insert(&ctx->refs_to_oids,\n> -\t\t\t\t\t\t decoration->name);\n> -\t\t\tsti->util = init_update_ref_record(decoration->name);\n> +\t\t\t\t\t\t ref);\n> +\t\t\tsti->util = init_update_ref_record(ref);\n> \t\t}\n> \n> \t\titem->offset_in_buf = base_offset;\n> @@ -6501,6 +6540,8 @@ static int add_decorations_to_list(const struct commit *commit,\n> \t\tdecoration = decoration->next;\n> \t}\n> \n> +\tstrbuf_release(&update_ref);\n> +\tfree(resolved_head_ref);\n> \treturn 0;\n> }\n> \n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> index 42ba8cc313..29447c0fc3 100755\n> --- a/t/t3404-rebase-interactive.sh\n> +++ b/t/t3404-rebase-interactive.sh\n> @@ -1978,7 +1978,7 @@ test_expect_success '--update-refs ignores non-branch decorations' '\n> \ttest_cmp expect actual\n> '\n> \n> -test_expect_failure '--update-refs skips branch symrefs to current branch' '\n> +test_expect_success '--update-refs skips branch symrefs to current branch' '\n> \ttest_when_finished \"\n> \t\ttest_might_fail git rebase --abort &&\n> \t\tgit checkout primary &&\n\n"},{"id":"544599","messageId":"pull.2126.v2.git.1780482436865.gitgitgadget@gmail.com","threadId":"65707","inReplyTo":"pull.2126.git.1779946921.gitgitgadget@gmail.com","subject":"[PATCH v2] rebase: skip branch symref aliases","fromName":"Son Luong Ngoc via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-06-03T10:27:16Z","receivedAt":"2026-06-03T10:27:20Z","isPatch":true,"body":"From: Son Luong Ngoc \n\ngit rebase --update-refs can fail after the normal rebase path has\nupdated the current branch when another local branch is a symref to it.\nThis can happen during a default-branch rename where refs/heads/main\npoints at refs/heads/master while users migrate.\n\nThe sequencer queues update-ref commands from local branch decorations.\nCommit 106b6885c7 (rebase: ignore non-branch update-refs) filters out\ndecorations that are not local branches, such as HEAD and tags. A branch\nsymref is different: it is still a local branch decoration, but if it\nresolves to another branch then that target branch is itself present in\nthe decoration list and will be updated as a concrete branch.\n\nSkip branch decorations whose symrefs resolve to refs/heads/*, because\nthose targets are already represented by concrete branch decorations.\nThis prevents aliases from scheduling a second update for the same\nbranch. Keep symrefs to non-branch targets on the existing path.\n\nPreserve the existing checked-out branch handling before applying these\nskips. Such refs still need a todo-list comment instead of an update-ref\ncommand, even when the checked-out ref is the branch being rebased or a\nbranch symref alias. Use a copy of the resolved HEAD ref so later ref\nresolution does not overwrite it.\n\nSigned-off-by: Son Luong Ngoc \n---\n rebase: handle --update-refs branch symrefs\n \n Changes since v1:\n \n * Squashed the regression test and fix into a single patch.\n * Reworked the implementation per Phillip's review: skip local branch\n symrefs that are not checked out when their targets resolve to\n refs/heads/*, while preserving the existing behavior for symrefs to\n non-branch targets.\n * Preserved checked-out branch comments before applying the new symref\n skip, so worktrees still get the existing todo-list warning instead\n of an update-ref command.\n * Folded the symref coverage into the existing \"--update-refs updates\n refs correctly\" test, covering both an alias of HEAD and an alias of\n another branch.\n * Kept the relationship to 106b6885c7 in the commit message: non-local\n decorations are already filtered, and this patch handles the\n remaining local branch decorations that are symref aliases.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2126%2Fsluongng%2Fsl%2Frebase-update-refs-symrefs-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2126/sluongng/sl/rebase-update-refs-symrefs-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/2126\n\nRange-diff vs v1:\n\n 1: a550923440 < -: ---------- t3404: add failing branch symref test\n 2: 0ab0a71744 ! 1: 68f698225c rebase: skip branch symref aliases\n @@ Metadata\n ## Commit message ##\n rebase: skip branch symref aliases\n \n - rebase --update-refs records local branch decorations before replaying\n - commits. If a decoration is a symbolic branch such as refs/heads/main\n - pointing at refs/heads/master, updating it later dereferences back to\n - master and can fail because the normal rebase path already moved that\n - branch.\n + git rebase --update-refs can fail after the normal rebase path has\n + updated the current branch when another local branch is a symref to it.\n + This can happen during a default-branch rename where refs/heads/main\n + points at refs/heads/master while users migrate.\n \n - Resolve local branch symref decorations to their referents before\n - queuing update-ref commands, and skip duplicates. This keeps branch\n - aliases from scheduling a second update for the same underlying branch\n - while still using the existing old-OID check for the single queued\n - update.\n + The sequencer queues update-ref commands from local branch decorations.\n + Commit 106b6885c7 (rebase: ignore non-branch update-refs) filters out\n + decorations that are not local branches, such as HEAD and tags. A branch\n + symref is different: it is still a local branch decoration, but if it\n + resolves to another branch then that target branch is itself present in\n + the decoration list and will be updated as a concrete branch.\n +\n + Skip branch decorations whose symrefs resolve to refs/heads/*, because\n + those targets are already represented by concrete branch decorations.\n + This prevents aliases from scheduling a second update for the same\n + branch. Keep symrefs to non-branch targets on the existing path.\n +\n + Preserve the existing checked-out branch handling before applying these\n + skips. Such refs still need a todo-list comment instead of an update-ref\n + command, even when the checked-out ref is the branch being rebased or a\n + branch symref alias. Use a copy of the resolved HEAD ref so later ref\n + resolution does not overwrite it.\n \n Signed-off-by: Son Luong Ngoc \n \n @@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,\n \tconst struct name_decoration *decoration = get_name_decoration(&commit->object);\n -\tconst char *head_ref = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),\n -\t\t\t\t\t\t \"HEAD\",\n -+\tstruct ref_store *refs = get_main_ref_store(the_repository);\n -+\tconst char *head_ref = refs_resolve_ref_unsafe(refs, \"HEAD\",\n - \t\t\t\t\t\t RESOLVE_REF_READING,\n +-\t\t\t\t\t\t RESOLVE_REF_READING,\n -\t\t\t\t\t\t NULL,\n -\t\t\t\t\t\t NULL);\n -+\t\t\t\t\t\t NULL, NULL);\n -+\tchar *resolved_head_ref = refs_resolve_refdup(refs, \"HEAD\",\n -+\t\t\t\t\t\t RESOLVE_REF_READING,\n -+\t\t\t\t\t\t NULL, NULL);\n -+\tstruct strbuf update_ref = STRBUF_INIT;\n ++\tstruct ref_store *refs = get_main_ref_store(the_repository);\n ++\tchar *head_ref = refs_resolve_refdup(refs, \"HEAD\",\n ++\t\t\t\t\t RESOLVE_REF_READING,\n ++\t\t\t\t\t NULL, NULL);\n \n \twhile (decoration) {\n \t\tstruct todo_item *item;\n \t\tconst char *path;\n -+\t\tconst char *ref = decoration->name;\n +\t\tconst char *resolved_ref;\n -+\t\tint is_symref = 0;\n +\t\tint flags = 0;\n \t\tsize_t base_offset = ctx->buf->len;\n \n \t\t/*\n -@@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,\n - \t\t * updated by the default rebase behavior.\n - \t\t * Exclude it from the list of refs to update,\n - \t\t * as well as any non-branch decorations.\n -+\t\t *\n -+\t\t * Resolve branch symrefs after checking for the current HEAD so\n -+\t\t * that aliases do not schedule duplicate updates for their\n -+\t\t * referents.\n -+\t\t *\n +-\t\t * If the branch is the current HEAD, then it will be\n +-\t\t * updated by the default rebase behavior.\n +-\t\t * Exclude it from the list of refs to update,\n +-\t\t * as well as any non-branch decorations.\n \t\t * Non-branch decorations may be present if the pretty format\n \t\t * includes \"%d\", which would have loaded all refs\n \t\t * into the global decoration table.\n @@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,\n +\t\t\tcontinue;\n +\t\t}\n +\n -+\t\tif (head_ref && !strcmp(head_ref, ref)) {\n ++\t\tpath = branch_checked_out(decoration->name);\n ++\n ++\t\t/*\n ++\t\t * If the branch is the current HEAD, then it will be\n ++\t\t * updated by the default rebase behavior. Exclude it from\n ++\t\t * the list of refs to update, unless it is checked out and\n ++\t\t * needs a comment in the todo list.\n ++\t\t */\n ++\t\tif (!path && head_ref && !strcmp(head_ref, decoration->name)) {\n +\t\t\tdecoration = decoration->next;\n +\t\t\tcontinue;\n +\t\t}\n +\n -+\t\tstrbuf_reset(&update_ref);\n -+\t\tresolved_ref = refs_resolve_ref_unsafe(refs, ref,\n -+\t\t\t\t\t\t RESOLVE_REF_READING |\n -+\t\t\t\t\t\t RESOLVE_REF_NO_RECURSE,\n ++\t\tresolved_ref = refs_resolve_ref_unsafe(refs, decoration->name,\n ++\t\t\t\t\t\t RESOLVE_REF_READING,\n +\t\t\t\t\t\t NULL, &flags);\n -+\t\tif ((flags & REF_ISSYMREF) && resolved_ref) {\n -+\t\t\tif (!starts_with(resolved_ref, \"refs/heads/\")) {\n -+\t\t\t\tdecoration = decoration->next;\n -+\t\t\t\tcontinue;\n -+\t\t\t}\n -+\n -+\t\t\tstrbuf_addstr(&update_ref, resolved_ref);\n -+\t\t\tref = update_ref.buf;\n -+\t\t\tis_symref = 1;\n -+\t\t}\n -+\n -+\t\tif ((is_symref && resolved_head_ref &&\n -+\t\t !strcmp(resolved_head_ref, ref)) ||\n -+\t\t string_list_has_string(&ctx->refs_to_oids, ref)) {\n ++\t\tif (!path && resolved_ref && (flags & REF_ISSYMREF) &&\n ++\t\t starts_with(resolved_ref, \"refs/heads/\")) {\n \t\t\tdecoration = decoration->next;\n \t\t\tcontinue;\n \t\t}\n @@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,\n \n \t\t/* If the branch is checked out, then leave a comment instead. */\n -\t\tif ((path = branch_checked_out(decoration->name))) {\n -+\t\tif ((path = branch_checked_out(ref))) {\n ++\t\tif (path) {\n \t\t\titem->command = TODO_COMMENT;\n \t\t\tstrbuf_commented_addf(ctx->buf, comment_line_str,\n \t\t\t\t\t \"Ref %s checked out at '%s'\\n\",\n --\t\t\t\t\t decoration->name, path);\n -+\t\t\t\t\t ref, path);\n - \t\t} else {\n - \t\t\tstruct string_list_item *sti;\n - \t\t\titem->command = TODO_UPDATE_REF;\n --\t\t\tstrbuf_addf(ctx->buf, \"%s\\n\", decoration->name);\n -+\t\t\tstrbuf_addf(ctx->buf, \"%s\\n\", ref);\n - \n - \t\t\tsti = string_list_insert(&ctx->refs_to_oids,\n --\t\t\t\t\t\t decoration->name);\n --\t\t\tsti->util = init_update_ref_record(decoration->name);\n -+\t\t\t\t\t\t ref);\n -+\t\t\tsti->util = init_update_ref_record(ref);\n - \t\t}\n - \n - \t\titem->offset_in_buf = base_offset;\n @@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,\n \t\tdecoration = decoration->next;\n \t}\n \n -+\tstrbuf_release(&update_ref);\n -+\tfree(resolved_head_ref);\n ++\tfree(head_ref);\n \treturn 0;\n }\n \n \n ## t/t3404-rebase-interactive.sh ##\n @@ t/t3404-rebase-interactive.sh: test_expect_success '--update-refs ignores non-branch decorations' '\n - \ttest_cmp expect actual\n '\n \n --test_expect_failure '--update-refs skips branch symrefs to current branch' '\n -+test_expect_success '--update-refs skips branch symrefs to current branch' '\n - \ttest_when_finished \"\n - \t\ttest_might_fail git rebase --abort &&\n - \t\tgit checkout primary &&\n + test_expect_success '--update-refs updates refs correctly' '\n ++\ttest_when_finished \"\n ++\t\ttest_might_fail git symbolic-ref -d refs/heads/no-conflict-branch-alias &&\n ++\t\ttest_might_fail git symbolic-ref -d refs/heads/second-alias\n ++\t\" &&\n + \tgit checkout -B update-refs no-conflict-branch &&\n + \tgit branch -f base HEAD~4 &&\n + \tgit branch -f first HEAD~3 &&\n + \tgit branch -f second HEAD~3 &&\n + \tgit branch -f third HEAD~1 &&\n ++\tgit symbolic-ref refs/heads/no-conflict-branch-alias \\\n ++\t\trefs/heads/no-conflict-branch &&\n ++\tgit symbolic-ref refs/heads/second-alias refs/heads/second &&\n + \ttest_commit extra2 fileX &&\n + \tgit commit --amend --fixup=L &&\n + \n +@@ t/t3404-rebase-interactive.sh: test_expect_success '--update-refs updates refs correctly' '\n + \n + \ttest_cmp_rev HEAD~3 refs/heads/first &&\n + \ttest_cmp_rev HEAD~3 refs/heads/second &&\n ++\ttest_cmp_rev HEAD~3 refs/heads/second-alias &&\n + \ttest_cmp_rev HEAD~1 refs/heads/third &&\n + \ttest_cmp_rev HEAD refs/heads/no-conflict-branch &&\n ++\ttest_cmp_rev HEAD refs/heads/no-conflict-branch-alias &&\n ++\ttest_write_lines refs/heads/no-conflict-branch >expect &&\n ++\tgit symbolic-ref refs/heads/no-conflict-branch-alias >actual &&\n ++\ttest_cmp expect actual &&\n ++\ttest_write_lines refs/heads/second >expect &&\n ++\tgit symbolic-ref refs/heads/second-alias >actual &&\n ++\ttest_cmp expect actual &&\n + \n + \tq_to_tab >expect <<-\\EOF &&\n + \tSuccessfully rebased and updated refs/heads/update-refs.\n\n\n sequencer.c | 43 +++++++++++++++++++++++++----------\n t/t3404-rebase-interactive.sh | 15 ++++++++++++\n 2 files changed, 46 insertions(+), 12 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 1ee4b2875b..6ab8b47108 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -6445,28 +6445,46 @@ static int add_decorations_to_list(const struct commit *commit,\n \t\t\t\t struct todo_add_branch_context *ctx)\n {\n \tconst struct name_decoration *decoration = get_name_decoration(&commit->object);\n-\tconst char *head_ref = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),\n-\t\t\t\t\t\t \"HEAD\",\n-\t\t\t\t\t\t RESOLVE_REF_READING,\n-\t\t\t\t\t\t NULL,\n-\t\t\t\t\t\t NULL);\n+\tstruct ref_store *refs = get_main_ref_store(the_repository);\n+\tchar *head_ref = refs_resolve_refdup(refs, \"HEAD\",\n+\t\t\t\t\t RESOLVE_REF_READING,\n+\t\t\t\t\t NULL, NULL);\n \n \twhile (decoration) {\n \t\tstruct todo_item *item;\n \t\tconst char *path;\n+\t\tconst char *resolved_ref;\n+\t\tint flags = 0;\n \t\tsize_t base_offset = ctx->buf->len;\n \n \t\t/*\n-\t\t * If the branch is the current HEAD, then it will be\n-\t\t * updated by the default rebase behavior.\n-\t\t * Exclude it from the list of refs to update,\n-\t\t * as well as any non-branch decorations.\n \t\t * Non-branch decorations may be present if the pretty format\n \t\t * includes \"%d\", which would have loaded all refs\n \t\t * into the global decoration table.\n \t\t */\n-\t\tif ((head_ref && !strcmp(head_ref, decoration->name)) ||\n-\t\t (decoration->type != DECORATION_REF_LOCAL)) {\n+\t\tif (decoration->type != DECORATION_REF_LOCAL) {\n+\t\t\tdecoration = decoration->next;\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tpath = branch_checked_out(decoration->name);\n+\n+\t\t/*\n+\t\t * If the branch is the current HEAD, then it will be\n+\t\t * updated by the default rebase behavior. Exclude it from\n+\t\t * the list of refs to update, unless it is checked out and\n+\t\t * needs a comment in the todo list.\n+\t\t */\n+\t\tif (!path && head_ref && !strcmp(head_ref, decoration->name)) {\n+\t\t\tdecoration = decoration->next;\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tresolved_ref = refs_resolve_ref_unsafe(refs, decoration->name,\n+\t\t\t\t\t\t RESOLVE_REF_READING,\n+\t\t\t\t\t\t NULL, &flags);\n+\t\tif (!path && resolved_ref && (flags & REF_ISSYMREF) &&\n+\t\t starts_with(resolved_ref, \"refs/heads/\")) {\n \t\t\tdecoration = decoration->next;\n \t\t\tcontinue;\n \t\t}\n@@ -6478,7 +6496,7 @@ static int add_decorations_to_list(const struct commit *commit,\n \t\tmemset(item, 0, sizeof(*item));\n \n \t\t/* If the branch is checked out, then leave a comment instead. */\n-\t\tif ((path = branch_checked_out(decoration->name))) {\n+\t\tif (path) {\n \t\t\titem->command = TODO_COMMENT;\n \t\t\tstrbuf_commented_addf(ctx->buf, comment_line_str,\n \t\t\t\t\t \"Ref %s checked out at '%s'\\n\",\n@@ -6501,6 +6519,7 @@ static int add_decorations_to_list(const struct commit *commit,\n \t\tdecoration = decoration->next;\n \t}\n \n+\tfree(head_ref);\n \treturn 0;\n }\n \ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 58b3bb0c27..bc0b6a31f7 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -1979,11 +1979,18 @@ test_expect_success '--update-refs ignores non-branch decorations' '\n '\n \n test_expect_success '--update-refs updates refs correctly' '\n+\ttest_when_finished \"\n+\t\ttest_might_fail git symbolic-ref -d refs/heads/no-conflict-branch-alias &&\n+\t\ttest_might_fail git symbolic-ref -d refs/heads/second-alias\n+\t\" &&\n \tgit checkout -B update-refs no-conflict-branch &&\n \tgit branch -f base HEAD~4 &&\n \tgit branch -f first HEAD~3 &&\n \tgit branch -f second HEAD~3 &&\n \tgit branch -f third HEAD~1 &&\n+\tgit symbolic-ref refs/heads/no-conflict-branch-alias \\\n+\t\trefs/heads/no-conflict-branch &&\n+\tgit symbolic-ref refs/heads/second-alias refs/heads/second &&\n \ttest_commit extra2 fileX &&\n \tgit commit --amend --fixup=L &&\n \n@@ -1991,8 +1998,16 @@ test_expect_success '--update-refs updates refs correctly' '\n \n \ttest_cmp_rev HEAD~3 refs/heads/first &&\n \ttest_cmp_rev HEAD~3 refs/heads/second &&\n+\ttest_cmp_rev HEAD~3 refs/heads/second-alias &&\n \ttest_cmp_rev HEAD~1 refs/heads/third &&\n \ttest_cmp_rev HEAD refs/heads/no-conflict-branch &&\n+\ttest_cmp_rev HEAD refs/heads/no-conflict-branch-alias &&\n+\ttest_write_lines refs/heads/no-conflict-branch >expect &&\n+\tgit symbolic-ref refs/heads/no-conflict-branch-alias >actual &&\n+\ttest_cmp expect actual &&\n+\ttest_write_lines refs/heads/second >expect &&\n+\tgit symbolic-ref refs/heads/second-alias >actual &&\n+\ttest_cmp expect actual &&\n \n \tq_to_tab >expect <<-\\EOF &&\n \tSuccessfully rebased and updated refs/heads/update-refs.\n\nbase-commit: c69baaf57ba26cf117c2b6793802877f19738b0d\n-- \ngitgitgadget\n"},{"id":"544748","messageId":"f982c386-e329-4ab0-b695-e540bcb9de3d@gmail.com","threadId":"65707","inReplyTo":"pull.2126.v2.git.1780482436865.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] rebase: skip branch symref aliases","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-06-04T15:37:39Z","receivedAt":"2026-06-04T15:37:52Z","isPatch":true,"body":"On 03/06/2026 11:27, Son Luong Ngoc via GitGitGadget wrote:\n> From: Son Luong Ngoc \n> \n> git rebase --update-refs can fail after the normal rebase path has\n> updated the current branch when another local branch is a symref to it.\n> This can happen during a default-branch rename where refs/heads/main\n> points at refs/heads/master while users migrate.\n> \n> The sequencer queues update-ref commands from local branch decorations.\n> Commit 106b6885c7 (rebase: ignore non-branch update-refs) filters out\n> decorations that are not local branches, such as HEAD and tags. A branch\n> symref is different: it is still a local branch decoration, but if it\n> resolves to another branch then that target branch is itself present in\n> the decoration list and will be updated as a concrete branch.\n> \n> Skip branch decorations whose symrefs resolve to refs/heads/*, because\n> those targets are already represented by concrete branch decorations.\n> This prevents aliases from scheduling a second update for the same\n> branch. Keep symrefs to non-branch targets on the existing path.\n\nMakes sense\n\n> Preserve the existing checked-out branch handling before applying these\n> skips. Such refs still need a todo-list comment instead of an update-ref\n> command, even when the checked-out ref is the branch being rebased or a\n> branch symref alias. Use a copy of the resolved HEAD ref so later ref\n> resolution does not overwrite it.\n\nI don't quite understand this. A symref that points to another branch \nshould always be skipped. When we look up which branches are checked out \n(see worktree.c:add_head_info()) we use\n\n \trefs_resolve_ref_unsafe(get_worktree_ref_store(wt),\n\t\t\t\t \"HEAD\",\n\t\t\t\t 0,\n\t\t\t\t &wt->head_oid, &flags);\n\nso it will never report a symref as being checked out - it always \nresolves any symrefs first.\n\nIf we have a symref pointing somewhere outside of \"refs/heads\" then we \nneed to check whether the target is checked out, not the symref itself. \nI'm not sure how likely that is to happen in practice.\n\n> diff --git a/sequencer.c b/sequencer.c\n> index 1ee4b2875b..6ab8b47108 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -6445,28 +6445,46 @@ static int add_decorations_to_list(const struct commit *commit,\n> \t\t\t\t struct todo_add_branch_context *ctx)\n> {\n> \tconst struct name_decoration *decoration = get_name_decoration(&commit->object);\n> -\tconst char *head_ref = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),\n> -\t\t\t\t\t\t \"HEAD\",\n> -\t\t\t\t\t\t RESOLVE_REF_READING,\n> -\t\t\t\t\t\t NULL,\n> -\t\t\t\t\t\t NULL);\n> +\tstruct ref_store *refs = get_main_ref_store(the_repository);\n> +\tchar *head_ref = refs_resolve_refdup(refs, \"HEAD\",\n> +\t\t\t\t\t RESOLVE_REF_READING,\n> +\t\t\t\t\t NULL, NULL);\n\nThis part and the test look good now\n> \twhile (decoration) {\n> \t\tstruct todo_item *item;\n> \t\tconst char *path;\n> +\t\tconst char *resolved_ref;\n> +\t\tint flags = 0;\n> \t\tsize_t base_offset = ctx->buf->len;\n> \n> \t\t/*\n> -\t\t * If the branch is the current HEAD, then it will be\n> -\t\t * updated by the default rebase behavior.\n> -\t\t * Exclude it from the list of refs to update,\n> -\t\t * as well as any non-branch decorations.\n> \t\t * Non-branch decorations may be present if the pretty format\n> \t\t * includes \"%d\", which would have loaded all refs\n> \t\t * into the global decoration table.\n> \t\t */\n> -\t\tif ((head_ref && !strcmp(head_ref, decoration->name)) ||\n> -\t\t (decoration->type != DECORATION_REF_LOCAL)) {\n> +\t\tif (decoration->type != DECORATION_REF_LOCAL) {\n> +\t\t\tdecoration = decoration->next;\n> +\t\t\tcontinue;\n> +\t\t}\n\nIf a decoration matches the current branch why don't we just skip it \nlike we used to? (As an aside the existing code in wrong because if the \nuser runs \"git rebase --update-refs \" HEAD does not \npoint to \"\" but lets not worry about that now)\n\n> +\t\tpath = branch_checked_out(decoration->name);\n\nAs I said above if the symref target is anther branch we should skip it \nand if the target is not a branch then we need to check if the target is \nchecked out so we need to resolve the ref before calling \nbranch_checked_out().\n\nThanks\n\nPhillip\n\n"},{"id":"548763","messageId":"pull.2126.v3.git.1784708107.gitgitgadget@gmail.com","threadId":"65707","inReplyTo":"pull.2126.v2.git.1780482436865.gitgitgadget@gmail.com","subject":"[PATCH v3 0/2] rebase: handle --update-refs branch symrefs","fromName":"Son Luong Ngoc via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-22T08:15:04Z","receivedAt":"2026-07-22T08:15:09Z","isPatch":true,"body":"git rebase --update-refs can finish rewriting the current branch and then\nfail while updating a local branch that is a symbolic ref. This can happen\nduring a default-branch rename where refs/heads/main points at\nrefs/heads/master while users migrate. The failure leaves refs partially\nupdated even though the main rebase has succeeded.\n\nResolve local branch decorations before adding update-ref commands. The\nfirst patch skips aliases whose targets are other branches and preserves the\nexisting handling of the current branch. The second patch keeps aliases to\nnon-branch refs supported while preventing duplicate and cross-worktree\nupdates to their resolved targets.\n\nChanges since v2:\n\n * Skip branch-to-branch symrefs before checked-out handling.\n * Restore the unconditional current-branch skip and keep an owned copy of\n the resolved HEAD name.\n * Check both a non-branch symref alias and its resolved target against\n checked-out reservations.\n * Deduplicate aliases that share a non-branch target.\n * Reserve resolved targets from other worktrees' in-progress update-refs\n state.\n * Split the branch-alias fix and non-branch safeguards into separate\n patches.\n * Rebase onto 48bbf81c29 (The 5th batch).\n\nThe focused t3400 and t3404 test suites pass with both the files and\nreftable backends.\n\nSon Luong Ngoc (2):\n rebase: skip branch symref aliases\n rebase: guard non-branch symref targets\n\n branch.c | 15 ++++++\n sequencer.c | 63 ++++++++++++++++++++-----\n t/t3400-rebase.sh | 2 +-\n t/t3404-rebase-interactive.sh | 88 +++++++++++++++++++++++++++++++++++\n 4 files changed, 155 insertions(+), 13 deletions(-)\n\n\nbase-commit: 48bbf81c29ca9a4479ec7850fe206518682cdb2f\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2126%2Fsluongng%2Fsl%2Frebase-update-refs-symrefs-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2126/sluongng/sl/rebase-update-refs-symrefs-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/2126\n\nRange-diff vs v2:\n\n 1: 68f698225c ! 1: b9a01e9141 rebase: skip branch symref aliases\n @@ Metadata\n ## Commit message ##\n rebase: skip branch symref aliases\n \n - git rebase --update-refs can fail after the normal rebase path has\n - updated the current branch when another local branch is a symref to it.\n - This can happen during a default-branch rename where refs/heads/main\n - points at refs/heads/master while users migrate.\n + git rebase --update-refs can finish rewriting the current branch and\n + then fail while updating a local branch that is a symbolic ref. This can\n + happen during a default-branch rename where refs/heads/main points at\n + refs/heads/master while users migrate.\n \n - The sequencer queues update-ref commands from local branch decorations.\n - Commit 106b6885c7 (rebase: ignore non-branch update-refs) filters out\n - decorations that are not local branches, such as HEAD and tags. A branch\n - symref is different: it is still a local branch decoration, but if it\n - resolves to another branch then that target branch is itself present in\n - the decoration list and will be updated as a concrete branch.\n + The problem is a partially applied ref update: the main rebase has\n + already succeeded when the later ref update fails.\n \n - Skip branch decorations whose symrefs resolve to refs/heads/*, because\n - those targets are already represented by concrete branch decorations.\n - This prevents aliases from scheduling a second update for the same\n - branch. Keep symrefs to non-branch targets on the existing path.\n + The sequencer queues updates from local branch decorations. Commit\n + 106b6885c7 (rebase: ignore non-branch update-refs) filters out\n + decorations such as HEAD and tags. A branch symref is still a local\n + branch decoration, but refs_update_ref() dereferences it, so an alias to\n + another branch duplicates the concrete branch update.\n \n - Preserve the existing checked-out branch handling before applying these\n - skips. Such refs still need a todo-list comment instead of an update-ref\n - command, even when the checked-out ref is the branch being rebased or a\n - branch symref alias. Use a copy of the resolved HEAD ref so later ref\n - resolution does not overwrite it.\n + Resolve local branch decorations before queuing them. Skip symrefs whose\n + targets are under refs/heads/ so that only the concrete branch update is\n + queued. Keep an owned copy of the resolved HEAD and skip the current\n + branch before checked-out handling so later ref resolution cannot change\n + the comparison.\n +\n + This prevents a successful rebase from being followed by a failed,\n + partially applied ref update while preserving each alias as a symref.\n \n Signed-off-by: Son Luong Ngoc \n \n @@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,\n \twhile (decoration) {\n \t\tstruct todo_item *item;\n \t\tconst char *path;\n -+\t\tconst char *resolved_ref;\n ++\t\tchar *resolved_ref;\n +\t\tint flags = 0;\n \t\tsize_t base_offset = ctx->buf->len;\n \n @@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,\n +\t\t\tcontinue;\n +\t\t}\n +\n -+\t\tpath = branch_checked_out(decoration->name);\n -+\n -+\t\t/*\n -+\t\t * If the branch is the current HEAD, then it will be\n -+\t\t * updated by the default rebase behavior. Exclude it from\n -+\t\t * the list of refs to update, unless it is checked out and\n -+\t\t * needs a comment in the todo list.\n -+\t\t */\n -+\t\tif (!path && head_ref && !strcmp(head_ref, decoration->name)) {\n ++\t\tresolved_ref = refs_resolve_refdup(refs, decoration->name,\n ++\t\t\t\t\t\t RESOLVE_REF_READING,\n ++\t\t\t\t\t\t NULL, &flags);\n ++\t\tif (resolved_ref && (flags & REF_ISSYMREF) &&\n ++\t\t starts_with(resolved_ref, \"refs/heads/\")) {\n ++\t\t\tfree(resolved_ref);\n +\t\t\tdecoration = decoration->next;\n +\t\t\tcontinue;\n +\t\t}\n +\n -+\t\tresolved_ref = refs_resolve_ref_unsafe(refs, decoration->name,\n -+\t\t\t\t\t\t RESOLVE_REF_READING,\n -+\t\t\t\t\t\t NULL, &flags);\n -+\t\tif (!path && resolved_ref && (flags & REF_ISSYMREF) &&\n -+\t\t starts_with(resolved_ref, \"refs/heads/\")) {\n ++\t\t/*\n ++\t\t * If the branch is the current HEAD, then it will be\n ++\t\t * updated by the default rebase behavior.\n ++\t\t */\n ++\t\tif (head_ref && !strcmp(head_ref, decoration->name)) {\n ++\t\t\tfree(resolved_ref);\n \t\t\tdecoration = decoration->next;\n \t\t\tcontinue;\n \t\t}\n + \n ++\t\tpath = branch_checked_out(decoration->name);\n ++\n + \t\tALLOC_GROW(ctx->items,\n + \t\t\tctx->items_nr + 1,\n + \t\t\tctx->items_alloc);\n @@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,\n \t\tmemset(item, 0, sizeof(*item));\n \n @@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,\n \t\t\tstrbuf_commented_addf(ctx->buf, comment_line_str,\n \t\t\t\t\t \"Ref %s checked out at '%s'\\n\",\n @@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,\n + \t\titem->arg_len = ctx->buf->len - base_offset;\n + \t\tctx->items_nr++;\n + \n ++\t\tfree(resolved_ref);\n \t\tdecoration = decoration->next;\n \t}\n \n @@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,\n }\n \n \n + ## t/t3400-rebase.sh ##\n +@@ t/t3400-rebase.sh: test_expect_success 'git rebase --update-ref with core.commentChar and branch on\n + \tGIT_SEQUENCE_EDITOR=\"cat >actual\" git -c core.commentChar=% \\\n + \t\t rebase -i --update-refs base &&\n + \ttest_grep \"% Ref refs/heads/wt-topic checked out at\" actual &&\n +-\ttest_grep \"% Ref refs/heads/topic2 checked out at\" actual\n ++\ttest_grep ! \"% Ref refs/heads/topic2 checked out at\" actual\n + '\n + \n + test_done\n +\n ## t/t3404-rebase-interactive.sh ##\n @@ t/t3404-rebase-interactive.sh: test_expect_success '--update-refs ignores non-branch decorations' '\n + \t) &&\n + \tgrep ^update-ref todo >actual &&\n + \ttest_write_lines \"update-ref refs/heads/no-conflict-branch\" >expect &&\n ++\ttest_grep ! \"^# Ref refs/heads/update-refs checked out\" todo &&\n + \ttest_cmp expect actual\n '\n \n test_expect_success '--update-refs updates refs correctly' '\n -: ---------- > 2: a653f56ea2 rebase: guard non-branch symref targets\n\n-- \ngitgitgadget\n"},{"id":"548764","messageId":"b9a01e9141d580606527cb1a658c7c72710fb013.1784708107.git.gitgitgadget@gmail.com","threadId":"65707","inReplyTo":"pull.2126.v3.git.1784708107.gitgitgadget@gmail.com","subject":"[PATCH v3 1/2] rebase: skip branch symref aliases","fromName":"Son Luong Ngoc via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-22T08:15:05Z","receivedAt":"2026-07-22T08:15:11Z","isPatch":true,"body":"From: Son Luong Ngoc \n\ngit rebase --update-refs can finish rewriting the current branch and\nthen fail while updating a local branch that is a symbolic ref. This can\nhappen during a default-branch rename where refs/heads/main points at\nrefs/heads/master while users migrate.\n\nThe problem is a partially applied ref update: the main rebase has\nalready succeeded when the later ref update fails.\n\nThe sequencer queues updates from local branch decorations. Commit\n106b6885c7 (rebase: ignore non-branch update-refs) filters out\ndecorations such as HEAD and tags. A branch symref is still a local\nbranch decoration, but refs_update_ref() dereferences it, so an alias to\nanother branch duplicates the concrete branch update.\n\nResolve local branch decorations before queuing them. Skip symrefs whose\ntargets are under refs/heads/ so that only the concrete branch update is\nqueued. Keep an owned copy of the resolved HEAD and skip the current\nbranch before checked-out handling so later ref resolution cannot change\nthe comparison.\n\nThis prevents a successful rebase from being followed by a failed,\npartially applied ref update while preserving each alias as a symref.\n\nSigned-off-by: Son Luong Ngoc \n---\n sequencer.c | 44 +++++++++++++++++++++++++----------\n t/t3400-rebase.sh | 2 +-\n t/t3404-rebase-interactive.sh | 16 +++++++++++++\n 3 files changed, 49 insertions(+), 13 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 1355a99a09..63aba60a08 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -6465,32 +6465,50 @@ static int add_decorations_to_list(const struct commit *commit,\n \t\t\t\t struct todo_add_branch_context *ctx)\n {\n \tconst struct name_decoration *decoration = get_name_decoration(&commit->object);\n-\tconst char *head_ref = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),\n-\t\t\t\t\t\t \"HEAD\",\n-\t\t\t\t\t\t RESOLVE_REF_READING,\n-\t\t\t\t\t\t NULL,\n-\t\t\t\t\t\t NULL);\n+\tstruct ref_store *refs = get_main_ref_store(the_repository);\n+\tchar *head_ref = refs_resolve_refdup(refs, \"HEAD\",\n+\t\t\t\t\t RESOLVE_REF_READING,\n+\t\t\t\t\t NULL, NULL);\n \n \twhile (decoration) {\n \t\tstruct todo_item *item;\n \t\tconst char *path;\n+\t\tchar *resolved_ref;\n+\t\tint flags = 0;\n \t\tsize_t base_offset = ctx->buf->len;\n \n \t\t/*\n-\t\t * If the branch is the current HEAD, then it will be\n-\t\t * updated by the default rebase behavior.\n-\t\t * Exclude it from the list of refs to update,\n-\t\t * as well as any non-branch decorations.\n \t\t * Non-branch decorations may be present if the pretty format\n \t\t * includes \"%d\", which would have loaded all refs\n \t\t * into the global decoration table.\n \t\t */\n-\t\tif ((head_ref && !strcmp(head_ref, decoration->name)) ||\n-\t\t (decoration->type != DECORATION_REF_LOCAL)) {\n+\t\tif (decoration->type != DECORATION_REF_LOCAL) {\n+\t\t\tdecoration = decoration->next;\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tresolved_ref = refs_resolve_refdup(refs, decoration->name,\n+\t\t\t\t\t\t RESOLVE_REF_READING,\n+\t\t\t\t\t\t NULL, &flags);\n+\t\tif (resolved_ref && (flags & REF_ISSYMREF) &&\n+\t\t starts_with(resolved_ref, \"refs/heads/\")) {\n+\t\t\tfree(resolved_ref);\n+\t\t\tdecoration = decoration->next;\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\t/*\n+\t\t * If the branch is the current HEAD, then it will be\n+\t\t * updated by the default rebase behavior.\n+\t\t */\n+\t\tif (head_ref && !strcmp(head_ref, decoration->name)) {\n+\t\t\tfree(resolved_ref);\n \t\t\tdecoration = decoration->next;\n \t\t\tcontinue;\n \t\t}\n \n+\t\tpath = branch_checked_out(decoration->name);\n+\n \t\tALLOC_GROW(ctx->items,\n \t\t\tctx->items_nr + 1,\n \t\t\tctx->items_alloc);\n@@ -6498,7 +6516,7 @@ static int add_decorations_to_list(const struct commit *commit,\n \t\tmemset(item, 0, sizeof(*item));\n \n \t\t/* If the branch is checked out, then leave a comment instead. */\n-\t\tif ((path = branch_checked_out(decoration->name))) {\n+\t\tif (path) {\n \t\t\titem->command = TODO_COMMENT;\n \t\t\tstrbuf_commented_addf(ctx->buf, comment_line_str,\n \t\t\t\t\t \"Ref %s checked out at '%s'\\n\",\n@@ -6518,9 +6536,11 @@ static int add_decorations_to_list(const struct commit *commit,\n \t\titem->arg_len = ctx->buf->len - base_offset;\n \t\tctx->items_nr++;\n \n+\t\tfree(resolved_ref);\n \t\tdecoration = decoration->next;\n \t}\n \n+\tfree(head_ref);\n \treturn 0;\n }\n \ndiff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh\nindex e62e07b894..1a02f6546b 100755\n--- a/t/t3400-rebase.sh\n+++ b/t/t3400-rebase.sh\n@@ -471,7 +471,7 @@ test_expect_success 'git rebase --update-ref with core.commentChar and branch on\n \tGIT_SEQUENCE_EDITOR=\"cat >actual\" git -c core.commentChar=% \\\n \t\t rebase -i --update-refs base &&\n \ttest_grep \"% Ref refs/heads/wt-topic checked out at\" actual &&\n-\ttest_grep \"% Ref refs/heads/topic2 checked out at\" actual\n+\ttest_grep ! \"% Ref refs/heads/topic2 checked out at\" actual\n '\n \n test_done\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex e64816770a..11afa8be56 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -1975,15 +1975,23 @@ test_expect_success '--update-refs ignores non-branch decorations' '\n \t) &&\n \tgrep ^update-ref todo >actual &&\n \ttest_write_lines \"update-ref refs/heads/no-conflict-branch\" >expect &&\n+\ttest_grep ! \"^# Ref refs/heads/update-refs checked out\" todo &&\n \ttest_cmp expect actual\n '\n \n test_expect_success '--update-refs updates refs correctly' '\n+\ttest_when_finished \"\n+\t\ttest_might_fail git symbolic-ref -d refs/heads/no-conflict-branch-alias &&\n+\t\ttest_might_fail git symbolic-ref -d refs/heads/second-alias\n+\t\" &&\n \tgit checkout -B update-refs no-conflict-branch &&\n \tgit branch -f base HEAD~4 &&\n \tgit branch -f first HEAD~3 &&\n \tgit branch -f second HEAD~3 &&\n \tgit branch -f third HEAD~1 &&\n+\tgit symbolic-ref refs/heads/no-conflict-branch-alias \\\n+\t\trefs/heads/no-conflict-branch &&\n+\tgit symbolic-ref refs/heads/second-alias refs/heads/second &&\n \ttest_commit extra2 fileX &&\n \tgit commit --amend --fixup=L &&\n \n@@ -1991,8 +1999,16 @@ test_expect_success '--update-refs updates refs correctly' '\n \n \ttest_cmp_rev HEAD~3 refs/heads/first &&\n \ttest_cmp_rev HEAD~3 refs/heads/second &&\n+\ttest_cmp_rev HEAD~3 refs/heads/second-alias &&\n \ttest_cmp_rev HEAD~1 refs/heads/third &&\n \ttest_cmp_rev HEAD refs/heads/no-conflict-branch &&\n+\ttest_cmp_rev HEAD refs/heads/no-conflict-branch-alias &&\n+\ttest_write_lines refs/heads/no-conflict-branch >expect &&\n+\tgit symbolic-ref refs/heads/no-conflict-branch-alias >actual &&\n+\ttest_cmp expect actual &&\n+\ttest_write_lines refs/heads/second >expect &&\n+\tgit symbolic-ref refs/heads/second-alias >actual &&\n+\ttest_cmp expect actual &&\n \n \tq_to_tab >expect <<-\\EOF &&\n \tSuccessfully rebased and updated refs/heads/update-refs.\n-- \ngitgitgadget\n\n"},{"id":"548765","messageId":"a653f56ea214e74ea71ba31f5378f9cbf8b04dde.1784708107.git.gitgitgadget@gmail.com","threadId":"65707","inReplyTo":"pull.2126.v3.git.1784708107.gitgitgadget@gmail.com","subject":"[PATCH v3 2/2] rebase: guard non-branch symref targets","fromName":"Son Luong Ngoc via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-22T08:15:06Z","receivedAt":"2026-07-22T08:15:12Z","isPatch":true,"body":"From: Son Luong Ngoc \n\nA local branch symbolic ref may point outside refs/heads/. Such an alias\ncannot be skipped like a branch-to-branch alias because its concrete\ntarget ref is absent from the local branch decoration list.\n\nHowever, queuing each alias independently can update the same target ref\nmore than once and make the second compare-and-swap fail. A reservation\nfrom another worktree can also name either an alias or its resolved\ntarget ref, so checking only one form can miss an in-progress update.\n\nFix these cases by checking both the literal alias and its resolved\ntarget ref against checked-out reservations. Deduplicate updates by\ntarget ref. Also reserve both forms when loading another worktree's\nupdate-refs state. This makes different aliases honor the same\nin-progress update.\n\nThis keeps non-branch symrefs supported without allowing duplicate or\ncross-worktree ref updates.\n\nSigned-off-by: Son Luong Ngoc \n---\n branch.c | 15 ++++++++\n sequencer.c | 19 +++++++++\n t/t3404-rebase-interactive.sh | 72 +++++++++++++++++++++++++++++++++++\n 3 files changed, 106 insertions(+)\n\ndiff --git a/branch.c b/branch.c\nindex 243db7d0fc..98a50d8368 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -442,10 +442,25 @@ static void prepare_checked_out_branches(void)\n \t\t\t\t\t\t &update_refs)) {\n \t\t\tstruct string_list_item *item;\n \t\t\tfor_each_string_list_item(item, &update_refs) {\n+\t\t\t\tchar *resolved_ref;\n+\t\t\t\tint flags = 0;\n+\n \t\t\t\told = strmap_put(¤t_checked_out_branches,\n \t\t\t\t\t\t item->string,\n \t\t\t\t\t\t xstrdup(wt->path));\n \t\t\t\tfree(old);\n+\n+\t\t\t\tresolved_ref = refs_resolve_refdup(\n+\t\t\t\t\tget_main_ref_store(the_repository),\n+\t\t\t\t\titem->string, RESOLVE_REF_READING,\n+\t\t\t\t\tNULL, &flags);\n+\t\t\t\tif (resolved_ref && (flags & REF_ISSYMREF)) {\n+\t\t\t\t\told = strmap_put(\n+\t\t\t\t\t\t¤t_checked_out_branches,\n+\t\t\t\t\t\tresolved_ref, xstrdup(wt->path));\n+\t\t\t\t\tfree(old);\n+\t\t\t\t}\n+\t\t\t\tfree(resolved_ref);\n \t\t\t}\n \t\t\tstring_list_clear(&update_refs, 1);\n \t\t}\ndiff --git a/sequencer.c b/sequencer.c\nindex 63aba60a08..040b5bf645 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -6459,6 +6459,7 @@ struct todo_add_branch_context {\n \tsize_t items_alloc;\n \tstruct strbuf *buf;\n \tstruct string_list refs_to_oids;\n+\tstruct string_list symref_update_targets;\n };\n \n static int add_decorations_to_list(const struct commit *commit,\n@@ -6473,6 +6474,7 @@ static int add_decorations_to_list(const struct commit *commit,\n \twhile (decoration) {\n \t\tstruct todo_item *item;\n \t\tconst char *path;\n+\t\tconst char *checked_ref;\n \t\tchar *resolved_ref;\n \t\tint flags = 0;\n \t\tsize_t base_offset = ctx->buf->len;\n@@ -6508,6 +6510,17 @@ static int add_decorations_to_list(const struct commit *commit,\n \t\t}\n \n \t\tpath = branch_checked_out(decoration->name);\n+\t\tif (!path && resolved_ref && (flags & REF_ISSYMREF)) {\n+\t\t\tchecked_ref = resolved_ref;\n+\t\t\tpath = branch_checked_out(checked_ref);\n+\t\t}\n+\t\tif (!path && resolved_ref && (flags & REF_ISSYMREF) &&\n+\t\t string_list_has_string(&ctx->symref_update_targets,\n+\t\t\t\t\t resolved_ref)) {\n+\t\t\tfree(resolved_ref);\n+\t\t\tdecoration = decoration->next;\n+\t\t\tcontinue;\n+\t\t}\n \n \t\tALLOC_GROW(ctx->items,\n \t\t\tctx->items_nr + 1,\n@@ -6523,6 +6536,10 @@ static int add_decorations_to_list(const struct commit *commit,\n \t\t\t\t\t decoration->name, path);\n \t\t} else {\n \t\t\tstruct string_list_item *sti;\n+\n+\t\t\tif (resolved_ref && (flags & REF_ISSYMREF))\n+\t\t\t\tstring_list_insert(&ctx->symref_update_targets,\n+\t\t\t\t\t\t resolved_ref);\n \t\t\titem->command = TODO_UPDATE_REF;\n \t\t\tstrbuf_addf(ctx->buf, \"%s\\n\", decoration->name);\n \n@@ -6554,6 +6571,7 @@ static int todo_list_add_update_ref_commands(struct todo_list *todo_list)\n \tstruct todo_add_branch_context ctx = {\n \t\t.buf = &todo_list->buf,\n \t\t.refs_to_oids = STRING_LIST_INIT_DUP,\n+\t\t.symref_update_targets = STRING_LIST_INIT_DUP,\n \t};\n \n \tctx.items_alloc = 2 * todo_list->nr + 1;\n@@ -6579,6 +6597,7 @@ static int todo_list_add_update_ref_commands(struct todo_list *todo_list)\n \tres = write_update_refs_state(&ctx.refs_to_oids);\n \n \tstring_list_clear(&ctx.refs_to_oids, 1);\n+\tstring_list_clear(&ctx.symref_update_targets, 0);\n \n \tif (res) {\n \t\t/* we failed, so clean up the new list. */\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 11afa8be56..110ed8ae63 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -2024,6 +2024,78 @@ test_expect_success '--update-refs updates refs correctly' '\n \ttest_cmp expect err.trimmed\n '\n \n+test_expect_success '--update-refs checks resolved non-branch symref target' '\n+\ttest_when_finished \"\n+\t\tgit worktree remove --force checked-out-target-wt &&\n+\t\tgit symbolic-ref -d refs/heads/non-branch-alias &&\n+\t\tgit tag -d checked-out-target\n+\t\" &&\n+\tgit tag checked-out-target HEAD~1 &&\n+\tgit symbolic-ref refs/heads/non-branch-alias refs/tags/checked-out-target &&\n+\tgit worktree add --detach checked-out-target-wt checked-out-target &&\n+\tgit -C checked-out-target-wt symbolic-ref HEAD refs/tags/checked-out-target &&\n+\n+\tGIT_SEQUENCE_EDITOR=\"cat >todo\" git rebase -i --update-refs HEAD~2 &&\n+\n+\ttest_grep \"^# Ref refs/heads/non-branch-alias checked out at\" todo &&\n+\ttest_write_lines refs/tags/checked-out-target >expect &&\n+\tgit symbolic-ref refs/heads/non-branch-alias >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--update-refs deduplicates non-branch symref targets' '\n+\ttest_when_finished \"\n+\t\tgit symbolic-ref -d refs/heads/non-branch-alias-one &&\n+\t\tgit symbolic-ref -d refs/heads/non-branch-alias-two &&\n+\t\tgit tag -d shared-non-branch-target\n+\t\" &&\n+\tgit tag shared-non-branch-target HEAD~1 &&\n+\tgit symbolic-ref refs/heads/non-branch-alias-one \\\n+\t\trefs/tags/shared-non-branch-target &&\n+\tgit symbolic-ref refs/heads/non-branch-alias-two \\\n+\t\trefs/tags/shared-non-branch-target &&\n+\n+\tGIT_SEQUENCE_EDITOR=: git rebase -i --force-rebase --update-refs HEAD~2 &&\n+\n+\ttest_cmp_rev HEAD~1 refs/heads/non-branch-alias-one &&\n+\ttest_cmp_rev HEAD~1 refs/heads/non-branch-alias-two &&\n+\ttest_write_lines refs/tags/shared-non-branch-target >expect &&\n+\tgit symbolic-ref refs/heads/non-branch-alias-one >actual &&\n+\ttest_cmp expect actual &&\n+\tgit symbolic-ref refs/heads/non-branch-alias-two >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--update-refs honors non-branch symref reservations' '\n+\ttest_when_finished \"\n+\t\ttest_might_fail git worktree remove --force reserved-target-wt &&\n+\t\ttest_might_fail git symbolic-ref -d \\\n+\t\t\trefs/heads/reserved-non-branch-alias-one &&\n+\t\ttest_might_fail git symbolic-ref -d \\\n+\t\t\trefs/heads/reserved-non-branch-alias-two &&\n+\t\ttest_might_fail git tag -d reserved-non-branch-target\n+\t\" &&\n+\tgit tag reserved-non-branch-target HEAD~1 &&\n+\tgit symbolic-ref refs/heads/reserved-non-branch-alias-one \\\n+\t\trefs/tags/reserved-non-branch-target &&\n+\tgit symbolic-ref refs/heads/reserved-non-branch-alias-two \\\n+\t\trefs/tags/reserved-non-branch-target &&\n+\tgit worktree add --detach reserved-target-wt HEAD &&\n+\twt_gitdir=$(git -C reserved-target-wt rev-parse --absolute-git-dir) &&\n+\tmkdir -p \"$wt_gitdir/rebase-merge\" &&\n+\told_oid=$(git rev-parse refs/heads/reserved-non-branch-alias-one) &&\n+\ttest_write_lines refs/heads/reserved-non-branch-alias-one \\\n+\t\t\"$old_oid\" \"$old_oid\" >\"$wt_gitdir/rebase-merge/update-refs\" &&\n+\n+\tGIT_SEQUENCE_EDITOR=\"cat >todo\" git rebase -i --update-refs HEAD~2 &&\n+\n+\ttest_grep \"^# Ref refs/heads/reserved-non-branch-alias-one checked out at\" \\\n+\t\ttodo &&\n+\ttest_grep \"^# Ref refs/heads/reserved-non-branch-alias-two checked out at\" \\\n+\t\ttodo &&\n+\ttest_grep ! \"^update-ref refs/heads/reserved-non-branch-alias\" todo\n+'\n+\n test_expect_success 'respect user edits to update-ref steps' '\n \tgit checkout -B update-refs-break no-conflict-branch &&\n \tgit branch -f base HEAD~4 &&\n-- \ngitgitgadget\n"},{"id":"548766","messageId":"CAL3xRKdoGpNOdbY7_dt3oSfn6728gf8uA4qa79Kcp1UuS=jpiQ@mail.gmail.com","threadId":"65707","inReplyTo":"f982c386-e329-4ab0-b695-e540bcb9de3d@gmail.com","subject":"Re: [PATCH v2] rebase: skip branch symref aliases","fromName":"Son Luong Ngoc","fromEmail":"sluongng@gmail.com","sentAt":"2026-07-22T08:16:56Z","receivedAt":"2026-07-22T08:16:57Z","isPatch":true,"body":"On 04/06/2026 16:46, Phillip Wood wrote:\n> A symref that points to another branch should always be skipped. When we\n> look up which branches are checked out (see worktree.c:add_head_info()) we\n> use\n>\n> \trefs_resolve_ref_unsafe(get_worktree_ref_store(wt),\n> \t\t\t\t \"HEAD\",\n> \t\t\t\t 0,\n> \t\t\t\t &wt->head_oid, &flags);\n>\n> so it will never report a symref as being checked out - it always resolves\n> any symrefs first.\n\nYes, this is the right distinction. Patch 1 now resolves each local\nbranch decoration before deciding whether to queue it. If the target is\nunder refs/heads/, the alias is skipped unconditionally and the concrete\nbranch decoration remains the only update that is queued.\n\n> If we have a symref pointing somewhere outside of \"refs/heads\" then we\n> need to check whether the target is checked out, not the symref itself.\n> I'm not sure how likely that is to happen in practice.\n\nPatch 2 handles that case separately. It checks both the literal alias\nand its resolved target ref against the checked-out reservations. I also\nadded a test with a non-branch target checked out in another worktree.\n\n> If a decoration matches the current branch why don't we just skip it like\n> we used to? (As an aside the existing code in wrong because if the user\n> runs \"git rebase --update-refs \" HEAD does not point to\n> \"\" but lets not worry about that now)\n\nAgreed. Patch 1 now skips the current branch before checked-out\nhandling, as the old code did.\n\nThe contrary expectation in t3400 came from head_ref pointing into a\nbuffer that was reused while resolving another decoration. That could\nmake the current-branch comparison fail. head_ref is now an owned copy.\nThe test expects the current branch to be omitted from the todo list.\n\nWhile adding the non-branch coverage, I found that two aliases to the\nsame target ref could queue the same update twice and make the second\ncompare-and-swap fail. Patch 2 deduplicates those updates by target ref.\nIt also records resolved target refs from other worktrees' in-progress\nupdate-refs state so that a different alias honors the same reservation.\n\nI split the reroll into these two patches so that the branch-alias fix\nand the non-branch safeguards can be reviewed independently.\n\nThanks for the review, and sorry for the slow response.\n\nThanks,\nSon\n\n\nOn Thu, 4 Jun 2026 16:37:39 +0100, Phillip Wood\n wrote:\n> On 03/06/2026 11:27, Son Luong Ngoc via GitGitGadget wrote:\n> > From: Son Luong Ngoc \n> >\n> > git rebase --update-refs can fail after the normal rebase path has\n> > updated the current branch when another local branch is a symref to it.\n> > This can happen during a default-branch rename where refs/heads/main\n> > points at refs/heads/master while users migrate.\n> >\n> > The sequencer queues update-ref commands from local branch decorations.\n> > Commit 106b6885c7 (rebase: ignore non-branch update-refs) filters out\n> > decorations that are not local branches, such as HEAD and tags. A branch\n> > symref is different: it is still a local branch decoration, but if it\n> > resolves to another branch then that target branch is itself present in\n> > the decoration list and will be updated as a concrete branch.\n> >\n> > Skip branch decorations whose symrefs resolve to refs/heads/*, because\n> > those targets are already represented by concrete branch decorations.\n> > This prevents aliases from scheduling a second update for the same\n> > branch. Keep symrefs to non-branch targets on the existing path.\n>\n> Makes sense\n>\n> > Preserve the existing checked-out branch handling before applying these\n> > skips. Such refs still need a todo-list comment instead of an update-ref\n> > command, even when the checked-out ref is the branch being rebased or a\n> > branch symref alias. Use a copy of the resolved HEAD ref so later ref\n> > resolution does not overwrite it.\n>\n> I don't quite understand this. A symref that points to another branch\n> should always be skipped. When we look up which branches are checked out\n> (see worktree.c:add_head_info()) we use\n>\n> refs_resolve_ref_unsafe(get_worktree_ref_store(wt),\n> \"HEAD\",\n> 0,\n> &wt->head_oid, &flags);\n>\n> so it will never report a symref as being checked out - it always\n> resolves any symrefs first.\n>\n> If we have a symref pointing somewhere outside of \"refs/heads\" then we\n> need to check whether the target is checked out, not the symref itself.\n> I'm not sure how likely that is to happen in practice.\n>\n> > diff --git a/sequencer.c b/sequencer.c\n> > index 1ee4b2875b..6ab8b47108 100644\n> > --- a/sequencer.c\n> > +++ b/sequencer.c\n> > @@ -6445,28 +6445,46 @@ static int add_decorations_to_list(const struct commit *commit,\n> > struct todo_add_branch_context *ctx)\n> > {\n> > const struct name_decoration *decoration = get_name_decoration(&commit->object);\n> > - const char *head_ref = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),\n> > - \"HEAD\",\n> > - RESOLVE_REF_READING,\n> > - NULL,\n> > - NULL);\n> > + struct ref_store *refs = get_main_ref_store(the_repository);\n> > + char *head_ref = refs_resolve_refdup(refs, \"HEAD\",\n> > + RESOLVE_REF_READING,\n> > + NULL, NULL);\n>\n> This part and the test look good now\n> > while (decoration) {\n> > struct todo_item *item;\n> > const char *path;\n> > + const char *resolved_ref;\n> > + int flags = 0;\n> > size_t base_offset = ctx->buf->len;\n> >\n> > /*\n> > - * If the branch is the current HEAD, then it will be\n> > - * updated by the default rebase behavior.\n> > - * Exclude it from the list of refs to update,\n> > - * as well as any non-branch decorations.\n> > * Non-branch decorations may be present if the pretty format\n> > * includes \"%d\", which would have loaded all refs\n> > * into the global decoration table.\n> > */\n> > - if ((head_ref && !strcmp(head_ref, decoration->name)) ||\n> > - (decoration->type != DECORATION_REF_LOCAL)) {\n> > + if (decoration->type != DECORATION_REF_LOCAL) {\n> > + decoration = decoration->next;\n> > + continue;\n> > + }\n>\n> If a decoration matches the current branch why don't we just skip it\n> like we used to? (As an aside the existing code in wrong because if the\n> user runs \"git rebase --update-refs \" HEAD does not\n> point to \"\" but lets not worry about that now)\n>\n> > + path = branch_checked_out(decoration->name);\n>\n> As I said above if the symref target is anther branch we should skip it\n> and if the target is not a branch then we need to check if the target is\n> checked out so we need to resolve the ref before calling\n> branch_checked_out().\n>\n> Thanks\n>\n> Phillip\n"},{"id":"548821","messageId":"5bece313-6ffb-450b-add1-29652b64de10@gmail.com","threadId":"65707","inReplyTo":"b9a01e9141d580606527cb1a658c7c72710fb013.1784708107.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 1/2] rebase: skip branch symref aliases","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-07-23T18:58:36Z","receivedAt":"2026-07-23T18:58:41Z","isPatch":true,"body":"On 22/07/2026 09:15, Son Luong Ngoc via GitGitGadget wrote:\n> From: Son Luong Ngoc \n> \n> git rebase --update-refs can finish rewriting the current branch and\n> then fail while updating a local branch that is a symbolic ref. This can\n> happen during a default-branch rename where refs/heads/main points at\n> refs/heads/master while users migrate.\n> \n> The problem is a partially applied ref update: the main rebase has\n> already succeeded when the later ref update fails.\n> \n> The sequencer queues updates from local branch decorations. Commit\n> 106b6885c7 (rebase: ignore non-branch update-refs) filters out\n> decorations such as HEAD and tags. A branch symref is still a local\n> branch decoration, but refs_update_ref() dereferences it, so an alias to\n> another branch duplicates the concrete branch update.\n> \n> Resolve local branch decorations before queuing them. Skip symrefs whose\n> targets are under refs/heads/ so that only the concrete branch update is\n> queued. Keep an owned copy of the resolved HEAD and skip the current\n> branch before checked-out handling so later ref resolution cannot change\n> the comparison.\n> \n> This prevents a successful rebase from being followed by a failed,\n> partially applied ref update while preserving each alias as a symref.\n\nThanks for re-rolling I'm pretty sure the logic is sound now but I'm a \nbit confused by a couple of things - see my comments below.\n\n> Signed-off-by: Son Luong Ngoc \n> ---\n> sequencer.c | 44 +++++++++++++++++++++++++----------\n> t/t3400-rebase.sh | 2 +-\n> t/t3404-rebase-interactive.sh | 16 +++++++++++++\n> 3 files changed, 49 insertions(+), 13 deletions(-)\n> \n> diff --git a/sequencer.c b/sequencer.c\n> index 1355a99a09..63aba60a08 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -6465,32 +6465,50 @@ static int add_decorations_to_list(const struct commit *commit,\n> \t\t\t\t struct todo_add_branch_context *ctx)\n> {\n> \tconst struct name_decoration *decoration = get_name_decoration(&commit->object);\n> -\tconst char *head_ref = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),\n> -\t\t\t\t\t\t \"HEAD\",\n> -\t\t\t\t\t\t RESOLVE_REF_READING,\n> -\t\t\t\t\t\t NULL,\n> -\t\t\t\t\t\t NULL);\n> +\tstruct ref_store *refs = get_main_ref_store(the_repository);\n> +\tchar *head_ref = refs_resolve_refdup(refs, \"HEAD\",\n> +\t\t\t\t\t RESOLVE_REF_READING,\n> +\t\t\t\t\t NULL, NULL);\n> \n> \twhile (decoration) {\n> \t\tstruct todo_item *item;\n> \t\tconst char *path;\n> +\t\tchar *resolved_ref;\n> +\t\tint flags = 0;\n> \t\tsize_t base_offset = ctx->buf->len;\n> \n> \t\t/*\n> -\t\t * If the branch is the current HEAD, then it will be\n> -\t\t * updated by the default rebase behavior.\n> -\t\t * Exclude it from the list of refs to update,\n> -\t\t * as well as any non-branch decorations.\n> \t\t * Non-branch decorations may be present if the pretty format\n> \t\t * includes \"%d\", which would have loaded all refs\n> \t\t * into the global decoration table.\n> \t\t */\n> -\t\tif ((head_ref && !strcmp(head_ref, decoration->name)) ||\n> -\t\t (decoration->type != DECORATION_REF_LOCAL)) {\n> +\t\tif (decoration->type != DECORATION_REF_LOCAL) {\n> +\t\t\tdecoration = decoration->next;\n> +\t\t\tcontinue;\n> +\t\t}\n\nIt would be nice to have a comment here explaining what we're doing. \nAlso I don't think we need to copy the refname so it would be more \nefficient to use refs_resolve_ref_unsafe().\n\n> +\t\tresolved_ref = refs_resolve_refdup(refs, decoration->name,\n> +\t\t\t\t\t\t RESOLVE_REF_READING,\n> +\t\t\t\t\t\t NULL, &flags);\n> +\t\tif (resolved_ref && (flags & REF_ISSYMREF) &&\n> +\t\t starts_with(resolved_ref, \"refs/heads/\")) {\n> +\t\t\tfree(resolved_ref);\n> +\t\t\tdecoration = decoration->next;\n> +\t\t\tcontinue;\n> +\t\t}\n\nWe skip any symbolic refs that point to another branch which is good.\n\n> +\t\t/*\n> +\t\t * If the branch is the current HEAD, then it will be\n> +\t\t * updated by the default rebase behavior.\n> +\t\t */\n> +\t\tif (head_ref && !strcmp(head_ref, decoration->name)) {\n> +\t\t\tfree(resolved_ref);\n> \t\t\tdecoration = decoration->next;\n> \t\t\tcontinue;\n> \t\t}\n\nThen we check to see if the decoration matches HEAD which we used to do \nabove - I'm not clear why we have moved this check.\n\n> +\t\tpath = branch_checked_out(decoration->name);\n> +\n\nThis belongs in the next patch I think.\n\n> diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh\n> index e62e07b894..1a02f6546b 100755\n> --- a/t/t3400-rebase.sh\n> +++ b/t/t3400-rebase.sh\n> @@ -471,7 +471,7 @@ test_expect_success 'git rebase --update-ref with core.commentChar and branch on\n\nAdding an extra context line shows\n\n\tgit checkout topic2> \tGIT_SEQUENCE_EDITOR=\"cat >actual\" git -c \ncore.commentChar=% \\\n> \t\t rebase -i --update-refs base &&\n> \ttest_grep \"% Ref refs/heads/wt-topic checked out at\" actual &&\n> -\ttest_grep \"% Ref refs/heads/topic2 checked out at\" actual\n> +\ttest_grep ! \"% Ref refs/heads/topic2 checked out at\" actual\n\nAs topic2 is checked out in the worktree where the rebase is running why \ndid this line appear before?\n\n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> index e64816770a..11afa8be56 100755\n> --- a/t/t3404-rebase-interactive.sh\n> +++ b/t/t3404-rebase-interactive.sh\n> @@ -1975,15 +1975,23 @@ test_expect_success '--update-refs ignores non-branch decorations' '\n> \t) &&\n> \tgrep ^update-ref todo >actual &&\n> \ttest_write_lines \"update-ref refs/heads/no-conflict-branch\" >expect &&\n> +\ttest_grep ! \"^# Ref refs/heads/update-refs checked out\" todo &&\n\nLets move this line below test_cmp so we keep that line next to the ones \nthat create the files that are being compared. Is this another case \nwhere we used to add this comment and no longer do so?\n\n> \ttest_cmp expect actual\n> '\n> \n> test_expect_success '--update-refs updates refs correctly' '\n> +\ttest_when_finished \"\n> +\t\ttest_might_fail git symbolic-ref -d refs/heads/no-conflict-branch-alias &&\n> +\t\ttest_might_fail git symbolic-ref -d refs/heads/second-alias\n> +\t\" &&\n> \tgit checkout -B update-refs no-conflict-branch &&\n> \tgit branch -f base HEAD~4 &&\n> \tgit branch -f first HEAD~3 &&\n> \tgit branch -f second HEAD~3 &&\n> \tgit branch -f third HEAD~1 &&\n> +\tgit symbolic-ref refs/heads/no-conflict-branch-alias \\\n> +\t\trefs/heads/no-conflict-branch &&\n> +\tgit symbolic-ref refs/heads/second-alias refs/heads/second &&\n> \ttest_commit extra2 fileX &&\n> \tgit commit --amend --fixup=L &&\n> \n> @@ -1991,8 +1999,16 @@ test_expect_success '--update-refs updates refs correctly' '\n> \n> \ttest_cmp_rev HEAD~3 refs/heads/first &&\n> \ttest_cmp_rev HEAD~3 refs/heads/second &&\n> +\ttest_cmp_rev HEAD~3 refs/heads/second-alias &&\n> \ttest_cmp_rev HEAD~1 refs/heads/third &&\n> \ttest_cmp_rev HEAD refs/heads/no-conflict-branch &&\n> +\ttest_cmp_rev HEAD refs/heads/no-conflict-branch-alias &&\n> +\ttest_write_lines refs/heads/no-conflict-branch >expect &&\n> +\tgit symbolic-ref refs/heads/no-conflict-branch-alias >actual &&\n> +\ttest_cmp expect actual &&\n> +\ttest_write_lines refs/heads/second >expect &&\n> +\tgit symbolic-ref refs/heads/second-alias >actual &&\n> +\ttest_cmp expect actual &&\n\nThis looks good - we check that \"rebase --update-refs\" succeeds withh \nbranches that are symrefs and also that those refs are untouched by the \nrebase.\n\nThanks\n\nPhillip\n\n> \tq_to_tab >expect <<-\\EOF &&\n> \tSuccessfully rebased and updated refs/heads/update-refs.\n\n"},{"id":"548867","messageId":"00e529b6-7ae7-463f-a4b3-0991e9411aba@gmail.com","threadId":"65707","inReplyTo":"5bece313-6ffb-450b-add1-29652b64de10@gmail.com","subject":"Re: [PATCH v3 1/2] rebase: skip branch symref aliases","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-07-24T09:55:18Z","receivedAt":"2026-07-24T09:55:25Z","isPatch":true,"body":"On 23/07/2026 19:58, Phillip Wood wrote:\n> On 22/07/2026 09:15, Son Luong Ngoc via GitGitGadget wrote:\n>> From: Son Luong Ngoc \n>>\n>> git rebase --update-refs can finish rewriting the current branch and\n>> then fail while updating a local branch that is a symbolic ref. This can\n>> happen during a default-branch rename where refs/heads/main points at\n>> refs/heads/master while users migrate.\n>>\n>> The problem is a partially applied ref update: the main rebase has\n>> already succeeded when the later ref update fails.\n>>\n>> The sequencer queues updates from local branch decorations. Commit\n>> 106b6885c7 (rebase: ignore non-branch update-refs) filters out\n>> decorations such as HEAD and tags. A branch symref is still a local\n>> branch decoration, but refs_update_ref() dereferences it, so an alias to\n>> another branch duplicates the concrete branch update.\n>>\n>> Resolve local branch decorations before queuing them. Skip symrefs whose\n>> targets are under refs/heads/ so that only the concrete branch update is\n>> queued. Keep an owned copy of the resolved HEAD and skip the current\n>> branch before checked-out handling so later ref resolution cannot change\n>> the comparison.\n>>\n>> This prevents a successful rebase from being followed by a failed,\n>> partially applied ref update while preserving each alias as a symref.\n> \n> Thanks for re-rolling I'm pretty sure the logic is sound now but I'm a \n> bit confused by a couple of things - see my comments below.\n> \n>> Signed-off-by: Son Luong Ngoc \n>> ---\n>>   sequencer.c                   | 44 +++++++++++++++++++++++++----------\n>>   t/t3400-rebase.sh             |  2 +-\n>>   t/t3404-rebase-interactive.sh | 16 +++++++++++++\n>>   3 files changed, 49 insertions(+), 13 deletions(-)\n>>\n>> diff --git a/sequencer.c b/sequencer.c\n>> index 1355a99a09..63aba60a08 100644\n>> --- a/sequencer.c\n>> +++ b/sequencer.c\n>> @@ -6465,32 +6465,50 @@ static int add_decorations_to_list(const \n>> struct commit *commit,\n>>                      struct todo_add_branch_context *ctx)\n>>   {\n>>       const struct name_decoration *decoration = \n>> get_name_decoration(&commit->object);\n>> -    const char *head_ref = \n>> refs_resolve_ref_unsafe(get_main_ref_store(the_repository),\n>> -                               \"HEAD\",\n>> -                               RESOLVE_REF_READING,\n>> -                               NULL,\n>> -                               NULL);\n>> +    struct ref_store *refs = get_main_ref_store(the_repository);\n>> +    char *head_ref = refs_resolve_refdup(refs, \"HEAD\",\n>> +                         RESOLVE_REF_READING,\n>> +                         NULL, NULL);\n>>       while (decoration) {\n>>           struct todo_item *item;\n>>           const char *path;\n>> +        char *resolved_ref;\n>> +        int flags = 0;\n>>           size_t base_offset = ctx->buf->len;\n>>           /*\n>> -         * If the branch is the current HEAD, then it will be\n>> -         * updated by the default rebase behavior.\n>> -         * Exclude it from the list of refs to update,\n>> -         * as well as any non-branch decorations.\n>>            * Non-branch decorations may be present if the pretty format\n>>            * includes \"%d\", which would have loaded all refs\n>>            * into the global decoration table.\n>>            */\n>> -        if ((head_ref && !strcmp(head_ref, decoration->name)) ||\n>> -            (decoration->type != DECORATION_REF_LOCAL)) {\n>> +        if (decoration->type != DECORATION_REF_LOCAL) {\n>> +            decoration = decoration->next;\n>> +            continue;\n>> +        }\n> \n> It would be nice to have a comment here explaining what we're doing. \n> Also I don't think we need to copy the refname so it would be more \n> efficient to use refs_resolve_ref_unsafe().\n\nLooking at this again we cannot use refs_resolve_ref_unsafe() because \nthe result would be overwritten by the call to refs_resolve_refdup() in \nbranch_checked_out().\n\n>> +        resolved_ref = refs_resolve_refdup(refs, decoration->name,\n>> +                              RESOLVE_REF_READING,\n>> +                              NULL, &flags);\n>> +        if (resolved_ref && (flags & REF_ISSYMREF) &&\n>> +            starts_with(resolved_ref, \"refs/heads/\")) {\n>> +            free(resolved_ref);\n>> +            decoration = decoration->next;\n>> +            continue;\n>> +        }\n> \n> We skip any symbolic refs that point to another branch which is good.\n> \n>> +        /*\n>> +         * If the branch is the current HEAD, then it will be\n>> +         * updated by the default rebase behavior.\n>> +         */\n>> +        if (head_ref && !strcmp(head_ref, decoration->name)) {\n>> +            free(resolved_ref);\n>>               decoration = decoration->next;\n>>               continue;\n>>           }\n> \n> Then we check to see if the decoration matches HEAD which we used to do \n> above - I'm not clear why we have moved this check.\n\nShould we be using \"resolved_ref\" instead of \"decoration->name\"? That \nwould explain why this was moved and would makes sense as we resolve \nsymrefs when reading HEAD. When HEAD points outside \"refs/heads/\" we'd \nthen skip updating any symrefs under \"refs/heads/\" that pointed to the \nsame ref as HEAD.\n\nThanks\n\nPhillip\n> \n>> +        path = branch_checked_out(decoration->name);\n>> +\n> \n> This belongs in the next patch I think.\n> \n>> diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh\n>> index e62e07b894..1a02f6546b 100755\n>> --- a/t/t3400-rebase.sh\n>> +++ b/t/t3400-rebase.sh\n>> @@ -471,7 +471,7 @@ test_expect_success 'git rebase --update-ref with \n>> core.commentChar and branch on\n> \n> Adding an extra context line shows\n> \n>     git checkout topic2>       GIT_SEQUENCE_EDITOR=\"cat >actual\" git -c \n> core.commentChar=% \\\n>>            rebase -i --update-refs base &&\n>>       test_grep \"% Ref refs/heads/wt-topic checked out at\" actual &&\n>> -    test_grep \"% Ref refs/heads/topic2 checked out at\" actual\n>> +    test_grep ! \"% Ref refs/heads/topic2 checked out at\" actual\n> \n> As topic2 is checked out in the worktree where the rebase is running why \n> did this line appear before?\n> \n>> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase- \n>> interactive.sh\n>> index e64816770a..11afa8be56 100755\n>> --- a/t/t3404-rebase-interactive.sh\n>> +++ b/t/t3404-rebase-interactive.sh\n>> @@ -1975,15 +1975,23 @@ test_expect_success '--update-refs ignores \n>> non-branch decorations' '\n>>       ) &&\n>>       grep ^update-ref todo >actual &&\n>>       test_write_lines \"update-ref refs/heads/no-conflict-branch\" \n>> >expect &&\n>> +    test_grep ! \"^# Ref refs/heads/update-refs checked out\" todo &&\n> \n> Lets move this line below test_cmp so we keep that line next to the ones \n> that create the files that are being compared. Is this another case \n> where we used to add this comment and no longer do so?\n> \n>>       test_cmp expect actual\n>>   '\n>>   test_expect_success '--update-refs updates refs correctly' '\n>> +    test_when_finished \"\n>> +        test_might_fail git symbolic-ref -d refs/heads/no-conflict- \n>> branch-alias &&\n>> +        test_might_fail git symbolic-ref -d refs/heads/second-alias\n>> +    \" &&\n>>       git checkout -B update-refs no-conflict-branch &&\n>>       git branch -f base HEAD~4 &&\n>>       git branch -f first HEAD~3 &&\n>>       git branch -f second HEAD~3 &&\n>>       git branch -f third HEAD~1 &&\n>> +    git symbolic-ref refs/heads/no-conflict-branch-alias \\\n>> +        refs/heads/no-conflict-branch &&\n>> +    git symbolic-ref refs/heads/second-alias refs/heads/second &&\n>>       test_commit extra2 fileX &&\n>>       git commit --amend --fixup=L &&\n>> @@ -1991,8 +1999,16 @@ test_expect_success '--update-refs updates refs \n>> correctly' '\n>>       test_cmp_rev HEAD~3 refs/heads/first &&\n>>       test_cmp_rev HEAD~3 refs/heads/second &&\n>> +    test_cmp_rev HEAD~3 refs/heads/second-alias &&\n>>       test_cmp_rev HEAD~1 refs/heads/third &&\n>>       test_cmp_rev HEAD refs/heads/no-conflict-branch &&\n>> +    test_cmp_rev HEAD refs/heads/no-conflict-branch-alias &&\n>> +    test_write_lines refs/heads/no-conflict-branch >expect &&\n>> +    git symbolic-ref refs/heads/no-conflict-branch-alias >actual &&\n>> +    test_cmp expect actual &&\n>> +    test_write_lines refs/heads/second >expect &&\n>> +    git symbolic-ref refs/heads/second-alias >actual &&\n>> +    test_cmp expect actual &&\n> \n> This looks good - we check that \"rebase --update-refs\" succeeds withh \n> branches that are symrefs and also that those refs are untouched by the \n> rebase.\n> \n> Thanks\n> \n> Phillip\n> \n>>       q_to_tab >expect <<-\\EOF &&\n>>       Successfully rebased and updated refs/heads/update-refs.\n> \n\n"},{"id":"548944","messageId":"amSSYagL0jTgzElD@mbp","threadId":"65707","inReplyTo":"00e529b6-7ae7-463f-a4b3-0991e9411aba@gmail.com","subject":"Re: [PATCH v3 1/2] rebase: skip branch symref aliases","fromName":"Erik Cervin-Edin","fromEmail":"erik@cervined.in","sentAt":"2026-07-25T11:21:53Z","receivedAt":"2026-07-25T11:21:59Z","isPatch":true,"body":"I ran into a minor regression that I tracked down to v2 of this patch\nseries. When I was running\n\n git rebase --interactive --update-refs\n\nI got a comment in my git-rebase-todo for the branch I was rebasing,\neven though it's just the current branch in the current worktree:\n\n # Ref refs/heads/main checked out at '/private/tmp/gittest'\n\nwhere I'd expect no entry at all, since the branch is updated by the\nrebase itself. After applying v3 of the series, the regression went\naway. Nevertheless, I thought I ought to share what I found.\n\nOn 26/07/24 10:55AM, Phillip Wood wrote:\n> > > + if (head_ref && !strcmp(head_ref, decoration->name)) {\n> > > + free(resolved_ref);\n> > > decoration = decoration->next;\n> > > continue;\n> > > }\n> > > +\n> > > + path = branch_checked_out(decoration->name);\n> >\n> > Then we check to see if the decoration matches HEAD which we used to do\n> > above - I'm not clear why we have moved this check.\n\nbranch_checked_out() can't tell \"checked out in another worktree\"\napart from \"checked out right here\", so `path` is never NULL for the\nbranch actually being rebased. In v2, the check above was instead:\n\n if (!path && head_ref && !strcmp(head_ref, decoration->name))\n continue;\n\nwhich made it a no-op for exactly that branch -- the regression I\nobserved. v3 drops the `!path` gate and moves `path` below it, which\nis why it's fixed.\n\n> > As topic2 is checked out in the worktree where the rebase is running\n> > why did this line appear before?\n\nThis might be the same symptom from another cause: on master, head_ref\ncomes from refs_resolve_ref_unsafe(), and as you note its buffer is\noverwritten inside branch_checked_out(). So by the time topic2 is\ncompared, wt-topic's lookup may already have clobbered head_ref,\nletting topic2 fall through to the comment. I haven't run the test to\nconfirm, though.\n\nThanks,\nErik\n"},{"id":"549021","messageId":"xmqq7bmhycxq.fsf@gitster.g","threadId":"65707","inReplyTo":"00e529b6-7ae7-463f-a4b3-0991e9411aba@gmail.com","subject":"Re: [PATCH v3 1/2] rebase: skip branch symref aliases","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-26T15:42:57Z","receivedAt":"2026-07-26T15:43:02Z","isPatch":true,"body":"Phillip Wood writes:\n\n>> Thanks for re-rolling I'm pretty sure the logic is sound now but I'm a \n>> bit confused by a couple of things - see my comments below.\n>> ...\n>> It would be nice to have a comment here explaining what we're doing. \n>> Also I don't think we need to copy the refname so it would be more \n>> efficient to use refs_resolve_ref_unsafe().\n>\n> Looking at this again we cannot use refs_resolve_ref_unsafe() because \n> the result would be overwritten by the call to refs_resolve_refdup() in \n> branch_checked_out().\n\nMakes sense. Thanks for raising a possible alternative and then\nclarifying that it is not quite workable.\n\n>>> +        /*\n>>> +         * If the branch is the current HEAD, then it will be\n>>> +         * updated by the default rebase behavior.\n>>> +         */\n>>> +        if (head_ref && !strcmp(head_ref, decoration->name)) {\n>>> +            free(resolved_ref);\n>>>               decoration = decoration->next;\n>>>               continue;\n>>>           }\n>> \n>> Then we check to see if the decoration matches HEAD which we used to do \n>> above - I'm not clear why we have moved this check.\n>\n> Should we be using \"resolved_ref\" instead of \"decoration->name\"? That \n> would explain why this was moved and would makes sense as we resolve \n> symrefs when reading HEAD. When HEAD points outside \"refs/heads/\" we'd \n> then skip updating any symrefs under \"refs/heads/\" that pointed to the \n> same ref as HEAD.\n\nYeah, decoration is very much end-user facing and if we can make\nbehavioural decision based on a more stable resolved_ref that would\nmake it easier to reason about.\n\nBut stepping back a bit, is having a HEAD that is a symref and\npoints outside \"refs/heads/\" an invalid state? Why are we catering\nto such a configuration to begin with?\n"},{"id":"549116","messageId":"8631114b-aa6f-446e-9710-92c400320eac@gmail.com","threadId":"65707","inReplyTo":"xmqq7bmhycxq.fsf@gitster.g","subject":"Re: [PATCH v3 1/2] rebase: skip branch symref aliases","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-07-28T09:36:33Z","receivedAt":"2026-07-28T09:36:36Z","isPatch":true,"body":"On 26/07/2026 16:42, Junio C Hamano wrote:\n> Phillip Wood writes:\n> \n>>> Thanks for re-rolling I'm pretty sure the logic is sound now but I'm a\n>>> bit confused by a couple of things - see my comments below.\n>>> ...\n>>> It would be nice to have a comment here explaining what we're doing.\n>>> Also I don't think we need to copy the refname so it would be more\n>>> efficient to use refs_resolve_ref_unsafe().\n>>\n>> Looking at this again we cannot use refs_resolve_ref_unsafe() because\n>> the result would be overwritten by the call to refs_resolve_refdup() in\n>> branch_checked_out().\n> \n> Makes sense. Thanks for raising a possible alternative and then\n> clarifying that it is not quite workable.\n> \n>>>> +        /*\n>>>> +         * If the branch is the current HEAD, then it will be\n>>>> +         * updated by the default rebase behavior.\n>>>> +         */\n>>>> +        if (head_ref && !strcmp(head_ref, decoration->name)) {\n>>>> +            free(resolved_ref);\n>>>>               decoration = decoration->next;\n>>>>               continue;\n>>>>           }\n>>>\n>>> Then we check to see if the decoration matches HEAD which we used to do\n>>> above - I'm not clear why we have moved this check.\n>>\n>> Should we be using \"resolved_ref\" instead of \"decoration->name\"? That\n>> would explain why this was moved and would makes sense as we resolve\n>> symrefs when reading HEAD. When HEAD points outside \"refs/heads/\" we'd\n>> then skip updating any symrefs under \"refs/heads/\" that pointed to the\n>> same ref as HEAD.\n> \n> Yeah, decoration is very much end-user facing and if we can make\n> behavioural decision based on a more stable resolved_ref that would\n> make it easier to reason about.\n> \n> But stepping back a bit, is having a HEAD that is a symref and\n> points outside \"refs/heads/\" an invalid state? Why are we catering\n> to such a configuration to begin with?\n\nWe allow HEAD to point to anything below \"refs/\" - see e9cc02f0e4 \n(symbolic-ref: allow refs/ in HEAD, 2009-02-13). I've not \ntested it but looking at the code I think rebase handles a non-branch \nHEAD correctly.\n\nIt would be nice if we didn't have to worry about non-branch HEADs, \nrecently Caleb reported on discord being surprised that git allows HEAD \nto point to a tag.\n\nThanks\n\nPhillip\n\n\n"},{"id":"549131","messageId":"xmqqpl07fb1u.fsf@gitster.g","threadId":"65707","inReplyTo":"8631114b-aa6f-446e-9710-92c400320eac@gmail.com","subject":"Re: [PATCH v3 1/2] rebase: skip branch symref aliases","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-28T14:23:09Z","receivedAt":"2026-07-28T14:23:11Z","isPatch":true,"body":"Phillip Wood writes:\n\n>> But stepping back a bit, is having a HEAD that is a symref and\n>> points outside \"refs/heads/\" an invalid state? Why are we catering\n>> to such a configuration to begin with?\n>\n> We allow HEAD to point to anything below \"refs/\" - see e9cc02f0e4 \n> (symbolic-ref: allow refs/ in HEAD, 2009-02-13).\n\nBut that was about a low level mechanism that must be more lenient\nto be usable as repair tools to recover from such a broken state,\nno? I thought the end-user facing commands like \"git checkout\" have\nbeen tightened long ago to stop users from getting into a situation\nthat needs repairing in the first place. And that was why I asked.\n\n"},{"id":"549187","messageId":"61291144-60da-4e37-83ef-fe09e91c4f51@gmail.com","threadId":"65707","inReplyTo":"xmqqpl07fb1u.fsf@gitster.g","subject":"Re: [PATCH v3 1/2] rebase: skip branch symref aliases","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-07-29T09:31:34Z","receivedAt":"2026-07-29T09:31:41Z","isPatch":true,"body":"On 28/07/2026 15:23, Junio C Hamano wrote:\n> Phillip Wood writes:\n> \n>>> But stepping back a bit, is having a HEAD that is a symref and\n>>> points outside \"refs/heads/\" an invalid state? Why are we catering\n>>> to such a configuration to begin with?\n>>\n>> We allow HEAD to point to anything below \"refs/\" - see e9cc02f0e4\n>> (symbolic-ref: allow refs/ in HEAD, 2009-02-13).\n> \n> But that was about a low level mechanism that must be more lenient\n> to be usable as repair tools to recover from such a broken state,\n> no?\n\nIt checks the new value of HEAD, not the old one so I don't think so. \nThe commit message talks about topgit using \"git symbolic-ref\" to set \nhead outside \"refs/heads/\" - peff had previously tried to tighten it to \nreject non-branch refs but that broke topgit. I've just had a quick look \nat the topgit code and still sets HEAD to point to \"refs/top-bases/...\" \nby default[1], although there are plans to start using \n\"refs/heads/{top-bases}/...\" instead.\n> I thought the end-user facing commands like \"git checkout\" have\n> been tightened long ago to stop users from getting into a situation\n> that needs repairing in the first place. And that was why I asked.\n\nYes \"git checkout\" detaches HEAD if you give a non-branch ref, but \nexternal tools can still use \"git symbolic-ref\" to bypass that. It \nappears topgit's rebase command is built around \"git rebase\"[2] so I \nthink we need to continue to support rebasing a non-branch HEAD.\n\nThanks\n\nPhillip\n\n[1] https://github.com/mackyle/topgit/blob/master/tg.sh#L2683\n[2] https://github.com/mackyle/topgit/blob/master/tg-rebase.sh#L56\n\n\n"},{"id":"549208","messageId":"xmqqwludan2m.fsf@gitster.g","threadId":"65707","inReplyTo":"61291144-60da-4e37-83ef-fe09e91c4f51@gmail.com","subject":"Re: [PATCH v3 1/2] rebase: skip branch symref aliases","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-29T14:26:57Z","receivedAt":"2026-07-29T14:26:59Z","isPatch":true,"body":"Phillip Wood writes:\n\n>> But that was about a low level mechanism that must be more lenient\n>> to be usable as repair tools to recover from such a broken state,\n>> no?\n>\n> It checks the new value of HEAD, not the old one so I don't think so. \n> The commit message talks about topgit using \"git symbolic-ref\" to set \n> head outside \"refs/heads/\" - peff had previously tried to tighten it to \n> reject non-branch refs but that broke topgit. I've just had a quick look \n> at the topgit code and still sets HEAD to point to \"refs/top-bases/...\" \n> by default[1], although there are plans to start using \n> \"refs/heads/{top-bases}/...\" instead.\n\nAh, that name vaguely rings a bell. Is it still in use, and now\nthey prevent us from forbidding funny characters like {} in the\nrefname? Sigh...\n\n> ... It \n> appears topgit's rebase command is built around \"git rebase\"[2] so I \n> think we need to continue to support rebasing a non-branch HEAD.\n\nSigh, again, but OK.\n\nThanks.\n"},{"id":"549298","messageId":"0844b4e0-679b-4c0a-bea1-5779b4d0489d@gmail.com","threadId":"65707","inReplyTo":"xmqqwludan2m.fsf@gitster.g","subject":"Re: [PATCH v3 1/2] rebase: skip branch symref aliases","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-07-30T13:10:31Z","receivedAt":"2026-07-30T13:10:35Z","isPatch":true,"body":"\n\nOn 29/07/2026 15:26, Junio C Hamano wrote:\n> Phillip Wood writes:\n> \n>>> But that was about a low level mechanism that must be more lenient\n>>> to be usable as repair tools to recover from such a broken state,\n>>> no?\n>>\n>> It checks the new value of HEAD, not the old one so I don't think so.\n>> The commit message talks about topgit using \"git symbolic-ref\" to set\n>> head outside \"refs/heads/\" - peff had previously tried to tighten it to\n>> reject non-branch refs but that broke topgit. I've just had a quick look\n>> at the topgit code and still sets HEAD to point to \"refs/top-bases/...\"\n>> by default[1], although there are plans to start using\n>> \"refs/heads/{top-bases}/...\" instead.\n> \n> Ah, that name vaguely rings a bell. Is it still in use, and now\n> they prevent us from forbidding funny characters like {} in the\n> refname? Sigh...\n\nYes, it still seems to be maintained, I guess they chose the funny \ncharacters to try and avoid name collisions because no-one would want \nthem in a \"normal\" branch name.\n\nThanks\n\nPhillip\n\n>> ... It\n>> appears topgit's rebase command is built around \"git rebase\"[2] so I\n>> think we need to continue to support rebasing a non-branch HEAD.\n> \n> Sigh, again, but OK.\n> \n> Thanks.\n> \n\n"},{"id":"549881","messageId":"xmqqh5l7huuc.fsf@gitster.g","threadId":"65707","inReplyTo":"0844b4e0-679b-4c0a-bea1-5779b4d0489d@gmail.com","subject":"Re: [PATCH v3 1/2] rebase: skip branch symref aliases","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-06T20:12:11Z","receivedAt":"2026-08-06T20:12:15Z","isPatch":true,"body":"Phillip Wood writes:\n\n> On 29/07/2026 15:26, Junio C Hamano wrote:\n>> Phillip Wood writes:\n>> \n>>>> But that was about a low level mechanism that must be more lenient\n>>>> to be usable as repair tools to recover from such a broken state,\n>>>> no?\n>>>\n>>> It checks the new value of HEAD, not the old one so I don't think so.\n>>> The commit message talks about topgit using \"git symbolic-ref\" to set\n>>> head outside \"refs/heads/\" - peff had previously tried to tighten it to\n>>> reject non-branch refs but that broke topgit. I've just had a quick look\n>>> at the topgit code and still sets HEAD to point to \"refs/top-bases/...\"\n>>> by default[1], although there are plans to start using\n>>> \"refs/heads/{top-bases}/...\" instead.\n>> \n>> Ah, that name vaguely rings a bell. Is it still in use, and now\n>> they prevent us from forbidding funny characters like {} in the\n>> refname? Sigh...\n>\n> Yes, it still seems to be maintained, I guess they chose the funny \n> characters to try and avoid name collisions because no-one would want \n> them in a \"normal\" branch name.\n>\n> Thanks\n\nI guess I dropped the ball here. So given that we are OK to see\nHEAD pointing outside refs/heads/, this subthread should be closed.\n\nMy understanding is that we still have an issue reported by Erik in\n that needs to be addressed before this topic\ncan move further?\n\nThanks.\n\n"},{"id":"550014","messageId":"98682fa4-55d9-4829-97f1-02e244b35266@gmail.com","threadId":"65707","inReplyTo":"a653f56ea214e74ea71ba31f5378f9cbf8b04dde.1784708107.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 2/2] rebase: guard non-branch symref targets","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-08-07T15:22:33Z","receivedAt":"2026-08-07T15:22:37Z","isPatch":true,"body":"On 22/07/2026 09:15, Son Luong Ngoc via GitGitGadget wrote:\n> From: Son Luong Ngoc \n> \n> A local branch symbolic ref may point outside refs/heads/. Such an alias\n> cannot be skipped like a branch-to-branch alias because its concrete\n> target ref is absent from the local branch decoration list.\n> \n> However, queuing each alias independently can update the same target ref\n> more than once and make the second compare-and-swap fail. A reservation\n> from another worktree can also name either an alias or its resolved\n> target ref, so checking only one form can miss an in-progress update.\n> \n> Fix these cases by checking both the literal alias and its resolved\n> target ref against checked-out reservations. Deduplicate updates by\n> target ref. Also reserve both forms when loading another worktree's\n> update-refs state. This makes different aliases honor the same\n> in-progress update.\n> \n> This keeps non-branch symrefs supported without allowing duplicate or\n> cross-worktree ref updates.\n\nI've left a couple of questions below. If you're short on time I'd \nsuggest we just concentrate on getting the first patch merged as that \nfixes the common case of one branch being a symlink to another. We can \nworry about two branches being symlinks to a ref outside \"refs/heads/*\" \nlater if it turns out to be a problem in practice.\n\n> diff --git a/branch.c b/branch.c\n> index 243db7d0fc..98a50d8368 100644\n> --- a/branch.c\n> +++ b/branch.c\n> @@ -442,10 +442,25 @@ static void prepare_checked_out_branches(void)\n> \t\t\t\t\t\t &update_refs)) {\n> \t\t\tstruct string_list_item *item;\n> \t\t\tfor_each_string_list_item(item, &update_refs) {\n> +\t\t\t\tchar *resolved_ref;\n> +\t\t\t\tint flags = 0;\n> +\n> \t\t\t\told = strmap_put(¤t_checked_out_branches,\n> \t\t\t\t\t\t item->string,\n> \t\t\t\t\t\t xstrdup(wt->path));\n> \t\t\t\tfree(old);\n> +\n> +\t\t\t\tresolved_ref = refs_resolve_refdup(\n> +\t\t\t\t\tget_main_ref_store(the_repository),\n> +\t\t\t\t\titem->string, RESOLVE_REF_READING,\n> +\t\t\t\t\tNULL, &flags);\n> +\t\t\t\tif (resolved_ref && (flags & REF_ISSYMREF)) {\n> +\t\t\t\t\told = strmap_put(\n> +\t\t\t\t\t\t¤t_checked_out_branches,\n> +\t\t\t\t\t\tresolved_ref, xstrdup(wt->path));\n> +\t\t\t\t\tfree(old);\n> +\t\t\t\t}\n> +\t\t\t\tfree(resolved_ref);\n> \t\t\t}\n\nAfter the last commit, when we prepare the todo list don't we skip any \nsymbolic refs and only record their target? That would mean there \nshouldn't be any symbolic refs to resolve here. I do wonder if the \nearlier part of this function should be storing the symref and its \ntarget when it walks all the worktree HEADs. If we have a branch \n\"refs/heads/feature\" and a symref \"refs/heads/symlink-to-feature\" is it \npossible to have them checkedout in different worktrees because we only \nadd HEAD to the list of checked out branches when we walk all the \nworktree HEADs?\n\n> \t\t\tstring_list_clear(&update_refs, 1);\n> \t\t}\n> diff --git a/sequencer.c b/sequencer.c\n> index 63aba60a08..040b5bf645 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -6459,6 +6459,7 @@ struct todo_add_branch_context {\n> \tsize_t items_alloc;\n> \tstruct strbuf *buf;\n> \tstruct string_list refs_to_oids;\n> +\tstruct string_list symref_update_targets;\n> };\n> \n> static int add_decorations_to_list(const struct commit *commit,\n> @@ -6473,6 +6474,7 @@ static int add_decorations_to_list(const struct commit *commit,\n> \twhile (decoration) {\n> \t\tstruct todo_item *item;\n> \t\tconst char *path;\n> +\t\tconst char *checked_ref;\n> \t\tchar *resolved_ref;\n> \t\tint flags = 0;\n> \t\tsize_t base_offset = ctx->buf->len;\n> @@ -6508,6 +6510,17 @@ static int add_decorations_to_list(const struct commit *commit,\n> \t\t}\n> \n> \t\tpath = branch_checked_out(decoration->name);\n> +\t\tif (!path && resolved_ref && (flags & REF_ISSYMREF)) {\n> +\t\t\tchecked_ref = resolved_ref;\n> +\t\t\tpath = branch_checked_out(checked_ref);\n> +\t\t}\n> +\t\tif (!path && resolved_ref && (flags & REF_ISSYMREF) &&\n> +\t\t string_list_has_string(&ctx->symref_update_targets,\n> +\t\t\t\t\t resolved_ref)) {\n> +\t\t\tfree(resolved_ref);\n> +\t\t\tdecoration = decoration->next;\n> +\t\t\tcontinue;\n> +\t\t}\n\nSo we check the to see if the symref or its target are checked out. \nThat's necessary because we might have stored a symref rather than its \ntarget in current_checked_out_branches above (which I think is probably \na bug). If two branches are symrefs to the same ref we'll only queue the \nupdate once which is good.\n\nThanks\n\nPhillip\n> \t\tALLOC_GROW(ctx->items,\n> \t\t\tctx->items_nr + 1,\n> @@ -6523,6 +6536,10 @@ static int add_decorations_to_list(const struct commit *commit,\n> \t\t\t\t\t decoration->name, path);\n> \t\t} else {\n> \t\t\tstruct string_list_item *sti;\n> +\n> +\t\t\tif (resolved_ref && (flags & REF_ISSYMREF))\n> +\t\t\t\tstring_list_insert(&ctx->symref_update_targets,\n> +\t\t\t\t\t\t resolved_ref);\n> \t\t\titem->command = TODO_UPDATE_REF;\n> \t\t\tstrbuf_addf(ctx->buf, \"%s\\n\", decoration->name);\n> \n> @@ -6554,6 +6571,7 @@ static int todo_list_add_update_ref_commands(struct todo_list *todo_list)\n> \tstruct todo_add_branch_context ctx = {\n> \t\t.buf = &todo_list->buf,\n> \t\t.refs_to_oids = STRING_LIST_INIT_DUP,\n> +\t\t.symref_update_targets = STRING_LIST_INIT_DUP,\n> \t};\n> \n> \tctx.items_alloc = 2 * todo_list->nr + 1;\n> @@ -6579,6 +6597,7 @@ static int todo_list_add_update_ref_commands(struct todo_list *todo_list)\n> \tres = write_update_refs_state(&ctx.refs_to_oids);\n> \n> \tstring_list_clear(&ctx.refs_to_oids, 1);\n> +\tstring_list_clear(&ctx.symref_update_targets, 0);\n> \n> \tif (res) {\n> \t\t/* we failed, so clean up the new list. */\n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> index 11afa8be56..110ed8ae63 100755\n> --- a/t/t3404-rebase-interactive.sh\n> +++ b/t/t3404-rebase-interactive.sh\n> @@ -2024,6 +2024,78 @@ test_expect_success '--update-refs updates refs correctly' '\n> \ttest_cmp expect err.trimmed\n> '\n> \n> +test_expect_success '--update-refs checks resolved non-branch symref target' '\n> +\ttest_when_finished \"\n> +\t\tgit worktree remove --force checked-out-target-wt &&\n> +\t\tgit symbolic-ref -d refs/heads/non-branch-alias &&\n> +\t\tgit tag -d checked-out-target\n> +\t\" &&\n> +\tgit tag checked-out-target HEAD~1 &&\n> +\tgit symbolic-ref refs/heads/non-branch-alias refs/tags/checked-out-target &&\n> +\tgit worktree add --detach checked-out-target-wt checked-out-target &&\n> +\tgit -C checked-out-target-wt symbolic-ref HEAD refs/tags/checked-out-target &&\n> +\n> +\tGIT_SEQUENCE_EDITOR=\"cat >todo\" git rebase -i --update-refs HEAD~2 &&\n> +\n> +\ttest_grep \"^# Ref refs/heads/non-branch-alias checked out at\" todo &&\n> +\ttest_write_lines refs/tags/checked-out-target >expect &&\n> +\tgit symbolic-ref refs/heads/non-branch-alias >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success '--update-refs deduplicates non-branch symref targets' '\n> +\ttest_when_finished \"\n> +\t\tgit symbolic-ref -d refs/heads/non-branch-alias-one &&\n> +\t\tgit symbolic-ref -d refs/heads/non-branch-alias-two &&\n> +\t\tgit tag -d shared-non-branch-target\n> +\t\" &&\n> +\tgit tag shared-non-branch-target HEAD~1 &&\n> +\tgit symbolic-ref refs/heads/non-branch-alias-one \\\n> +\t\trefs/tags/shared-non-branch-target &&\n> +\tgit symbolic-ref refs/heads/non-branch-alias-two \\\n> +\t\trefs/tags/shared-non-branch-target &&\n> +\n> +\tGIT_SEQUENCE_EDITOR=: git rebase -i --force-rebase --update-refs HEAD~2 &&\n> +\n> +\ttest_cmp_rev HEAD~1 refs/heads/non-branch-alias-one &&\n> +\ttest_cmp_rev HEAD~1 refs/heads/non-branch-alias-two &&\n> +\ttest_write_lines refs/tags/shared-non-branch-target >expect &&\n> +\tgit symbolic-ref refs/heads/non-branch-alias-one >actual &&\n> +\ttest_cmp expect actual &&\n> +\tgit symbolic-ref refs/heads/non-branch-alias-two >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success '--update-refs honors non-branch symref reservations' '\n> +\ttest_when_finished \"\n> +\t\ttest_might_fail git worktree remove --force reserved-target-wt &&\n> +\t\ttest_might_fail git symbolic-ref -d \\\n> +\t\t\trefs/heads/reserved-non-branch-alias-one &&\n> +\t\ttest_might_fail git symbolic-ref -d \\\n> +\t\t\trefs/heads/reserved-non-branch-alias-two &&\n> +\t\ttest_might_fail git tag -d reserved-non-branch-target\n> +\t\" &&\n> +\tgit tag reserved-non-branch-target HEAD~1 &&\n> +\tgit symbolic-ref refs/heads/reserved-non-branch-alias-one \\\n> +\t\trefs/tags/reserved-non-branch-target &&\n> +\tgit symbolic-ref refs/heads/reserved-non-branch-alias-two \\\n> +\t\trefs/tags/reserved-non-branch-target &&\n> +\tgit worktree add --detach reserved-target-wt HEAD &&\n> +\twt_gitdir=$(git -C reserved-target-wt rev-parse --absolute-git-dir) &&\n> +\tmkdir -p \"$wt_gitdir/rebase-merge\" &&\n> +\told_oid=$(git rev-parse refs/heads/reserved-non-branch-alias-one) &&\n> +\ttest_write_lines refs/heads/reserved-non-branch-alias-one \\\n> +\t\t\"$old_oid\" \"$old_oid\" >\"$wt_gitdir/rebase-merge/update-refs\" &&\n> +\n> +\tGIT_SEQUENCE_EDITOR=\"cat >todo\" git rebase -i --update-refs HEAD~2 &&\n> +\n> +\ttest_grep \"^# Ref refs/heads/reserved-non-branch-alias-one checked out at\" \\\n> +\t\ttodo &&\n> +\ttest_grep \"^# Ref refs/heads/reserved-non-branch-alias-two checked out at\" \\\n> +\t\ttodo &&\n> +\ttest_grep ! \"^update-ref refs/heads/reserved-non-branch-alias\" todo\n> +'\n> +\n> test_expect_success 'respect user edits to update-ref steps' '\n> \tgit checkout -B update-refs-break no-conflict-branch &&\n> \tgit branch -f base HEAD~4 &&\n\n"},{"id":"550016","messageId":"1eba5fb2-ab76-41e9-955d-e283256ad25d@gmail.com","threadId":"65707","inReplyTo":"xmqqh5l7huuc.fsf@gitster.g","subject":"Re: [PATCH v3 1/2] rebase: skip branch symref aliases","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-08-07T15:28:18Z","receivedAt":"2026-08-07T15:28:21Z","isPatch":true,"body":"On 06/08/2026 21:12, Junio C Hamano wrote:\n> Phillip Wood writes:\n> \n>> On 29/07/2026 15:26, Junio C Hamano wrote:\n>>> Phillip Wood writes:\n>>>\n>>>>> But that was about a low level mechanism that must be more lenient\n>>>>> to be usable as repair tools to recover from such a broken state,\n>>>>> no?\n>>>>\n>>>> It checks the new value of HEAD, not the old one so I don't think so.\n>>>> The commit message talks about topgit using \"git symbolic-ref\" to set\n>>>> head outside \"refs/heads/\" - peff had previously tried to tighten it to\n>>>> reject non-branch refs but that broke topgit. I've just had a quick look\n>>>> at the topgit code and still sets HEAD to point to \"refs/top-bases/...\"\n>>>> by default[1], although there are plans to start using\n>>>> \"refs/heads/{top-bases}/...\" instead.\n>>>\n>>> Ah, that name vaguely rings a bell. Is it still in use, and now\n>>> they prevent us from forbidding funny characters like {} in the\n>>> refname? Sigh...\n>>\n>> Yes, it still seems to be maintained, I guess they chose the funny\n>> characters to try and avoid name collisions because no-one would want\n>> them in a \"normal\" branch name.\n>>\n>> Thanks\n> \n> I guess I dropped the ball here. So given that we are OK to see\n> HEAD pointing outside refs/heads/, this subthread should be closed.\n> \n> My understanding is that we still have an issue reported by Erik in\n> that needs to be addressed before this topic\n> can move further?\n\nMaybe I misunderstood but I thought Erik was pointing out a bug in v2 \nthat was fixed in v3. I do think we want a re-roll though for\n\n >> + /*\n >> + * If the branch is the current HEAD, then it will be\n >> + * updated by the default rebase behavior.\n >> + */\n >> + if (head_ref && !strcmp(head_ref, decoration->name)) {\n >> + free(resolved_ref);\n >> decoration = decoration->next;\n >> continue;\n >> }\n > [...]\n > Should we be using \"resolved_ref\" instead of \"decoration->name\"? That\n > would explain why this was moved and would makes sense as we resolve\n > symrefs when reading HEAD. When HEAD points outside \"refs/heads/\" \nwe'd > then skip updating any symrefs under \"refs/heads/\" that pointed \nto the > same ref as HEAD.\n\nin [1]. I've also just left some comments on the second patch\n\nThanks\n\nPhillip\n\n[1] \nhttps://lore.kernel.org/git/00e529b6-7ae7-463f-a4b3-0991e9411aba@gmail.com\n"}]}