[PATCH v3 0/2] rebase: handle --update-refs branch symrefs

["Son Luong Ngoc via GitGitGadget" <gitgitgadget@gmail.com>]

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 failure leaves refs partially
updated even though the main rebase has succeeded.

Resolve local branch decorations before adding update-ref commands. The
first patch skips aliases whose targets are other branches and preserves the
existing handling of the current branch. The second patch keeps aliases to
non-branch refs supported while preventing duplicate and cross-worktree
updates to their resolved targets.

Changes since v2:

 * Skip branch-to-branch symrefs before checked-out handling.
 * Restore the unconditional current-branch skip and keep an owned copy of
   the resolved HEAD name.
 * Check both a non-branch symref alias and its resolved target against
   checked-out reservations.
 * Deduplicate aliases that share a non-branch target.
 * Reserve resolved targets from other worktrees' in-progress update-refs
   state.
 * Split the branch-alias fix and non-branch safeguards into separate
   patches.
 * Rebase onto 48bbf81c29 (The 5th batch).

The focused t3400 and t3404 test suites pass with both the files and
reftable backends.

Son Luong Ngoc (2):
  rebase: skip branch symref aliases
  rebase: guard non-branch symref targets

 branch.c                      | 15 ++++++
 sequencer.c                   | 63 ++++++++++++++++++++-----
 t/t3400-rebase.sh             |  2 +-
 t/t3404-rebase-interactive.sh | 88 +++++++++++++++++++++++++++++++++++
 4 files changed, 155 insertions(+), 13 deletions(-)


base-commit: 48bbf81c29ca9a4479ec7850fe206518682cdb2f
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2126%2Fsluongng%2Fsl%2Frebase-update-refs-symrefs-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2126/sluongng/sl/rebase-update-refs-symrefs-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/2126

Range-diff vs v2:

 1:  68f698225c ! 1:  b9a01e9141 rebase: skip branch symref aliases
     @@ Metadata
       ## Commit message ##
          rebase: skip branch symref aliases
      
     -    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.
     +    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 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.
     +    The problem is a partially applied ref update: the main rebase has
     +    already succeeded when the later ref update fails.
      
     -    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.
     +    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.
      
     -    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.
     +    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>
      
     @@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,
       	while (decoration) {
       		struct todo_item *item;
       		const char *path;
     -+		const char *resolved_ref;
     ++		char *resolved_ref;
      +		int flags = 0;
       		size_t base_offset = ctx->buf->len;
       
     @@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,
      +			continue;
      +		}
      +
     -+		path = branch_checked_out(decoration->name);
     -+
     -+		/*
     -+		 * 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, unless it is checked out and
     -+		 * needs a comment in the todo list.
     -+		 */
     -+		if (!path && head_ref && !strcmp(head_ref, decoration->name)) {
     ++		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;
      +		}
      +
     -+		resolved_ref = refs_resolve_ref_unsafe(refs, decoration->name,
     -+						       RESOLVE_REF_READING,
     -+						       NULL, &flags);
     -+		if (!path && resolved_ref && (flags & REF_ISSYMREF) &&
     -+		    starts_with(resolved_ref, "refs/heads/")) {
     ++		/*
     ++		 * 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);
      @@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,
       		memset(item, 0, sizeof(*item));
       
     @@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,
       			strbuf_commented_addf(ctx->buf, comment_line_str,
       					      "Ref %s checked out at '%s'\n",
      @@ sequencer.c: 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;
       	}
       
     @@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,
       }
       
      
     + ## t/t3400-rebase.sh ##
     +@@ t/t3400-rebase.sh: 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
     +
       ## t/t3404-rebase-interactive.sh ##
      @@ t/t3404-rebase-interactive.sh: 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' '
 -:  ---------- > 2:  a653f56ea2 rebase: guard non-branch symref targets

-- 
gitgitgadget