Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 52 additions & 11 deletions sequencer.c
Original file line number Diff line number Diff line change
Expand Up @@ -6445,28 +6445,67 @@ static int add_decorations_to_list(const struct commit *commit,
struct todo_add_branch_context *ctx)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Kristoffer Haugsbakk" wrote on the Git mailing list (how to reply to this email):

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

This style of fixing a bug by:

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

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

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

> --
> gitgitgadget

{
const struct name_decoration *decoration = get_name_decoration(&commit->object);
const char *head_ref = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
"HEAD",
struct ref_store *refs = get_main_ref_store(the_repository);
const char *head_ref = refs_resolve_ref_unsafe(refs, "HEAD",
RESOLVE_REF_READING,
NULL,
NULL);
NULL, NULL);
char *resolved_head_ref = refs_resolve_refdup(refs, "HEAD",
RESOLVE_REF_READING,
NULL, NULL);
struct strbuf update_ref = STRBUF_INIT;

while (decoration) {
struct todo_item *item;
const char *path;
const char *ref = decoration->name;
const char *resolved_ref;
int is_symref = 0;
int flags = 0;
size_t base_offset = ctx->buf->len;

/*
* 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.
*
* Resolve branch symrefs after checking for the current HEAD so
* that aliases do not schedule duplicate updates for their
* referents.
*
* Non-branch decorations may be present if the pretty format
* includes "%d", which would have loaded all refs
* into the global decoration table.
*/
if ((head_ref && !strcmp(head_ref, decoration->name)) ||
(decoration->type != DECORATION_REF_LOCAL)) {
if (decoration->type != DECORATION_REF_LOCAL) {
decoration = decoration->next;
continue;
}

if (head_ref && !strcmp(head_ref, ref)) {
decoration = decoration->next;
continue;
}

strbuf_reset(&update_ref);
resolved_ref = refs_resolve_ref_unsafe(refs, ref,
RESOLVE_REF_READING |
RESOLVE_REF_NO_RECURSE,
NULL, &flags);
if ((flags & REF_ISSYMREF) && resolved_ref) {
if (!starts_with(resolved_ref, "refs/heads/")) {
decoration = decoration->next;
continue;
}

strbuf_addstr(&update_ref, resolved_ref);
ref = update_ref.buf;
is_symref = 1;
}

if ((is_symref && resolved_head_ref &&
!strcmp(resolved_head_ref, ref)) ||
string_list_has_string(&ctx->refs_to_oids, ref)) {
decoration = decoration->next;
continue;
}
Expand All @@ -6478,19 +6517,19 @@ static int add_decorations_to_list(const struct commit *commit,
memset(item, 0, sizeof(*item));

/* If the branch is checked out, then leave a comment instead. */
if ((path = branch_checked_out(decoration->name))) {
if ((path = branch_checked_out(ref))) {
item->command = TODO_COMMENT;
strbuf_commented_addf(ctx->buf, comment_line_str,
"Ref %s checked out at '%s'\n",
decoration->name, path);
ref, path);
} else {
struct string_list_item *sti;
item->command = TODO_UPDATE_REF;
strbuf_addf(ctx->buf, "%s\n", decoration->name);
strbuf_addf(ctx->buf, "%s\n", ref);

sti = string_list_insert(&ctx->refs_to_oids,
decoration->name);
sti->util = init_update_ref_record(decoration->name);
ref);
sti->util = init_update_ref_record(ref);
}

item->offset_in_buf = base_offset;
Expand All @@ -6501,6 +6540,8 @@ static int add_decorations_to_list(const struct commit *commit,
decoration = decoration->next;
}

strbuf_release(&update_ref);
free(resolved_head_ref);
return 0;
}

Expand Down
25 changes: 25 additions & 0 deletions t/t3404-rebase-interactive.sh
Original file line number Diff line number Diff line change
Expand Up @@ -1978,6 +1978,31 @@ test_expect_success '--update-refs ignores non-branch decorations' '
test_cmp expect actual
'

test_expect_success '--update-refs skips branch symrefs to current branch' '
test_when_finished "
test_might_fail git rebase --abort &&
git checkout primary &&
test_might_fail git symbolic-ref -d refs/heads/update-refs-symref-alias &&
test_might_fail git branch -D update-refs-symref update-refs-symref-base
" &&
git checkout -B update-refs-symref-base primary &&
test_commit --no-tag update-refs-symref-base symref-base.t &&
git checkout -B update-refs-symref &&
test_commit --no-tag update-refs-symref-topic symref-topic.t &&
git checkout update-refs-symref-base &&
test_commit --no-tag update-refs-symref-newbase symref-newbase.t &&
git checkout update-refs-symref &&
git symbolic-ref refs/heads/update-refs-symref-alias refs/heads/update-refs-symref &&

git rebase --update-refs update-refs-symref-base 2>err &&

test_cmp_rev update-refs-symref-base update-refs-symref^ &&
test_cmp_rev refs/heads/update-refs-symref refs/heads/update-refs-symref-alias &&
test_write_lines refs/heads/update-refs-symref >expect &&
git symbolic-ref refs/heads/update-refs-symref-alias >actual &&
test_cmp expect actual
'

test_expect_success '--update-refs updates refs correctly' '
git checkout -B update-refs no-conflict-branch &&
git branch -f base HEAD~4 &&
Expand Down
Loading