From 6c8f77cb71c7e0c820704b1725331f4601d8876e Mon Sep 17 00:00:00 2001 From: Philippe Blain Date: Sun, 23 Feb 2025 11:59:05 -0500 Subject: [PATCH 1/3] rebase -r: do create merge commit after empty resolution When a user runs 'git rebase --continue' to conclude a conflicted merge during a 'git rebase -r' invocation, we do not create a merge commit if the resolution was empty (i.e. if the index and HEAD are identical). We simply continue the rebase as if no 'merge' instruction had been given. This is confusing since all commits from the side branch are absent from the rebased history. What's more, if that 'merge' is the last instruction in the todo list, we fail to remove the merge state, such that running 'git status' shows we are still merging after the rebase has concluded. This happens because in 'sequencer.c::commit_staged_changes', we exit early before calling 'run_git_commit' if 'is_clean' is true, i.e. if nothing is staged. Fix this by also checking for the presence of MERGE_HEAD before exiting early, such that we do call 'run_git_commit' when MERGE_HEAD is present. This also ensures that we unlink git_path_merge_head later in 'commit_staged_changes' to clear the merge state. Make sure to also remove MERGE_HEAD when a merge command fails to start. We already remove MERGE_MSG since e032abd5a0 (rebase: fix rewritten list for failed pick, 2023-09-06). Removing MERGE_HEAD ensures that in this situation, upon 'git rebase --continue' we still exit early in 'commit_staged_changes', without calling 'run_git_commit'. This is already covered by t5407.11, which fails without this change because we enter 'run_git_commit' and then fail to find 'rebase_path_message'. Signed-off-by: Philippe Blain --- sequencer.c | 3 ++- t/t3418-rebase-continue.sh | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/sequencer.c b/sequencer.c index ad0ab75c8d4dd7..2baaf716a3cc89 100644 --- a/sequencer.c +++ b/sequencer.c @@ -4349,6 +4349,7 @@ static int do_merge(struct repository *r, error(_("could not even attempt to merge '%.*s'"), merge_arg_len, arg); unlink(git_path_merge_msg(r)); + unlink(git_path_merge_head(r)); goto leave_merge; } /* @@ -5364,7 +5365,7 @@ static int commit_staged_changes(struct repository *r, flags |= AMEND_MSG; } - if (is_clean) { + if (is_clean && !file_exists(git_path_merge_head(r))) { if (refs_ref_exists(get_main_ref_store(r), "CHERRY_PICK_HEAD") && refs_delete_ref(get_main_ref_store(r), "", diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh index 127216f7225aa4..f4b459fea16af2 100755 --- a/t/t3418-rebase-continue.sh +++ b/t/t3418-rebase-continue.sh @@ -111,6 +111,30 @@ test_expect_success 'rebase -r passes merge strategy options correctly' ' git rebase --continue ' +test_expect_success '--continue creates merge commit after empty resolution' ' + git reset --hard main && + git checkout -b rebase_i_merge && + test_commit unrelated && + git checkout -b rebase_i_merge_side && + test_commit side2 main.txt && + git checkout rebase_i_merge && + test_commit side1 main.txt && + PICK=$(git rev-parse --short rebase_i_merge) && + test_must_fail git merge rebase_i_merge_side && + echo side1 >main.txt && + git add main.txt && + test_tick && + git commit --no-edit && + FAKE_LINES="1 2 3 5 6 7 8 9 10 11" && + export FAKE_LINES && + test_must_fail git rebase -ir main && + echo side1 >main.txt && + git add main.txt && + git rebase --continue && + git log --merges >out && + test_grep "Merge branch '\''rebase_i_merge_side'\''" out +' + test_expect_success '--skip after failed fixup cleans commit message' ' test_when_finished "test_might_fail git rebase --abort" && git checkout -b with-conflicting-fixup && From e297b71ba123b642c2e724d7dda475fa52dfdeaa Mon Sep 17 00:00:00 2001 From: Philippe Blain Date: Wed, 15 Jan 2025 19:51:27 -0500 Subject: [PATCH 2/3] wt-status: also abbreviate 'merge' and 'fixup -C' lines during rebase When "git status" is invoked during a rebase, we print the last commands done and the next commands to do, and abbreviate commit hashes found in those lines. However, we only abbreviate hashes in 'pick', 'squash' and plain 'fixup' lines, not those in 'merge -C' and 'fixup -C' lines, as the parsing done in wt-status.c::abbrev_oid_in_line is not prepared for such lines. Improve the parsing done by this function by special casing 'fixup' and 'merge' such that the hash to abbreviate is the string found in the third field of 'split', instead of the second one for other commands. Introduce a 'hash' strbuf pointer to point to the correct field in all cases. Signed-off-by: Philippe Blain --- wt-status.c | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/wt-status.c b/wt-status.c index 1da5732f57b115..d11d9f9f142fe7 100644 --- a/wt-status.c +++ b/wt-status.c @@ -1342,9 +1342,11 @@ static int split_commit_in_progress(struct wt_status *s) /* * Turn - * "pick d6a2f0303e897ec257dd0e0a39a5ccb709bc2047 some message" + * "pick d6a2f0303e897ec257dd0e0a39a5ccb709bc2047 some message" and + * "merge -C d6a2f0303e897ec257dd0e0a39a5ccb709bc2047 some-branch" * into - * "pick d6a2f03 some message" + * "pick d6a2f03 some message" and + * "merge -C d6a2f03 some-branch" * * The function assumes that the line does not contain useless spaces * before or after the command. @@ -1360,20 +1362,31 @@ static void abbrev_oid_in_line(struct strbuf *line) starts_with(line->buf, "l ")) return; - split = strbuf_split_max(line, ' ', 3); + split = strbuf_split_max(line, ' ', 4); if (split[0] && split[1]) { struct object_id oid; - + struct strbuf *hash; + + if ((!strcmp(split[0]->buf, "merge ") || + !strcmp(split[0]->buf, "m " ) || + !strcmp(split[0]->buf, "fixup ") || + !strcmp(split[0]->buf, "f " )) && + (!strcmp(split[1]->buf, "-C ") || + !strcmp(split[1]->buf, "-c "))) { + hash = split[2]; + } else { + hash = split[1]; + } /* * strbuf_split_max left a space. Trim it and re-add * it after abbreviation. */ - strbuf_trim(split[1]); - if (!repo_get_oid(the_repository, split[1]->buf, &oid)) { - strbuf_reset(split[1]); - strbuf_add_unique_abbrev(split[1], &oid, + strbuf_trim(hash); + if (!repo_get_oid(the_repository, hash->buf, &oid)) { + strbuf_reset(hash); + strbuf_add_unique_abbrev(hash, &oid, DEFAULT_ABBREV); - strbuf_addch(split[1], ' '); + strbuf_addch(hash, ' '); strbuf_reset(line); for (i = 0; split[i]; i++) strbuf_addbuf(line, split[i]); From db01acdd062a17b1cca62428eba8c3ed62ca7c6a Mon Sep 17 00:00:00 2001 From: Philippe Blain Date: Mon, 10 Feb 2025 10:25:23 -0500 Subject: [PATCH 3/3] wt-status: suggest 'git rebase --continue' to conclude 'merge' instruction Since 982288e9bd (status: rebase and merge can be in progress at the same time, 2018-11-12), when a merge is in progress as part of a 'git rebase -r' operation, 'wt_longstatus_print_state' shows information about the in-progress rebase (via show_rebase_information), and then calls 'show_merge_in_progress' to help the user conclude the merge. This function suggests using 'git commit' to do so, but this throws away the authorship information from the original merge, which is not ideal. Using 'git rebase --continue' instead preserves the authorship information, since we enter 'sequencer.c:run_git_commit' which calls read_env_script to read the author-script file. Note however that this only works when a merge was scheduled using a 'merge' instruction in the rebase todo list. Indeed, when using 'exec git merge', the state files necessary for 'git rebase --continue' are not present, and one must use 'git commit' (or 'git merge --continue') in that case. Be more helpful to the user by suggesting either 'git rebase --continue', when the merge was scheduled using a 'merge' instruction, and 'git commit' otherwise. As such, add a 'merge_during_rebase_in_progress' field to 'struct wt_status_state', and detect this situation in wt_status_check_rebase by looking at the last command done. Adjust wt_longstatus_print_state to check this field and suggest 'git rebase --continue' if a merge came from a 'merge' instruction, by calling show_rebase_in_progress directly. Add two tests for the new behaviour, using 'merge' and 'exec git merge' instructions. Signed-off-by: Philippe Blain --- t/t7512-status-help.sh | 75 ++++++++++++++++++++++++++++++++++++++++++ wt-status.c | 18 +++++++--- wt-status.h | 1 + 3 files changed, 90 insertions(+), 4 deletions(-) diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh index 802f8f704c62eb..b37e99625b4e65 100755 --- a/t/t7512-status-help.sh +++ b/t/t7512-status-help.sh @@ -183,6 +183,81 @@ EOF test_cmp expected actual ' +test_expect_success 'status during rebase -ir after conflicted merge (exec git merge)' ' + git reset --hard main && + git checkout -b rebase_i_merge && + test_commit unrelated && + git checkout -b rebase_i_merge_side && + test_commit side2 main.txt && + git checkout rebase_i_merge && + test_commit side1 main.txt && + PICK=$(git rev-parse --short rebase_i_merge) && + test_must_fail git merge rebase_i_merge_side && + echo side1 >main.txt && + git add main.txt && + test_tick && + git commit --no-edit && + MERGE=$(git rev-parse --short rebase_i_merge) && + ONTO=$(git rev-parse --short main) && + test_when_finished "git rebase --abort" && + FAKE_LINES="1 2 3 5 6 7 8 9 10 exec_git_merge_refs/rewritten/rebase-i-merge-side" && + export FAKE_LINES && + test_must_fail git rebase -ir main && + cat >expect <..." to mark resolution) + both modified: main.txt + +no changes added to commit (use "git add" and/or "git commit -a") +EOF + git status --untracked-files=no >actual && + test_cmp expect actual +' + +test_expect_success 'status during rebase -ir after replaying conflicted merge (merge)' ' + PICK=$(git rev-parse --short :/side1) && + UNRELATED=$(git rev-parse --short :/unrelated) && + MERGE=$(git rev-parse --short rebase_i_merge) && + ONTO=$(git rev-parse --short main) && + test_when_finished "git rebase --abort" && + FAKE_LINES="1 2 3 5 6 7 8 9 10 11 4" && + export FAKE_LINES && + test_must_fail git rebase -ir main && + cat >expect <..." to mark resolution) + both modified: main.txt + +no changes added to commit (use "git add" and/or "git commit -a") +EOF + git status --untracked-files=no >actual && + test_cmp expect actual +' + test_expect_success 'status when rebasing -i in edit mode' ' git reset --hard main && diff --git a/wt-status.c b/wt-status.c index d11d9f9f142fe7..f15495039e3eb1 100644 --- a/wt-status.c +++ b/wt-status.c @@ -1744,6 +1744,7 @@ int wt_status_check_rebase(const struct worktree *wt, struct wt_status_state *state) { struct stat st; + struct string_list have_done = STRING_LIST_INIT_DUP; if (!stat(worktree_git_path(the_repository, wt, "rebase-apply"), &st)) { if (!stat(worktree_git_path(the_repository, wt, "rebase-apply/applying"), &st)) { @@ -1760,8 +1761,12 @@ int wt_status_check_rebase(const struct worktree *wt, state->rebase_interactive_in_progress = 1; else state->rebase_in_progress = 1; + read_rebase_todolist("rebase-merge/done", &have_done); + if (have_done.nr > 0 && starts_with(have_done.items[have_done.nr - 1].string, "merge")) + state->merge_during_rebase_in_progress = 1; state->branch = get_branch(wt, "rebase-merge/head-name"); state->onto = get_branch(wt, "rebase-merge/onto"); + string_list_clear(&have_done, 0); } else return 0; return 1; @@ -1855,10 +1860,15 @@ static void wt_longstatus_print_state(struct wt_status *s) if (state->merge_in_progress) { if (state->rebase_interactive_in_progress) { - show_rebase_information(s, state_color); - fputs("\n", s->fp); - } - show_merge_in_progress(s, state_color); + if (state->merge_during_rebase_in_progress) + show_rebase_in_progress(s, state_color); + else { + show_rebase_information(s, state_color); + fputs("\n", s->fp); + show_merge_in_progress(s, state_color); + } + } else + show_merge_in_progress(s, state_color); } else if (state->am_in_progress) show_am_in_progress(s, state_color); else if (state->rebase_in_progress || state->rebase_interactive_in_progress) diff --git a/wt-status.h b/wt-status.h index 4e377ce62b8b28..84bedfcd48f6cb 100644 --- a/wt-status.h +++ b/wt-status.h @@ -87,6 +87,7 @@ struct wt_status_state { int am_empty_patch; int rebase_in_progress; int rebase_interactive_in_progress; + int merge_during_rebase_in_progress; int cherry_pick_in_progress; int bisect_in_progress; int revert_in_progress;