[PATCH v3 0/2] rebase: handle --update-refs branch symrefs
"Son Luong Ngoc via GitGitGadget" <[email protected]>
| Newsgroups | org.kernel.vger.git |
|---|---|
| Message-ID | <[email protected]> |
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 <[email protected]>
@@ 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