Merge branch 'js/rebase-r-strategy' into jch

gitster · gitster · commit aa2d3a6afc39 · 2019-09-16T14:01:01.000-07:00
"git rebase --rebase-merges" learned to drive different merge
strategies and pass strategy specific options to them.

* js/rebase-r-strategy:
  t3427: accelerate this test by using fast-export and fast-import
  rebase -r: do not (re-)generate root commits with `--root` *and* `--onto`
  t3418: test `rebase -r` with merge strategies
  t/lib-rebase: prepare for testing `git rebase --rebase-merges`
  rebase -r: support merge strategies other than `recursive`
  t3427: fix another incorrect assumption
  t3427: accommodate for the `rebase --merge` backend having been replaced
  t3427: fix erroneous assumption
  t3427: condense the unnecessarily repetitive test cases into three
  t3427: move the `filter-branch` invocation into the `setup` case
  t3427: simplify the `setup` test case significantly
  t3427: add a clarifying comment
  rebase: fold git-rebase--common into the -p backend
  sequencer: the `am` and `rebase--interactive` scripts are gone
  .gitignore: there is no longer a built-in `git-rebase--interactive`
  t3400: stop referring to the scripted rebase
  Drop unused git-rebase--am.sh
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
@@ -543,8 +543,6 @@ In addition, the following pairs of options are incompatible:
  * --preserve-merges and --interactive
  * --preserve-merges and --signoff
  * --preserve-merges and --rebase-merges
- * --rebase-merges and --strategy
- * --rebase-merges and --strategy-option
 
 BEHAVIORAL DIFFERENCES
 -----------------------
diff --git a/builtin/rebase.c b/builtin/rebase.c
@@ -62,7 +62,7 @@ struct rebase_options {
 	const char *onto_name;
 	const char *revisions;
 	const char *switch_to;
-	int root;
+	int root, root_with_onto;
 	struct object_id *squash_onto;
 	struct commit *restrict_revision;
 	int dont_finish_rebase;
@@ -374,6 +374,7 @@ static int run_rebase_interactive(struct rebase_options *opts,
 	flags |= abbreviate_commands ? TODO_LIST_ABBREVIATE_CMDS : 0;
 	flags |= opts->rebase_merges ? TODO_LIST_REBASE_MERGES : 0;
 	flags |= opts->rebase_cousins > 0 ? TODO_LIST_REBASE_COUSINS : 0;
+	flags |= opts->root_with_onto ? TODO_LIST_ROOT_WITH_ONTO : 0;
 	flags |= command == ACTION_SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0;
 
 	switch (command) {
@@ -1833,15 +1834,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 			      "'--reschedule-failed-exec'"));
 	}
 
-	if (options.rebase_merges) {
-		if (strategy_options.nr)
-			die(_("cannot combine '--rebase-merges' with "
-			      "'--strategy-option'"));
-		if (options.strategy)
-			die(_("cannot combine '--rebase-merges' with "
-			      "'--strategy'"));
-	}
-
 	if (!options.root) {
 		if (argc < 1) {
 			struct branch *branch;
@@ -1872,7 +1864,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 			options.squash_onto = &squash_onto;
 			options.onto_name = squash_onto_name =
 				xstrdup(oid_to_hex(&squash_onto));
-		}
+		} else
+			options.root_with_onto = 1;
+
 		options.upstream_name = NULL;
 		options.upstream = NULL;
 		if (argc > 1)
diff --git a/sequencer.c b/sequencer.c
@@ -3364,6 +3364,9 @@ static int do_merge(struct repository *r,
 	struct commit *head_commit, *merge_commit, *i;
 	struct commit_list *bases, *j, *reversed = NULL;
 	struct commit_list *to_merge = NULL, **tail = &to_merge;
+	const char *strategy = !opts->xopts_nr &&
+		(!opts->strategy || !strcmp(opts->strategy, "recursive")) ?
+		NULL : opts->strategy;
 	struct merge_options o;
 	int merge_arg_len, oneline_offset, can_fast_forward, ret, k;
 	static struct lock_file lock;
@@ -3516,7 +3519,7 @@ static int do_merge(struct repository *r,
 		goto leave_merge;
 	}
 
-	if (to_merge->next) {
+	if (strategy || to_merge->next) {
 		/* Octopus merge */
 		struct child_process cmd = CHILD_PROCESS_INIT;
 
@@ -3530,7 +3533,14 @@ static int do_merge(struct repository *r,
 		cmd.git_cmd = 1;
 		argv_array_push(&cmd.args, "merge");
 		argv_array_push(&cmd.args, "-s");
-		argv_array_push(&cmd.args, "octopus");
+		if (!strategy)
+			argv_array_push(&cmd.args, "octopus");
+		else {
+			argv_array_push(&cmd.args, strategy);
+			for (k = 0; k < opts->xopts_nr; k++)
+				argv_array_pushf(&cmd.args,
+						 "-X%s", opts->xopts[k]);
+		}
 		argv_array_push(&cmd.args, "--no-edit");
 		argv_array_push(&cmd.args, "--no-ff");
 		argv_array_push(&cmd.args, "--no-log");
@@ -4554,6 +4564,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,
 {
 	int keep_empty = flags & TODO_LIST_KEEP_EMPTY;
 	int rebase_cousins = flags & TODO_LIST_REBASE_COUSINS;
+	int root_with_onto = flags & TODO_LIST_ROOT_WITH_ONTO;
 	struct strbuf buf = STRBUF_INIT, oneline = STRBUF_INIT;
 	struct strbuf label = STRBUF_INIT;
 	struct commit_list *commits = NULL, **tail = &commits, *iter;
@@ -4720,7 +4731,8 @@ static int make_script_with_merges(struct pretty_print_context *pp,
 
 		if (!commit)
 			strbuf_addf(out, "%s %s\n", cmd_reset,
-				    rebase_cousins ? "onto" : "[new root]");
+				    rebase_cousins || root_with_onto ?
+				    "onto" : "[new root]");
 		else {
 			const char *to = NULL;
 
diff --git a/sequencer.h b/sequencer.h
@@ -143,6 +143,12 @@ int sequencer_remove_state(struct replay_opts *opts);
  */
 #define TODO_LIST_REBASE_COUSINS (1U << 4)
 #define TODO_LIST_APPEND_TODO_HELP (1U << 5)
+/*
+ * When generating a script that rebases merges with `--root` *and* with
+ * `--onto`, we do not want to re-generate the root commits.
+ */
+#define TODO_LIST_ROOT_WITH_ONTO (1U << 6)
+
 
 int sequencer_make_script(struct repository *r, struct strbuf *out, int argc,
 			  const char **argv, unsigned flags);
diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
@@ -44,10 +44,10 @@ set_fake_editor () {
 	rm -f "$1"
 	echo 'rebase -i script before editing:'
 	cat "$1".tmp
-	action=pick
+	action=\&
 	for line in $FAKE_LINES; do
 		case $line in
-		pick|p|squash|s|fixup|f|edit|e|reword|r|drop|d)
+		pick|p|squash|s|fixup|f|edit|e|reword|r|drop|d|label|l|reset|r|merge|m)
 			action="$line";;
 		exec_*|x_*|break|b)
 			echo "$line" | sed 's/_/ /g' >> "$1";;
@@ -58,11 +58,12 @@ set_fake_editor () {
 		bad)
 			action="badcmd";;
 		fakesha)
+			test \& != "$action" || action=pick
 			echo "$action XXXXXXX False commit" >> "$1"
 			action=pick;;
 		*)
-			sed -n "${line}s/^pick/$action/p" < "$1".tmp >> "$1"
-			action=pick;;
+			sed -n "${line}s/^[a-z][a-z]*/$action/p" < "$1".tmp >> "$1"
+			action=\&;;
 		esac
 	done
 	echo 'rebase -i script after editing:'
diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
@@ -120,6 +120,20 @@ test_expect_success REBASE_P 'rebase passes merge strategy options correctly' '
 	git rebase --continue
 '
 
+test_expect_success 'rebase -r passes merge strategy options correctly' '
+	rm -fr .git/rebase-* &&
+	git reset --hard commit-new-file-F3-on-topic-branch &&
+	test_commit merge-theirs &&
+	git reset --hard HEAD^ &&
+	test_commit some-other-commit &&
+	test_tick &&
+	git merge --no-ff merge-theirs &&
+	FAKE_LINES="1 3 edit 4 5 7 8 9" git rebase -i -f -r -m \
+		-s recursive --strategy-option=theirs HEAD~2 &&
+	test_commit force-change-ours &&
+	git rebase --continue
+'
+
 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 &&
diff --git a/t/t3422-rebase-incompatible-options.sh b/t/t3422-rebase-incompatible-options.sh
@@ -76,14 +76,4 @@ test_expect_success REBASE_P \
 	test_must_fail git rebase --preserve-merges --rebase-merges A
 '
 
-test_expect_success '--rebase-merges incompatible with --strategy' '
-	git checkout B^0 &&
-	test_must_fail git rebase --rebase-merges -s resolve A
-'
-
-test_expect_success '--rebase-merges incompatible with --strategy-option' '
-	git checkout B^0 &&
-	test_must_fail git rebase --rebase-merges -Xignore-space-change A
-'
-
 test_done
diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh
@@ -11,113 +11,99 @@ commit_message() {
 	git log --pretty=format:%s -1 "$1"
 }
 
+# There are a few bugs in the rebase with regards to the subtree strategy, and
+# this test script tries to document them.  First, the following commit history
+# is generated (the onelines are shown, time flows from left to right):
+#
+# master1 - master2 - master3
+#                             \
+# README ---------------------- Add subproject master - master4 - files_subtree/master5
+#
+# Where the merge moves the files master[123].t into the subdirectory
+# files_subtree/ and master4 as well as files_subtree/master5 add files to that
+# directory directly.
+#
+# Then, in subsequent test cases, `git filter-branch` is used to distill just
+# the commits that touch files_subtree/. To give it a final pre-rebase touch,
+# an empty commit is added on top. The pre-rebase commit history looks like
+# this:
+#
+# Add subproject master - master4 - files_subtree/master5 - Empty commit
+#
+# where the root commit adds three files: master1.t, master2.t and master3.t.
+#
+# This commit history is then rebased onto `master3` with the
+# `-Xsubtree=files_subtree` option in three different ways:
+#
+# 1. using `--preserve-merges`
+# 2. using `--preserve-merges` and --keep-empty
+# 3. without specifying a rebase backend
+
 test_expect_success 'setup' '
 	test_commit README &&
-	mkdir files &&
-	(
-		cd files &&
-		git init &&
-		test_commit master1 &&
-		test_commit master2 &&
-		test_commit master3
-	) &&
-	git fetch files master &&
-	git branch files-master FETCH_HEAD &&
-	git read-tree --prefix=files_subtree files-master &&
-	git checkout -- files_subtree &&
-	tree=$(git write-tree) &&
-	head=$(git rev-parse HEAD) &&
-	rev=$(git rev-parse --verify files-master^0) &&
-	commit=$(git commit-tree -p $head -p $rev -m "Add subproject master" $tree) &&
-	git update-ref HEAD $commit &&
-	(
-		cd files_subtree &&
-		test_commit master4
-	) &&
-	test_commit files_subtree/master5
-'
 
-# FAILURE: Does not preserve master4.
-test_expect_failure REBASE_P \
-	'Rebase -Xsubtree --preserve-merges --onto commit 4' '
-	reset_rebase &&
-	git checkout -b rebase-preserve-merges-4 master &&
-	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
-	git commit -m "Empty commit" --allow-empty &&
-	git rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master &&
-	verbose test "$(commit_message HEAD~)" = "files_subtree/master4"
+	git init files &&
+	test_commit -C files master1 &&
+	test_commit -C files master2 &&
+	test_commit -C files master3 &&
+
+	: perform subtree merge into files_subtree/ &&
+	git fetch files refs/heads/master:refs/heads/files-master &&
+	git merge -s ours --no-commit --allow-unrelated-histories \
+		files-master &&
+	git read-tree --prefix=files_subtree -u files-master &&
+	git commit -m "Add subproject master" &&
+
+	: add two extra commits to rebase &&
+	test_commit -C files_subtree master4 &&
+	test_commit files_subtree/master5 &&
+
+	git checkout -b to-rebase &&
+	git fast-export --no-data HEAD -- files_subtree/ |
+		sed -e "s%\([0-9a-f]\{40\} \)files_subtree/%\1%" |
+		git fast-import --force --quiet &&
+	git reset --hard &&
+	git commit -m "Empty commit" --allow-empty
 '
 
-# FAILURE: Does not preserve master5.
-test_expect_failure REBASE_P \
-	'Rebase -Xsubtree --preserve-merges --onto commit 5' '
+# FAILURE: Does not preserve master4.
+test_expect_failure REBASE_P 'Rebase -Xsubtree --preserve-merges --onto commit' '
 	reset_rebase &&
-	git checkout -b rebase-preserve-merges-5 master &&
-	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
-	git commit -m "Empty commit" --allow-empty &&
+	git checkout -b rebase-preserve-merges to-rebase &&
 	git rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master &&
+	verbose test "$(commit_message HEAD~)" = "master4" &&
 	verbose test "$(commit_message HEAD)" = "files_subtree/master5"
 '
 
 # FAILURE: Does not preserve master4.
-test_expect_failure REBASE_P \
-	'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit 4' '
-	reset_rebase &&
-	git checkout -b rebase-keep-empty-4 master &&
-	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
-	git commit -m "Empty commit" --allow-empty &&
-	git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master &&
-	verbose test "$(commit_message HEAD~2)" = "files_subtree/master4"
-'
-
-# FAILURE: Does not preserve master5.
-test_expect_failure REBASE_P \
-	'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit 5' '
-	reset_rebase &&
-	git checkout -b rebase-keep-empty-5 master &&
-	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
-	git commit -m "Empty commit" --allow-empty &&
-	git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master &&
-	verbose test "$(commit_message HEAD~)" = "files_subtree/master5"
-'
-
-# FAILURE: Does not preserve Empty.
-test_expect_failure REBASE_P \
-	'Rebase -Xsubtree --keep-empty --preserve-merges --onto empty commit' '
+test_expect_failure REBASE_P 'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit' '
 	reset_rebase &&
-	git checkout -b rebase-keep-empty-empty master &&
-	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
-	git commit -m "Empty commit" --allow-empty &&
+	git checkout -b rebase-keep-empty to-rebase &&
 	git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master &&
+	verbose test "$(commit_message HEAD~2)" = "master4" &&
+	verbose test "$(commit_message HEAD~)" = "files_subtree/master5" &&
 	verbose test "$(commit_message HEAD)" = "Empty commit"
 '
 
-# FAILURE: fatal: Could not parse object
-test_expect_failure 'Rebase -Xsubtree --onto commit 4' '
+test_expect_success 'Rebase -Xsubtree --keep-empty --onto commit' '
 	reset_rebase &&
-	git checkout -b rebase-onto-4 master &&
-	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
-	git commit -m "Empty commit" --allow-empty &&
-	git rebase -Xsubtree=files_subtree --onto files-master master &&
-	verbose test "$(commit_message HEAD~2)" = "files_subtree/master4"
+	git checkout -b rebase-onto to-rebase &&
+	test_must_fail git rebase -Xsubtree=files_subtree --keep-empty --onto files-master master &&
+	: first pick results in no changes &&
+	git rebase --continue &&
+	verbose test "$(commit_message HEAD~2)" = "master4" &&
+	verbose test "$(commit_message HEAD~)" = "files_subtree/master5" &&
+	verbose test "$(commit_message HEAD)" = "Empty commit"
 '
 
-# FAILURE: fatal: Could not parse object
-test_expect_failure 'Rebase -Xsubtree --onto commit 5' '
-	reset_rebase &&
-	git checkout -b rebase-onto-5 master &&
-	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
-	git commit -m "Empty commit" --allow-empty &&
-	git rebase -Xsubtree=files_subtree --onto files-master master &&
-	verbose test "$(commit_message HEAD~)" = "files_subtree/master5"
-'
-# FAILURE: fatal: Could not parse object
-test_expect_failure 'Rebase -Xsubtree --onto empty commit' '
+test_expect_success 'Rebase -Xsubtree --keep-empty --rebase-merges --onto commit' '
 	reset_rebase &&
-	git checkout -b rebase-onto-empty master &&
-	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
-	git commit -m "Empty commit" --allow-empty &&
-	git rebase -Xsubtree=files_subtree --onto files-master master &&
+	git checkout -b rebase-merges-onto to-rebase &&
+	test_must_fail git rebase -Xsubtree=files_subtree --keep-empty --rebase-merges --onto files-master --root &&
+	: first pick results in no changes &&
+	git rebase --continue &&
+	verbose test "$(commit_message HEAD~2)" = "master4" &&
+	verbose test "$(commit_message HEAD~)" = "files_subtree/master5" &&
 	verbose test "$(commit_message HEAD)" = "Empty commit"
 '
 
diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh
@@ -441,4 +441,25 @@ test_expect_success '--continue after resolving conflicts after a merge' '
 	test_path_is_missing .git/MERGE_HEAD
 '
 
+test_expect_success '--rebase-merges with strategies' '
+	git checkout -b with-a-strategy F &&
+	test_tick &&
+	git merge -m "Merge conflicting-G" conflicting-G &&
+
+	: first, test with a merge strategy option &&
+	git rebase -ir -Xtheirs G &&
+	echo conflicting-G >expect &&
+	test_cmp expect G.t &&
+
+	: now, try with a merge strategy other than recursive &&
+	git reset --hard @{1} &&
+	write_script git-merge-override <<-\EOF &&
+	echo overridden$1 >>G.t
+	git add G.t
+	EOF
+	PATH="$PWD:$PATH" git rebase -ir -s override -Xxopt G &&
+	test_write_lines G overridden--xopt >expect &&
+	test_cmp expect G.t
+'
+
 test_done
