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.

The problem is a partially applied ref update: the main rebase has
already succeeded when the later ref update fails.

The sequencer queues updates from local branch decorations. Commit
106b6885c7 (rebase: ignore non-branch update-refs) filters out
decorations such as HEAD and tags. A branch symref is still a local
branch decoration, but refs_update_ref() dereferences it, so an alias to
another branch duplicates the concrete branch update.

Resolve local branch decorations before queuing them. Skip symrefs whose
targets are under refs/heads/ so that only the concrete branch update is
queued. 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.

This prevents a successful rebase from being followed by a failed,
partially applied ref update while preserving each alias as a symref.

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

diff --git a/sequencer.c b/sequencer.c
index 1ee4b2875b25a08f6589314aae11ab5e3f35eecc..17f5baab62ff70de4a58bb7d5193007579023dc9 100644 (file)
--- a/sequencer.c
+++ b/sequencer.c
@@ -6445,32 +6445,50 @@ 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);
 
        while (decoration) {
                struct todo_item *item;
                const char *path;
+               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;
+               }
+
+               resolved_ref = refs_resolve_refdup(refs, decoration->name,
+                                                     RESOLVE_REF_READING,
+                                                     NULL, &flags);
+               if (resolved_ref && (flags & REF_ISSYMREF) &&
+                   starts_with(resolved_ref, "refs/heads/")) {
+                       free(resolved_ref);
+                       decoration = decoration->next;
+                       continue;
+               }
+
+               /*
+                * If the branch is the current HEAD, then it will be
+                * updated by the default rebase behavior.
+                */
+               if (head_ref && !strcmp(head_ref, decoration->name)) {
+                       free(resolved_ref);
                        decoration = decoration->next;
                        continue;
                }
 
+               path = branch_checked_out(decoration->name);
+
                ALLOC_GROW(ctx->items,
                        ctx->items_nr + 1,
                        ctx->items_alloc);
@@ -6478,7 +6496,7 @@ 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) {
                        item->command = TODO_COMMENT;
                        strbuf_commented_addf(ctx->buf, comment_line_str,
                                              "Ref %s checked out at '%s'\n",
@@ -6498,9 +6516,11 @@ static int add_decorations_to_list(const struct commit *commit,
                item->arg_len = ctx->buf->len - base_offset;
                ctx->items_nr++;
 
+               free(resolved_ref);
                decoration = decoration->next;
        }
 
+       free(head_ref);
        return 0;
 }
 

diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index c0c00fbb7b1e4ed52434c7af1b6f0e56fbe66614..be120bdcd1a41df50d6f7f7f58e2a56f9d4013b5 100755 (executable)
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -471,7 +471,7 @@ test_expect_success 'git rebase --update-ref with core.commentChar and branch on
        GIT_SEQUENCE_EDITOR="cat >actual" git -c core.commentChar=% \
                 rebase -i --update-refs base &&
        test_grep "% Ref refs/heads/wt-topic checked out at" actual &&
-       test_grep "% Ref refs/heads/topic2 checked out at" actual
+       test_grep ! "% Ref refs/heads/topic2 checked out at" actual
 '
 
 test_done

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 58b3bb0c271aaebab4381795c9d9dcf8ba3a7593..b05d93884668b543e9115bc676959a536dbd9f95 100755 (executable)
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1975,15 +1975,23 @@ test_expect_success '--update-refs ignores non-branch decorations' '
        ) &&
        grep ^update-ref todo >actual &&
        test_write_lines "update-ref refs/heads/no-conflict-branch" >expect &&
+       test_grep ! "^# Ref refs/heads/update-refs checked out" todo &&
        test_cmp expect actual
 '
 
 test_expect_success '--update-refs updates refs correctly' '
+       test_when_finished "
+               test_might_fail git symbolic-ref -d refs/heads/no-conflict-branch-alias &&
+               test_might_fail git symbolic-ref -d refs/heads/second-alias
+       " &&
        git checkout -B update-refs no-conflict-branch &&
        git branch -f base HEAD~4 &&
        git branch -f first HEAD~3 &&
        git branch -f second HEAD~3 &&
        git branch -f third HEAD~1 &&
+       git symbolic-ref refs/heads/no-conflict-branch-alias \
+               refs/heads/no-conflict-branch &&
+       git symbolic-ref refs/heads/second-alias refs/heads/second &&
        test_commit extra2 fileX &&
        git commit --amend --fixup=L &&
 
@@ -1991,8 +1999,16 @@ test_expect_success '--update-refs updates refs correctly' '
 
        test_cmp_rev HEAD~3 refs/heads/first &&
        test_cmp_rev HEAD~3 refs/heads/second &&
+       test_cmp_rev HEAD~3 refs/heads/second-alias &&
        test_cmp_rev HEAD~1 refs/heads/third &&
        test_cmp_rev HEAD refs/heads/no-conflict-branch &&
+       test_cmp_rev HEAD refs/heads/no-conflict-branch-alias &&
+       test_write_lines refs/heads/no-conflict-branch >expect &&
+       git symbolic-ref refs/heads/no-conflict-branch-alias >actual &&
+       test_cmp expect actual &&
+       test_write_lines refs/heads/second >expect &&
+       git symbolic-ref refs/heads/second-alias >actual &&
+       test_cmp expect actual &&
 
        q_to_tab >expect <<-\EOF &&
        Successfully rebased and updated refs/heads/update-refs.