{"thread":{"id":"66539","subject":"[PATCH] sequencer: don't clobber HEAD ref name in worktree lookup","startedAt":"2026-10-14T16:58:24Z","lastAt":"2026-10-15T15:24:51Z","messageCount":3,"participants":["Andy Koppe","Junio C Hamano","Phillip Wood"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"555049","messageId":"20261014165824.2214234-1-andy.koppe@gmail.com","threadId":"66539","inReplyTo":null,"subject":"[PATCH] sequencer: don't clobber HEAD ref name in worktree lookup","fromName":"Andy Koppe","fromEmail":"andy.koppe@gmail.com","sentAt":"2026-10-14T16:58:24Z","receivedAt":"2026-10-14T16:58:24Z","isPatch":true,"sender":{"key":"andy.koppe@gmail.com","avatar":"https://avatars.githubusercontent.com/u/223411?v=4"},"body":"add_decorations_to_list() skips the branch that HEAD points to, because\nthe rebase updates it anyway, as per aa37f3e1d89 (sequencer: ignore HEAD\nref under --update-refs, 2022-07-19).\n\nHowever, it keeps the name of that branch in the static buffer returned\nby refs_resolve_ref_unsafe(). If another branch on the same commit is\nchecked out in a different worktree, branch_checked_out() overwrites\nthat buffer. If the current branch comes after the other branch in the\nlist of decorations, it gets a \"checked out\" comment in the todo list\ninstead of being skipped, and the branch whose name ended up in the\nbuffer is skipped in its place.\n\nDecorations are processed in reverse alphabetical order, so this depends\non the branch names. The test added in 94304b9f48e (sequencer: comment\nchecked-out branch properly, 2024-11-25) happens to trigger the problem\nand expects its result.\n\nFix it by using refs_resolve_refdup(), correct that test's expectation,\nand add a test with checked-out branches sorting both before and after\nthe current branch.\n\nSigned-off-by: Andy Koppe <andy.koppe@gmail.com>\n---\n sequencer.c       | 11 ++++++-----\n t/t3400-rebase.sh | 21 ++++++++++++++++++++-\n 2 files changed, 26 insertions(+), 6 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 6dae43e4db..c672b6e127 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -6577,11 +6577,11 @@ 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+\tchar *head_ref = refs_resolve_refdup(get_main_ref_store(the_repository),\n+\t\t\t\t\t     \"HEAD\",\n+\t\t\t\t\t     RESOLVE_REF_READING,\n+\t\t\t\t\t     NULL,\n+\t\t\t\t\t     NULL);\n \n \twhile (decoration) {\n \t\tstruct todo_item *item;\n@@ -6633,6 +6633,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/t3400-rebase.sh b/t/t3400-rebase.sh\nindex accaaba4d4..0e7ef33eb1 100755\n--- a/t/t3400-rebase.sh\n+++ b/t/t3400-rebase.sh\n@@ -483,7 +483,26 @@ 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\" actual\n+'\n+\n+test_expect_success 'git rebase --update-refs ignores current branch with branches on worktrees' '\n+\ttest_when_finished git branch -D base current a-topic z-topic &&\n+\ttest_when_finished git checkout main &&\n+\ttest_when_finished git worktree remove z-topic &&\n+\ttest_when_finished git worktree remove a-topic &&\n+\tgit checkout -b base main &&\n+\tgit checkout -b current &&\n+\ttest_commit msg4 &&\n+\tgit worktree add a-topic &&\n+\tgit worktree add z-topic &&\n+\tgit checkout base &&\n+\ttest_commit msg5 &&\n+\tgit checkout current &&\n+\tGIT_SEQUENCE_EDITOR=\"cat >actual\" git rebase -i --update-refs base &&\n+\ttest_grep \"# Ref refs/heads/a-topic checked out at\" actual &&\n+\ttest_grep \"# Ref refs/heads/z-topic checked out at\" actual &&\n+\ttest_grep ! \"refs/heads/current\" actual\n '\n \n test_done\n-- \n2.56.0\n\n\n"},{"id":"555067","messageId":"xmqqik34qd6s.fsf@gitster.g","threadId":"66539","inReplyTo":"20261014165824.2214234-1-andy.koppe@gmail.com","subject":"Re: [PATCH] sequencer: don't clobber HEAD ref name in worktree lookup","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-10-14T19:44:59Z","receivedAt":"2026-10-14T19:44:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andy Koppe <andy.koppe@gmail.com> writes:\n\n> add_decorations_to_list() skips the branch that HEAD points to, because\n> the rebase updates it anyway, as per aa37f3e1d89 (sequencer: ignore HEAD\n> ref under --update-refs, 2022-07-19).\n>\n> However, it keeps the name of that branch in the static buffer returned\n> by refs_resolve_ref_unsafe(). If another branch on the same commit is\n> checked out in a different worktree, branch_checked_out() overwrites\n> that buffer. If the current branch comes after the other branch in the\n> list of decorations, it gets a \"checked out\" comment in the todo list\n> instead of being skipped, and the branch whose name ended up in the\n> buffer is skipped in its place.\n\nSo obviously broken original, so cleanly corrected.  I very much\nlike it.\n\nWill queue.  thanks.\n\n>\n> Decorations are processed in reverse alphabetical order, so this depends\n> on the branch names. The test added in 94304b9f48e (sequencer: comment\n> checked-out branch properly, 2024-11-25) happens to trigger the problem\n> and expects its result.\n>\n> Fix it by using refs_resolve_refdup(), correct that test's expectation,\n> and add a test with checked-out branches sorting both before and after\n> the current branch.\n>\n> Signed-off-by: Andy Koppe <andy.koppe@gmail.com>\n> ---\n>  sequencer.c       | 11 ++++++-----\n>  t/t3400-rebase.sh | 21 ++++++++++++++++++++-\n>  2 files changed, 26 insertions(+), 6 deletions(-)\n>\n> diff --git a/sequencer.c b/sequencer.c\n> index 6dae43e4db..c672b6e127 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -6577,11 +6577,11 @@ 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> +\tchar *head_ref = refs_resolve_refdup(get_main_ref_store(the_repository),\n> +\t\t\t\t\t     \"HEAD\",\n> +\t\t\t\t\t     RESOLVE_REF_READING,\n> +\t\t\t\t\t     NULL,\n> +\t\t\t\t\t     NULL);\n>  \n>  \twhile (decoration) {\n>  \t\tstruct todo_item *item;\n> @@ -6633,6 +6633,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>  \n> diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh\n> index accaaba4d4..0e7ef33eb1 100755\n> --- a/t/t3400-rebase.sh\n> +++ b/t/t3400-rebase.sh\n> @@ -483,7 +483,26 @@ 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\" actual\n> +'\n> +\n> +test_expect_success 'git rebase --update-refs ignores current branch with branches on worktrees' '\n> +\ttest_when_finished git branch -D base current a-topic z-topic &&\n> +\ttest_when_finished git checkout main &&\n> +\ttest_when_finished git worktree remove z-topic &&\n> +\ttest_when_finished git worktree remove a-topic &&\n> +\tgit checkout -b base main &&\n> +\tgit checkout -b current &&\n> +\ttest_commit msg4 &&\n> +\tgit worktree add a-topic &&\n> +\tgit worktree add z-topic &&\n> +\tgit checkout base &&\n> +\ttest_commit msg5 &&\n> +\tgit checkout current &&\n> +\tGIT_SEQUENCE_EDITOR=\"cat >actual\" git rebase -i --update-refs base &&\n> +\ttest_grep \"# Ref refs/heads/a-topic checked out at\" actual &&\n> +\ttest_grep \"# Ref refs/heads/z-topic checked out at\" actual &&\n> +\ttest_grep ! \"refs/heads/current\" actual\n>  '\n>  \n>  test_done\n\n"},{"id":"555156","messageId":"837b7d72-a767-4c51-9693-8fd95d14a58d@gmail.com","threadId":"66539","inReplyTo":"xmqqik34qd6s.fsf@gitster.g","subject":"Re: [PATCH] sequencer: don't clobber HEAD ref name in worktree lookup","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-10-15T15:24:51Z","receivedAt":"2026-10-15T15:24:51Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 14/10/2026 20:44, Junio C Hamano wrote:\n> Andy Koppe <andy.koppe@gmail.com> writes:\n> >> add_decorations_to_list() skips the branch that HEAD points to, because\n>> the rebase updates it anyway, as per aa37f3e1d89 (sequencer: ignore HEAD\n>> ref under --update-refs, 2022-07-19).\n>>\n>> However, it keeps the name of that branch in the static buffer returned\n>> by refs_resolve_ref_unsafe(). If another branch on the same commit is\n>> checked out in a different worktree, branch_checked_out() overwrites\n>> that buffer. If the current branch comes after the other branch in the\n>> list of decorations, it gets a \"checked out\" comment in the todo list\n>> instead of being skipped, and the branch whose name ended up in the\n>> buffer is skipped in its place.\n> > So obviously broken original, so cleanly corrected.  I very much\n> like it.\n\nYes, this is nicely explained and I appreciate the new test\n\nThanks\n\nPhillip\n\n> Will queue.  thanks.\n> >>\n>> Decorations are processed in reverse alphabetical order, so this depends\n>> on the branch names. The test added in 94304b9f48e (sequencer: comment\n>> checked-out branch properly, 2024-11-25) happens to trigger the problem\n>> and expects its result.\n>>\n>> Fix it by using refs_resolve_refdup(), correct that test's expectation,\n>> and add a test with checked-out branches sorting both before and after\n>> the current branch.\n>>\n>> Signed-off-by: Andy Koppe <andy.koppe@gmail.com>\n>> ---\n>>   sequencer.c       | 11 ++++++-----\n>>   t/t3400-rebase.sh | 21 ++++++++++++++++++++-\n>>   2 files changed, 26 insertions(+), 6 deletions(-)\n>>\n>> diff --git a/sequencer.c b/sequencer.c\n>> index 6dae43e4db..c672b6e127 100644\n>> --- a/sequencer.c\n>> +++ b/sequencer.c\n>> @@ -6577,11 +6577,11 @@ 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>> +\tchar *head_ref = refs_resolve_refdup(get_main_ref_store(the_repository),\n>> +\t\t\t\t\t     \"HEAD\",\n>> +\t\t\t\t\t     RESOLVE_REF_READING,\n>> +\t\t\t\t\t     NULL,\n>> +\t\t\t\t\t     NULL);\n>>   >>   \twhile (decoration) {\n>>   \t\tstruct todo_item *item;\n>> @@ -6633,6 +6633,7 @@ static int add_decorations_to_list(const struct commit *commit,\n>>   \t\tdecoration = decoration->next;\n>>   \t}\n>>   >> +\tfree(head_ref);\n>>   \treturn 0;\n>>   }\n>>   >> diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh\n>> index accaaba4d4..0e7ef33eb1 100755\n>> --- a/t/t3400-rebase.sh\n>> +++ b/t/t3400-rebase.sh\n>> @@ -483,7 +483,26 @@ 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\" actual\n>> +'\n>> +\n>> +test_expect_success 'git rebase --update-refs ignores current branch with branches on worktrees' '\n>> +\ttest_when_finished git branch -D base current a-topic z-topic &&\n>> +\ttest_when_finished git checkout main &&\n>> +\ttest_when_finished git worktree remove z-topic &&\n>> +\ttest_when_finished git worktree remove a-topic &&\n>> +\tgit checkout -b base main &&\n>> +\tgit checkout -b current &&\n>> +\ttest_commit msg4 &&\n>> +\tgit worktree add a-topic &&\n>> +\tgit worktree add z-topic &&\n>> +\tgit checkout base &&\n>> +\ttest_commit msg5 &&\n>> +\tgit checkout current &&\n>> +\tGIT_SEQUENCE_EDITOR=\"cat >actual\" git rebase -i --update-refs base &&\n>> +\ttest_grep \"# Ref refs/heads/a-topic checked out at\" actual &&\n>> +\ttest_grep \"# Ref refs/heads/z-topic checked out at\" actual &&\n>> +\ttest_grep ! \"refs/heads/current\" actual\n>>   '\n>>   >>   test_done\n\n\n"}]}