git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Son Luong Ngoc <sluongng@gmail.com>
Date
Jul 22, 2026, 08:16 UTC
Message-ID
<CAL3xRKdoGpNOdbY7_dt3oSfn6728gf8uA4qa79Kcp1UuS=jpiQ@mail.gmail.com>
In-Reply-To
<f982c386-e329-4ab0-b695-e540bcb9de3d@gmail.com>
On 04/06/2026 16:46, Phillip Wood wrote:
Show 11 quoted lines
> A symref that points to another branch should always be skipped. When we
> look up which branches are checked out (see worktree.c:add_head_info()) we
> use
>
> 	refs_resolve_ref_unsafe(get_worktree_ref_store(wt),
> 				 "HEAD",
> 				 0,
> 				 &wt->head_oid, &flags);
>
> so it will never report a symref as being checked out - it always resolves
> any symrefs first.

Yes, this is the right distinction. Patch 1 now resolves each local branch decoration before deciding whether to queue it. If the target is under refs/heads/, the alias is skipped unconditionally and the concrete branch decoration remains the only update that is queued.

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

Patch 2 handles that case separately. It checks both the literal alias and its resolved target ref against the checked-out reservations. I also added a test with a non-branch target checked out in another worktree.

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

Agreed. Patch 1 now skips the current branch before checked-out handling, as the old code did.

The contrary expectation in t3400 came from head_ref pointing into a buffer that was reused while resolving another decoration. That could make the current-branch comparison fail. head_ref is now an owned copy. The test expects the current branch to be omitted from the todo list.

While adding the non-branch coverage, I found that two aliases to the same target ref could queue the same update twice and make the second compare-and-swap fail. Patch 2 deduplicates those updates by target ref. It also records resolved target refs from other worktrees' in-progress update-refs state so that a different alias honors the same reservation.

I split the reroll into these two patches so that the branch-alias fix and the non-branch safeguards can be reviewed independently.

Thanks for the review, and sorry for the slow response.

Thanks, Son

On Thu, 4 Jun 2026 16:37:39 +0100, Phillip Wood <phillip.wood123@gmail.com> wrote:

Show 101 quoted lines
> On 03/06/2026 11:27, Son Luong Ngoc via GitGitGadget wrote:
> > From: Son Luong Ngoc <sluongng@gmail.com>
> >
> > git rebase --update-refs can fail after the normal rebase path has
> > updated the current branch when another local branch is a symref to it.
> > This can happen during a default-branch rename where refs/heads/main
> > points at refs/heads/master while users migrate.
> >
> > The sequencer queues update-ref commands from local branch decorations.
> > Commit 106b6885c7 (rebase: ignore non-branch update-refs) filters out
> > decorations that are not local branches, such as HEAD and tags. A branch
> > symref is different: it is still a local branch decoration, but if it
> > resolves to another branch then that target branch is itself present in
> > the decoration list and will be updated as a concrete branch.
> >
> > Skip branch decorations whose symrefs resolve to refs/heads/*, because
> > those targets are already represented by concrete branch decorations.
> > This prevents aliases from scheduling a second update for the same
> > branch. Keep symrefs to non-branch targets on the existing path.
>
> Makes sense
>
> > Preserve the existing checked-out branch handling before applying these
> > skips. Such refs still need a todo-list comment instead of an update-ref
> > command, even when the checked-out ref is the branch being rebased or a
> > branch symref alias. Use a copy of the resolved HEAD ref so later ref
> > resolution does not overwrite it.
>
> I don't quite understand this. A symref that points to another branch
> should always be skipped. When we look up which branches are checked out
> (see worktree.c:add_head_info()) we use
>
> refs_resolve_ref_unsafe(get_worktree_ref_store(wt),
> "HEAD",
> 0,
> &wt->head_oid, &flags);
>
> so it will never report a symref as being checked out - it always
> resolves any symrefs first.
>
> If we have a symref pointing somewhere outside of "refs/heads" then we
> need to check whether the target is checked out, not the symref itself.
> I'm not sure how likely that is to happen in practice.
>
> > diff --git a/sequencer.c b/sequencer.c
> > index 1ee4b2875b..6ab8b47108 100644
> > --- a/sequencer.c
> > +++ b/sequencer.c
> > @@ -6445,28 +6445,46 @@ static int add_decorations_to_list(const struct commit *commit,
> > struct todo_add_branch_context *ctx)
> > {
> > const struct name_decoration *decoration = get_name_decoration(&commit->object);
> > - const char *head_ref = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
> > - "HEAD",
> > - RESOLVE_REF_READING,
> > - NULL,
> > - NULL);
> > + struct ref_store *refs = get_main_ref_store(the_repository);
> > + char *head_ref = refs_resolve_refdup(refs, "HEAD",
> > + RESOLVE_REF_READING,
> > + NULL, NULL);
>
> This part and the test look good now
> > while (decoration) {
> > struct todo_item *item;
> > const char *path;
> > + const char *resolved_ref;
> > + int flags = 0;
> > size_t base_offset = ctx->buf->len;
> >
> > /*
> > - * If the branch is the current HEAD, then it will be
> > - * updated by the default rebase behavior.
> > - * Exclude it from the list of refs to update,
> > - * as well as any non-branch decorations.
> > * Non-branch decorations may be present if the pretty format
> > * includes "%d", which would have loaded all refs
> > * into the global decoration table.
> > */
> > - if ((head_ref && !strcmp(head_ref, decoration->name)) ||
> > - (decoration->type != DECORATION_REF_LOCAL)) {
> > + if (decoration->type != DECORATION_REF_LOCAL) {
> > + decoration = decoration->next;
> > + continue;
> > + }
>
> If a decoration matches the current branch why don't we just skip it
> like we used to? (As an aside the existing code in wrong because if the
> user runs "git rebase --update-refs <upstream> <branch>" HEAD does not
> point to "<branch>" but lets not worry about that now)
>
> > + path = branch_checked_out(decoration->name);
>
> As I said above if the symref target is anther branch we should skip it
> and if the target is not a branch then we need to check if the target is
> checked out so we need to resolve the ref before calling
> branch_checked_out().
>
> Thanks
>
> Phillip
Previous: Phillip WoodNext: Son Luong Ngoc via GitGitGadget
Message 10 of 25 in “rebase: handle --update-refs branch symrefs”
  1. 0/2 rebase: handle --update-refs branch symrefsSon Luong Ngoc via GitGitGadget, May 28, 2026
  2. 1/2 t3404: add failing branch symref testSon Luong Ngoc via GitGitGadget, May 28, 2026
  3. Phillip WoodJun 1, 2026
  4. 2/2 rebase: skip branch symref aliasesSon Luong Ngoc via GitGitGadget, May 28, 2026
  5. Kristoffer HaugsbakkMay 28, 2026
  6. Phillip WoodJun 1, 2026
  7. Junio C HamanoMay 28, 2026
  8. rebase: skip branch symref aliasesSon Luong Ngoc via GitGitGadget, Jun 3, 2026
  9. Phillip WoodJun 4, 2026
  10. Son Luong NgocJul 22, 2026
  11. 0/2 rebase: handle --update-refs branch symrefsSon Luong Ngoc via GitGitGadget, Jul 22, 2026
  12. 1/2 rebase: skip branch symref aliasesSon Luong Ngoc via GitGitGadget, Jul 22, 2026
  13. Phillip WoodJul 23, 2026
  14. Phillip WoodJul 24, 2026
  15. Erik Cervin-EdinJul 25, 2026
  16. Junio C HamanoJul 26, 2026
  17. Phillip WoodJul 28, 2026
  18. Junio C HamanoJul 28, 2026
  19. Phillip WoodJul 29, 2026
  20. Junio C HamanoJul 29, 2026
  21. Phillip WoodJul 30, 2026
  22. Junio C HamanoAug 6, 2026
  23. Phillip WoodAug 7, 2026
  24. 2/2 rebase: guard non-branch symref targetsSon Luong Ngoc via GitGitGadget, Jul 22, 2026
  25. Phillip WoodAug 7, 2026

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.