From 54f947ebc5da419d20f2cbe5b07bab3672c3f0ec Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 12 Nov 2018 16:47:15 +0100 Subject: [PATCH 1/5] rebase -r: demonstrate bug with conflicting merges When calling `merge` on a branch that has already been merged, that `merge` is skipped quietly, but currently a MERGE_HEAD file is being left behind and will then be grabbed by the next `pick` (that did not want to create a *merge* commit). Demonstrate this. Signed-off-by: Johannes Schindelin --- t/t3430-rebase-merges.sh | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh index aa7bfc88ec..1f08a33687 100755 --- a/t/t3430-rebase-merges.sh +++ b/t/t3430-rebase-merges.sh @@ -396,4 +396,20 @@ test_expect_success 'with --autosquash and --exec' ' grep "G: +G" actual ' +test_expect_failure '--continue after resolving conflicts after a merge' ' + git checkout -b already-has-g E && + git cherry-pick E..G && + test_commit H2 && + + git checkout -b conflicts-in-merge H && + test_commit H2 H2.t conflicts H2-conflict && + test_must_fail git rebase -r already-has-g && + grep conflicts H2.t && + echo resolved >H2.t && + git add -u && + git rebase --continue && + test_must_fail git rev-parse --verify HEAD^2 && + test_path_is_missing .git/MERGE_HEAD +' + test_done From 769f8a818ffb4333522026be824ba4979b77e354 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 12 Nov 2018 21:39:48 +0100 Subject: [PATCH 2/5] rebase -r: do not write MERGE_HEAD unless needed When we detect that a `merge` can be skipped because the merged commit is already an ancestor of HEAD, we do not need to commit, therefore writing the MERGE_HEAD file is useless. It is actually worse than useless: a subsequent `git commit` will pick it up and think that we want to merge that commit, still. To avoid that, move the code that writes the MERGE_HEAD file to a location where we already know that the `merge` cannot be skipped. Signed-off-by: Johannes Schindelin --- sequencer.c | 8 ++++---- t/t3430-rebase-merges.sh | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/sequencer.c b/sequencer.c index ad2193507b..bec191aa69 100644 --- a/sequencer.c +++ b/sequencer.c @@ -3184,10 +3184,6 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len, } merge_commit = to_merge->item; - write_message(oid_to_hex(&merge_commit->object.oid), GIT_SHA1_HEXSZ, - git_path_merge_head(the_repository), 0); - write_message("no-ff", 5, git_path_merge_mode(the_repository), 0); - bases = get_merge_bases(head_commit, merge_commit); if (bases && !oidcmp(&merge_commit->object.oid, &bases->item->object.oid)) { @@ -3196,6 +3192,10 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len, goto leave_merge; } + write_message(oid_to_hex(&merge_commit->object.oid), GIT_SHA1_HEXSZ, + git_path_merge_head(the_repository), 0); + write_message("no-ff", 5, git_path_merge_mode(the_repository), 0); + for (j = bases; j; j = j->next) commit_list_insert(j->item, &reversed); free_commit_list(bases); diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh index 1f08a33687..cc5646836f 100755 --- a/t/t3430-rebase-merges.sh +++ b/t/t3430-rebase-merges.sh @@ -396,7 +396,7 @@ test_expect_success 'with --autosquash and --exec' ' grep "G: +G" actual ' -test_expect_failure '--continue after resolving conflicts after a merge' ' +test_expect_success '--continue after resolving conflicts after a merge' ' git checkout -b already-has-g E && git cherry-pick E..G && test_commit H2 && From 94e041abbd9484877990301bbddcaa6fa9e15c36 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 12 Nov 2018 21:44:11 +0100 Subject: [PATCH 3/5] rebase -i: include MERGE_HEAD into files to clean up Every once in a while, the interactive rebase makes sure that no stale files are lying around. These days, we need to include MERGE_HEAD into that set of files, as the `merge` command will generate them. Signed-off-by: Johannes Schindelin --- sequencer.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sequencer.c b/sequencer.c index bec191aa69..98b41ae890 100644 --- a/sequencer.c +++ b/sequencer.c @@ -3434,6 +3434,7 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts) unlink(rebase_path_author_script()); unlink(rebase_path_stopped_sha()); unlink(rebase_path_amend()); + unlink(git_path_merge_head(the_repository)); delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF); } if (item->command <= TODO_SQUASH) { @@ -3790,6 +3791,7 @@ static int commit_staged_changes(struct replay_opts *opts, opts, flags)) return error(_("could not commit staged changes.")); unlink(rebase_path_amend()); + unlink(git_path_merge_head(the_repository)); if (final_fixup) { unlink(rebase_path_fixup_msg()); unlink(rebase_path_squash_msg()); From eff1e5951172b69dc6c7bf47bab23fdbb4bb6adc Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 12 Nov 2018 21:47:06 +0100 Subject: [PATCH 4/5] built-in rebase --skip/--abort: clean up stale .git/ files The scripted version of the rebase used to execute `git reset --hard` when skipping or aborting. When we ported this to C, we did update the worktree and some reflogs, but we failed to imitate `git reset --hard`'s behavior regarding files in .git/ such as MERGE_HEAD. Let's address this oversight. Signed-off-by: Johannes Schindelin --- builtin/rebase.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/builtin/rebase.c b/builtin/rebase.c index b4881ebb22..321ad0142f 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -22,6 +22,7 @@ #include "wt-status.h" #include "revision.h" #include "rerere.h" +#include "branch.h" static char const * const builtin_rebase_usage[] = { N_("git rebase [-i] [options] [--exec ] [--onto ] " @@ -1001,6 +1002,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) if (reset_head(NULL, "reset", NULL, 0, NULL, NULL) < 0) die(_("could not discard worktree changes")); + remove_branch_state(); if (read_basic_state(&options)) exit(1); goto run_rebase; @@ -1018,6 +1020,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) options.head_name, 0, NULL, NULL) < 0) die(_("could not move back to %s"), oid_to_hex(&options.orig_head)); + remove_branch_state(); ret = finish_rebase(&options); goto cleanup; } From 07a0e6fd9ba9227eb74fe323e32009aca15ca68c Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 12 Nov 2018 21:49:26 +0100 Subject: [PATCH 5/5] status: rebase and merge can be in progress at the same time Since `git rebase -r` was introduced, that is possible. But our machinery did not think that possible, and failed to say anything about the rebase in progress when in the middle of a merge. Let's work around that in the minimal fashion. Signed-off-by: Johannes Schindelin --- wt-status.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/wt-status.c b/wt-status.c index 5ffab61015..30b81576a3 100644 --- a/wt-status.c +++ b/wt-status.c @@ -1553,6 +1553,7 @@ void wt_status_get_state(struct wt_status_state *state, struct object_id oid; if (!stat(git_path_merge_head(the_repository), &st)) { + wt_status_check_rebase(NULL, state); state->merge_in_progress = 1; } else if (wt_status_check_rebase(NULL, state)) { ; /* all set */ @@ -1576,9 +1577,13 @@ static void wt_longstatus_print_state(struct wt_status *s, struct wt_status_state *state) { const char *state_color = color(WT_STATUS_HEADER, s); - if (state->merge_in_progress) + if (state->merge_in_progress) { + if (state->rebase_interactive_in_progress) { + show_rebase_information(s, state, state_color); + fputs("\n", s->fp); + } show_merge_in_progress(s, state, state_color); - else if (state->am_in_progress) + } else if (state->am_in_progress) show_am_in_progress(s, state, state_color); else if (state->rebase_in_progress || state->rebase_interactive_in_progress) show_rebase_in_progress(s, state, state_color);