rebase: guard non-branch symref targets

A local branch symbolic ref may point outside refs/heads/. Such an alias
cannot be skipped like a branch-to-branch alias because its concrete
target ref is absent from the local branch decoration list.

However, queuing each alias independently can update the same target ref
more than once and make the second compare-and-swap fail. A reservation
from another worktree can also name either an alias or its resolved
target ref, so checking only one form can miss an in-progress update.

Fix these cases by checking both the literal alias and its resolved
target ref against checked-out reservations. Deduplicate updates by
target ref. Also reserve both forms when loading another worktree's
update-refs state. This makes different aliases honor the same
in-progress update.

This keeps non-branch symrefs supported without allowing duplicate or
cross-worktree ref updates.

Signed-off-by: Son Luong Ngoc <sluongng@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/branch.c b/branch.c
index 243db7d0fc02266b9ec79cd8cb90f620c0ce1970..98a50d836802017b5141a3e207248d690cc0adc4 100644 (file)
--- a/branch.c
+++ b/branch.c
@@ -442,10 +442,25 @@ static void prepare_checked_out_branches(void)
                                                     &update_refs)) {
                        struct string_list_item *item;
                        for_each_string_list_item(item, &update_refs) {
+                               char *resolved_ref;
+                               int flags = 0;
+
                                old = strmap_put(&current_checked_out_branches,
                                                 item->string,
                                                 xstrdup(wt->path));
                                free(old);
+
+                               resolved_ref = refs_resolve_refdup(
+                                       get_main_ref_store(the_repository),
+                                       item->string, RESOLVE_REF_READING,
+                                       NULL, &flags);
+                               if (resolved_ref && (flags & REF_ISSYMREF)) {
+                                       old = strmap_put(
+                                               &current_checked_out_branches,
+                                               resolved_ref, xstrdup(wt->path));
+                                       free(old);
+                               }
+                               free(resolved_ref);
                        }
                        string_list_clear(&update_refs, 1);
                }

diff --git a/sequencer.c b/sequencer.c
index 17f5baab62ff70de4a58bb7d5193007579023dc9..73c7d7f0c7860c43d42fb1b6432b71848e80798a 100644 (file)
--- a/sequencer.c
+++ b/sequencer.c
@@ -6439,6 +6439,7 @@ struct todo_add_branch_context {
        size_t items_alloc;
        struct strbuf *buf;
        struct string_list refs_to_oids;
+       struct string_list symref_update_targets;
 };
 
 static int add_decorations_to_list(const struct commit *commit,
@@ -6453,6 +6454,7 @@ static int add_decorations_to_list(const struct commit *commit,
        while (decoration) {
                struct todo_item *item;
                const char *path;
+               const char *checked_ref;
                char *resolved_ref;
                int flags = 0;
                size_t base_offset = ctx->buf->len;
@@ -6488,6 +6490,17 @@ static int add_decorations_to_list(const struct commit *commit,
                }
 
                path = branch_checked_out(decoration->name);
+               if (!path && resolved_ref && (flags & REF_ISSYMREF)) {
+                       checked_ref = resolved_ref;
+                       path = branch_checked_out(checked_ref);
+               }
+               if (!path && resolved_ref && (flags & REF_ISSYMREF) &&
+                   string_list_has_string(&ctx->symref_update_targets,
+                                          resolved_ref)) {
+                       free(resolved_ref);
+                       decoration = decoration->next;
+                       continue;
+               }
 
                ALLOC_GROW(ctx->items,
                        ctx->items_nr + 1,
@@ -6503,6 +6516,10 @@ static int add_decorations_to_list(const struct commit *commit,
                                              decoration->name, path);
                } else {
                        struct string_list_item *sti;
+
+                       if (resolved_ref && (flags & REF_ISSYMREF))
+                               string_list_insert(&ctx->symref_update_targets,
+                                                  resolved_ref);
                        item->command = TODO_UPDATE_REF;
                        strbuf_addf(ctx->buf, "%s\n", decoration->name);
 
@@ -6534,6 +6551,7 @@ static int todo_list_add_update_ref_commands(struct todo_list *todo_list)
        struct todo_add_branch_context ctx = {
                .buf = &todo_list->buf,
                .refs_to_oids = STRING_LIST_INIT_DUP,
+               .symref_update_targets = STRING_LIST_INIT_DUP,
        };
 
        ctx.items_alloc = 2 * todo_list->nr + 1;
@@ -6559,6 +6577,7 @@ static int todo_list_add_update_ref_commands(struct todo_list *todo_list)
        res = write_update_refs_state(&ctx.refs_to_oids);
 
        string_list_clear(&ctx.refs_to_oids, 1);
+       string_list_clear(&ctx.symref_update_targets, 0);
 
        if (res) {
                /* we failed, so clean up the new list. */

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index b05d93884668b543e9115bc676959a536dbd9f95..54981c909bb9b65f6b7ded2ff33550d2207e98e2 100755 (executable)
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -2024,6 +2024,78 @@ test_expect_success '--update-refs updates refs correctly' '
        test_cmp expect err.trimmed
 '
 
+test_expect_success '--update-refs checks resolved non-branch symref target' '
+       test_when_finished "
+               git worktree remove --force checked-out-target-wt &&
+               git symbolic-ref -d refs/heads/non-branch-alias &&
+               git tag -d checked-out-target
+       " &&
+       git tag checked-out-target HEAD~1 &&
+       git symbolic-ref refs/heads/non-branch-alias refs/tags/checked-out-target &&
+       git worktree add --detach checked-out-target-wt checked-out-target &&
+       git -C checked-out-target-wt symbolic-ref HEAD refs/tags/checked-out-target &&
+
+       GIT_SEQUENCE_EDITOR="cat >todo" git rebase -i --update-refs HEAD~2 &&
+
+       test_grep "^# Ref refs/heads/non-branch-alias checked out at" todo &&
+       test_write_lines refs/tags/checked-out-target >expect &&
+       git symbolic-ref refs/heads/non-branch-alias >actual &&
+       test_cmp expect actual
+'
+
+test_expect_success '--update-refs deduplicates non-branch symref targets' '
+       test_when_finished "
+               git symbolic-ref -d refs/heads/non-branch-alias-one &&
+               git symbolic-ref -d refs/heads/non-branch-alias-two &&
+               git tag -d shared-non-branch-target
+       " &&
+       git tag shared-non-branch-target HEAD~1 &&
+       git symbolic-ref refs/heads/non-branch-alias-one \
+               refs/tags/shared-non-branch-target &&
+       git symbolic-ref refs/heads/non-branch-alias-two \
+               refs/tags/shared-non-branch-target &&
+
+       GIT_SEQUENCE_EDITOR=: git rebase -i --force-rebase --update-refs HEAD~2 &&
+
+       test_cmp_rev HEAD~1 refs/heads/non-branch-alias-one &&
+       test_cmp_rev HEAD~1 refs/heads/non-branch-alias-two &&
+       test_write_lines refs/tags/shared-non-branch-target >expect &&
+       git symbolic-ref refs/heads/non-branch-alias-one >actual &&
+       test_cmp expect actual &&
+       git symbolic-ref refs/heads/non-branch-alias-two >actual &&
+       test_cmp expect actual
+'
+
+test_expect_success '--update-refs honors non-branch symref reservations' '
+       test_when_finished "
+               test_might_fail git worktree remove --force reserved-target-wt &&
+               test_might_fail git symbolic-ref -d \
+                       refs/heads/reserved-non-branch-alias-one &&
+               test_might_fail git symbolic-ref -d \
+                       refs/heads/reserved-non-branch-alias-two &&
+               test_might_fail git tag -d reserved-non-branch-target
+       " &&
+       git tag reserved-non-branch-target HEAD~1 &&
+       git symbolic-ref refs/heads/reserved-non-branch-alias-one \
+               refs/tags/reserved-non-branch-target &&
+       git symbolic-ref refs/heads/reserved-non-branch-alias-two \
+               refs/tags/reserved-non-branch-target &&
+       git worktree add --detach reserved-target-wt HEAD &&
+       wt_gitdir=$(git -C reserved-target-wt rev-parse --absolute-git-dir) &&
+       mkdir -p "$wt_gitdir/rebase-merge" &&
+       old_oid=$(git rev-parse refs/heads/reserved-non-branch-alias-one) &&
+       test_write_lines refs/heads/reserved-non-branch-alias-one \
+               "$old_oid" "$old_oid" >"$wt_gitdir/rebase-merge/update-refs" &&
+
+       GIT_SEQUENCE_EDITOR="cat >todo" git rebase -i --update-refs HEAD~2 &&
+
+       test_grep "^# Ref refs/heads/reserved-non-branch-alias-one checked out at" \
+               todo &&
+       test_grep "^# Ref refs/heads/reserved-non-branch-alias-two checked out at" \
+               todo &&
+       test_grep ! "^update-ref refs/heads/reserved-non-branch-alias" todo
+'
+
 test_expect_success 'respect user edits to update-ref steps' '
        git checkout -B update-refs-break no-conflict-branch &&
        git branch -f base HEAD~4 &&