Skip to content

Commit e89289c

Browse files
sluongngsluongng
committed
rebase: skip branch symref aliases
git rebase --update-refs can finish rewriting the current branch and then fail while updating a local branch that is a symbolic ref. This can happen during a default-branch rename where refs/heads/main points at refs/heads/master while users migrate. Fix this because the post-rebase failure leaves refs partially updated even though the main rebase has succeeded. The sequencer queues updates from local branch decorations. Commit 106b688 (rebase: ignore non-branch update-refs) filters out decorations that are not local branches, such as HEAD and tags. A branch symref is still a local branch decoration, but refs_update_ref() dereferences it. A symref to another branch therefore duplicates the concrete branch update. Multiple aliases to the same non-branch ref can likewise queue duplicate updates, causing the second compare-and-swap to fail. Resolve each local branch decoration before queuing it. Skip symrefs whose referents are under refs/heads/, since the concrete branch decoration already represents that update. Keep an owned copy of the resolved HEAD and skip the current branch before checked-out handling so later ref resolution cannot change the comparison. For symrefs to non-branch targets, check both the literal alias and its resolved referent against checked-out reservations. Deduplicate queued updates by referent so multiple aliases cannot update the same ref twice. When loading another worktree's update-refs state, reserve both the literal symref and its resolved target so different aliases honor the same in-progress update. Cover branch aliases, current-branch handling, checked-out non-branch targets, duplicate non-branch aliases, and cross-worktree reservations. Signed-off-by: Son Luong Ngoc <sluongng@gmail.com>
1 parent 48bbf81 commit e89289c

4 files changed

Lines changed: 155 additions & 13 deletions

File tree

‎branch.c‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -442,10 +442,25 @@ static void prepare_checked_out_branches(void)
442442
&update_refs)) {
443443
struct string_list_item *item;
444444
for_each_string_list_item(item, &update_refs) {
445+
char *resolved_ref;
446+
int flags = 0;
447+
445448
old = strmap_put(&current_checked_out_branches,
446449
item->string,
447450
xstrdup(wt->path));
448451
free(old);
452+
453+
resolved_ref = refs_resolve_refdup(
454+
get_main_ref_store(the_repository),
455+
item->string, RESOLVE_REF_READING,
456+
NULL, &flags);
457+
if (resolved_ref && (flags & REF_ISSYMREF)) {
458+
old = strmap_put(
459+
&current_checked_out_branches,
460+
resolved_ref, xstrdup(wt->path));
461+
free(old);
462+
}
463+
free(resolved_ref);
449464
}
450465
string_list_clear(&update_refs, 1);
451466
}

‎sequencer.c‎

Lines changed: 51 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6459,34 +6459,65 @@ struct todo_add_branch_context {
64596459
size_t items_alloc;
64606460
struct strbuf *buf;
64616461
struct string_list refs_to_oids;
6462+
struct string_list symref_update_targets;
64626463
};
64636464

64646465
static int add_decorations_to_list(const struct commit *commit,
64656466
struct todo_add_branch_context *ctx)
64666467
{
64676468
const struct name_decoration *decoration = get_name_decoration(&commit->object);
6468-
const char *head_ref = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
6469-
"HEAD",
6470-
RESOLVE_REF_READING,
6471-
NULL,
6472-
NULL);
6469+
struct ref_store *refs = get_main_ref_store(the_repository);
6470+
char *head_ref = refs_resolve_refdup(refs, "HEAD",
6471+
RESOLVE_REF_READING,
6472+
NULL, NULL);
64736473

64746474
while (decoration) {
64756475
struct todo_item *item;
64766476
const char *path;
6477+
const char *checked_ref;
6478+
char *resolved_ref;
6479+
int flags = 0;
64776480
size_t base_offset = ctx->buf->len;
64786481

64796482
/*
6480-
* If the branch is the current HEAD, then it will be
6481-
* updated by the default rebase behavior.
6482-
* Exclude it from the list of refs to update,
6483-
* as well as any non-branch decorations.
64846483
* Non-branch decorations may be present if the pretty format
64856484
* includes "%d", which would have loaded all refs
64866485
* into the global decoration table.
64876486
*/
6488-
if ((head_ref && !strcmp(head_ref, decoration->name)) ||
6489-
(decoration->type != DECORATION_REF_LOCAL)) {
6487+
if (decoration->type != DECORATION_REF_LOCAL) {
6488+
decoration = decoration->next;
6489+
continue;
6490+
}
6491+
6492+
resolved_ref = refs_resolve_refdup(refs, decoration->name,
6493+
RESOLVE_REF_READING,
6494+
NULL, &flags);
6495+
if (resolved_ref && (flags & REF_ISSYMREF) &&
6496+
starts_with(resolved_ref, "refs/heads/")) {
6497+
free(resolved_ref);
6498+
decoration = decoration->next;
6499+
continue;
6500+
}
6501+
6502+
/*
6503+
* If the branch is the current HEAD, then it will be
6504+
* updated by the default rebase behavior.
6505+
*/
6506+
if (head_ref && !strcmp(head_ref, decoration->name)) {
6507+
free(resolved_ref);
6508+
decoration = decoration->next;
6509+
continue;
6510+
}
6511+
6512+
path = branch_checked_out(decoration->name);
6513+
if (!path && resolved_ref && (flags & REF_ISSYMREF)) {
6514+
checked_ref = resolved_ref;
6515+
path = branch_checked_out(checked_ref);
6516+
}
6517+
if (!path && resolved_ref && (flags & REF_ISSYMREF) &&
6518+
string_list_has_string(&ctx->symref_update_targets,
6519+
resolved_ref)) {
6520+
free(resolved_ref);
64906521
decoration = decoration->next;
64916522
continue;
64926523
}
@@ -6498,13 +6529,17 @@ static int add_decorations_to_list(const struct commit *commit,
64986529
memset(item, 0, sizeof(*item));
64996530

65006531
/* If the branch is checked out, then leave a comment instead. */
6501-
if ((path = branch_checked_out(decoration->name))) {
6532+
if (path) {
65026533
item->command = TODO_COMMENT;
65036534
strbuf_commented_addf(ctx->buf, comment_line_str,
65046535
"Ref %s checked out at '%s'\n",
65056536
decoration->name, path);
65066537
} else {
65076538
struct string_list_item *sti;
6539+
6540+
if (resolved_ref && (flags & REF_ISSYMREF))
6541+
string_list_insert(&ctx->symref_update_targets,
6542+
resolved_ref);
65086543
item->command = TODO_UPDATE_REF;
65096544
strbuf_addf(ctx->buf, "%s\n", decoration->name);
65106545

@@ -6518,9 +6553,11 @@ static int add_decorations_to_list(const struct commit *commit,
65186553
item->arg_len = ctx->buf->len - base_offset;
65196554
ctx->items_nr++;
65206555

6556+
free(resolved_ref);
65216557
decoration = decoration->next;
65226558
}
65236559

6560+
free(head_ref);
65246561
return 0;
65256562
}
65266563

@@ -6534,6 +6571,7 @@ static int todo_list_add_update_ref_commands(struct todo_list *todo_list)
65346571
struct todo_add_branch_context ctx = {
65356572
.buf = &todo_list->buf,
65366573
.refs_to_oids = STRING_LIST_INIT_DUP,
6574+
.symref_update_targets = STRING_LIST_INIT_DUP,
65376575
};
65386576

65396577
ctx.items_alloc = 2 * todo_list->nr + 1;
@@ -6559,6 +6597,7 @@ static int todo_list_add_update_ref_commands(struct todo_list *todo_list)
65596597
res = write_update_refs_state(&ctx.refs_to_oids);
65606598

65616599
string_list_clear(&ctx.refs_to_oids, 1);
6600+
string_list_clear(&ctx.symref_update_targets, 0);
65626601

65636602
if (res) {
65646603
/* we failed, so clean up the new list. */

‎t/t3400-rebase.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -471,7 +471,7 @@ test_expect_success 'git rebase --update-ref with core.commentChar and branch on
471471
GIT_SEQUENCE_EDITOR="cat >actual" git -c core.commentChar=% \
472472
rebase -i --update-refs base &&
473473
test_grep "% Ref refs/heads/wt-topic checked out at" actual &&
474-
test_grep "% Ref refs/heads/topic2 checked out at" actual
474+
test_grep ! "% Ref refs/heads/topic2 checked out at" actual
475475
'
476476

477477
test_done

‎t/t3404-rebase-interactive.sh‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1975,24 +1975,40 @@ test_expect_success '--update-refs ignores non-branch decorations' '
19751975
) &&
19761976
grep ^update-ref todo >actual &&
19771977
test_write_lines "update-ref refs/heads/no-conflict-branch" >expect &&
1978+
test_grep ! "^# Ref refs/heads/update-refs checked out" todo &&
19781979
test_cmp expect actual
19791980
'
19801981

19811982
test_expect_success '--update-refs updates refs correctly' '
1983+
test_when_finished "
1984+
test_might_fail git symbolic-ref -d refs/heads/no-conflict-branch-alias &&
1985+
test_might_fail git symbolic-ref -d refs/heads/second-alias
1986+
" &&
19821987
git checkout -B update-refs no-conflict-branch &&
19831988
git branch -f base HEAD~4 &&
19841989
git branch -f first HEAD~3 &&
19851990
git branch -f second HEAD~3 &&
19861991
git branch -f third HEAD~1 &&
1992+
git symbolic-ref refs/heads/no-conflict-branch-alias \
1993+
refs/heads/no-conflict-branch &&
1994+
git symbolic-ref refs/heads/second-alias refs/heads/second &&
19871995
test_commit extra2 fileX &&
19881996
git commit --amend --fixup=L &&
19891997
19901998
git rebase -i --autosquash --update-refs primary 2>err &&
19911999
19922000
test_cmp_rev HEAD~3 refs/heads/first &&
19932001
test_cmp_rev HEAD~3 refs/heads/second &&
2002+
test_cmp_rev HEAD~3 refs/heads/second-alias &&
19942003
test_cmp_rev HEAD~1 refs/heads/third &&
19952004
test_cmp_rev HEAD refs/heads/no-conflict-branch &&
2005+
test_cmp_rev HEAD refs/heads/no-conflict-branch-alias &&
2006+
test_write_lines refs/heads/no-conflict-branch >expect &&
2007+
git symbolic-ref refs/heads/no-conflict-branch-alias >actual &&
2008+
test_cmp expect actual &&
2009+
test_write_lines refs/heads/second >expect &&
2010+
git symbolic-ref refs/heads/second-alias >actual &&
2011+
test_cmp expect actual &&
19962012
19972013
q_to_tab >expect <<-\EOF &&
19982014
Successfully rebased and updated refs/heads/update-refs.
@@ -2008,6 +2024,78 @@ test_expect_success '--update-refs updates refs correctly' '
20082024
test_cmp expect err.trimmed
20092025
'
20102026

2027+
test_expect_success '--update-refs checks resolved non-branch symref target' '
2028+
test_when_finished "
2029+
git worktree remove --force checked-out-target-wt &&
2030+
git symbolic-ref -d refs/heads/non-branch-alias &&
2031+
git tag -d checked-out-target
2032+
" &&
2033+
git tag checked-out-target HEAD~1 &&
2034+
git symbolic-ref refs/heads/non-branch-alias refs/tags/checked-out-target &&
2035+
git worktree add --detach checked-out-target-wt checked-out-target &&
2036+
git -C checked-out-target-wt symbolic-ref HEAD refs/tags/checked-out-target &&
2037+
2038+
GIT_SEQUENCE_EDITOR="cat >todo" git rebase -i --update-refs HEAD~2 &&
2039+
2040+
test_grep "^# Ref refs/heads/non-branch-alias checked out at" todo &&
2041+
test_write_lines refs/tags/checked-out-target >expect &&
2042+
git symbolic-ref refs/heads/non-branch-alias >actual &&
2043+
test_cmp expect actual
2044+
'
2045+
2046+
test_expect_success '--update-refs deduplicates non-branch symref targets' '
2047+
test_when_finished "
2048+
git symbolic-ref -d refs/heads/non-branch-alias-one &&
2049+
git symbolic-ref -d refs/heads/non-branch-alias-two &&
2050+
git tag -d shared-non-branch-target
2051+
" &&
2052+
git tag shared-non-branch-target HEAD~1 &&
2053+
git symbolic-ref refs/heads/non-branch-alias-one \
2054+
refs/tags/shared-non-branch-target &&
2055+
git symbolic-ref refs/heads/non-branch-alias-two \
2056+
refs/tags/shared-non-branch-target &&
2057+
2058+
GIT_SEQUENCE_EDITOR=: git rebase -i --force-rebase --update-refs HEAD~2 &&
2059+
2060+
test_cmp_rev HEAD~1 refs/heads/non-branch-alias-one &&
2061+
test_cmp_rev HEAD~1 refs/heads/non-branch-alias-two &&
2062+
test_write_lines refs/tags/shared-non-branch-target >expect &&
2063+
git symbolic-ref refs/heads/non-branch-alias-one >actual &&
2064+
test_cmp expect actual &&
2065+
git symbolic-ref refs/heads/non-branch-alias-two >actual &&
2066+
test_cmp expect actual
2067+
'
2068+
2069+
test_expect_success '--update-refs honors non-branch symref reservations' '
2070+
test_when_finished "
2071+
test_might_fail git worktree remove --force reserved-target-wt &&
2072+
test_might_fail git symbolic-ref -d \
2073+
refs/heads/reserved-non-branch-alias-one &&
2074+
test_might_fail git symbolic-ref -d \
2075+
refs/heads/reserved-non-branch-alias-two &&
2076+
test_might_fail git tag -d reserved-non-branch-target
2077+
" &&
2078+
git tag reserved-non-branch-target HEAD~1 &&
2079+
git symbolic-ref refs/heads/reserved-non-branch-alias-one \
2080+
refs/tags/reserved-non-branch-target &&
2081+
git symbolic-ref refs/heads/reserved-non-branch-alias-two \
2082+
refs/tags/reserved-non-branch-target &&
2083+
git worktree add --detach reserved-target-wt HEAD &&
2084+
wt_gitdir=$(git -C reserved-target-wt rev-parse --absolute-git-dir) &&
2085+
mkdir -p "$wt_gitdir/rebase-merge" &&
2086+
old_oid=$(git rev-parse refs/heads/reserved-non-branch-alias-one) &&
2087+
test_write_lines refs/heads/reserved-non-branch-alias-one \
2088+
"$old_oid" "$old_oid" >"$wt_gitdir/rebase-merge/update-refs" &&
2089+
2090+
GIT_SEQUENCE_EDITOR="cat >todo" git rebase -i --update-refs HEAD~2 &&
2091+
2092+
test_grep "^# Ref refs/heads/reserved-non-branch-alias-one checked out at" \
2093+
todo &&
2094+
test_grep "^# Ref refs/heads/reserved-non-branch-alias-two checked out at" \
2095+
todo &&
2096+
test_grep ! "^update-ref refs/heads/reserved-non-branch-alias" todo
2097+
'
2098+
20112099
test_expect_success 'respect user edits to update-ref steps' '
20122100
git checkout -B update-refs-break no-conflict-branch &&
20132101
git branch -f base HEAD~4 &&

0 commit comments

Comments
 (0)