[PATCH 0/5] Assorted fixes revolving around rebase and merges

STALE2891d

6 messages, 1 author, 2018-11-12 · open the first message on its own page

[PATCH 0/5] Assorted fixes revolving around rebase and merges

From: Johannes Schindelin via GitGitGadget <hidden>
Date: 2018-11-12 23:25:59

I noticed a couple of weeks ago that I had bogus merge commits after my
rebases, where the original commits had been regular commits.

This set me out on the adventure that is reflected in this patch series.

Of course, the thing I wanted to fix is demonstrated by 1/5 and fixed in
2/5. But while at it, I ran into other issues and fixed them since I was at
it anyway.

Johannes Schindelin (5):
  rebase -r: demonstrate bug with conflicting merges
  rebase -r: do not write MERGE_HEAD unless needed
  rebase -i: include MERGE_HEAD into files to clean up
  built-in rebase --skip/--abort: clean up stale .git/<name> files
  status: rebase and merge can be in progress at the same time

 builtin/rebase.c         |  3 +++
 sequencer.c              | 10 ++++++----
 t/t3430-rebase-merges.sh | 16 ++++++++++++++++
 wt-status.c              |  9 +++++++--
 4 files changed, 32 insertions(+), 6 deletions(-)


base-commit: 8858448bb49332d353febc078ce4a3abcc962efe
Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-75%2Fdscho%2Frebase-r-and-merge-head-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-75/dscho/rebase-r-and-merge-head-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/75
-- 
gitgitgadget

[PATCH 1/5] rebase -r: demonstrate bug with conflicting merges

From: Johannes Schindelin via GitGitGadget <hidden>
Date: 2018-11-12 23:26:00

From: Johannes Schindelin <redacted>

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 <redacted>
---
 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
-- 
gitgitgadget

[PATCH 2/5] rebase -r: do not write MERGE_HEAD unless needed

From: Johannes Schindelin via GitGitGadget <hidden>
Date: 2018-11-12 23:26:02

From: Johannes Schindelin <redacted>

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 <redacted>
---
 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 9e1ab3a2a7..7a9cd81afb 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3191,10 +3191,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 && oideq(&merge_commit->object.oid,
 			   &bases->item->object.oid)) {
@@ -3203,6 +3199,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 &&
-- 
gitgitgadget

[PATCH 3/5] rebase -i: include MERGE_HEAD into files to clean up

From: Johannes Schindelin via GitGitGadget <hidden>
Date: 2018-11-12 23:26:03

From: Johannes Schindelin <redacted>

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 <redacted>
---
 sequencer.c | 2 ++
 1 file changed, 2 insertions(+)
diff --git a/sequencer.c b/sequencer.c
index 7a9cd81afb..2f526390ac 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3459,6 +3459,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_BREAK)
@@ -3829,6 +3830,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());
-- 
gitgitgadget

[PATCH 4/5] built-in rebase --skip/--abort: clean up stale .git/<name> files

From: Johannes Schindelin via GitGitGadget <hidden>
Date: 2018-11-12 23:26:03

From: Johannes Schindelin <redacted>

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 <redacted>
---
 builtin/rebase.c | 3 +++
 1 file changed, 3 insertions(+)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 0ee06aa363..017dce1b50 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -23,6 +23,7 @@
 #include "revision.h"
 #include "commit-reach.h"
 #include "rerere.h"
+#include "branch.h"
 
 static char const * const builtin_rebase_usage[] = {
 	N_("git rebase [-i] [options] [--exec <cmd>] [--onto <newbase>] "
@@ -1002,6 +1003,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;
@@ -1019,6 +1021,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;
 	}
-- 
gitgitgadget

[PATCH 5/5] status: rebase and merge can be in progress at the same time

From: Johannes Schindelin via GitGitGadget <hidden>
Date: 2018-11-12 23:26:05

From: Johannes Schindelin <redacted>

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 <redacted>
---
 wt-status.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/wt-status.c b/wt-status.c
index 187568a112..a24711374c 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1559,6 +1559,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 */
@@ -1583,9 +1584,13 @@ static void wt_longstatus_print_state(struct wt_status *s)
 	const char *state_color = color(WT_STATUS_HEADER, s);
 	struct wt_status_state *state = &s->state;
 
-	if (state->merge_in_progress)
+	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);
-	else if (state->am_in_progress)
+	} else if (state->am_in_progress)
 		show_am_in_progress(s, state_color);
 	else if (state->rebase_in_progress || state->rebase_interactive_in_progress)
 		show_rebase_in_progress(s, state_color);
-- 
gitgitgadget
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help