Skip to contentSkip to conversation

Debug merge-recursive.[ch]

PATCH Superseded Elijah Newren via GitGitGadget 4 participants 31 messages Mar 31, 2025 – Apr 8, 2025

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

cover Debug merge-recursive.[ch]

As a wise man once told me, "Deleted code is debugged code!"

This series does some preparation, then moves the code shared between merge-recursive and merge-ort from the former to the latter, and then debugs the remainder of merge-recursive.[ch].

Series overview:

* Patches 1-5: Preparation; switch remaining callers of merge-recursive.[ch] functions to merge-ort equivalents, and add sole remaining missing feature (diff-algorithm selection)
* Patch 6: Nuke merge-recursive.[ch]
* Patch 7-8: Cleanup testsuite; we don't need GIT_TEST_MERGE_ALGORITHM anymore

While the diffstat might look large, the non-test code changes are actually pretty small. The drivers of the big diffstat are:

* We move a significant chunk of shared code from merge-recursive.[ch] to merge-ort.[ch], without modifying it
* We delete (the remainder of) merge-recursive.[ch]
* We rip out all the temporary GIT_TEST_MERGE_ALGORITHM stuff designed to let us reuse tests between recursive and ort

Elijah Newren (8):
checkout: replace merge_trees() with merge_ort_nonrecursive() builtin/merge-recursive: switch to using merge_ort_generic()
merge-ort: enable diff-algorithms other than histogram
sequencer: switch non-recursive merges over to ort
merge, sequencer: switch recursive merges over to ort merge-recursive.[ch]: thoroughly debug these
tests: remove GIT_TEST_MERGE_ALGORITHM and test_expect_merge_algorithm builtin/{merge,rebase,revert}: remove GIT_TEST_MERGE_ALGORITHM

 Documentation/merge-strategies.adoc           |   51 +-
 Documentation/technical/sparse-checkout.adoc  |    2 -
 Makefile                                      |    1 -
 builtin/checkout.c                            |   10 +-
 builtin/merge-recursive.c                     |    4 +-
 builtin/merge.c                               |   23 +-
 builtin/rebase.c                              |    5 -
 builtin/revert.c                              |    2 -
 ci/run-build-and-tests.sh                     |    1 -
 merge-ort-wrappers.h                          |    2 +-
 merge-ort.c                                   |  162 +-
 merge-ort.h                                   |   60 +-
 merge-recursive.c                             | 4079 -----------------
 merge-recursive.h                             |  132 -
 meson.build                                   |    1 -
 sequencer.c                                   |   58 +-
 t/lib-merge.sh                                |   13 -
 t/t1092-sparse-checkout-compatibility.sh      |    2 -
 t/t2501-cwd-empty.sh                          |    2 -
 t/t3512-cherry-pick-submodule.sh              |    5 -
 t/t3513-revert-submodule.sh                   |    4 -
 t/t4069-remerge-diff.sh                       |    7 -
 t/t4301-merge-tree-write-tree.sh              |    7 -
 t/t5572-pull-submodule.sh                     |    5 -
 t/t6400-merge-df.sh                           |   14 +-
 t/t6402-merge-rename.sh                       |  125 +-
 t/t6404-recursive-merge.sh                    |   21 +-
 t/t6406-merge-attr.sh                         |    7 +-
 t/t6416-recursive-corner-cases.sh             |  194 +-
 t/t6421-merge-partial-clone.sh                |    7 +-
 t/t6422-merge-rename-corner-cases.sh          |   31 +-
 t/t6423-merge-rename-directories.sh           |  517 +--
 t/t6424-merge-unrelated-index-changes.sh      |    8 +-
 t/t6426-merge-skip-unneeded-updates.sh        |    4 +-
 t/t6428-merge-conflicts-sparse.sh             |    2 -
 t/t6430-merge-recursive.sh                    |   46 +-
 t/t6434-merge-recursive-rename-options.sh     |   16 +-
 t/t6436-merge-overwrite.sh                    |   17 +-
 t/t6437-submodule-merge.sh                    |   65 +-
 t/t6438-submodule-directory-file-conflicts.sh |    5 -
 t/t6439-merge-co-error-msgs.sh                |    2 +-
 t/t7402-submodule-rebase.sh                   |    7 +-
 t/t7602-merge-octopus-many.sh                 |    9 +-
 t/t7610-mergetool.sh                          |   40 +-
 t/t7615-diff-algo-with-mergy-operations.sh    |    2 -
 t/test-lib.sh                                 |    2 -
 46 files changed, 538 insertions(+), 5241 deletions(-)
 delete mode 100644 merge-recursive.c
 delete mode 100644 merge-recursive.h
 delete mode 100644 t/lib-merge.sh

base-commit: 5b97a56fa0e7d580dc8865b73107407c9b3f0eff
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1898%2Fnewren%2Fendit-quote-debugging-unquote-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1898/newren/endit-quote-debugging-unquote-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1898

Signature · 1 line
gitgitgadget
#1 lore raw

PATCH 1/8 checkout: replace merge_trees() with merge_ort_nonrecursive()

From: Elijah Newren <[email protected]>

Replace the use of merge_trees() from merge-recursive.[ch] with the merge-ort equivalent.

---
 builtin/checkout.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
builtin/checkout.c +5 -5
index 01ea9ff8b28..67879e72362 100644
@@ -17,7 +17,7 @@
 #include "merge-ll.h"
 #include "lockfile.h"
 #include "mem-pool.h"
-#include "merge-recursive.h"
+#include "merge-ort-wrappers.h"
 #include "object-name.h"
 #include "object-store-ll.h"
 #include "parse-options.h"
@@ -907,10 +907,10 @@ static int merge_working_tree(const struct checkout_opts *opts,
 			o.branch1 = new_branch_info->name;
 			o.branch2 = "local";
 			o.conflict_style = opts->conflict_style;
-			ret = merge_trees(&o,
-					  new_tree,
-					  work,
-					  old_tree);
+			ret = merge_ort_nonrecursive(&o,
+						     new_tree,
+						     work,
+						     old_tree);
 			if (ret < 0)
 				exit(128);
 			ret = reset_tree(new_tree,
Signature · 1 line
gitgitgadget
#2 lore raw

PATCH 2/8 builtin/merge-recursive: switch to using merge_ort_generic()

In reply to Elijah Newren via GitGitGadget

From: Elijah Newren <[email protected]>

Switch from merge-recursive to merge-ort. Adjust the following testcases due to the switch:

* t6430: most of the test differences here were due to improved D/F conflict handling explained in more detail in ef527787089c (merge
tests: expect improved directory/file conflict handling in ort, 2020-10-26). These changes weren't made to this test back in that commit simply because I had been looking at git merge rather than git merge-recursive. The final test in this testsuite, though, was expunged because it was looking for specific output, and the calls to output_commit_title() were discarded from merge_ort_internal() in its adaptation from merge_recursive_internal(); see 8119214f4e70 (merge-ort: implement merge_incore_recursive(), 2020-12-16).

* t6436: This test is built entirely around rename/delete conflicts, which had a suboptimal handling under merge-recursive. As explained in more detail in commits 1f3c9ba707 ("t6425: be more flexible with rename/delete conflict messages", 2020-08-10) and 727c75b23f ("t6404,
t6423: expect improved rename/delete handling in ort backend", 2020-10-26), rename/delete conflicts should each have two entries in the index rather than just one. Adjust the expectations for all the tests in this testcase to see the two entries per rename/delete conflict.

* t6424: merge-recursive had a special check-if-toplevel-trees-match check that it ran at the beginning on both the merge-base and the other side being merged in. In such a case, it exited early and printed an "Already up to date." message. merge-ort got rid of this, and instead checks the merge base tree matching the other side throughout the tree instead of just at the toplevel, allowing it to avoid recursing into various subtrees. As part of that, it got rid of the specialty toplevel message. That message hasn't been missed for years from git merge, so I don't think it is necessary to keep it just for git merge-recursive, especially since the latter is rarely used. (git itself only references it in the testsuite, whereas it used to power one of the three rebase backends that existed once upon a time.)

---
 builtin/merge-recursive.c                 |  4 +--
 t/t6424-merge-unrelated-index-changes.sh  |  4 ++-
 t/t6430-merge-recursive.sh                | 43 +++++------------------
 t/t6434-merge-recursive-rename-options.sh | 16 +++++----
 4 files changed, 22 insertions(+), 45 deletions(-)
builtin/merge-recursive.c +2 -2
index abfc060e28a..03b5100cfae 100644
@@ -3,7 +3,7 @@
 #include "advice.h"
 #include "gettext.h"
 #include "hash.h"
-#include "merge-recursive.h"
+#include "merge-ort-wrappers.h"
 #include "object-name.h"
 
 static const char builtin_merge_recursive_usage[] =
@@ -89,7 +89,7 @@ int cmd_merge_recursive(int argc,
 	if (o.verbosity >= 3)
 		printf(_("Merging %s with %s\n"), o.branch1, o.branch2);
 
-	failed = merge_recursive_generic(&o, &h1, &h2, bases_count, bases, &result);
+	failed = merge_ort_generic(&o, &h1, &h2, bases_count, bases, &result);
 
 	free(better1);
 	free(better2);
t/t6424-merge-unrelated-index-changes.sh +3 -1
index 7677c5f08d0..084bfe7e6ce 100755
@@ -176,9 +176,11 @@ test_expect_success 'merge-recursive, when index==head but head!=HEAD' '
 	# Make index match B
 	git diff C B -- | git apply --cached &&
 	test_when_finished "git clean -fd" &&  # Do not leave untracked around
+	git write-tree >index-before &&
 	# Merge B & F, with B as "head"
 	git merge-recursive A -- B F > out &&
-	test_grep "Already up to date" out
+	git write-tree >index-after &&
+	test_cmp index-before index-after
 '
 
 test_expect_success 'recursive, when file has staged changes not matching HEAD nor what a merge would give' '
t/t6430-merge-recursive.sh +8 -35

77 lines not on this page · raw message

t/t6434-merge-recursive-rename-options.sh +9 -7
index 6e913c30a13..5a6f74839cb 100755
@@ -34,7 +34,9 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 get_expected_stages () {
 	git checkout rename -- $1-new &&
 	git ls-files --stage $1-new >expected-stages-undetected-$1 &&
-	sed "s/ 0	/ 2	/" <expected-stages-undetected-$1 \
+	git ls-tree HEAD^ $1-old >tmp &&
+	git ls-tree HEAD  $1-new >>tmp &&
+	cat tmp | awk '{print $1 " " $3 " " NR "\t" '$1'"-new"}' \
 		>expected-stages-detected-$1 &&
 	git read-tree -u --reset HEAD
 }
@@ -51,11 +53,11 @@ rename_undetected () {
 
 check_common () {
 	git ls-files --stage >stages-actual &&
-	test_line_count = 4 stages-actual
+	test_line_count = $1 stages-actual
 }
 
 check_threshold_0 () {
-	check_common &&
+	check_common 8 &&
 	rename_detected 0 &&
 	rename_detected 1 &&
 	rename_detected 2 &&
@@ -63,7 +65,7 @@ check_threshold_0 () {
 }
 
 check_threshold_1 () {
-	check_common &&
+	check_common 7 &&
 	rename_undetected 0 &&
 	rename_detected 1 &&
 	rename_detected 2 &&
@@ -71,7 +73,7 @@ check_threshold_1 () {
 }
 
 check_threshold_2 () {
-	check_common &&
+	check_common 6 &&
 	rename_undetected 0 &&
 	rename_undetected 1 &&
 	rename_detected 2 &&
@@ -79,7 +81,7 @@ check_threshold_2 () {
 }
 
 check_exact_renames () {
-	check_common &&
+	check_common 5 &&
 	rename_undetected 0 &&
 	rename_undetected 1 &&
 	rename_undetected 2 &&
@@ -87,7 +89,7 @@ check_exact_renames () {
 }
 
 check_no_renames () {
-	check_common &&
+	check_common 4 &&
 	rename_undetected 0 &&
 	rename_undetected 1 &&
 	rename_undetected 2 &&
Signature · 1 line
gitgitgadget
#3 lore raw

PATCH 3/8 merge-ort: enable diff-algorithms other than histogram

In reply to Elijah Newren via GitGitGadget

From: Elijah Newren <[email protected]>

The ort merge strategy has always used the histogram diff algorithm. The recursive merge strategy, in contrast, defaults to the myers diff algorithm, while allowing it to be changed.

Change the ort merge strategy to allow different diff algorithms, by removing the hard coded value in merge_start() and instead just making it a default in init_merge_options(). Technically, this also changes the default diff algorithm for the recursive backend too, but we're going to remove the final callers of the recursive backend in the next two commits.

---
 Documentation/merge-strategies.adoc | 29 +++++++++++++++--------------
 merge-ort.c                         |  3 ---
 merge-recursive.c                   |  1 +
 3 files changed, 16 insertions(+), 17 deletions(-)
Documentation/merge-strategies.adoc +15 -14
index 59f5ae36ccb..8c87dd70210 100644
@@ -87,6 +87,20 @@ no-renames;;
 	configuration variable.
 	See also linkgit:git-diff[1] `--no-renames`.
 
+histogram;;
+	Deprecated synonym for `diff-algorithm=histogram`.
+
+patience;;
+	Deprecated synonym for `diff-algorithm=patience`.
+
+diff-algorithm=[histogram|minimal|myers|patience];;
+	Use a different diff algorithm while merging, which can help
+	avoid mismerges that occur due to unimportant matching lines
+	(such as braces from distinct functions).  See also
+	linkgit:git-diff[1] `--diff-algorithm`.  Note that `ort`
+	defaults to `diff-algorithm=histogram`, while regular diffs
+	currently default to the `diff.algorithm` config setting.
+
 subtree[=<path>];;
 	This option is a more advanced form of 'subtree' strategy, where
 	the strategy makes a guess on how two trees must be shifted to
@@ -111,20 +125,7 @@ recursive::
 For a path that is a submodule, the same caution as 'ort' applies to this
 strategy.
 +
-The 'recursive' strategy takes the same options as 'ort'.  However,
-there are two additional options that 'ort' ignores (not documented
-above) that are potentially useful with the 'recursive' strategy:
-
-patience;;
-	Deprecated synonym for `diff-algorithm=patience`.
-
-diff-algorithm=[patience|minimal|histogram|myers];;
-	Use a different diff algorithm while merging, which can help
-	avoid mismerges that occur due to unimportant matching lines
-	(such as braces from distinct functions).  See also
-	linkgit:git-diff[1] `--diff-algorithm`.  Note that `ort`
-	specifically uses `diff-algorithm=histogram`, while `recursive`
-	defaults to the `diff.algorithm` config setting.
+The 'recursive' strategy takes the same options as 'ort'.
 
 resolve::
 	This can only resolve two heads (i.e. the current branch
merge-ort.c +0 -3
index 2b7d86aa4ec..14a7ae4a6bf 100644
@@ -4957,9 +4957,6 @@ static void merge_start(struct merge_options *opt, struct merge_result *result)
 	}
 	trace2_region_leave("merge", "sanity checks", opt->repo);
 
-	/* Default to histogram diff.  Actually, just hardcode it...for now. */
-	opt->xdl_opts = DIFF_WITH_ALG(opt, HISTOGRAM_DIFF);
-
 	/* Handle attr direction stuff for renormalization */
 	if (opt->renormalize)
 		git_attr_set_direction(GIT_ATTR_CHECKOUT);
merge-recursive.c +1 -0
index 884ccf99a58..f3df127ad9b 100644
@@ -3981,6 +3981,7 @@ static void init_merge_options(struct merge_options *opt,
 	opt->renormalize = 0;
 
 	opt->conflict_style = -1;
+	opt->xdl_opts = DIFF_WITH_ALG(opt, HISTOGRAM_DIFF);
 
 	merge_recursive_config(opt, ui);
 	merge_verbosity = getenv("GIT_MERGE_VERBOSITY");
Signature · 1 line
gitgitgadget
#4 lore raw

PATCH 4/8 sequencer: switch non-recursive merges over to ort

In reply to Elijah Newren via GitGitGadget

From: Elijah Newren <[email protected]>

The do_recursive_merge() function, which is somewhat misleadingly named since its purpose in life is to do a *non*-recursive merge, had code to allow either using the recursive or ort backends. The default has been ort for a very long time, let's just remove the code path for allowing the recursive backend to be selected.

---
 sequencer.c | 35 +++++++++++++----------------------
 1 file changed, 13 insertions(+), 22 deletions(-)
sequencer.c +13 -22
index ad0ab75c8d4..b5d91fd3515 100644
@@ -781,28 +781,19 @@ static int do_recursive_merge(struct repository *r,
 	for (i = 0; i < opts->xopts.nr; i++)
 		parse_merge_opt(&o, opts->xopts.v[i]);
 
-	if (!opts->strategy || !strcmp(opts->strategy, "ort")) {
-		memset(&result, 0, sizeof(result));
-		merge_incore_nonrecursive(&o, base_tree, head_tree, next_tree,
-					    &result);
-		show_output = !is_rebase_i(opts) || !result.clean;
-		/*
-		 * TODO: merge_switch_to_result will update index/working tree;
-		 * we only really want to do that if !result.clean || this is
-		 * the final patch to be picked.  But determining this is the
-		 * final patch would take some work, and "head_tree" would need
-		 * to be replace with the tree the index matched before we
-		 * started doing any picks.
-		 */
-		merge_switch_to_result(&o, head_tree, &result, 1, show_output);
-		clean = result.clean;
-	} else {
-		ensure_full_index(r->index);
-		clean = merge_trees(&o, head_tree, next_tree, base_tree);
-		if (is_rebase_i(opts) && clean <= 0)
-			fputs(o.obuf.buf, stdout);
-		strbuf_release(&o.obuf);
-	}
+	memset(&result, 0, sizeof(result));
+	merge_incore_nonrecursive(&o, base_tree, head_tree, next_tree, &result);
+	show_output = !is_rebase_i(opts) || !result.clean;
+	/*
+	 * TODO: merge_switch_to_result will update index/working tree;
+	 * we only really want to do that if !result.clean || this is
+	 * the final patch to be picked.  But determining this is the
+	 * final patch would take some work, and "head_tree" would need
+	 * to be replace with the tree the index matched before we
+	 * started doing any picks.
+	 */
+	merge_switch_to_result(&o, head_tree, &result, 1, show_output);
+	clean = result.clean;
 	if (clean < 0) {
 		rollback_lock_file(&index_lock);
 		return clean;
Signature · 1 line
gitgitgadget
#5 lore raw

PATCH 5/8 merge, sequencer: switch recursive merges over to ort

In reply to Elijah Newren via GitGitGadget

From: Elijah Newren <[email protected]>

More precisely, replace calls to merge_recursive() with merge_ort_recursive().

Also change t7615 to quit calling out recursive; it is not needed anymore, and we are in fact using ort now.

---
 builtin/merge.c                            |  9 ++-------
 sequencer.c                                | 23 ++++++++--------------
 t/t7615-diff-algo-with-mergy-operations.sh |  2 --
 3 files changed, 10 insertions(+), 24 deletions(-)
builtin/merge.c +2 -7
index ba9faf126aa..c0bbdab7104 100644
@@ -39,7 +39,6 @@
 #include "rerere.h"
 #include "help.h"
 #include "merge.h"
-#include "merge-recursive.h"
 #include "merge-ort-wrappers.h"
 #include "resolve-undo.h"
 #include "remote.h"
@@ -750,12 +749,8 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
 
 		repo_hold_locked_index(the_repository, &lock,
 				       LOCK_DIE_ON_ERROR);
-		if (!strcmp(strategy, "ort"))
-			clean = merge_ort_recursive(&o, head, remoteheads->item,
-						    reversed, &result);
-		else
-			clean = merge_recursive(&o, head, remoteheads->item,
-						reversed, &result);
+		clean = merge_ort_recursive(&o, head, remoteheads->item,
+					    reversed, &result);
 		free_commit_list(reversed);
 		strbuf_release(&o.obuf);
 
sequencer.c +8 -15
index b5d91fd3515..8bb49735891 100644
@@ -4319,20 +4319,13 @@ static int do_merge(struct repository *r,
 	o.branch2 = ref_name.buf;
 	o.buffer_output = 2;
 
-	if (!opts->strategy || !strcmp(opts->strategy, "ort")) {
-		/*
-		 * TODO: Should use merge_incore_recursive() and
-		 * merge_switch_to_result(), skipping the call to
-		 * merge_switch_to_result() when we don't actually need to
-		 * update the index and working copy immediately.
-		 */
-		ret = merge_ort_recursive(&o,
-					  head_commit, merge_commit, bases,
-					  &i);
-	} else {
-		ret = merge_recursive(&o, head_commit, merge_commit, bases,
-				      &i);
-	}
+	/*
+	 * TODO: Should use merge_incore_recursive() and
+	 * merge_switch_to_result(), skipping the call to
+	 * merge_switch_to_result() when we don't actually need to
+	 * update the index and working copy immediately.
+	 */
+	ret = merge_ort_recursive(&o, head_commit, merge_commit, bases, &i);
 	if (ret <= 0)
 		fputs(o.obuf.buf, stdout);
 	strbuf_release(&o.obuf);
@@ -4343,7 +4336,7 @@ static int do_merge(struct repository *r,
 		goto leave_merge;
 	}
 	/*
-	 * The return value of merge_recursive() is 1 on clean, and 0 on
+	 * The return value of merge_ort_recursive() is 1 on clean, and 0 on
 	 * unclean merge.
 	 *
 	 * Let's reverse that, so that do_merge() returns 0 upon success and
t/t7615-diff-algo-with-mergy-operations.sh +0 -2
index ac5863e788c..5822d02d517 100755
@@ -22,8 +22,6 @@ test_expect_success 'setup' '
 	git tag c2
 '
 
-GIT_TEST_MERGE_ALGORITHM=recursive
-
 test_expect_success 'merge c2 to c1 with recursive merge strategy fails with the current default myers diff algorithm' '
 	git reset --hard c1 &&
 	test_must_fail git merge -s recursive -Xdiff-algorithm=myers c2
Signature · 1 line
gitgitgadget
#6 lore raw

PATCH 6/8 merge-recursive.[ch]: thoroughly debug these

In reply to Elijah Newren via GitGitGadget

From: Elijah Newren <[email protected]>

As a wise man once told me, "Deleted code is debugged code!" So, move the functions that are shared between merge-recursive and merge-ort from the former to the latter, and then debug the remainder of merge-recursive.[ch].

---
 Documentation/merge-strategies.adoc          |   24 +-
 Documentation/technical/sparse-checkout.adoc |    2 -
 Makefile                                     |    1 -
 merge-ort-wrappers.h                         |    2 +-
 merge-ort.c                                  |  159 +
 merge-ort.h                                  |   60 +-
 merge-recursive.c                            | 4080 ------------------
 merge-recursive.h                            |  132 -
 meson.build                                  |    1 -
 9 files changed, 225 insertions(+), 4236 deletions(-)
 delete mode 100644 merge-recursive.c
 delete mode 100644 merge-recursive.h
Documentation/merge-strategies.adoc +6 -18
index 8c87dd70210..9e034f447e7 100644
@@ -109,23 +109,11 @@ subtree[=<path>];;
 	two trees to match.
 
 recursive::
-	This can only resolve two heads using a 3-way merge
-	algorithm.  When there is more than one common
-	ancestor that can be used for 3-way merge, it creates a
-	merged tree of the common ancestors and uses that as
-	the reference tree for the 3-way merge.  This has been
-	reported to result in fewer merge conflicts without
-	causing mismerges by tests done on actual merge commits
-	taken from Linux 2.6 kernel development history.
-	Additionally this can detect and handle merges involving
-	renames.  It does not make use of detected copies.  This was
-	the default strategy for resolving two heads from Git v0.99.9k
-	until v2.33.0.
-+
-For a path that is a submodule, the same caution as 'ort' applies to this
-strategy.
-+
-The 'recursive' strategy takes the same options as 'ort'.
+	This is now a synonym for `ort`.  It was an alternative
+	implementation until v2.49.0, but was redirected to mean `ort`
+	in v2.50.0.  The previous recursive strategy was the default
+	strategy for resolving two heads from Git v0.99.9k until
+	v2.33.0.
 
 resolve::
 	This can only resolve two heads (i.e. the current branch
@@ -146,7 +134,7 @@ ours::
 	ignoring all changes from all other branches.  It is meant to
 	be used to supersede old development history of side
 	branches.  Note that this is different from the -Xours option to
-	the 'recursive' merge strategy.
+	the 'ort' merge strategy.
 
 subtree::
 	This is a modified `ort` strategy. When merging trees A and
Documentation/technical/sparse-checkout.adoc +0 -2
index d9686593548..dc2e763bbec 100644
@@ -356,8 +356,6 @@ understanding these differences can be beneficial.
     The behavior for these commands somewhat depends upon the merge
     strategy being used:
       * `ort` behaves as described above
-      * `recursive` tries to not vivify files unnecessarily, but does sometimes
-	vivify files without conflicts.
       * `octopus` and `resolve` will always vivify any file changed in the merge
 	relative to the first parent, which is rather suboptimal.
 
Makefile +0 -1
index ac32d2d0bda..4fbd29cc7e9 100644
@@ -1069,7 +1069,6 @@ LIB_OBJS += merge-blobs.o
 LIB_OBJS += merge-ll.o
 LIB_OBJS += merge-ort.o
 LIB_OBJS += merge-ort-wrappers.o
-LIB_OBJS += merge-recursive.o
 LIB_OBJS += merge.o
 LIB_OBJS += midx.o
 LIB_OBJS += midx-write.o
merge-ort-wrappers.h +1 -1
index aeffa1c87b4..b7e1ced9d7f 100644
@@ -1,7 +1,7 @@
 #ifndef MERGE_ORT_WRAPPERS_H
 #define MERGE_ORT_WRAPPERS_H
 
-#include "merge-recursive.h"
+#include "merge-ort.h"
 
 /*
  * rename-detecting three-way merge, no recursion.
merge-ort.c +159 -0

168 lines not on this page · raw message

merge-ort.h +59 -1

79 lines not on this page · raw message

merge-recursive.c deleted +0 -4080

2,694 lines not on this page · raw message

merge-recursive.h deleted +0 -132

132 lines not on this page · raw message

meson.build +0 -1
index e98cfa4909f..8448df9d15e 100644

Lines of this file are not shown. View the raw message.

The rest of this diff is not shown. View the raw message.

Signature · 1 line
gitgitgadget

Part of the diff is not shown here. View the raw message.

#7 lore raw

PATCH 7/8 tests: remove GIT_TEST_MERGE_ALGORITHM and test_expect_merge_algorithm

In reply to Elijah Newren via GitGitGadget

From: Elijah Newren <[email protected]>

Both of these existed to allow use to reuse all the merge-related tests in the testsuite while easily flipping between the 'recursive' and the 'ort' backends. Now that we have removed merge-recursive and remapped 'recursive' to mean 'ort', we don't need this scaffolding anymore.

---
 ci/run-build-and-tests.sh                     |   1 -
 t/lib-merge.sh                                |  13 -
 t/t1092-sparse-checkout-compatibility.sh      |   2 -
 t/t2501-cwd-empty.sh                          |   2 -
 t/t3512-cherry-pick-submodule.sh              |   5 -
 t/t3513-revert-submodule.sh                   |   4 -
 t/t4069-remerge-diff.sh                       |   7 -
 t/t4301-merge-tree-write-tree.sh              |   7 -
 t/t5572-pull-submodule.sh                     |   5 -
 t/t6400-merge-df.sh                           |  14 +-
 t/t6402-merge-rename.sh                       | 125 +----
 t/t6404-recursive-merge.sh                    |  21 +-
 t/t6406-merge-attr.sh                         |   7 +-
 t/t6416-recursive-corner-cases.sh             | 194 ++-----
 t/t6421-merge-partial-clone.sh                |   7 +-
 t/t6422-merge-rename-corner-cases.sh          |  31 +-
 t/t6423-merge-rename-directories.sh           | 517 ++++--------------
 t/t6424-merge-unrelated-index-changes.sh      |   4 +-
 t/t6426-merge-skip-unneeded-updates.sh        |   4 +-
 t/t6428-merge-conflicts-sparse.sh             |   2 -
 t/t6430-merge-recursive.sh                    |   3 +-
 t/t6436-merge-overwrite.sh                    |  17 +-
 t/t6437-submodule-merge.sh                    |  65 +--
 t/t6438-submodule-directory-file-conflicts.sh |   5 -
 t/t6439-merge-co-error-msgs.sh                |   2 +-
 t/t7402-submodule-rebase.sh                   |   7 +-
 t/t7602-merge-octopus-many.sh                 |   9 +-
 t/t7610-mergetool.sh                          |  40 +-
 t/test-lib.sh                                 |   2 -
 29 files changed, 248 insertions(+), 874 deletions(-)
 delete mode 100644 t/lib-merge.sh
ci/run-build-and-tests.sh +0 -1
index 1c698467235..f99b7db2ee8 100755
@@ -20,7 +20,6 @@ linux-breaking-changes)
 linux-TEST-vars)
 	export OPENSSL_SHA1_UNSAFE=YesPlease
 	export GIT_TEST_SPLIT_INDEX=yes
-	export GIT_TEST_MERGE_ALGORITHM=recursive
 	export GIT_TEST_FULL_IN_PACK_ARRAY=true
 	export GIT_TEST_OE_SIZE=10
 	export GIT_TEST_OE_DELTA_SIZE=5
t/lib-merge.sh deleted +0 -13
deleted file mode 100644
index 8734ebfc17f..00000000000
@@ -1,13 +0,0 @@
-# Helper functions used by merge tests.
-
-test_expect_merge_algorithm () {
-	status_for_recursive=$1 status_for_ort=$2
-	shift 2
-
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		test_expect_${status_for_ort} "$@"
-	else
-		test_expect_${status_for_recursive} "$@"
-	fi
-}
t/t1092-sparse-checkout-compatibility.sh +0 -2
index a4c7c41fc00..f9b448792cb 100755
@@ -1486,7 +1486,6 @@ test_expect_success 'sparse-index is not expanded' '
 	ensure_not_expanded checkout -f update-deep &&
 	test_config -C sparse-index pull.twohead ort &&
 	(
-		sane_unset GIT_TEST_MERGE_ALGORITHM &&
 		for OPERATION in "merge -m merge" cherry-pick rebase
 		do
 			ensure_not_expanded merge -m merge update-folder1 &&
@@ -1506,7 +1505,6 @@ test_expect_success 'sparse-index is not expanded: merge conflict in cone' '
 	done &&
 
 	(
-		sane_unset GIT_TEST_MERGE_ALGORITHM &&
 		git -C sparse-index config pull.twohead ort &&
 		ensure_not_expanded ! merge -m merged expand-right
 	)
t/t2501-cwd-empty.sh +0 -2
index f6d8d7d03d7..be9140bbaa4 100755
@@ -117,8 +117,6 @@ test_expect_success 'merge fails if cwd needs to be removed; recursive friendly'
 	grep "Refusing to remove the current working directory" error
 '
 
-GIT_TEST_MERGE_ALGORITHM=ort
-
 test_expect_success 'merge fails if cwd needs to be removed' '
 	test_required_dir_removal git merge fd_conflict
 '
t/t3512-cherry-pick-submodule.sh +0 -5
index f22d1ddead1..85a79326977 100755
@@ -8,11 +8,6 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-submodule-update.sh
 
-if test "$GIT_TEST_MERGE_ALGORITHM" != ort
-then
-	KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1
-	KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1
-fi
 test_submodule_switch "cherry-pick"
 
 test_expect_success 'unrelated submodule/file conflict is ignored' '
t/t3513-revert-submodule.sh +0 -4
index 8bfe3ed2467..32e15c72eee 100755
@@ -30,10 +30,6 @@ git_revert () {
 	git revert HEAD
 }
 
-if test "$GIT_TEST_MERGE_ALGORITHM" != ort
-then
-	KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1
-fi
 test_submodule_switch_func "git_revert"
 
 test_done
t/t4069-remerge-diff.sh +0 -7
index c6c94aef14d..966882ce912 100755
@@ -4,13 +4,6 @@ test_description='remerge-diff handling'
 
 . ./test-lib.sh
 
-# This test is ort-specific
-if test "${GIT_TEST_MERGE_ALGORITHM}" != ort
-then
-	skip_all="GIT_TEST_MERGE_ALGORITHM != ort"
-	test_done
-fi
-
 test_expect_success 'setup basic merges' '
 	test_write_lines 1 2 3 4 5 6 7 8 9 >numbers &&
 	git add numbers &&
t/t4301-merge-tree-write-tree.sh +0 -7
index 44f7d077593..f9c5883a7f7 100755
@@ -4,13 +4,6 @@ test_description='git merge-tree --write-tree'
 
 . ./test-lib.sh
 
-# This test is ort-specific
-if test "$GIT_TEST_MERGE_ALGORITHM" != "ort"
-then
-	skip_all="GIT_TEST_MERGE_ALGORITHM != ort"
-	test_done
-fi
-
 test_expect_success setup '
 	test_write_lines 1 2 3 4 5 >numbers &&
 	echo hello >greeting &&
t/t5572-pull-submodule.sh +0 -5
index f7650e8475e..45f384dd328 100755
@@ -45,11 +45,6 @@ git_pull_noff () {
 	$2 git pull --no-ff
 }
 
-if test "$GIT_TEST_MERGE_ALGORITHM" != ort
-then
-	KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1
-	KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1
-fi
 test_submodule_switch_func "git_pull_noff"
 
 test_expect_success 'setup' '
t/t6400-merge-df.sh +2 -12
index 3de4ef6bd9e..b46fe494409 100755
@@ -84,12 +84,7 @@ test_expect_success 'modify/delete + directory/file conflict' '
 
 	test_stdout_line_count = 5 git ls-files -s &&
 	test_stdout_line_count = 4 git ls-files -u &&
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		test_stdout_line_count = 0 git ls-files -o
-	else
-		test_stdout_line_count = 1 git ls-files -o
-	fi &&
+	test_stdout_line_count = 0 git ls-files -o &&
 
 	test_path_is_file letters/file &&
 	test_path_is_file letters.txt &&
@@ -105,12 +100,7 @@ test_expect_success 'modify/delete + directory/file conflict; other way' '
 
 	test_stdout_line_count = 5 git ls-files -s &&
 	test_stdout_line_count = 4 git ls-files -u &&
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		test_stdout_line_count = 0 git ls-files -o
-	else
-		test_stdout_line_count = 1 git ls-files -o
-	fi &&
+	test_stdout_line_count = 0 git ls-files -o &&
 
 	test_path_is_file letters/file &&
 	test_path_is_file letters.txt &&
t/t6402-merge-rename.sh +28 -97

177 lines not on this page · raw message

t/t6404-recursive-merge.sh +3 -18
index ae687f2ce54..346f3608b98 100755
@@ -108,12 +108,7 @@ test_expect_success 'refuse to merge binary files' '
 	printf "\0\0" >binary-file &&
 	git add binary-file &&
 	git commit -m binary2 &&
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		test_must_fail git merge F >merge_output
-	else
-		test_must_fail git merge F 2>merge_output
-	fi &&
+	test_must_fail git merge F >merge_output &&
 	grep "Cannot merge binary files: binary-file (HEAD vs. F)" merge_output
 '
 
@@ -129,22 +124,12 @@ test_expect_success 'mark rename/delete as unmerged' '
 	test_tick &&
 	git commit -m rename &&
 	test_must_fail git merge delete &&
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		test 2 = $(git ls-files --unmerged | wc -l)
-	else
-		test 1 = $(git ls-files --unmerged | wc -l)
-	fi &&
+	test 2 = $(git ls-files --unmerged | wc -l) &&
 	git rev-parse --verify :2:a2 &&
 	test_must_fail git rev-parse --verify :3:a2 &&
 	git checkout -f delete &&
 	test_must_fail git merge rename &&
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		test 2 = $(git ls-files --unmerged | wc -l)
-	else
-		test 1 = $(git ls-files --unmerged | wc -l)
-	fi &&
+	test 2 = $(git ls-files --unmerged | wc -l) &&
 	test_must_fail git rev-parse --verify :2:a2 &&
 	git rev-parse --verify :3:a2
 '
t/t6406-merge-attr.sh +1 -6
index 66e01464b5e..8f6fbef0024 100755
@@ -259,12 +259,7 @@ test_expect_success 'binary files with union attribute' '
 	printf "two\0" >bin.txt &&
 	git commit -am two &&
 
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		test_must_fail git merge bin-main >output
-	else
-		test_must_fail git merge bin-main 2>output
-	fi &&
+	test_must_fail git merge bin-main >output &&
 	grep -i "warning.*cannot merge.*HEAD vs. bin-main" output
 '
 
t/t6416-recursive-corner-cases.sh +55 -139

278 lines not on this page · raw message

t/t6421-merge-partial-clone.sh +3 -4
index b99f29ef9ba..6eb51285a0c 100755
@@ -27,7 +27,6 @@ test_description="limiting blob downloads when merging with partial clones"
 #                     files that might be renamed into each other's paths.)
 
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/lib-merge.sh
 
 test_setup_repo () {
 	test -d server && return
@@ -207,7 +206,7 @@ test_setup_repo () {
 #
 #   Summary: 2 fetches (1 for 2 objects, 1 for 1 object)
 #
-test_expect_merge_algorithm failure success 'Objects downloaded for single relevant rename' '
+test_expect_success 'Objects downloaded for single relevant rename' '
 	test_setup_repo &&
 	git clone --sparse --filter=blob:none "file://$(pwd)/server" objects-single &&
 	(
@@ -297,7 +296,7 @@ test_expect_merge_algorithm failure success 'Objects downloaded for single relev
 #      this are not all that common.)
 #   Summary: 1 fetches for 6 objects
 #
-test_expect_merge_algorithm failure success 'Objects downloaded when a directory rename triggered' '
+test_expect_success 'Objects downloaded when a directory rename triggered' '
 	test_setup_repo &&
 	git clone --sparse --filter=blob:none "file://$(pwd)/server" objects-dir &&
 	(
@@ -399,7 +398,7 @@ test_expect_merge_algorithm failure success 'Objects downloaded when a directory
 #
 #   Summary: 4 fetches (1 for 6 objects, 1 for 8, 1 for 3, 1 for 2)
 #
-test_expect_merge_algorithm failure success 'Objects downloaded with lots of renames and modifications' '
+test_expect_success 'Objects downloaded with lots of renames and modifications' '
 	test_setup_repo &&
 	git clone --sparse --filter=blob:none "file://$(pwd)/server" objects-many &&
 	(
t/t6422-merge-rename-corner-cases.sh +7 -24
index 62b49c67e22..9cbe7ca7824 100755
@@ -7,7 +7,6 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/lib-merge.sh
 
 test_setup_rename_delete_untracked () {
 	git init rename-delete-untracked &&
@@ -316,12 +315,7 @@ test_expect_success 'rename/directory conflict + clean content merge' '
 		git ls-files -u >out &&
 		test_line_count = 1 out &&
 		git ls-files -o >out &&
-		if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-		then
-			test_line_count = 1 out
-		else
-			test_line_count = 2 out
-		fi &&
+		test_line_count = 1 out &&
 
 		echo 0 >expect &&
 		git cat-file -p base:file >>expect &&
@@ -350,12 +344,7 @@ test_expect_success 'rename/directory conflict + content merge conflict' '
 		git ls-files -u >out &&
 		test_line_count = 3 out &&
 		git ls-files -o >out &&
-		if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-		then
-			test_line_count = 1 out
-		else
-			test_line_count = 2 out
-		fi &&
+		test_line_count = 1 out &&
 
 		git cat-file -p left-conflict:newfile >left &&
 		git cat-file -p base:file    >base &&
@@ -369,14 +358,8 @@ test_expect_success 'rename/directory conflict + content merge conflict' '
 
 		git rev-parse >expect   \
 			base:file       left-conflict:newfile right:file &&
-		if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-		then
-			git rev-parse >actual \
-				:1:newfile~HEAD :2:newfile~HEAD :3:newfile~HEAD
-		else
-			git rev-parse >actual \
-				:1:newfile      :2:newfile      :3:newfile
-		fi &&
+		git rev-parse >actual \
+			:1:newfile~HEAD :2:newfile~HEAD :3:newfile~HEAD &&
 		test_cmp expect actual &&
 
 		test_path_is_file newfile/realfile &&
@@ -896,7 +879,7 @@ test_setup_rad () {
 	)
 }
 
-test_expect_merge_algorithm failure success 'rad-check: rename/add/delete conflict' '
+test_expect_success 'rad-check: rename/add/delete conflict' '
 	test_setup_rad &&
 	(
 		cd rad &&
@@ -969,7 +952,7 @@ test_setup_rrdd () {
 	)
 }
 
-test_expect_merge_algorithm failure success 'rrdd-check: rename/rename(2to1)/delete/delete conflict' '
+test_expect_success 'rrdd-check: rename/rename(2to1)/delete/delete conflict' '
 	test_setup_rrdd &&
 	(
 		cd rrdd &&
@@ -1058,7 +1041,7 @@ test_setup_mod6 () {
 	)
 }
 
-test_expect_merge_algorithm failure success 'mod6-check: chains of rename/rename(1to2) and rename/rename(2to1)' '
+test_expect_success 'mod6-check: chains of rename/rename(1to2) and rename/rename(2to1)' '
 	test_setup_mod6 &&
 	(
 		cd mod6 &&
t/t6423-merge-rename-directories.sh +105 -412

673 lines not on this page · raw message

t/t6424-merge-unrelated-index-changes.sh +1 -3
index 084bfe7e6ce..ddc7524f6c1 100755
@@ -296,10 +296,8 @@ test_expect_success 'with multiple strategies, recursive or ort failure do not e
 	git add a &&
 	git rev-parse :a >expect &&
 
-	sane_unset GIT_TEST_MERGE_ALGORITHM &&
-	test_must_fail git merge -s recursive -s ort -s octopus C^0 >output 2>&1 &&
+	test_must_fail git merge -s ort -s octopus C^0 >output 2>&1 &&
 
-	grep "Trying merge strategy recursive..." output &&
 	grep "Trying merge strategy ort..." output &&
 	grep "Trying merge strategy octopus..." output &&
 	grep "No merge strategy handled the merge." output &&
t/t6426-merge-skip-unneeded-updates.sh +1 -3
index b059475ed03..404cd3f2ca1 100755
@@ -23,8 +23,6 @@ test_description="merge cases"
 #                     files that might be renamed into each other's paths.)
 
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/lib-merge.sh
-
 
 ###########################################################################
 # SECTION 1: Cases involving no renames (one side has subset of changes of
@@ -663,7 +661,7 @@ test_setup_4a () {
 #   correct requires doing the merge in-memory first, then realizing that no
 #   updates to the file are necessary, and thus that we can just leave the path
 #   alone.
-test_expect_merge_algorithm failure success '4a: Change on A, change on B subset of A, dirty mods present' '
+test_expect_success '4a: Change on A, change on B subset of A, dirty mods present' '
 	test_setup_4a &&
 	(
 		cd 4a &&
t/t6428-merge-conflicts-sparse.sh +0 -2
index 9919c3fa7cd..c86e02c6e23 100755
@@ -23,8 +23,6 @@ test_description="merge cases"
 #                     files that might be renamed into each other's paths.)
 
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/lib-merge.sh
-
 
 # Testcase basic, conflicting changes in 'numerals'
 
t/t6430-merge-recursive.sh +1 -2
index c539a68fd07..e59560a5dd9 100755
@@ -6,7 +6,6 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/lib-merge.sh
 
 test_expect_success 'setup 1' '
 
@@ -645,7 +644,7 @@ test_expect_success 'merge-recursive copy vs. rename' '
 	test_cmp expected actual
 '
 
-test_expect_merge_algorithm failure success 'merge-recursive rename vs. rename/symlink' '
+test_expect_success 'merge-recursive rename vs. rename/symlink' '
 
 	git checkout -f rename &&
 	git merge rename-ln &&
t/t6436-merge-overwrite.sh +4 -13
index 4f4376421e7..70b5d2d6944 100755
@@ -101,19 +101,10 @@ test_expect_success 'will not overwrite unstaged changes in renamed file' '
 	git mv c1.c other.c &&
 	git commit -m rename &&
 	cp important other.c &&
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		test_must_fail git merge c1a >out 2>err &&
-		test_grep "would be overwritten by merge" err &&
-		test_cmp important other.c &&
-		test_path_is_missing .git/MERGE_HEAD
-	else
-		test_must_fail git merge c1a >out &&
-		test_grep "Refusing to lose dirty file at other.c" out &&
-		test_path_is_file other.c~HEAD &&
-		test $(git hash-object other.c~HEAD) = $(git rev-parse c1a:c1.c) &&
-		test_cmp important other.c
-	fi
+	test_must_fail git merge c1a >out 2>err &&
+	test_grep "would be overwritten by merge" err &&
+	test_cmp important other.c &&
+	test_path_is_missing .git/MERGE_HEAD
 '
 
 test_expect_success 'will not overwrite untracked subtree' '
t/t6437-submodule-merge.sh +20 -45

114 lines not on this page · raw message

t/t6438-submodule-directory-file-conflicts.sh +0 -5
index 8df67a0ef99..53d83c828a7 100755
@@ -12,11 +12,6 @@ test_submodule_switch "merge --ff"
 
 test_submodule_switch "merge --ff-only"
 
-if test "$GIT_TEST_MERGE_ALGORITHM" != ort
-then
-	KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1
-	KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1
-fi
 test_submodule_switch "merge --no-ff"
 
 test_done
t/t6439-merge-co-error-msgs.sh +1 -1
index 55bd744a3f4..643c9368e0e 100755
@@ -47,7 +47,7 @@ test_expect_success 'untracked files overwritten by merge (fast and non-fast for
 		export GIT_MERGE_VERBOSITY &&
 		test_must_fail git merge branch 2>out2
 	) &&
-	echo "Merge with strategy ${GIT_TEST_MERGE_ALGORITHM:-ort} failed." >>expect &&
+	echo "Merge with strategy ort failed." >>expect &&
 	test_cmp out2 expect &&
 	git reset --hard HEAD^
 '
t/t7402-submodule-rebase.sh +2 -5
index 25b33a1e875..06cee3432fe 100755
@@ -124,11 +124,8 @@ test_expect_success 'rebasing submodule that should conflict' '
 		echo "160000 $(git rev-parse HEAD) 3	submodule"
 	) >expect &&
 	test_cmp expect actual &&
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-    then
-		sub_expect="go to submodule (submodule), and either merge commit $(git -C submodule rev-parse --short HEAD^0)" &&
-		grep "$sub_expect" actual_output
-	fi
+	sub_expect="go to submodule (submodule), and either merge commit $(git -C submodule rev-parse --short HEAD^0)" &&
+	grep "$sub_expect" actual_output
 '
 
 test_done
t/t7602-merge-octopus-many.sh +3 -6
index ff085b086cc..42f675b7399 100755
@@ -77,12 +77,9 @@ Merge made by the 'recursive' strategy.
 EOF
 
 test_expect_success 'merge reduces irrelevant remote heads' '
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		mv expected expected.tmp &&
-		sed s/recursive/ort/ expected.tmp >expected &&
-		rm expected.tmp
-	fi &&
+	mv expected expected.tmp &&
+	sed s/recursive/ort/ expected.tmp >expected &&
+	rm expected.tmp &&
 	GIT_MERGE_VERBOSITY=0 git merge c4 c5 >actual &&
 	test_cmp expected actual
 '
t/t7610-mergetool.sh +11 -29
index c077aba7ced..957f8e20ba3 100755
@@ -535,14 +535,9 @@ test_expect_success 'file vs modified submodule' '
 	yes "" | git mergetool file1 file2 spaced\ name subdir/file3 &&
 	yes "" | git mergetool both &&
 	yes "d" | git mergetool file11 file12 &&
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		yes "c" | git mergetool submod~HEAD &&
-		git rm submod &&
-		git mv submod~HEAD submod
-	else
-		yes "l" | git mergetool submod
-	fi &&
+	yes "c" | git mergetool submod~HEAD &&
+	git rm submod &&
+	git mv submod~HEAD submod &&
 	git submodule update -N &&
 	echo "not a submodule" >expect &&
 	test_cmp expect submod &&
@@ -559,15 +554,10 @@ test_expect_success 'file vs modified submodule' '
 	yes "" | git mergetool file1 file2 spaced\ name subdir/file3 &&
 	yes "" | git mergetool both &&
 	yes "d" | git mergetool file11 file12 &&
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		mv submod submod.orig &&
-		git rm --cached submod &&
-		yes "c" | git mergetool submod~test19 &&
-		git mv submod~test19 submod
-	else
-		yes "r" | git mergetool submod
-	fi &&
+	mv submod submod.orig &&
+	git rm --cached submod &&
+	yes "c" | git mergetool submod~test19 &&
+	git mv submod~test19 submod &&
 	test -d submod.orig &&
 	git submodule update -N &&
 	echo "not a submodule" >expect &&
@@ -585,10 +575,7 @@ test_expect_success 'file vs modified submodule' '
 	yes "" | git mergetool both &&
 	yes "d" | git mergetool file11 file12 &&
 	yes "l" | git mergetool submod &&
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		yes "d" | git mergetool submod~test19
-	fi &&
+	yes "d" | git mergetool submod~test19 &&
 	echo "main submodule" >expect &&
 	test_cmp expect submod/bar &&
 	git submodule update -N &&
@@ -686,14 +673,9 @@ test_expect_success 'directory vs modified submodule' '
 	test_must_fail git merge main &&
 	test -n "$(git ls-files -u)" &&
 	test ! -e submod.orig &&
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		yes "r" | git mergetool submod~main &&
-		git mv submod submod.orig &&
-		git mv submod~main submod
-	else
-		yes "r" | git mergetool submod
-	fi &&
+	yes "r" | git mergetool submod~main &&
+	git mv submod submod.orig &&
+	git mv submod~main submod &&
 	test -d submod.orig &&
 	echo "not a submodule" >expect &&
 	test_cmp expect submod.orig/file16 &&
t/test-lib.sh +0 -2
index 9001ed3a647..5952eb56e27 100644
@@ -544,8 +544,6 @@ GIT_DEFAULT_HASH="${GIT_TEST_DEFAULT_HASH:-sha1}"
 export GIT_DEFAULT_HASH
 GIT_DEFAULT_REF_FORMAT="${GIT_TEST_DEFAULT_REF_FORMAT:-files}"
 export GIT_DEFAULT_REF_FORMAT
-GIT_TEST_MERGE_ALGORITHM="${GIT_TEST_MERGE_ALGORITHM:-ort}"
-export GIT_TEST_MERGE_ALGORITHM
 
 # Tests using GIT_TRACE typically don't want <timestamp> <file>:<line> output
 GIT_TRACE_BARE=1
Signature · 1 line
gitgitgadget
#10 lore raw

PATCH 8/8 builtin/{merge,rebase,revert}: remove GIT_TEST_MERGE_ALGORITHM

In reply to Elijah Newren via GitGitGadget

From: Elijah Newren <[email protected]>

This environment variable existed to allow the testsuite to reuse all the merge-related tests in the testsuite while easily flipping between the 'recursive' and the 'ort' backends. Now that we have removed merge-recursive and remapped 'recursive' to mean 'ort', we don't need this scaffolding anymore. Remove it from these three builtins.

---
 builtin/merge.c  | 14 +-------------
 builtin/rebase.c |  5 -----
 builtin/revert.c |  2 --
 3 files changed, 1 insertion(+), 20 deletions(-)
builtin/merge.c +1 -13
index c0bbdab7104..9efd585842f 100644
@@ -170,7 +170,7 @@ static struct strategy *get_strategy(const char *name)
 	struct strategy *ret;
 	static struct cmdnames main_cmds = {0}, other_cmds = {0};
 	static int loaded;
-	char *default_strategy = getenv("GIT_TEST_MERGE_ALGORITHM");
+	char *default_strategy = NULL;
 
 	if (!name)
 		return NULL;
@@ -1311,12 +1311,6 @@ int cmd_merge(int argc,
 	if (branch)
 		skip_prefix(branch, "refs/heads/", &branch);
 
-	if (!pull_twohead) {
-		char *default_strategy = getenv("GIT_TEST_MERGE_ALGORITHM");
-		if (default_strategy && !strcmp(default_strategy, "ort"))
-			pull_twohead = xstrdup("ort");
-	}
-
 	init_diff_ui_defaults();
 	git_config(git_merge_config, NULL);
 
@@ -1517,12 +1511,6 @@ int cmd_merge(int argc,
 			fast_forward = FF_NO;
 	}
 
-	if (!use_strategies && !pull_twohead &&
-	    remoteheads && !remoteheads->next) {
-		char *default_strategy = getenv("GIT_TEST_MERGE_ALGORITHM");
-		if (default_strategy)
-			append_strategy(get_strategy(default_strategy));
-	}
 	if (!use_strategies) {
 		if (!remoteheads)
 			; /* already up-to-date */
builtin/rebase.c +0 -5
index d4715ed35d7..e83193ac73e 100644
@@ -1575,11 +1575,6 @@ int cmd_rebase(int argc,
 			    options.default_backend);
 	}
 
-	if (options.type == REBASE_MERGE &&
-	    !options.strategy &&
-	    getenv("GIT_TEST_MERGE_ALGORITHM"))
-		options.strategy = xstrdup(getenv("GIT_TEST_MERGE_ALGORITHM"));
-
 	switch (options.type) {
 	case REBASE_MERGE:
 		options.state_dir = merge_dir();
builtin/revert.c +0 -2
index aca6c293cdf..2654f769a88 100644
@@ -252,8 +252,6 @@ static int run_sequencer(int argc, const char **argv, const char *prefix,
 		free(opts->strategy);
 		opts->strategy = xstrdup_or_null(strategy);
 	}
-	if (!opts->strategy && getenv("GIT_TEST_MERGE_ALGORITHM"))
-		opts->strategy = xstrdup(getenv("GIT_TEST_MERGE_ALGORITHM"));
 	free(options);
 
 	if (cmd == 'q') {
Signature · 1 line
gitgitgadget
#8 lore raw

Re: [PATCH 7/8] tests: remove GIT_TEST_MERGE_ALGORITHM and test_expect_merge_algorithm

In reply to Elijah Newren via GitGitGadget
Quoted 1 line · Elijah Newren via GitGitGadget

Both of these existed to allow use to reuse all the merge-related tests

s/use/us/

Quoted 5 lines

in the testsuite while easily flipping between the 'recursive' and the 'ort' backends. Now that we have removed merge-recursive and remapped 'recursive' to mean 'ort', we don't need this scaffolding anymore.

#9 lore raw

Re: [PATCH 7/8] tests: remove GIT_TEST_MERGE_ALGORITHM and test_expect_merge_algorithm

Quoted 6 lines · Eric Sunshine
Quoted 1 line · Elijah Newren via GitGitGadget

Both of these existed to allow use to reuse all the merge-related tests

s/use/us/

Indeed, thanks!

Quoted 5 lines
Quoted 5 lines

in the testsuite while easily flipping between the 'recursive' and the 'ort' backends. Now that we have removed merge-recursive and remapped 'recursive' to mean 'ort', we don't need this scaffolding anymore.

#11 lore raw

v2 cover Debug merge-recursive.[ch]

In reply to Elijah Newren via GitGitGadget

Changes since v1:

* fixed a couple of typos in commit messages

Original cover letter:

As a wise man once told me, "Deleted code is debugged code!"

This series does some preparation, then moves the code shared between merge-recursive and merge-ort from the former to the latter, and then debugs the remainder of merge-recursive.[ch].

Series overview:

* Patches 1-5: Preparation; switch remaining callers of merge-recursive.[ch] functions to merge-ort equivalents, and add sole remaining missing feature (diff-algorithm selection)
* Patch 6: Nuke merge-recursive.[ch]
* Patch 7-8: Cleanup testsuite; we don't need GIT_TEST_MERGE_ALGORITHM anymore

While the diffstat might look large, the non-test code changes are actually pretty small. The drivers of the big diffstat are:

* We move a significant chunk of shared code from merge-recursive.[ch] to merge-ort.[ch], without modifying it
* We delete (the remainder of) merge-recursive.[ch]
* We rip out all the temporary GIT_TEST_MERGE_ALGORITHM stuff designed to let us reuse tests between recursive and ort

Elijah Newren (8):
checkout: replace merge_trees() with merge_ort_nonrecursive() builtin/merge-recursive: switch to using merge_ort_generic()
merge-ort: enable diff-algorithms other than histogram
sequencer: switch non-recursive merges over to ort
merge, sequencer: switch recursive merges over to ort merge-recursive.[ch]: thoroughly debug these
tests: remove GIT_TEST_MERGE_ALGORITHM and test_expect_merge_algorithm builtin/{merge,rebase,revert}: remove GIT_TEST_MERGE_ALGORITHM

 Documentation/merge-strategies.adoc           |   51 +-
 Documentation/technical/sparse-checkout.adoc  |    2 -
 Makefile                                      |    1 -
 builtin/checkout.c                            |   10 +-
 builtin/merge-recursive.c                     |    4 +-
 builtin/merge.c                               |   23 +-
 builtin/rebase.c                              |    5 -
 builtin/revert.c                              |    2 -
 ci/run-build-and-tests.sh                     |    1 -
 merge-ort-wrappers.h                          |    2 +-
 merge-ort.c                                   |  162 +-
 merge-ort.h                                   |   60 +-
 merge-recursive.c                             | 4079 -----------------
 merge-recursive.h                             |  132 -
 meson.build                                   |    1 -
 sequencer.c                                   |   58 +-
 t/lib-merge.sh                                |   13 -
 t/t1092-sparse-checkout-compatibility.sh      |    2 -
 t/t2501-cwd-empty.sh                          |    2 -
 t/t3512-cherry-pick-submodule.sh              |    5 -
 t/t3513-revert-submodule.sh                   |    4 -
 t/t4069-remerge-diff.sh                       |    7 -
 t/t4301-merge-tree-write-tree.sh              |    7 -
 t/t5572-pull-submodule.sh                     |    5 -
 t/t6400-merge-df.sh                           |   14 +-
 t/t6402-merge-rename.sh                       |  125 +-
 t/t6404-recursive-merge.sh                    |   21 +-
 t/t6406-merge-attr.sh                         |    7 +-
 t/t6416-recursive-corner-cases.sh             |  194 +-
 t/t6421-merge-partial-clone.sh                |    7 +-
 t/t6422-merge-rename-corner-cases.sh          |   31 +-
 t/t6423-merge-rename-directories.sh           |  517 +--
 t/t6424-merge-unrelated-index-changes.sh      |    8 +-
 t/t6426-merge-skip-unneeded-updates.sh        |    4 +-
 t/t6428-merge-conflicts-sparse.sh             |    2 -
 t/t6430-merge-recursive.sh                    |   46 +-
 t/t6434-merge-recursive-rename-options.sh     |   16 +-
 t/t6436-merge-overwrite.sh                    |   17 +-
 t/t6437-submodule-merge.sh                    |   65 +-
 t/t6438-submodule-directory-file-conflicts.sh |    5 -
 t/t6439-merge-co-error-msgs.sh                |    2 +-
 t/t7402-submodule-rebase.sh                   |    7 +-
 t/t7602-merge-octopus-many.sh                 |    9 +-
 t/t7610-mergetool.sh                          |   40 +-
 t/t7615-diff-algo-with-mergy-operations.sh    |    2 -
 t/test-lib.sh                                 |    2 -
 46 files changed, 538 insertions(+), 5241 deletions(-)
 delete mode 100644 merge-recursive.c
 delete mode 100644 merge-recursive.h
 delete mode 100644 t/lib-merge.sh

base-commit: 5b97a56fa0e7d580dc8865b73107407c9b3f0eff
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1898%2Fnewren%2Fendit-quote-debugging-unquote-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1898/newren/endit-quote-debugging-unquote-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1898

Range-diff vs v1:

 1:  0e150ee9065 = 1:  0e150ee9065 checkout: replace merge_trees() with merge_ort_nonrecursive()
 2:  44be41da6c5 ! 2:  b093d74968b builtin/merge-recursive: switch to using merge_ort_generic()

@@ Commit message
adaptation from merge_recursive_internal(); see 8119214f4e70 (merge-ort: implement merge_incore_recursive(), 2020-12-16).

- * t6436: This test is built entirely around rename/delete conflicts,
+ * t6434: This test is built entirely around rename/delete conflicts,
which had a suboptimal handling under merge-recursive. As explained in more detail in commits 1f3c9ba707 ("t6425: be more flexible with rename/delete conflict messages", 2020-08-10) and 727c75b23f ("t6404,

 3:  3945c471b0c = 3:  cf774437123 merge-ort: enable diff-algorithms other than histogram
 4:  39ff4860fcd = 4:  6203589ac17 sequencer: switch non-recursive merges over to ort
 5:  91398ffd1e1 = 5:  8821f22d5ea merge, sequencer: switch recursive merges over to ort
 6:  6ef536cdbcd = 6:  770f611c2b6 merge-recursive.[ch]: thoroughly debug these
 7:  0b6bcd225dc ! 7:  a6501ee85fa tests: remove GIT_TEST_MERGE_ALGORITHM and test_expect_merge_algorithm

@@ Metadata
## Commit message ##
tests: remove GIT_TEST_MERGE_ALGORITHM and test_expect_merge_algorithm

- Both of these existed to allow use to reuse all the merge-related tests
+ Both of these existed to allow us to reuse all the merge-related tests
in the testsuite while easily flipping between the 'recursive' and the 'ort' backends. Now that we have removed merge-recursive and remapped 'recursive' to mean 'ort', we don't need this scaffolding anymore.
8: 3abcfe6faf0 = 8: d1dea986646 builtin/{merge,rebase,revert}: remove GIT_TEST_MERGE_ALGORITHM

Signature · 1 line
gitgitgadget
#12 lore raw

PATCH v2 1/8 checkout: replace merge_trees() with merge_ort_nonrecursive()

From: Elijah Newren <[email protected]>

Replace the use of merge_trees() from merge-recursive.[ch] with the merge-ort equivalent.

---
 builtin/checkout.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
builtin/checkout.c +5 -5
index 01ea9ff8b28..67879e72362 100644
@@ -17,7 +17,7 @@
 #include "merge-ll.h"
 #include "lockfile.h"
 #include "mem-pool.h"
-#include "merge-recursive.h"
+#include "merge-ort-wrappers.h"
 #include "object-name.h"
 #include "object-store-ll.h"
 #include "parse-options.h"
@@ -907,10 +907,10 @@ static int merge_working_tree(const struct checkout_opts *opts,
 			o.branch1 = new_branch_info->name;
 			o.branch2 = "local";
 			o.conflict_style = opts->conflict_style;
-			ret = merge_trees(&o,
-					  new_tree,
-					  work,
-					  old_tree);
+			ret = merge_ort_nonrecursive(&o,
+						     new_tree,
+						     work,
+						     old_tree);
 			if (ret < 0)
 				exit(128);
 			ret = reset_tree(new_tree,
Signature · 1 line
gitgitgadget
#13 lore raw

PATCH v2 2/8 builtin/merge-recursive: switch to using merge_ort_generic()

In reply to Elijah Newren via GitGitGadget

From: Elijah Newren <[email protected]>

Switch from merge-recursive to merge-ort. Adjust the following testcases due to the switch:

* t6430: most of the test differences here were due to improved D/F conflict handling explained in more detail in ef527787089c (merge
tests: expect improved directory/file conflict handling in ort, 2020-10-26). These changes weren't made to this test back in that commit simply because I had been looking at git merge rather than git merge-recursive. The final test in this testsuite, though, was expunged because it was looking for specific output, and the calls to output_commit_title() were discarded from merge_ort_internal() in its adaptation from merge_recursive_internal(); see 8119214f4e70 (merge-ort: implement merge_incore_recursive(), 2020-12-16).

* t6434: This test is built entirely around rename/delete conflicts, which had a suboptimal handling under merge-recursive. As explained in more detail in commits 1f3c9ba707 ("t6425: be more flexible with rename/delete conflict messages", 2020-08-10) and 727c75b23f ("t6404,
t6423: expect improved rename/delete handling in ort backend", 2020-10-26), rename/delete conflicts should each have two entries in the index rather than just one. Adjust the expectations for all the tests in this testcase to see the two entries per rename/delete conflict.

* t6424: merge-recursive had a special check-if-toplevel-trees-match check that it ran at the beginning on both the merge-base and the other side being merged in. In such a case, it exited early and printed an "Already up to date." message. merge-ort got rid of this, and instead checks the merge base tree matching the other side throughout the tree instead of just at the toplevel, allowing it to avoid recursing into various subtrees. As part of that, it got rid of the specialty toplevel message. That message hasn't been missed for years from git merge, so I don't think it is necessary to keep it just for git merge-recursive, especially since the latter is rarely used. (git itself only references it in the testsuite, whereas it used to power one of the three rebase backends that existed once upon a time.)

---
 builtin/merge-recursive.c                 |  4 +--
 t/t6424-merge-unrelated-index-changes.sh  |  4 ++-
 t/t6430-merge-recursive.sh                | 43 +++++------------------
 t/t6434-merge-recursive-rename-options.sh | 16 +++++----
 4 files changed, 22 insertions(+), 45 deletions(-)
builtin/merge-recursive.c +2 -2
index abfc060e28a..03b5100cfae 100644
@@ -3,7 +3,7 @@
 #include "advice.h"
 #include "gettext.h"
 #include "hash.h"
-#include "merge-recursive.h"
+#include "merge-ort-wrappers.h"
 #include "object-name.h"
 
 static const char builtin_merge_recursive_usage[] =
@@ -89,7 +89,7 @@ int cmd_merge_recursive(int argc,
 	if (o.verbosity >= 3)
 		printf(_("Merging %s with %s\n"), o.branch1, o.branch2);
 
-	failed = merge_recursive_generic(&o, &h1, &h2, bases_count, bases, &result);
+	failed = merge_ort_generic(&o, &h1, &h2, bases_count, bases, &result);
 
 	free(better1);
 	free(better2);
t/t6424-merge-unrelated-index-changes.sh +3 -1
index 7677c5f08d0..084bfe7e6ce 100755
@@ -176,9 +176,11 @@ test_expect_success 'merge-recursive, when index==head but head!=HEAD' '
 	# Make index match B
 	git diff C B -- | git apply --cached &&
 	test_when_finished "git clean -fd" &&  # Do not leave untracked around
+	git write-tree >index-before &&
 	# Merge B & F, with B as "head"
 	git merge-recursive A -- B F > out &&
-	test_grep "Already up to date" out
+	git write-tree >index-after &&
+	test_cmp index-before index-after
 '
 
 test_expect_success 'recursive, when file has staged changes not matching HEAD nor what a merge would give' '
t/t6430-merge-recursive.sh +8 -35

77 lines not on this page · raw message

t/t6434-merge-recursive-rename-options.sh +9 -7
index 6e913c30a13..5a6f74839cb 100755
@@ -34,7 +34,9 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 get_expected_stages () {
 	git checkout rename -- $1-new &&
 	git ls-files --stage $1-new >expected-stages-undetected-$1 &&
-	sed "s/ 0	/ 2	/" <expected-stages-undetected-$1 \
+	git ls-tree HEAD^ $1-old >tmp &&
+	git ls-tree HEAD  $1-new >>tmp &&
+	cat tmp | awk '{print $1 " " $3 " " NR "\t" '$1'"-new"}' \
 		>expected-stages-detected-$1 &&
 	git read-tree -u --reset HEAD
 }
@@ -51,11 +53,11 @@ rename_undetected () {
 
 check_common () {
 	git ls-files --stage >stages-actual &&
-	test_line_count = 4 stages-actual
+	test_line_count = $1 stages-actual
 }
 
 check_threshold_0 () {
-	check_common &&
+	check_common 8 &&
 	rename_detected 0 &&
 	rename_detected 1 &&
 	rename_detected 2 &&
@@ -63,7 +65,7 @@ check_threshold_0 () {
 }
 
 check_threshold_1 () {
-	check_common &&
+	check_common 7 &&
 	rename_undetected 0 &&
 	rename_detected 1 &&
 	rename_detected 2 &&
@@ -71,7 +73,7 @@ check_threshold_1 () {
 }
 
 check_threshold_2 () {
-	check_common &&
+	check_common 6 &&
 	rename_undetected 0 &&
 	rename_undetected 1 &&
 	rename_detected 2 &&
@@ -79,7 +81,7 @@ check_threshold_2 () {
 }
 
 check_exact_renames () {
-	check_common &&
+	check_common 5 &&
 	rename_undetected 0 &&
 	rename_undetected 1 &&
 	rename_undetected 2 &&
@@ -87,7 +89,7 @@ check_exact_renames () {
 }
 
 check_no_renames () {
-	check_common &&
+	check_common 4 &&
 	rename_undetected 0 &&
 	rename_undetected 1 &&
 	rename_undetected 2 &&
Signature · 1 line
gitgitgadget
#14 lore raw

PATCH v2 3/8 merge-ort: enable diff-algorithms other than histogram

In reply to Elijah Newren via GitGitGadget

From: Elijah Newren <[email protected]>

The ort merge strategy has always used the histogram diff algorithm. The recursive merge strategy, in contrast, defaults to the myers diff algorithm, while allowing it to be changed.

Change the ort merge strategy to allow different diff algorithms, by removing the hard coded value in merge_start() and instead just making it a default in init_merge_options(). Technically, this also changes the default diff algorithm for the recursive backend too, but we're going to remove the final callers of the recursive backend in the next two commits.

---
 Documentation/merge-strategies.adoc | 29 +++++++++++++++--------------
 merge-ort.c                         |  3 ---
 merge-recursive.c                   |  1 +
 3 files changed, 16 insertions(+), 17 deletions(-)
Documentation/merge-strategies.adoc +15 -14
index 59f5ae36ccb..8c87dd70210 100644
@@ -87,6 +87,20 @@ no-renames;;
 	configuration variable.
 	See also linkgit:git-diff[1] `--no-renames`.
 
+histogram;;
+	Deprecated synonym for `diff-algorithm=histogram`.
+
+patience;;
+	Deprecated synonym for `diff-algorithm=patience`.
+
+diff-algorithm=[histogram|minimal|myers|patience];;
+	Use a different diff algorithm while merging, which can help
+	avoid mismerges that occur due to unimportant matching lines
+	(such as braces from distinct functions).  See also
+	linkgit:git-diff[1] `--diff-algorithm`.  Note that `ort`
+	defaults to `diff-algorithm=histogram`, while regular diffs
+	currently default to the `diff.algorithm` config setting.
+
 subtree[=<path>];;
 	This option is a more advanced form of 'subtree' strategy, where
 	the strategy makes a guess on how two trees must be shifted to
@@ -111,20 +125,7 @@ recursive::
 For a path that is a submodule, the same caution as 'ort' applies to this
 strategy.
 +
-The 'recursive' strategy takes the same options as 'ort'.  However,
-there are two additional options that 'ort' ignores (not documented
-above) that are potentially useful with the 'recursive' strategy:
-
-patience;;
-	Deprecated synonym for `diff-algorithm=patience`.
-
-diff-algorithm=[patience|minimal|histogram|myers];;
-	Use a different diff algorithm while merging, which can help
-	avoid mismerges that occur due to unimportant matching lines
-	(such as braces from distinct functions).  See also
-	linkgit:git-diff[1] `--diff-algorithm`.  Note that `ort`
-	specifically uses `diff-algorithm=histogram`, while `recursive`
-	defaults to the `diff.algorithm` config setting.
+The 'recursive' strategy takes the same options as 'ort'.
 
 resolve::
 	This can only resolve two heads (i.e. the current branch
merge-ort.c +0 -3
index 2b7d86aa4ec..14a7ae4a6bf 100644
@@ -4957,9 +4957,6 @@ static void merge_start(struct merge_options *opt, struct merge_result *result)
 	}
 	trace2_region_leave("merge", "sanity checks", opt->repo);
 
-	/* Default to histogram diff.  Actually, just hardcode it...for now. */
-	opt->xdl_opts = DIFF_WITH_ALG(opt, HISTOGRAM_DIFF);
-
 	/* Handle attr direction stuff for renormalization */
 	if (opt->renormalize)
 		git_attr_set_direction(GIT_ATTR_CHECKOUT);
merge-recursive.c +1 -0
index 884ccf99a58..f3df127ad9b 100644
@@ -3981,6 +3981,7 @@ static void init_merge_options(struct merge_options *opt,
 	opt->renormalize = 0;
 
 	opt->conflict_style = -1;
+	opt->xdl_opts = DIFF_WITH_ALG(opt, HISTOGRAM_DIFF);
 
 	merge_recursive_config(opt, ui);
 	merge_verbosity = getenv("GIT_MERGE_VERBOSITY");
Signature · 1 line
gitgitgadget
#15 lore raw

PATCH v2 4/8 sequencer: switch non-recursive merges over to ort

In reply to Elijah Newren via GitGitGadget

From: Elijah Newren <[email protected]>

The do_recursive_merge() function, which is somewhat misleadingly named since its purpose in life is to do a *non*-recursive merge, had code to allow either using the recursive or ort backends. The default has been ort for a very long time, let's just remove the code path for allowing the recursive backend to be selected.

---
 sequencer.c | 35 +++++++++++++----------------------
 1 file changed, 13 insertions(+), 22 deletions(-)
sequencer.c +13 -22
index ad0ab75c8d4..b5d91fd3515 100644
@@ -781,28 +781,19 @@ static int do_recursive_merge(struct repository *r,
 	for (i = 0; i < opts->xopts.nr; i++)
 		parse_merge_opt(&o, opts->xopts.v[i]);
 
-	if (!opts->strategy || !strcmp(opts->strategy, "ort")) {
-		memset(&result, 0, sizeof(result));
-		merge_incore_nonrecursive(&o, base_tree, head_tree, next_tree,
-					    &result);
-		show_output = !is_rebase_i(opts) || !result.clean;
-		/*
-		 * TODO: merge_switch_to_result will update index/working tree;
-		 * we only really want to do that if !result.clean || this is
-		 * the final patch to be picked.  But determining this is the
-		 * final patch would take some work, and "head_tree" would need
-		 * to be replace with the tree the index matched before we
-		 * started doing any picks.
-		 */
-		merge_switch_to_result(&o, head_tree, &result, 1, show_output);
-		clean = result.clean;
-	} else {
-		ensure_full_index(r->index);
-		clean = merge_trees(&o, head_tree, next_tree, base_tree);
-		if (is_rebase_i(opts) && clean <= 0)
-			fputs(o.obuf.buf, stdout);
-		strbuf_release(&o.obuf);
-	}
+	memset(&result, 0, sizeof(result));
+	merge_incore_nonrecursive(&o, base_tree, head_tree, next_tree, &result);
+	show_output = !is_rebase_i(opts) || !result.clean;
+	/*
+	 * TODO: merge_switch_to_result will update index/working tree;
+	 * we only really want to do that if !result.clean || this is
+	 * the final patch to be picked.  But determining this is the
+	 * final patch would take some work, and "head_tree" would need
+	 * to be replace with the tree the index matched before we
+	 * started doing any picks.
+	 */
+	merge_switch_to_result(&o, head_tree, &result, 1, show_output);
+	clean = result.clean;
 	if (clean < 0) {
 		rollback_lock_file(&index_lock);
 		return clean;
Signature · 1 line
gitgitgadget
#16 lore raw

PATCH v2 5/8 merge, sequencer: switch recursive merges over to ort

In reply to Elijah Newren via GitGitGadget

From: Elijah Newren <[email protected]>

More precisely, replace calls to merge_recursive() with merge_ort_recursive().

Also change t7615 to quit calling out recursive; it is not needed anymore, and we are in fact using ort now.

---
 builtin/merge.c                            |  9 ++-------
 sequencer.c                                | 23 ++++++++--------------
 t/t7615-diff-algo-with-mergy-operations.sh |  2 --
 3 files changed, 10 insertions(+), 24 deletions(-)
builtin/merge.c +2 -7
index ba9faf126aa..c0bbdab7104 100644
@@ -39,7 +39,6 @@
 #include "rerere.h"
 #include "help.h"
 #include "merge.h"
-#include "merge-recursive.h"
 #include "merge-ort-wrappers.h"
 #include "resolve-undo.h"
 #include "remote.h"
@@ -750,12 +749,8 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
 
 		repo_hold_locked_index(the_repository, &lock,
 				       LOCK_DIE_ON_ERROR);
-		if (!strcmp(strategy, "ort"))
-			clean = merge_ort_recursive(&o, head, remoteheads->item,
-						    reversed, &result);
-		else
-			clean = merge_recursive(&o, head, remoteheads->item,
-						reversed, &result);
+		clean = merge_ort_recursive(&o, head, remoteheads->item,
+					    reversed, &result);
 		free_commit_list(reversed);
 		strbuf_release(&o.obuf);
 
sequencer.c +8 -15
index b5d91fd3515..8bb49735891 100644
@@ -4319,20 +4319,13 @@ static int do_merge(struct repository *r,
 	o.branch2 = ref_name.buf;
 	o.buffer_output = 2;
 
-	if (!opts->strategy || !strcmp(opts->strategy, "ort")) {
-		/*
-		 * TODO: Should use merge_incore_recursive() and
-		 * merge_switch_to_result(), skipping the call to
-		 * merge_switch_to_result() when we don't actually need to
-		 * update the index and working copy immediately.
-		 */
-		ret = merge_ort_recursive(&o,
-					  head_commit, merge_commit, bases,
-					  &i);
-	} else {
-		ret = merge_recursive(&o, head_commit, merge_commit, bases,
-				      &i);
-	}
+	/*
+	 * TODO: Should use merge_incore_recursive() and
+	 * merge_switch_to_result(), skipping the call to
+	 * merge_switch_to_result() when we don't actually need to
+	 * update the index and working copy immediately.
+	 */
+	ret = merge_ort_recursive(&o, head_commit, merge_commit, bases, &i);
 	if (ret <= 0)
 		fputs(o.obuf.buf, stdout);
 	strbuf_release(&o.obuf);
@@ -4343,7 +4336,7 @@ static int do_merge(struct repository *r,
 		goto leave_merge;
 	}
 	/*
-	 * The return value of merge_recursive() is 1 on clean, and 0 on
+	 * The return value of merge_ort_recursive() is 1 on clean, and 0 on
 	 * unclean merge.
 	 *
 	 * Let's reverse that, so that do_merge() returns 0 upon success and
t/t7615-diff-algo-with-mergy-operations.sh +0 -2
index ac5863e788c..5822d02d517 100755
@@ -22,8 +22,6 @@ test_expect_success 'setup' '
 	git tag c2
 '
 
-GIT_TEST_MERGE_ALGORITHM=recursive
-
 test_expect_success 'merge c2 to c1 with recursive merge strategy fails with the current default myers diff algorithm' '
 	git reset --hard c1 &&
 	test_must_fail git merge -s recursive -Xdiff-algorithm=myers c2
Signature · 1 line
gitgitgadget
#17 lore raw

PATCH v2 6/8 merge-recursive.[ch]: thoroughly debug these

In reply to Elijah Newren via GitGitGadget

From: Elijah Newren <[email protected]>

As a wise man once told me, "Deleted code is debugged code!" So, move the functions that are shared between merge-recursive and merge-ort from the former to the latter, and then debug the remainder of merge-recursive.[ch].

---
 Documentation/merge-strategies.adoc          |   24 +-
 Documentation/technical/sparse-checkout.adoc |    2 -
 Makefile                                     |    1 -
 merge-ort-wrappers.h                         |    2 +-
 merge-ort.c                                  |  159 +
 merge-ort.h                                  |   60 +-
 merge-recursive.c                            | 4080 ------------------
 merge-recursive.h                            |  132 -
 meson.build                                  |    1 -
 9 files changed, 225 insertions(+), 4236 deletions(-)
 delete mode 100644 merge-recursive.c
 delete mode 100644 merge-recursive.h
Documentation/merge-strategies.adoc +6 -18
index 8c87dd70210..9e034f447e7 100644
@@ -109,23 +109,11 @@ subtree[=<path>];;
 	two trees to match.
 
 recursive::
-	This can only resolve two heads using a 3-way merge
-	algorithm.  When there is more than one common
-	ancestor that can be used for 3-way merge, it creates a
-	merged tree of the common ancestors and uses that as
-	the reference tree for the 3-way merge.  This has been
-	reported to result in fewer merge conflicts without
-	causing mismerges by tests done on actual merge commits
-	taken from Linux 2.6 kernel development history.
-	Additionally this can detect and handle merges involving
-	renames.  It does not make use of detected copies.  This was
-	the default strategy for resolving two heads from Git v0.99.9k
-	until v2.33.0.
-+
-For a path that is a submodule, the same caution as 'ort' applies to this
-strategy.
-+
-The 'recursive' strategy takes the same options as 'ort'.
+	This is now a synonym for `ort`.  It was an alternative
+	implementation until v2.49.0, but was redirected to mean `ort`
+	in v2.50.0.  The previous recursive strategy was the default
+	strategy for resolving two heads from Git v0.99.9k until
+	v2.33.0.
 
 resolve::
 	This can only resolve two heads (i.e. the current branch
@@ -146,7 +134,7 @@ ours::
 	ignoring all changes from all other branches.  It is meant to
 	be used to supersede old development history of side
 	branches.  Note that this is different from the -Xours option to
-	the 'recursive' merge strategy.
+	the 'ort' merge strategy.
 
 subtree::
 	This is a modified `ort` strategy. When merging trees A and
Documentation/technical/sparse-checkout.adoc +0 -2
index d9686593548..dc2e763bbec 100644
@@ -356,8 +356,6 @@ understanding these differences can be beneficial.
     The behavior for these commands somewhat depends upon the merge
     strategy being used:
       * `ort` behaves as described above
-      * `recursive` tries to not vivify files unnecessarily, but does sometimes
-	vivify files without conflicts.
       * `octopus` and `resolve` will always vivify any file changed in the merge
 	relative to the first parent, which is rather suboptimal.
 
Makefile +0 -1
index ac32d2d0bda..4fbd29cc7e9 100644
@@ -1069,7 +1069,6 @@ LIB_OBJS += merge-blobs.o
 LIB_OBJS += merge-ll.o
 LIB_OBJS += merge-ort.o
 LIB_OBJS += merge-ort-wrappers.o
-LIB_OBJS += merge-recursive.o
 LIB_OBJS += merge.o
 LIB_OBJS += midx.o
 LIB_OBJS += midx-write.o
merge-ort-wrappers.h +1 -1
index aeffa1c87b4..b7e1ced9d7f 100644
@@ -1,7 +1,7 @@
 #ifndef MERGE_ORT_WRAPPERS_H
 #define MERGE_ORT_WRAPPERS_H
 
-#include "merge-recursive.h"
+#include "merge-ort.h"
 
 /*
  * rename-detecting three-way merge, no recursion.
merge-ort.c +159 -0

168 lines not on this page · raw message

merge-ort.h +59 -1

79 lines not on this page · raw message

merge-recursive.c deleted +0 -4080

2,694 lines not on this page · raw message

merge-recursive.h deleted +0 -132

132 lines not on this page · raw message

meson.build +0 -1
index e98cfa4909f..8448df9d15e 100644

Lines of this file are not shown. View the raw message.

The rest of this diff is not shown. View the raw message.

Signature · 1 line
gitgitgadget

Part of the diff is not shown here. View the raw message.

#18 lore raw

PATCH v2 7/8 tests: remove GIT_TEST_MERGE_ALGORITHM and test_expect_merge_algorithm

In reply to Elijah Newren via GitGitGadget

From: Elijah Newren <[email protected]>

Both of these existed to allow us to reuse all the merge-related tests in the testsuite while easily flipping between the 'recursive' and the 'ort' backends. Now that we have removed merge-recursive and remapped 'recursive' to mean 'ort', we don't need this scaffolding anymore.

---
 ci/run-build-and-tests.sh                     |   1 -
 t/lib-merge.sh                                |  13 -
 t/t1092-sparse-checkout-compatibility.sh      |   2 -
 t/t2501-cwd-empty.sh                          |   2 -
 t/t3512-cherry-pick-submodule.sh              |   5 -
 t/t3513-revert-submodule.sh                   |   4 -
 t/t4069-remerge-diff.sh                       |   7 -
 t/t4301-merge-tree-write-tree.sh              |   7 -
 t/t5572-pull-submodule.sh                     |   5 -
 t/t6400-merge-df.sh                           |  14 +-
 t/t6402-merge-rename.sh                       | 125 +----
 t/t6404-recursive-merge.sh                    |  21 +-
 t/t6406-merge-attr.sh                         |   7 +-
 t/t6416-recursive-corner-cases.sh             | 194 ++-----
 t/t6421-merge-partial-clone.sh                |   7 +-
 t/t6422-merge-rename-corner-cases.sh          |  31 +-
 t/t6423-merge-rename-directories.sh           | 517 ++++--------------
 t/t6424-merge-unrelated-index-changes.sh      |   4 +-
 t/t6426-merge-skip-unneeded-updates.sh        |   4 +-
 t/t6428-merge-conflicts-sparse.sh             |   2 -
 t/t6430-merge-recursive.sh                    |   3 +-
 t/t6436-merge-overwrite.sh                    |  17 +-
 t/t6437-submodule-merge.sh                    |  65 +--
 t/t6438-submodule-directory-file-conflicts.sh |   5 -
 t/t6439-merge-co-error-msgs.sh                |   2 +-
 t/t7402-submodule-rebase.sh                   |   7 +-
 t/t7602-merge-octopus-many.sh                 |   9 +-
 t/t7610-mergetool.sh                          |  40 +-
 t/test-lib.sh                                 |   2 -
 29 files changed, 248 insertions(+), 874 deletions(-)
 delete mode 100644 t/lib-merge.sh
ci/run-build-and-tests.sh +0 -1
index 1c698467235..f99b7db2ee8 100755
@@ -20,7 +20,6 @@ linux-breaking-changes)
 linux-TEST-vars)
 	export OPENSSL_SHA1_UNSAFE=YesPlease
 	export GIT_TEST_SPLIT_INDEX=yes
-	export GIT_TEST_MERGE_ALGORITHM=recursive
 	export GIT_TEST_FULL_IN_PACK_ARRAY=true
 	export GIT_TEST_OE_SIZE=10
 	export GIT_TEST_OE_DELTA_SIZE=5
t/lib-merge.sh deleted +0 -13
deleted file mode 100644
index 8734ebfc17f..00000000000
@@ -1,13 +0,0 @@
-# Helper functions used by merge tests.
-
-test_expect_merge_algorithm () {
-	status_for_recursive=$1 status_for_ort=$2
-	shift 2
-
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		test_expect_${status_for_ort} "$@"
-	else
-		test_expect_${status_for_recursive} "$@"
-	fi
-}
t/t1092-sparse-checkout-compatibility.sh +0 -2
index a4c7c41fc00..f9b448792cb 100755
@@ -1486,7 +1486,6 @@ test_expect_success 'sparse-index is not expanded' '
 	ensure_not_expanded checkout -f update-deep &&
 	test_config -C sparse-index pull.twohead ort &&
 	(
-		sane_unset GIT_TEST_MERGE_ALGORITHM &&
 		for OPERATION in "merge -m merge" cherry-pick rebase
 		do
 			ensure_not_expanded merge -m merge update-folder1 &&
@@ -1506,7 +1505,6 @@ test_expect_success 'sparse-index is not expanded: merge conflict in cone' '
 	done &&
 
 	(
-		sane_unset GIT_TEST_MERGE_ALGORITHM &&
 		git -C sparse-index config pull.twohead ort &&
 		ensure_not_expanded ! merge -m merged expand-right
 	)
t/t2501-cwd-empty.sh +0 -2
index f6d8d7d03d7..be9140bbaa4 100755
@@ -117,8 +117,6 @@ test_expect_success 'merge fails if cwd needs to be removed; recursive friendly'
 	grep "Refusing to remove the current working directory" error
 '
 
-GIT_TEST_MERGE_ALGORITHM=ort
-
 test_expect_success 'merge fails if cwd needs to be removed' '
 	test_required_dir_removal git merge fd_conflict
 '
t/t3512-cherry-pick-submodule.sh +0 -5
index f22d1ddead1..85a79326977 100755
@@ -8,11 +8,6 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-submodule-update.sh
 
-if test "$GIT_TEST_MERGE_ALGORITHM" != ort
-then
-	KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1
-	KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1
-fi
 test_submodule_switch "cherry-pick"
 
 test_expect_success 'unrelated submodule/file conflict is ignored' '
t/t3513-revert-submodule.sh +0 -4
index 8bfe3ed2467..32e15c72eee 100755
@@ -30,10 +30,6 @@ git_revert () {
 	git revert HEAD
 }
 
-if test "$GIT_TEST_MERGE_ALGORITHM" != ort
-then
-	KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1
-fi
 test_submodule_switch_func "git_revert"
 
 test_done
t/t4069-remerge-diff.sh +0 -7
index c6c94aef14d..966882ce912 100755
@@ -4,13 +4,6 @@ test_description='remerge-diff handling'
 
 . ./test-lib.sh
 
-# This test is ort-specific
-if test "${GIT_TEST_MERGE_ALGORITHM}" != ort
-then
-	skip_all="GIT_TEST_MERGE_ALGORITHM != ort"
-	test_done
-fi
-
 test_expect_success 'setup basic merges' '
 	test_write_lines 1 2 3 4 5 6 7 8 9 >numbers &&
 	git add numbers &&
t/t4301-merge-tree-write-tree.sh +0 -7
index 44f7d077593..f9c5883a7f7 100755
@@ -4,13 +4,6 @@ test_description='git merge-tree --write-tree'
 
 . ./test-lib.sh
 
-# This test is ort-specific
-if test "$GIT_TEST_MERGE_ALGORITHM" != "ort"
-then
-	skip_all="GIT_TEST_MERGE_ALGORITHM != ort"
-	test_done
-fi
-
 test_expect_success setup '
 	test_write_lines 1 2 3 4 5 >numbers &&
 	echo hello >greeting &&
t/t5572-pull-submodule.sh +0 -5
index f7650e8475e..45f384dd328 100755
@@ -45,11 +45,6 @@ git_pull_noff () {
 	$2 git pull --no-ff
 }
 
-if test "$GIT_TEST_MERGE_ALGORITHM" != ort
-then
-	KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1
-	KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1
-fi
 test_submodule_switch_func "git_pull_noff"
 
 test_expect_success 'setup' '
t/t6400-merge-df.sh +2 -12
index 3de4ef6bd9e..b46fe494409 100755
@@ -84,12 +84,7 @@ test_expect_success 'modify/delete + directory/file conflict' '
 
 	test_stdout_line_count = 5 git ls-files -s &&
 	test_stdout_line_count = 4 git ls-files -u &&
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		test_stdout_line_count = 0 git ls-files -o
-	else
-		test_stdout_line_count = 1 git ls-files -o
-	fi &&
+	test_stdout_line_count = 0 git ls-files -o &&
 
 	test_path_is_file letters/file &&
 	test_path_is_file letters.txt &&
@@ -105,12 +100,7 @@ test_expect_success 'modify/delete + directory/file conflict; other way' '
 
 	test_stdout_line_count = 5 git ls-files -s &&
 	test_stdout_line_count = 4 git ls-files -u &&
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		test_stdout_line_count = 0 git ls-files -o
-	else
-		test_stdout_line_count = 1 git ls-files -o
-	fi &&
+	test_stdout_line_count = 0 git ls-files -o &&
 
 	test_path_is_file letters/file &&
 	test_path_is_file letters.txt &&
t/t6402-merge-rename.sh +28 -97

177 lines not on this page · raw message

t/t6404-recursive-merge.sh +3 -18
index ae687f2ce54..346f3608b98 100755
@@ -108,12 +108,7 @@ test_expect_success 'refuse to merge binary files' '
 	printf "\0\0" >binary-file &&
 	git add binary-file &&
 	git commit -m binary2 &&
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		test_must_fail git merge F >merge_output
-	else
-		test_must_fail git merge F 2>merge_output
-	fi &&
+	test_must_fail git merge F >merge_output &&
 	grep "Cannot merge binary files: binary-file (HEAD vs. F)" merge_output
 '
 
@@ -129,22 +124,12 @@ test_expect_success 'mark rename/delete as unmerged' '
 	test_tick &&
 	git commit -m rename &&
 	test_must_fail git merge delete &&
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		test 2 = $(git ls-files --unmerged | wc -l)
-	else
-		test 1 = $(git ls-files --unmerged | wc -l)
-	fi &&
+	test 2 = $(git ls-files --unmerged | wc -l) &&
 	git rev-parse --verify :2:a2 &&
 	test_must_fail git rev-parse --verify :3:a2 &&
 	git checkout -f delete &&
 	test_must_fail git merge rename &&
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		test 2 = $(git ls-files --unmerged | wc -l)
-	else
-		test 1 = $(git ls-files --unmerged | wc -l)
-	fi &&
+	test 2 = $(git ls-files --unmerged | wc -l) &&
 	test_must_fail git rev-parse --verify :2:a2 &&
 	git rev-parse --verify :3:a2
 '
t/t6406-merge-attr.sh +1 -6
index 66e01464b5e..8f6fbef0024 100755
@@ -259,12 +259,7 @@ test_expect_success 'binary files with union attribute' '
 	printf "two\0" >bin.txt &&
 	git commit -am two &&
 
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		test_must_fail git merge bin-main >output
-	else
-		test_must_fail git merge bin-main 2>output
-	fi &&
+	test_must_fail git merge bin-main >output &&
 	grep -i "warning.*cannot merge.*HEAD vs. bin-main" output
 '
 
t/t6416-recursive-corner-cases.sh +55 -139

278 lines not on this page · raw message

t/t6421-merge-partial-clone.sh +3 -4
index b99f29ef9ba..6eb51285a0c 100755
@@ -27,7 +27,6 @@ test_description="limiting blob downloads when merging with partial clones"
 #                     files that might be renamed into each other's paths.)
 
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/lib-merge.sh
 
 test_setup_repo () {
 	test -d server && return
@@ -207,7 +206,7 @@ test_setup_repo () {
 #
 #   Summary: 2 fetches (1 for 2 objects, 1 for 1 object)
 #
-test_expect_merge_algorithm failure success 'Objects downloaded for single relevant rename' '
+test_expect_success 'Objects downloaded for single relevant rename' '
 	test_setup_repo &&
 	git clone --sparse --filter=blob:none "file://$(pwd)/server" objects-single &&
 	(
@@ -297,7 +296,7 @@ test_expect_merge_algorithm failure success 'Objects downloaded for single relev
 #      this are not all that common.)
 #   Summary: 1 fetches for 6 objects
 #
-test_expect_merge_algorithm failure success 'Objects downloaded when a directory rename triggered' '
+test_expect_success 'Objects downloaded when a directory rename triggered' '
 	test_setup_repo &&
 	git clone --sparse --filter=blob:none "file://$(pwd)/server" objects-dir &&
 	(
@@ -399,7 +398,7 @@ test_expect_merge_algorithm failure success 'Objects downloaded when a directory
 #
 #   Summary: 4 fetches (1 for 6 objects, 1 for 8, 1 for 3, 1 for 2)
 #
-test_expect_merge_algorithm failure success 'Objects downloaded with lots of renames and modifications' '
+test_expect_success 'Objects downloaded with lots of renames and modifications' '
 	test_setup_repo &&
 	git clone --sparse --filter=blob:none "file://$(pwd)/server" objects-many &&
 	(
t/t6422-merge-rename-corner-cases.sh +7 -24
index 62b49c67e22..9cbe7ca7824 100755
@@ -7,7 +7,6 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/lib-merge.sh
 
 test_setup_rename_delete_untracked () {
 	git init rename-delete-untracked &&
@@ -316,12 +315,7 @@ test_expect_success 'rename/directory conflict + clean content merge' '
 		git ls-files -u >out &&
 		test_line_count = 1 out &&
 		git ls-files -o >out &&
-		if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-		then
-			test_line_count = 1 out
-		else
-			test_line_count = 2 out
-		fi &&
+		test_line_count = 1 out &&
 
 		echo 0 >expect &&
 		git cat-file -p base:file >>expect &&
@@ -350,12 +344,7 @@ test_expect_success 'rename/directory conflict + content merge conflict' '
 		git ls-files -u >out &&
 		test_line_count = 3 out &&
 		git ls-files -o >out &&
-		if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-		then
-			test_line_count = 1 out
-		else
-			test_line_count = 2 out
-		fi &&
+		test_line_count = 1 out &&
 
 		git cat-file -p left-conflict:newfile >left &&
 		git cat-file -p base:file    >base &&
@@ -369,14 +358,8 @@ test_expect_success 'rename/directory conflict + content merge conflict' '
 
 		git rev-parse >expect   \
 			base:file       left-conflict:newfile right:file &&
-		if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-		then
-			git rev-parse >actual \
-				:1:newfile~HEAD :2:newfile~HEAD :3:newfile~HEAD
-		else
-			git rev-parse >actual \
-				:1:newfile      :2:newfile      :3:newfile
-		fi &&
+		git rev-parse >actual \
+			:1:newfile~HEAD :2:newfile~HEAD :3:newfile~HEAD &&
 		test_cmp expect actual &&
 
 		test_path_is_file newfile/realfile &&
@@ -896,7 +879,7 @@ test_setup_rad () {
 	)
 }
 
-test_expect_merge_algorithm failure success 'rad-check: rename/add/delete conflict' '
+test_expect_success 'rad-check: rename/add/delete conflict' '
 	test_setup_rad &&
 	(
 		cd rad &&
@@ -969,7 +952,7 @@ test_setup_rrdd () {
 	)
 }
 
-test_expect_merge_algorithm failure success 'rrdd-check: rename/rename(2to1)/delete/delete conflict' '
+test_expect_success 'rrdd-check: rename/rename(2to1)/delete/delete conflict' '
 	test_setup_rrdd &&
 	(
 		cd rrdd &&
@@ -1058,7 +1041,7 @@ test_setup_mod6 () {
 	)
 }
 
-test_expect_merge_algorithm failure success 'mod6-check: chains of rename/rename(1to2) and rename/rename(2to1)' '
+test_expect_success 'mod6-check: chains of rename/rename(1to2) and rename/rename(2to1)' '
 	test_setup_mod6 &&
 	(
 		cd mod6 &&
t/t6423-merge-rename-directories.sh +105 -412

673 lines not on this page · raw message

t/t6424-merge-unrelated-index-changes.sh +1 -3
index 084bfe7e6ce..ddc7524f6c1 100755
@@ -296,10 +296,8 @@ test_expect_success 'with multiple strategies, recursive or ort failure do not e
 	git add a &&
 	git rev-parse :a >expect &&
 
-	sane_unset GIT_TEST_MERGE_ALGORITHM &&
-	test_must_fail git merge -s recursive -s ort -s octopus C^0 >output 2>&1 &&
+	test_must_fail git merge -s ort -s octopus C^0 >output 2>&1 &&
 
-	grep "Trying merge strategy recursive..." output &&
 	grep "Trying merge strategy ort..." output &&
 	grep "Trying merge strategy octopus..." output &&
 	grep "No merge strategy handled the merge." output &&
t/t6426-merge-skip-unneeded-updates.sh +1 -3
index b059475ed03..404cd3f2ca1 100755
@@ -23,8 +23,6 @@ test_description="merge cases"
 #                     files that might be renamed into each other's paths.)
 
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/lib-merge.sh
-
 
 ###########################################################################
 # SECTION 1: Cases involving no renames (one side has subset of changes of
@@ -663,7 +661,7 @@ test_setup_4a () {
 #   correct requires doing the merge in-memory first, then realizing that no
 #   updates to the file are necessary, and thus that we can just leave the path
 #   alone.
-test_expect_merge_algorithm failure success '4a: Change on A, change on B subset of A, dirty mods present' '
+test_expect_success '4a: Change on A, change on B subset of A, dirty mods present' '
 	test_setup_4a &&
 	(
 		cd 4a &&
t/t6428-merge-conflicts-sparse.sh +0 -2
index 9919c3fa7cd..c86e02c6e23 100755
@@ -23,8 +23,6 @@ test_description="merge cases"
 #                     files that might be renamed into each other's paths.)
 
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/lib-merge.sh
-
 
 # Testcase basic, conflicting changes in 'numerals'
 
t/t6430-merge-recursive.sh +1 -2
index c539a68fd07..e59560a5dd9 100755
@@ -6,7 +6,6 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/lib-merge.sh
 
 test_expect_success 'setup 1' '
 
@@ -645,7 +644,7 @@ test_expect_success 'merge-recursive copy vs. rename' '
 	test_cmp expected actual
 '
 
-test_expect_merge_algorithm failure success 'merge-recursive rename vs. rename/symlink' '
+test_expect_success 'merge-recursive rename vs. rename/symlink' '
 
 	git checkout -f rename &&
 	git merge rename-ln &&
t/t6436-merge-overwrite.sh +4 -13
index 4f4376421e7..70b5d2d6944 100755
@@ -101,19 +101,10 @@ test_expect_success 'will not overwrite unstaged changes in renamed file' '
 	git mv c1.c other.c &&
 	git commit -m rename &&
 	cp important other.c &&
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		test_must_fail git merge c1a >out 2>err &&
-		test_grep "would be overwritten by merge" err &&
-		test_cmp important other.c &&
-		test_path_is_missing .git/MERGE_HEAD
-	else
-		test_must_fail git merge c1a >out &&
-		test_grep "Refusing to lose dirty file at other.c" out &&
-		test_path_is_file other.c~HEAD &&
-		test $(git hash-object other.c~HEAD) = $(git rev-parse c1a:c1.c) &&
-		test_cmp important other.c
-	fi
+	test_must_fail git merge c1a >out 2>err &&
+	test_grep "would be overwritten by merge" err &&
+	test_cmp important other.c &&
+	test_path_is_missing .git/MERGE_HEAD
 '
 
 test_expect_success 'will not overwrite untracked subtree' '
t/t6437-submodule-merge.sh +20 -45

114 lines not on this page · raw message

t/t6438-submodule-directory-file-conflicts.sh +0 -5
index 8df67a0ef99..53d83c828a7 100755
@@ -12,11 +12,6 @@ test_submodule_switch "merge --ff"
 
 test_submodule_switch "merge --ff-only"
 
-if test "$GIT_TEST_MERGE_ALGORITHM" != ort
-then
-	KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1
-	KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1
-fi
 test_submodule_switch "merge --no-ff"
 
 test_done
t/t6439-merge-co-error-msgs.sh +1 -1
index 55bd744a3f4..643c9368e0e 100755
@@ -47,7 +47,7 @@ test_expect_success 'untracked files overwritten by merge (fast and non-fast for
 		export GIT_MERGE_VERBOSITY &&
 		test_must_fail git merge branch 2>out2
 	) &&
-	echo "Merge with strategy ${GIT_TEST_MERGE_ALGORITHM:-ort} failed." >>expect &&
+	echo "Merge with strategy ort failed." >>expect &&
 	test_cmp out2 expect &&
 	git reset --hard HEAD^
 '
t/t7402-submodule-rebase.sh +2 -5
index 25b33a1e875..06cee3432fe 100755
@@ -124,11 +124,8 @@ test_expect_success 'rebasing submodule that should conflict' '
 		echo "160000 $(git rev-parse HEAD) 3	submodule"
 	) >expect &&
 	test_cmp expect actual &&
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-    then
-		sub_expect="go to submodule (submodule), and either merge commit $(git -C submodule rev-parse --short HEAD^0)" &&
-		grep "$sub_expect" actual_output
-	fi
+	sub_expect="go to submodule (submodule), and either merge commit $(git -C submodule rev-parse --short HEAD^0)" &&
+	grep "$sub_expect" actual_output
 '
 
 test_done
t/t7602-merge-octopus-many.sh +3 -6
index ff085b086cc..42f675b7399 100755
@@ -77,12 +77,9 @@ Merge made by the 'recursive' strategy.
 EOF
 
 test_expect_success 'merge reduces irrelevant remote heads' '
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		mv expected expected.tmp &&
-		sed s/recursive/ort/ expected.tmp >expected &&
-		rm expected.tmp
-	fi &&
+	mv expected expected.tmp &&
+	sed s/recursive/ort/ expected.tmp >expected &&
+	rm expected.tmp &&
 	GIT_MERGE_VERBOSITY=0 git merge c4 c5 >actual &&
 	test_cmp expected actual
 '
t/t7610-mergetool.sh +11 -29
index c077aba7ced..957f8e20ba3 100755
@@ -535,14 +535,9 @@ test_expect_success 'file vs modified submodule' '
 	yes "" | git mergetool file1 file2 spaced\ name subdir/file3 &&
 	yes "" | git mergetool both &&
 	yes "d" | git mergetool file11 file12 &&
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		yes "c" | git mergetool submod~HEAD &&
-		git rm submod &&
-		git mv submod~HEAD submod
-	else
-		yes "l" | git mergetool submod
-	fi &&
+	yes "c" | git mergetool submod~HEAD &&
+	git rm submod &&
+	git mv submod~HEAD submod &&
 	git submodule update -N &&
 	echo "not a submodule" >expect &&
 	test_cmp expect submod &&
@@ -559,15 +554,10 @@ test_expect_success 'file vs modified submodule' '
 	yes "" | git mergetool file1 file2 spaced\ name subdir/file3 &&
 	yes "" | git mergetool both &&
 	yes "d" | git mergetool file11 file12 &&
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		mv submod submod.orig &&
-		git rm --cached submod &&
-		yes "c" | git mergetool submod~test19 &&
-		git mv submod~test19 submod
-	else
-		yes "r" | git mergetool submod
-	fi &&
+	mv submod submod.orig &&
+	git rm --cached submod &&
+	yes "c" | git mergetool submod~test19 &&
+	git mv submod~test19 submod &&
 	test -d submod.orig &&
 	git submodule update -N &&
 	echo "not a submodule" >expect &&
@@ -585,10 +575,7 @@ test_expect_success 'file vs modified submodule' '
 	yes "" | git mergetool both &&
 	yes "d" | git mergetool file11 file12 &&
 	yes "l" | git mergetool submod &&
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		yes "d" | git mergetool submod~test19
-	fi &&
+	yes "d" | git mergetool submod~test19 &&
 	echo "main submodule" >expect &&
 	test_cmp expect submod/bar &&
 	git submodule update -N &&
@@ -686,14 +673,9 @@ test_expect_success 'directory vs modified submodule' '
 	test_must_fail git merge main &&
 	test -n "$(git ls-files -u)" &&
 	test ! -e submod.orig &&
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		yes "r" | git mergetool submod~main &&
-		git mv submod submod.orig &&
-		git mv submod~main submod
-	else
-		yes "r" | git mergetool submod
-	fi &&
+	yes "r" | git mergetool submod~main &&
+	git mv submod submod.orig &&
+	git mv submod~main submod &&
 	test -d submod.orig &&
 	echo "not a submodule" >expect &&
 	test_cmp expect submod.orig/file16 &&
t/test-lib.sh +0 -2
index 9001ed3a647..5952eb56e27 100644
@@ -544,8 +544,6 @@ GIT_DEFAULT_HASH="${GIT_TEST_DEFAULT_HASH:-sha1}"
 export GIT_DEFAULT_HASH
 GIT_DEFAULT_REF_FORMAT="${GIT_TEST_DEFAULT_REF_FORMAT:-files}"
 export GIT_DEFAULT_REF_FORMAT
-GIT_TEST_MERGE_ALGORITHM="${GIT_TEST_MERGE_ALGORITHM:-ort}"
-export GIT_TEST_MERGE_ALGORITHM
 
 # Tests using GIT_TRACE typically don't want <timestamp> <file>:<line> output
 GIT_TRACE_BARE=1
Signature · 1 line
gitgitgadget
#19 lore raw

PATCH v2 8/8 builtin/{merge,rebase,revert}: remove GIT_TEST_MERGE_ALGORITHM

In reply to Elijah Newren via GitGitGadget

From: Elijah Newren <[email protected]>

This environment variable existed to allow the testsuite to reuse all the merge-related tests in the testsuite while easily flipping between the 'recursive' and the 'ort' backends. Now that we have removed merge-recursive and remapped 'recursive' to mean 'ort', we don't need this scaffolding anymore. Remove it from these three builtins.

---
 builtin/merge.c  | 14 +-------------
 builtin/rebase.c |  5 -----
 builtin/revert.c |  2 --
 3 files changed, 1 insertion(+), 20 deletions(-)
builtin/merge.c +1 -13
index c0bbdab7104..9efd585842f 100644
@@ -170,7 +170,7 @@ static struct strategy *get_strategy(const char *name)
 	struct strategy *ret;
 	static struct cmdnames main_cmds = {0}, other_cmds = {0};
 	static int loaded;
-	char *default_strategy = getenv("GIT_TEST_MERGE_ALGORITHM");
+	char *default_strategy = NULL;
 
 	if (!name)
 		return NULL;
@@ -1311,12 +1311,6 @@ int cmd_merge(int argc,
 	if (branch)
 		skip_prefix(branch, "refs/heads/", &branch);
 
-	if (!pull_twohead) {
-		char *default_strategy = getenv("GIT_TEST_MERGE_ALGORITHM");
-		if (default_strategy && !strcmp(default_strategy, "ort"))
-			pull_twohead = xstrdup("ort");
-	}
-
 	init_diff_ui_defaults();
 	git_config(git_merge_config, NULL);
 
@@ -1517,12 +1511,6 @@ int cmd_merge(int argc,
 			fast_forward = FF_NO;
 	}
 
-	if (!use_strategies && !pull_twohead &&
-	    remoteheads && !remoteheads->next) {
-		char *default_strategy = getenv("GIT_TEST_MERGE_ALGORITHM");
-		if (default_strategy)
-			append_strategy(get_strategy(default_strategy));
-	}
 	if (!use_strategies) {
 		if (!remoteheads)
 			; /* already up-to-date */
builtin/rebase.c +0 -5
index d4715ed35d7..e83193ac73e 100644
@@ -1575,11 +1575,6 @@ int cmd_rebase(int argc,
 			    options.default_backend);
 	}
 
-	if (options.type == REBASE_MERGE &&
-	    !options.strategy &&
-	    getenv("GIT_TEST_MERGE_ALGORITHM"))
-		options.strategy = xstrdup(getenv("GIT_TEST_MERGE_ALGORITHM"));
-
 	switch (options.type) {
 	case REBASE_MERGE:
 		options.state_dir = merge_dir();
builtin/revert.c +0 -2
index aca6c293cdf..2654f769a88 100644
@@ -252,8 +252,6 @@ static int run_sequencer(int argc, const char **argv, const char *prefix,
 		free(opts->strategy);
 		opts->strategy = xstrdup_or_null(strategy);
 	}
-	if (!opts->strategy && getenv("GIT_TEST_MERGE_ALGORITHM"))
-		opts->strategy = xstrdup(getenv("GIT_TEST_MERGE_ALGORITHM"));
 	free(options);
 
 	if (cmd == 'q') {
Signature · 1 line
gitgitgadget
#20 lore raw
In reply to Elijah Newren via GitGitGadget
Quoted 3 lines · Elijah Newren via GitGitGadget

This series does some preparation, then moves the code shared between merge-recursive and merge-ort from the former to the latter, and then debugs the remainder of merge-recursive.[ch].

Help unconfusing me. When we have bugs in our code, the action we take consists of two parts, i.e. first we find them, and then we fix them. To me, the verb "debug" refers only to the earlier half, and never the latter.

But the code in the later part of this series is not only to find or expose existing bugs, but also fixing them, right?

I've already named the topic with "debug" in its name while queuing the original iteration of this series, as I was on vacation and did not want to spend more than minimum braincycles on naming, but now I am back, I sense that the use of the word, and the proposed log message for 6/8, are overly suboptimal. If you are referring to fixing remaining bugs, "Debug the remainder of merge-recursive.[ch]" is not how we usually describe our fixes.

I suspect that the overall sentiment behind this series is ...

Such and such bugs existed in the older backend, but now the newer backend is used when the older one is asked, and the newer backend does not share these bugs, we can simply remove the buggy code specific to the older backend.

... but I find it somewhat disturbing to stay totally silent about "such and such bugs existed" part.

Thanks.

#21 lore raw
Quoted 8 lines · Junio C Hamano
Quoted 3 lines · Elijah Newren via GitGitGadget

This series does some preparation, then moves the code shared between merge-recursive and merge-ort from the former to the latter, and then debugs the remainder of merge-recursive.[ch].

Help unconfusing me.

It was an attempt at humor, which importantly relied on the sentence you stripped out immediately before this part you quoted, namely:

Quoted 1 line
Quoted 1 line

As a wise man once told me, "Deleted code is debugged code!"

With this sentence as context, "debug the code" was a funny way of saying "delete the code". Because if it's deleted, we are no longer affected by any bugs contained within it (and that'd be true for both known bugs and latent ones we hadn't triggered yet).

Quoted 8 lines

I've already named the topic with "debug" in its name while queuing the original iteration of this series, as I was on vacation and did not want to spend more than minimum braincycles on naming, but now I am back, I sense that the use of the word, and the proposed log message for 6/8, are overly suboptimal. If you are referring to fixing remaining bugs, "Debug the remainder of merge-recursive.[ch]" is not how we usually describe our fixes.

Quoted 6 lines

I suspect that the overall sentiment behind this series is ...

Such and such bugs existed in the older backend, but now the newer backend is used when the older one is asked, and the newer backend does not share these bugs, we can simply remove the buggy code specific to the older backend.

I'd say the sentiment is more:

merge-ort was always meant to be a replacement for merge-recursive (and has various advantages -- worktree-less and index-less operation, faster, fixes some bugs); let's convert the rest of the callers over and then clean up by deleting merge-recursive (as well as removing the extra test scaffolding added long ago to aid in the conversion).

In other words the bug fixes, while real, were not the thrust of the story. I showed a handful of people my existing commit message and cover letter, all of whom found it a humorous way of stating that we're finally replacing merge-recursive. But, since this intent wasn't obvious to everyone, I can re-roll with some clarification.

#22 lore raw

v3 cover Debug merge-recursive.[ch]

In reply to Elijah Newren via GitGitGadget

Changes since v1:

* fixed a couple of typos in commit messages

Changes since v2.

* clarified the commit message for patch 6, and the cover letter

Clarified cover letter:

As a wise man once told me, "Deleted code is debugged code!"

This series does some preparation, then moves the code shared between merge-recursive and merge-ort from the former to the latter, and then "debugs" the remainder of merge-recursive.[ch].

Joking aside, merge-ort was always intended to replace merge-recursive. It did replace it as the default years ago in commit 6a5fb966720f (Change default merge backend from recursive to ort, 2021-08-04), and was noted to be better on correctness, extensibility, and performance issues. In this series, we convert the remaining callers of merge-recursive over to merge-ort equivalents, move the shared code between merge-recursive and merge-ort from the former to the latter, and then delete the remainder of merge-recursive.[ch] as it has now been fully replaced. We then clean up some test framework scaffolding that was put into place to allow us to reuse the recursive tests as ort tests.

Series overview:

* Patches 1-5: Preparation; switch remaining callers of merge-recursive.[ch] functions to merge-ort equivalents, and add sole remaining missing feature (diff-algorithm selection)
* Patch 6: Nuke merge-recursive.[ch]
* Patch 7-8: Cleanup testsuite; we don't need GIT_TEST_MERGE_ALGORITHM anymore

While the diffstat might look large, the non-test code changes are actually pretty small. The drivers of the big diffstat are:

* We move a significant chunk of shared code from merge-recursive.[ch] to merge-ort.[ch], without modifying it
* We delete (the remainder of) merge-recursive.[ch]
* We rip out all the temporary GIT_TEST_MERGE_ALGORITHM stuff designed to let us reuse tests between recursive and ort

Elijah Newren (8):
checkout: replace merge_trees() with merge_ort_nonrecursive() builtin/merge-recursive: switch to using merge_ort_generic()
merge-ort: enable diff-algorithms other than histogram
sequencer: switch non-recursive merges over to ort
merge, sequencer: switch recursive merges over to ort merge-recursive.[ch]: thoroughly debug these
tests: remove GIT_TEST_MERGE_ALGORITHM and test_expect_merge_algorithm builtin/{merge,rebase,revert}: remove GIT_TEST_MERGE_ALGORITHM

 Documentation/merge-strategies.adoc           |   51 +-
 Documentation/technical/sparse-checkout.adoc  |    2 -
 Makefile                                      |    1 -
 builtin/checkout.c                            |   10 +-
 builtin/merge-recursive.c                     |    4 +-
 builtin/merge.c                               |   23 +-
 builtin/rebase.c                              |    5 -
 builtin/revert.c                              |    2 -
 ci/run-build-and-tests.sh                     |    1 -
 merge-ort-wrappers.h                          |    2 +-
 merge-ort.c                                   |  162 +-
 merge-ort.h                                   |   60 +-
 merge-recursive.c                             | 4079 -----------------
 merge-recursive.h                             |  132 -
 meson.build                                   |    1 -
 sequencer.c                                   |   58 +-
 t/lib-merge.sh                                |   13 -
 t/t1092-sparse-checkout-compatibility.sh      |    2 -
 t/t2501-cwd-empty.sh                          |    2 -
 t/t3512-cherry-pick-submodule.sh              |    5 -
 t/t3513-revert-submodule.sh                   |    4 -
 t/t4069-remerge-diff.sh                       |    7 -
 t/t4301-merge-tree-write-tree.sh              |    7 -
 t/t5572-pull-submodule.sh                     |    5 -
 t/t6400-merge-df.sh                           |   14 +-
 t/t6402-merge-rename.sh                       |  125 +-
 t/t6404-recursive-merge.sh                    |   21 +-
 t/t6406-merge-attr.sh                         |    7 +-
 t/t6416-recursive-corner-cases.sh             |  194 +-
 t/t6421-merge-partial-clone.sh                |    7 +-
 t/t6422-merge-rename-corner-cases.sh          |   31 +-
 t/t6423-merge-rename-directories.sh           |  517 +--
 t/t6424-merge-unrelated-index-changes.sh      |    8 +-
 t/t6426-merge-skip-unneeded-updates.sh        |    4 +-
 t/t6428-merge-conflicts-sparse.sh             |    2 -
 t/t6430-merge-recursive.sh                    |   46 +-
 t/t6434-merge-recursive-rename-options.sh     |   16 +-
 t/t6436-merge-overwrite.sh                    |   17 +-
 t/t6437-submodule-merge.sh                    |   65 +-
 t/t6438-submodule-directory-file-conflicts.sh |    5 -
 t/t6439-merge-co-error-msgs.sh                |    2 +-
 t/t7402-submodule-rebase.sh                   |    7 +-
 t/t7602-merge-octopus-many.sh                 |    9 +-
 t/t7610-mergetool.sh                          |   40 +-
 t/t7615-diff-algo-with-mergy-operations.sh    |    2 -
 t/test-lib.sh                                 |    2 -
 46 files changed, 538 insertions(+), 5241 deletions(-)
 delete mode 100644 merge-recursive.c
 delete mode 100644 merge-recursive.h
 delete mode 100644 t/lib-merge.sh

base-commit: 5b97a56fa0e7d580dc8865b73107407c9b3f0eff
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1898%2Fnewren%2Fendit-quote-debugging-unquote-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1898/newren/endit-quote-debugging-unquote-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1898

Range-diff vs v2:

 1:  0e150ee9065 = 1:  0e150ee9065 checkout: replace merge_trees() with merge_ort_nonrecursive()
 2:  b093d74968b = 2:  b093d74968b builtin/merge-recursive: switch to using merge_ort_generic()
 3:  cf774437123 = 3:  cf774437123 merge-ort: enable diff-algorithms other than histogram
 4:  6203589ac17 = 4:  6203589ac17 sequencer: switch non-recursive merges over to ort
 5:  8821f22d5ea = 5:  8821f22d5ea merge, sequencer: switch recursive merges over to ort
 6:  770f611c2b6 ! 6:  e696cb472bd merge-recursive.[ch]: thoroughly debug these

@@ Commit message
the former to the latter, and then debug the remainder of merge-recursive.[ch].

+ Joking aside, merge-ort was always intended to replace merge-recursive.
+ It has numerous advantages over merge-recursive (operates much faster,
+ can operate without a worktree or index, and fixes a number of known
+ bugs and suboptimal merges). Since we have now replaced all callers of
+ merge-recursive with equivalent functions from merge-ort, move the
+ shared functions from the former to the latter, and delete the remainder
+ of merge-recursive.[ch].
+

## Documentation/merge-strategies.adoc ##

 7:  a6501ee85fa = 7:  5dce9096e23 tests: remove GIT_TEST_MERGE_ALGORITHM and test_expect_merge_algorithm
 8:  d1dea986646 = 8:  bf2d462c4e5 builtin/{merge,rebase,revert}: remove GIT_TEST_MERGE_ALGORITHM
Signature · 1 line
gitgitgadget
#23 lore raw

PATCH v3 1/8 checkout: replace merge_trees() with merge_ort_nonrecursive()

From: Elijah Newren <[email protected]>

Replace the use of merge_trees() from merge-recursive.[ch] with the merge-ort equivalent.

---
 builtin/checkout.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
builtin/checkout.c +5 -5
index 01ea9ff8b28..67879e72362 100644
@@ -17,7 +17,7 @@
 #include "merge-ll.h"
 #include "lockfile.h"
 #include "mem-pool.h"
-#include "merge-recursive.h"
+#include "merge-ort-wrappers.h"
 #include "object-name.h"
 #include "object-store-ll.h"
 #include "parse-options.h"
@@ -907,10 +907,10 @@ static int merge_working_tree(const struct checkout_opts *opts,
 			o.branch1 = new_branch_info->name;
 			o.branch2 = "local";
 			o.conflict_style = opts->conflict_style;
-			ret = merge_trees(&o,
-					  new_tree,
-					  work,
-					  old_tree);
+			ret = merge_ort_nonrecursive(&o,
+						     new_tree,
+						     work,
+						     old_tree);
 			if (ret < 0)
 				exit(128);
 			ret = reset_tree(new_tree,
Signature · 1 line
gitgitgadget
#24 lore raw

PATCH v3 2/8 builtin/merge-recursive: switch to using merge_ort_generic()

In reply to Elijah Newren via GitGitGadget

From: Elijah Newren <[email protected]>

Switch from merge-recursive to merge-ort. Adjust the following testcases due to the switch:

* t6430: most of the test differences here were due to improved D/F conflict handling explained in more detail in ef527787089c (merge
tests: expect improved directory/file conflict handling in ort, 2020-10-26). These changes weren't made to this test back in that commit simply because I had been looking at git merge rather than git merge-recursive. The final test in this testsuite, though, was expunged because it was looking for specific output, and the calls to output_commit_title() were discarded from merge_ort_internal() in its adaptation from merge_recursive_internal(); see 8119214f4e70 (merge-ort: implement merge_incore_recursive(), 2020-12-16).

* t6434: This test is built entirely around rename/delete conflicts, which had a suboptimal handling under merge-recursive. As explained in more detail in commits 1f3c9ba707 ("t6425: be more flexible with rename/delete conflict messages", 2020-08-10) and 727c75b23f ("t6404,
t6423: expect improved rename/delete handling in ort backend", 2020-10-26), rename/delete conflicts should each have two entries in the index rather than just one. Adjust the expectations for all the tests in this testcase to see the two entries per rename/delete conflict.

* t6424: merge-recursive had a special check-if-toplevel-trees-match check that it ran at the beginning on both the merge-base and the other side being merged in. In such a case, it exited early and printed an "Already up to date." message. merge-ort got rid of this, and instead checks the merge base tree matching the other side throughout the tree instead of just at the toplevel, allowing it to avoid recursing into various subtrees. As part of that, it got rid of the specialty toplevel message. That message hasn't been missed for years from git merge, so I don't think it is necessary to keep it just for git merge-recursive, especially since the latter is rarely used. (git itself only references it in the testsuite, whereas it used to power one of the three rebase backends that existed once upon a time.)

---
 builtin/merge-recursive.c                 |  4 +--
 t/t6424-merge-unrelated-index-changes.sh  |  4 ++-
 t/t6430-merge-recursive.sh                | 43 +++++------------------
 t/t6434-merge-recursive-rename-options.sh | 16 +++++----
 4 files changed, 22 insertions(+), 45 deletions(-)
builtin/merge-recursive.c +2 -2
index abfc060e28a..03b5100cfae 100644
@@ -3,7 +3,7 @@
 #include "advice.h"
 #include "gettext.h"
 #include "hash.h"
-#include "merge-recursive.h"
+#include "merge-ort-wrappers.h"
 #include "object-name.h"
 
 static const char builtin_merge_recursive_usage[] =
@@ -89,7 +89,7 @@ int cmd_merge_recursive(int argc,
 	if (o.verbosity >= 3)
 		printf(_("Merging %s with %s\n"), o.branch1, o.branch2);
 
-	failed = merge_recursive_generic(&o, &h1, &h2, bases_count, bases, &result);
+	failed = merge_ort_generic(&o, &h1, &h2, bases_count, bases, &result);
 
 	free(better1);
 	free(better2);
t/t6424-merge-unrelated-index-changes.sh +3 -1
index 7677c5f08d0..084bfe7e6ce 100755
@@ -176,9 +176,11 @@ test_expect_success 'merge-recursive, when index==head but head!=HEAD' '
 	# Make index match B
 	git diff C B -- | git apply --cached &&
 	test_when_finished "git clean -fd" &&  # Do not leave untracked around
+	git write-tree >index-before &&
 	# Merge B & F, with B as "head"
 	git merge-recursive A -- B F > out &&
-	test_grep "Already up to date" out
+	git write-tree >index-after &&
+	test_cmp index-before index-after
 '
 
 test_expect_success 'recursive, when file has staged changes not matching HEAD nor what a merge would give' '
t/t6430-merge-recursive.sh +8 -35

77 lines not on this page · raw message

t/t6434-merge-recursive-rename-options.sh +9 -7
index 6e913c30a13..5a6f74839cb 100755
@@ -34,7 +34,9 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 get_expected_stages () {
 	git checkout rename -- $1-new &&
 	git ls-files --stage $1-new >expected-stages-undetected-$1 &&
-	sed "s/ 0	/ 2	/" <expected-stages-undetected-$1 \
+	git ls-tree HEAD^ $1-old >tmp &&
+	git ls-tree HEAD  $1-new >>tmp &&
+	cat tmp | awk '{print $1 " " $3 " " NR "\t" '$1'"-new"}' \
 		>expected-stages-detected-$1 &&
 	git read-tree -u --reset HEAD
 }
@@ -51,11 +53,11 @@ rename_undetected () {
 
 check_common () {
 	git ls-files --stage >stages-actual &&
-	test_line_count = 4 stages-actual
+	test_line_count = $1 stages-actual
 }
 
 check_threshold_0 () {
-	check_common &&
+	check_common 8 &&
 	rename_detected 0 &&
 	rename_detected 1 &&
 	rename_detected 2 &&
@@ -63,7 +65,7 @@ check_threshold_0 () {
 }
 
 check_threshold_1 () {
-	check_common &&
+	check_common 7 &&
 	rename_undetected 0 &&
 	rename_detected 1 &&
 	rename_detected 2 &&
@@ -71,7 +73,7 @@ check_threshold_1 () {
 }
 
 check_threshold_2 () {
-	check_common &&
+	check_common 6 &&
 	rename_undetected 0 &&
 	rename_undetected 1 &&
 	rename_detected 2 &&
@@ -79,7 +81,7 @@ check_threshold_2 () {
 }
 
 check_exact_renames () {
-	check_common &&
+	check_common 5 &&
 	rename_undetected 0 &&
 	rename_undetected 1 &&
 	rename_undetected 2 &&
@@ -87,7 +89,7 @@ check_exact_renames () {
 }
 
 check_no_renames () {
-	check_common &&
+	check_common 4 &&
 	rename_undetected 0 &&
 	rename_undetected 1 &&
 	rename_undetected 2 &&
Signature · 1 line
gitgitgadget
#25 lore raw

PATCH v3 3/8 merge-ort: enable diff-algorithms other than histogram

In reply to Elijah Newren via GitGitGadget

From: Elijah Newren <[email protected]>

The ort merge strategy has always used the histogram diff algorithm. The recursive merge strategy, in contrast, defaults to the myers diff algorithm, while allowing it to be changed.

Change the ort merge strategy to allow different diff algorithms, by removing the hard coded value in merge_start() and instead just making it a default in init_merge_options(). Technically, this also changes the default diff algorithm for the recursive backend too, but we're going to remove the final callers of the recursive backend in the next two commits.

---
 Documentation/merge-strategies.adoc | 29 +++++++++++++++--------------
 merge-ort.c                         |  3 ---
 merge-recursive.c                   |  1 +
 3 files changed, 16 insertions(+), 17 deletions(-)
Documentation/merge-strategies.adoc +15 -14
index 59f5ae36ccb..8c87dd70210 100644
@@ -87,6 +87,20 @@ no-renames;;
 	configuration variable.
 	See also linkgit:git-diff[1] `--no-renames`.
 
+histogram;;
+	Deprecated synonym for `diff-algorithm=histogram`.
+
+patience;;
+	Deprecated synonym for `diff-algorithm=patience`.
+
+diff-algorithm=[histogram|minimal|myers|patience];;
+	Use a different diff algorithm while merging, which can help
+	avoid mismerges that occur due to unimportant matching lines
+	(such as braces from distinct functions).  See also
+	linkgit:git-diff[1] `--diff-algorithm`.  Note that `ort`
+	defaults to `diff-algorithm=histogram`, while regular diffs
+	currently default to the `diff.algorithm` config setting.
+
 subtree[=<path>];;
 	This option is a more advanced form of 'subtree' strategy, where
 	the strategy makes a guess on how two trees must be shifted to
@@ -111,20 +125,7 @@ recursive::
 For a path that is a submodule, the same caution as 'ort' applies to this
 strategy.
 +
-The 'recursive' strategy takes the same options as 'ort'.  However,
-there are two additional options that 'ort' ignores (not documented
-above) that are potentially useful with the 'recursive' strategy:
-
-patience;;
-	Deprecated synonym for `diff-algorithm=patience`.
-
-diff-algorithm=[patience|minimal|histogram|myers];;
-	Use a different diff algorithm while merging, which can help
-	avoid mismerges that occur due to unimportant matching lines
-	(such as braces from distinct functions).  See also
-	linkgit:git-diff[1] `--diff-algorithm`.  Note that `ort`
-	specifically uses `diff-algorithm=histogram`, while `recursive`
-	defaults to the `diff.algorithm` config setting.
+The 'recursive' strategy takes the same options as 'ort'.
 
 resolve::
 	This can only resolve two heads (i.e. the current branch
merge-ort.c +0 -3
index 2b7d86aa4ec..14a7ae4a6bf 100644
@@ -4957,9 +4957,6 @@ static void merge_start(struct merge_options *opt, struct merge_result *result)
 	}
 	trace2_region_leave("merge", "sanity checks", opt->repo);
 
-	/* Default to histogram diff.  Actually, just hardcode it...for now. */
-	opt->xdl_opts = DIFF_WITH_ALG(opt, HISTOGRAM_DIFF);
-
 	/* Handle attr direction stuff for renormalization */
 	if (opt->renormalize)
 		git_attr_set_direction(GIT_ATTR_CHECKOUT);
merge-recursive.c +1 -0
index 884ccf99a58..f3df127ad9b 100644
@@ -3981,6 +3981,7 @@ static void init_merge_options(struct merge_options *opt,
 	opt->renormalize = 0;
 
 	opt->conflict_style = -1;
+	opt->xdl_opts = DIFF_WITH_ALG(opt, HISTOGRAM_DIFF);
 
 	merge_recursive_config(opt, ui);
 	merge_verbosity = getenv("GIT_MERGE_VERBOSITY");
Signature · 1 line
gitgitgadget
#26 lore raw

PATCH v3 4/8 sequencer: switch non-recursive merges over to ort

In reply to Elijah Newren via GitGitGadget

From: Elijah Newren <[email protected]>

The do_recursive_merge() function, which is somewhat misleadingly named since its purpose in life is to do a *non*-recursive merge, had code to allow either using the recursive or ort backends. The default has been ort for a very long time, let's just remove the code path for allowing the recursive backend to be selected.

---
 sequencer.c | 35 +++++++++++++----------------------
 1 file changed, 13 insertions(+), 22 deletions(-)
sequencer.c +13 -22
index ad0ab75c8d4..b5d91fd3515 100644
@@ -781,28 +781,19 @@ static int do_recursive_merge(struct repository *r,
 	for (i = 0; i < opts->xopts.nr; i++)
 		parse_merge_opt(&o, opts->xopts.v[i]);
 
-	if (!opts->strategy || !strcmp(opts->strategy, "ort")) {
-		memset(&result, 0, sizeof(result));
-		merge_incore_nonrecursive(&o, base_tree, head_tree, next_tree,
-					    &result);
-		show_output = !is_rebase_i(opts) || !result.clean;
-		/*
-		 * TODO: merge_switch_to_result will update index/working tree;
-		 * we only really want to do that if !result.clean || this is
-		 * the final patch to be picked.  But determining this is the
-		 * final patch would take some work, and "head_tree" would need
-		 * to be replace with the tree the index matched before we
-		 * started doing any picks.
-		 */
-		merge_switch_to_result(&o, head_tree, &result, 1, show_output);
-		clean = result.clean;
-	} else {
-		ensure_full_index(r->index);
-		clean = merge_trees(&o, head_tree, next_tree, base_tree);
-		if (is_rebase_i(opts) && clean <= 0)
-			fputs(o.obuf.buf, stdout);
-		strbuf_release(&o.obuf);
-	}
+	memset(&result, 0, sizeof(result));
+	merge_incore_nonrecursive(&o, base_tree, head_tree, next_tree, &result);
+	show_output = !is_rebase_i(opts) || !result.clean;
+	/*
+	 * TODO: merge_switch_to_result will update index/working tree;
+	 * we only really want to do that if !result.clean || this is
+	 * the final patch to be picked.  But determining this is the
+	 * final patch would take some work, and "head_tree" would need
+	 * to be replace with the tree the index matched before we
+	 * started doing any picks.
+	 */
+	merge_switch_to_result(&o, head_tree, &result, 1, show_output);
+	clean = result.clean;
 	if (clean < 0) {
 		rollback_lock_file(&index_lock);
 		return clean;
Signature · 1 line
gitgitgadget
#27 lore raw

PATCH v3 5/8 merge, sequencer: switch recursive merges over to ort

In reply to Elijah Newren via GitGitGadget

From: Elijah Newren <[email protected]>

More precisely, replace calls to merge_recursive() with merge_ort_recursive().

Also change t7615 to quit calling out recursive; it is not needed anymore, and we are in fact using ort now.

---
 builtin/merge.c                            |  9 ++-------
 sequencer.c                                | 23 ++++++++--------------
 t/t7615-diff-algo-with-mergy-operations.sh |  2 --
 3 files changed, 10 insertions(+), 24 deletions(-)
builtin/merge.c +2 -7
index ba9faf126aa..c0bbdab7104 100644
@@ -39,7 +39,6 @@
 #include "rerere.h"
 #include "help.h"
 #include "merge.h"
-#include "merge-recursive.h"
 #include "merge-ort-wrappers.h"
 #include "resolve-undo.h"
 #include "remote.h"
@@ -750,12 +749,8 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
 
 		repo_hold_locked_index(the_repository, &lock,
 				       LOCK_DIE_ON_ERROR);
-		if (!strcmp(strategy, "ort"))
-			clean = merge_ort_recursive(&o, head, remoteheads->item,
-						    reversed, &result);
-		else
-			clean = merge_recursive(&o, head, remoteheads->item,
-						reversed, &result);
+		clean = merge_ort_recursive(&o, head, remoteheads->item,
+					    reversed, &result);
 		free_commit_list(reversed);
 		strbuf_release(&o.obuf);
 
sequencer.c +8 -15
index b5d91fd3515..8bb49735891 100644
@@ -4319,20 +4319,13 @@ static int do_merge(struct repository *r,
 	o.branch2 = ref_name.buf;
 	o.buffer_output = 2;
 
-	if (!opts->strategy || !strcmp(opts->strategy, "ort")) {
-		/*
-		 * TODO: Should use merge_incore_recursive() and
-		 * merge_switch_to_result(), skipping the call to
-		 * merge_switch_to_result() when we don't actually need to
-		 * update the index and working copy immediately.
-		 */
-		ret = merge_ort_recursive(&o,
-					  head_commit, merge_commit, bases,
-					  &i);
-	} else {
-		ret = merge_recursive(&o, head_commit, merge_commit, bases,
-				      &i);
-	}
+	/*
+	 * TODO: Should use merge_incore_recursive() and
+	 * merge_switch_to_result(), skipping the call to
+	 * merge_switch_to_result() when we don't actually need to
+	 * update the index and working copy immediately.
+	 */
+	ret = merge_ort_recursive(&o, head_commit, merge_commit, bases, &i);
 	if (ret <= 0)
 		fputs(o.obuf.buf, stdout);
 	strbuf_release(&o.obuf);
@@ -4343,7 +4336,7 @@ static int do_merge(struct repository *r,
 		goto leave_merge;
 	}
 	/*
-	 * The return value of merge_recursive() is 1 on clean, and 0 on
+	 * The return value of merge_ort_recursive() is 1 on clean, and 0 on
 	 * unclean merge.
 	 *
 	 * Let's reverse that, so that do_merge() returns 0 upon success and
t/t7615-diff-algo-with-mergy-operations.sh +0 -2
index ac5863e788c..5822d02d517 100755
@@ -22,8 +22,6 @@ test_expect_success 'setup' '
 	git tag c2
 '
 
-GIT_TEST_MERGE_ALGORITHM=recursive
-
 test_expect_success 'merge c2 to c1 with recursive merge strategy fails with the current default myers diff algorithm' '
 	git reset --hard c1 &&
 	test_must_fail git merge -s recursive -Xdiff-algorithm=myers c2
Signature · 1 line
gitgitgadget
#28 lore raw

PATCH v3 6/8 merge-recursive.[ch]: thoroughly debug these

In reply to Elijah Newren via GitGitGadget

From: Elijah Newren <[email protected]>

As a wise man once told me, "Deleted code is debugged code!" So, move the functions that are shared between merge-recursive and merge-ort from the former to the latter, and then debug the remainder of merge-recursive.[ch].

Joking aside, merge-ort was always intended to replace merge-recursive. It has numerous advantages over merge-recursive (operates much faster, can operate without a worktree or index, and fixes a number of known bugs and suboptimal merges). Since we have now replaced all callers of merge-recursive with equivalent functions from merge-ort, move the shared functions from the former to the latter, and delete the remainder of merge-recursive.[ch].

---
 Documentation/merge-strategies.adoc          |   24 +-
 Documentation/technical/sparse-checkout.adoc |    2 -
 Makefile                                     |    1 -
 merge-ort-wrappers.h                         |    2 +-
 merge-ort.c                                  |  159 +
 merge-ort.h                                  |   60 +-
 merge-recursive.c                            | 4080 ------------------
 merge-recursive.h                            |  132 -
 meson.build                                  |    1 -
 9 files changed, 225 insertions(+), 4236 deletions(-)
 delete mode 100644 merge-recursive.c
 delete mode 100644 merge-recursive.h
Documentation/merge-strategies.adoc +6 -18
index 8c87dd70210..9e034f447e7 100644
@@ -109,23 +109,11 @@ subtree[=<path>];;
 	two trees to match.
 
 recursive::
-	This can only resolve two heads using a 3-way merge
-	algorithm.  When there is more than one common
-	ancestor that can be used for 3-way merge, it creates a
-	merged tree of the common ancestors and uses that as
-	the reference tree for the 3-way merge.  This has been
-	reported to result in fewer merge conflicts without
-	causing mismerges by tests done on actual merge commits
-	taken from Linux 2.6 kernel development history.
-	Additionally this can detect and handle merges involving
-	renames.  It does not make use of detected copies.  This was
-	the default strategy for resolving two heads from Git v0.99.9k
-	until v2.33.0.
-+
-For a path that is a submodule, the same caution as 'ort' applies to this
-strategy.
-+
-The 'recursive' strategy takes the same options as 'ort'.
+	This is now a synonym for `ort`.  It was an alternative
+	implementation until v2.49.0, but was redirected to mean `ort`
+	in v2.50.0.  The previous recursive strategy was the default
+	strategy for resolving two heads from Git v0.99.9k until
+	v2.33.0.
 
 resolve::
 	This can only resolve two heads (i.e. the current branch
@@ -146,7 +134,7 @@ ours::
 	ignoring all changes from all other branches.  It is meant to
 	be used to supersede old development history of side
 	branches.  Note that this is different from the -Xours option to
-	the 'recursive' merge strategy.
+	the 'ort' merge strategy.
 
 subtree::
 	This is a modified `ort` strategy. When merging trees A and
Documentation/technical/sparse-checkout.adoc +0 -2
index d9686593548..dc2e763bbec 100644
@@ -356,8 +356,6 @@ understanding these differences can be beneficial.
     The behavior for these commands somewhat depends upon the merge
     strategy being used:
       * `ort` behaves as described above
-      * `recursive` tries to not vivify files unnecessarily, but does sometimes
-	vivify files without conflicts.
       * `octopus` and `resolve` will always vivify any file changed in the merge
 	relative to the first parent, which is rather suboptimal.
 
Makefile +0 -1
index ac32d2d0bda..4fbd29cc7e9 100644
@@ -1069,7 +1069,6 @@ LIB_OBJS += merge-blobs.o
 LIB_OBJS += merge-ll.o
 LIB_OBJS += merge-ort.o
 LIB_OBJS += merge-ort-wrappers.o
-LIB_OBJS += merge-recursive.o
 LIB_OBJS += merge.o
 LIB_OBJS += midx.o
 LIB_OBJS += midx-write.o
merge-ort-wrappers.h +1 -1
index aeffa1c87b4..b7e1ced9d7f 100644
@@ -1,7 +1,7 @@
 #ifndef MERGE_ORT_WRAPPERS_H
 #define MERGE_ORT_WRAPPERS_H
 
-#include "merge-recursive.h"
+#include "merge-ort.h"
 
 /*
  * rename-detecting three-way merge, no recursion.
merge-ort.c +159 -0

168 lines not on this page · raw message

merge-ort.h +59 -1

79 lines not on this page · raw message

merge-recursive.c deleted +0 -4080

2,694 lines not on this page · raw message

merge-recursive.h deleted +0 -132

132 lines not on this page · raw message

meson.build +0 -1
index e98cfa4909f..8448df9d15e 100644

Lines of this file are not shown. View the raw message.

The rest of this diff is not shown. View the raw message.

Signature · 1 line
gitgitgadget

Part of the diff is not shown here. View the raw message.

#29 lore raw

PATCH v3 7/8 tests: remove GIT_TEST_MERGE_ALGORITHM and test_expect_merge_algorithm

In reply to Elijah Newren via GitGitGadget

From: Elijah Newren <[email protected]>

Both of these existed to allow us to reuse all the merge-related tests in the testsuite while easily flipping between the 'recursive' and the 'ort' backends. Now that we have removed merge-recursive and remapped 'recursive' to mean 'ort', we don't need this scaffolding anymore.

---
 ci/run-build-and-tests.sh                     |   1 -
 t/lib-merge.sh                                |  13 -
 t/t1092-sparse-checkout-compatibility.sh      |   2 -
 t/t2501-cwd-empty.sh                          |   2 -
 t/t3512-cherry-pick-submodule.sh              |   5 -
 t/t3513-revert-submodule.sh                   |   4 -
 t/t4069-remerge-diff.sh                       |   7 -
 t/t4301-merge-tree-write-tree.sh              |   7 -
 t/t5572-pull-submodule.sh                     |   5 -
 t/t6400-merge-df.sh                           |  14 +-
 t/t6402-merge-rename.sh                       | 125 +----
 t/t6404-recursive-merge.sh                    |  21 +-
 t/t6406-merge-attr.sh                         |   7 +-
 t/t6416-recursive-corner-cases.sh             | 194 ++-----
 t/t6421-merge-partial-clone.sh                |   7 +-
 t/t6422-merge-rename-corner-cases.sh          |  31 +-
 t/t6423-merge-rename-directories.sh           | 517 ++++--------------
 t/t6424-merge-unrelated-index-changes.sh      |   4 +-
 t/t6426-merge-skip-unneeded-updates.sh        |   4 +-
 t/t6428-merge-conflicts-sparse.sh             |   2 -
 t/t6430-merge-recursive.sh                    |   3 +-
 t/t6436-merge-overwrite.sh                    |  17 +-
 t/t6437-submodule-merge.sh                    |  65 +--
 t/t6438-submodule-directory-file-conflicts.sh |   5 -
 t/t6439-merge-co-error-msgs.sh                |   2 +-
 t/t7402-submodule-rebase.sh                   |   7 +-
 t/t7602-merge-octopus-many.sh                 |   9 +-
 t/t7610-mergetool.sh                          |  40 +-
 t/test-lib.sh                                 |   2 -
 29 files changed, 248 insertions(+), 874 deletions(-)
 delete mode 100644 t/lib-merge.sh
ci/run-build-and-tests.sh +0 -1
index 1c698467235..f99b7db2ee8 100755
@@ -20,7 +20,6 @@ linux-breaking-changes)
 linux-TEST-vars)
 	export OPENSSL_SHA1_UNSAFE=YesPlease
 	export GIT_TEST_SPLIT_INDEX=yes
-	export GIT_TEST_MERGE_ALGORITHM=recursive
 	export GIT_TEST_FULL_IN_PACK_ARRAY=true
 	export GIT_TEST_OE_SIZE=10
 	export GIT_TEST_OE_DELTA_SIZE=5
t/lib-merge.sh deleted +0 -13
deleted file mode 100644
index 8734ebfc17f..00000000000
@@ -1,13 +0,0 @@
-# Helper functions used by merge tests.
-
-test_expect_merge_algorithm () {
-	status_for_recursive=$1 status_for_ort=$2
-	shift 2
-
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		test_expect_${status_for_ort} "$@"
-	else
-		test_expect_${status_for_recursive} "$@"
-	fi
-}
t/t1092-sparse-checkout-compatibility.sh +0 -2
index a4c7c41fc00..f9b448792cb 100755
@@ -1486,7 +1486,6 @@ test_expect_success 'sparse-index is not expanded' '
 	ensure_not_expanded checkout -f update-deep &&
 	test_config -C sparse-index pull.twohead ort &&
 	(
-		sane_unset GIT_TEST_MERGE_ALGORITHM &&
 		for OPERATION in "merge -m merge" cherry-pick rebase
 		do
 			ensure_not_expanded merge -m merge update-folder1 &&
@@ -1506,7 +1505,6 @@ test_expect_success 'sparse-index is not expanded: merge conflict in cone' '
 	done &&
 
 	(
-		sane_unset GIT_TEST_MERGE_ALGORITHM &&
 		git -C sparse-index config pull.twohead ort &&
 		ensure_not_expanded ! merge -m merged expand-right
 	)
t/t2501-cwd-empty.sh +0 -2
index f6d8d7d03d7..be9140bbaa4 100755
@@ -117,8 +117,6 @@ test_expect_success 'merge fails if cwd needs to be removed; recursive friendly'
 	grep "Refusing to remove the current working directory" error
 '
 
-GIT_TEST_MERGE_ALGORITHM=ort
-
 test_expect_success 'merge fails if cwd needs to be removed' '
 	test_required_dir_removal git merge fd_conflict
 '
t/t3512-cherry-pick-submodule.sh +0 -5
index f22d1ddead1..85a79326977 100755
@@ -8,11 +8,6 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-submodule-update.sh
 
-if test "$GIT_TEST_MERGE_ALGORITHM" != ort
-then
-	KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1
-	KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1
-fi
 test_submodule_switch "cherry-pick"
 
 test_expect_success 'unrelated submodule/file conflict is ignored' '
t/t3513-revert-submodule.sh +0 -4
index 8bfe3ed2467..32e15c72eee 100755
@@ -30,10 +30,6 @@ git_revert () {
 	git revert HEAD
 }
 
-if test "$GIT_TEST_MERGE_ALGORITHM" != ort
-then
-	KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1
-fi
 test_submodule_switch_func "git_revert"
 
 test_done
t/t4069-remerge-diff.sh +0 -7
index c6c94aef14d..966882ce912 100755
@@ -4,13 +4,6 @@ test_description='remerge-diff handling'
 
 . ./test-lib.sh
 
-# This test is ort-specific
-if test "${GIT_TEST_MERGE_ALGORITHM}" != ort
-then
-	skip_all="GIT_TEST_MERGE_ALGORITHM != ort"
-	test_done
-fi
-
 test_expect_success 'setup basic merges' '
 	test_write_lines 1 2 3 4 5 6 7 8 9 >numbers &&
 	git add numbers &&
t/t4301-merge-tree-write-tree.sh +0 -7
index 44f7d077593..f9c5883a7f7 100755
@@ -4,13 +4,6 @@ test_description='git merge-tree --write-tree'
 
 . ./test-lib.sh
 
-# This test is ort-specific
-if test "$GIT_TEST_MERGE_ALGORITHM" != "ort"
-then
-	skip_all="GIT_TEST_MERGE_ALGORITHM != ort"
-	test_done
-fi
-
 test_expect_success setup '
 	test_write_lines 1 2 3 4 5 >numbers &&
 	echo hello >greeting &&
t/t5572-pull-submodule.sh +0 -5
index f7650e8475e..45f384dd328 100755
@@ -45,11 +45,6 @@ git_pull_noff () {
 	$2 git pull --no-ff
 }
 
-if test "$GIT_TEST_MERGE_ALGORITHM" != ort
-then
-	KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1
-	KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1
-fi
 test_submodule_switch_func "git_pull_noff"
 
 test_expect_success 'setup' '
t/t6400-merge-df.sh +2 -12
index 3de4ef6bd9e..b46fe494409 100755
@@ -84,12 +84,7 @@ test_expect_success 'modify/delete + directory/file conflict' '
 
 	test_stdout_line_count = 5 git ls-files -s &&
 	test_stdout_line_count = 4 git ls-files -u &&
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		test_stdout_line_count = 0 git ls-files -o
-	else
-		test_stdout_line_count = 1 git ls-files -o
-	fi &&
+	test_stdout_line_count = 0 git ls-files -o &&
 
 	test_path_is_file letters/file &&
 	test_path_is_file letters.txt &&
@@ -105,12 +100,7 @@ test_expect_success 'modify/delete + directory/file conflict; other way' '
 
 	test_stdout_line_count = 5 git ls-files -s &&
 	test_stdout_line_count = 4 git ls-files -u &&
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		test_stdout_line_count = 0 git ls-files -o
-	else
-		test_stdout_line_count = 1 git ls-files -o
-	fi &&
+	test_stdout_line_count = 0 git ls-files -o &&
 
 	test_path_is_file letters/file &&
 	test_path_is_file letters.txt &&
t/t6402-merge-rename.sh +28 -97

177 lines not on this page · raw message

t/t6404-recursive-merge.sh +3 -18
index ae687f2ce54..346f3608b98 100755
@@ -108,12 +108,7 @@ test_expect_success 'refuse to merge binary files' '
 	printf "\0\0" >binary-file &&
 	git add binary-file &&
 	git commit -m binary2 &&
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		test_must_fail git merge F >merge_output
-	else
-		test_must_fail git merge F 2>merge_output
-	fi &&
+	test_must_fail git merge F >merge_output &&
 	grep "Cannot merge binary files: binary-file (HEAD vs. F)" merge_output
 '
 
@@ -129,22 +124,12 @@ test_expect_success 'mark rename/delete as unmerged' '
 	test_tick &&
 	git commit -m rename &&
 	test_must_fail git merge delete &&
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		test 2 = $(git ls-files --unmerged | wc -l)
-	else
-		test 1 = $(git ls-files --unmerged | wc -l)
-	fi &&
+	test 2 = $(git ls-files --unmerged | wc -l) &&
 	git rev-parse --verify :2:a2 &&
 	test_must_fail git rev-parse --verify :3:a2 &&
 	git checkout -f delete &&
 	test_must_fail git merge rename &&
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		test 2 = $(git ls-files --unmerged | wc -l)
-	else
-		test 1 = $(git ls-files --unmerged | wc -l)
-	fi &&
+	test 2 = $(git ls-files --unmerged | wc -l) &&
 	test_must_fail git rev-parse --verify :2:a2 &&
 	git rev-parse --verify :3:a2
 '
t/t6406-merge-attr.sh +1 -6
index 66e01464b5e..8f6fbef0024 100755
@@ -259,12 +259,7 @@ test_expect_success 'binary files with union attribute' '
 	printf "two\0" >bin.txt &&
 	git commit -am two &&
 
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		test_must_fail git merge bin-main >output
-	else
-		test_must_fail git merge bin-main 2>output
-	fi &&
+	test_must_fail git merge bin-main >output &&
 	grep -i "warning.*cannot merge.*HEAD vs. bin-main" output
 '
 
t/t6416-recursive-corner-cases.sh +55 -139

278 lines not on this page · raw message

t/t6421-merge-partial-clone.sh +3 -4
index b99f29ef9ba..6eb51285a0c 100755
@@ -27,7 +27,6 @@ test_description="limiting blob downloads when merging with partial clones"
 #                     files that might be renamed into each other's paths.)
 
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/lib-merge.sh
 
 test_setup_repo () {
 	test -d server && return
@@ -207,7 +206,7 @@ test_setup_repo () {
 #
 #   Summary: 2 fetches (1 for 2 objects, 1 for 1 object)
 #
-test_expect_merge_algorithm failure success 'Objects downloaded for single relevant rename' '
+test_expect_success 'Objects downloaded for single relevant rename' '
 	test_setup_repo &&
 	git clone --sparse --filter=blob:none "file://$(pwd)/server" objects-single &&
 	(
@@ -297,7 +296,7 @@ test_expect_merge_algorithm failure success 'Objects downloaded for single relev
 #      this are not all that common.)
 #   Summary: 1 fetches for 6 objects
 #
-test_expect_merge_algorithm failure success 'Objects downloaded when a directory rename triggered' '
+test_expect_success 'Objects downloaded when a directory rename triggered' '
 	test_setup_repo &&
 	git clone --sparse --filter=blob:none "file://$(pwd)/server" objects-dir &&
 	(
@@ -399,7 +398,7 @@ test_expect_merge_algorithm failure success 'Objects downloaded when a directory
 #
 #   Summary: 4 fetches (1 for 6 objects, 1 for 8, 1 for 3, 1 for 2)
 #
-test_expect_merge_algorithm failure success 'Objects downloaded with lots of renames and modifications' '
+test_expect_success 'Objects downloaded with lots of renames and modifications' '
 	test_setup_repo &&
 	git clone --sparse --filter=blob:none "file://$(pwd)/server" objects-many &&
 	(
t/t6422-merge-rename-corner-cases.sh +7 -24
index 62b49c67e22..9cbe7ca7824 100755
@@ -7,7 +7,6 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/lib-merge.sh
 
 test_setup_rename_delete_untracked () {
 	git init rename-delete-untracked &&
@@ -316,12 +315,7 @@ test_expect_success 'rename/directory conflict + clean content merge' '
 		git ls-files -u >out &&
 		test_line_count = 1 out &&
 		git ls-files -o >out &&
-		if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-		then
-			test_line_count = 1 out
-		else
-			test_line_count = 2 out
-		fi &&
+		test_line_count = 1 out &&
 
 		echo 0 >expect &&
 		git cat-file -p base:file >>expect &&
@@ -350,12 +344,7 @@ test_expect_success 'rename/directory conflict + content merge conflict' '
 		git ls-files -u >out &&
 		test_line_count = 3 out &&
 		git ls-files -o >out &&
-		if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-		then
-			test_line_count = 1 out
-		else
-			test_line_count = 2 out
-		fi &&
+		test_line_count = 1 out &&
 
 		git cat-file -p left-conflict:newfile >left &&
 		git cat-file -p base:file    >base &&
@@ -369,14 +358,8 @@ test_expect_success 'rename/directory conflict + content merge conflict' '
 
 		git rev-parse >expect   \
 			base:file       left-conflict:newfile right:file &&
-		if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-		then
-			git rev-parse >actual \
-				:1:newfile~HEAD :2:newfile~HEAD :3:newfile~HEAD
-		else
-			git rev-parse >actual \
-				:1:newfile      :2:newfile      :3:newfile
-		fi &&
+		git rev-parse >actual \
+			:1:newfile~HEAD :2:newfile~HEAD :3:newfile~HEAD &&
 		test_cmp expect actual &&
 
 		test_path_is_file newfile/realfile &&
@@ -896,7 +879,7 @@ test_setup_rad () {
 	)
 }
 
-test_expect_merge_algorithm failure success 'rad-check: rename/add/delete conflict' '
+test_expect_success 'rad-check: rename/add/delete conflict' '
 	test_setup_rad &&
 	(
 		cd rad &&
@@ -969,7 +952,7 @@ test_setup_rrdd () {
 	)
 }
 
-test_expect_merge_algorithm failure success 'rrdd-check: rename/rename(2to1)/delete/delete conflict' '
+test_expect_success 'rrdd-check: rename/rename(2to1)/delete/delete conflict' '
 	test_setup_rrdd &&
 	(
 		cd rrdd &&
@@ -1058,7 +1041,7 @@ test_setup_mod6 () {
 	)
 }
 
-test_expect_merge_algorithm failure success 'mod6-check: chains of rename/rename(1to2) and rename/rename(2to1)' '
+test_expect_success 'mod6-check: chains of rename/rename(1to2) and rename/rename(2to1)' '
 	test_setup_mod6 &&
 	(
 		cd mod6 &&
t/t6423-merge-rename-directories.sh +105 -412

673 lines not on this page · raw message

t/t6424-merge-unrelated-index-changes.sh +1 -3
index 084bfe7e6ce..ddc7524f6c1 100755
@@ -296,10 +296,8 @@ test_expect_success 'with multiple strategies, recursive or ort failure do not e
 	git add a &&
 	git rev-parse :a >expect &&
 
-	sane_unset GIT_TEST_MERGE_ALGORITHM &&
-	test_must_fail git merge -s recursive -s ort -s octopus C^0 >output 2>&1 &&
+	test_must_fail git merge -s ort -s octopus C^0 >output 2>&1 &&
 
-	grep "Trying merge strategy recursive..." output &&
 	grep "Trying merge strategy ort..." output &&
 	grep "Trying merge strategy octopus..." output &&
 	grep "No merge strategy handled the merge." output &&
t/t6426-merge-skip-unneeded-updates.sh +1 -3
index b059475ed03..404cd3f2ca1 100755
@@ -23,8 +23,6 @@ test_description="merge cases"
 #                     files that might be renamed into each other's paths.)
 
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/lib-merge.sh
-
 
 ###########################################################################
 # SECTION 1: Cases involving no renames (one side has subset of changes of
@@ -663,7 +661,7 @@ test_setup_4a () {
 #   correct requires doing the merge in-memory first, then realizing that no
 #   updates to the file are necessary, and thus that we can just leave the path
 #   alone.
-test_expect_merge_algorithm failure success '4a: Change on A, change on B subset of A, dirty mods present' '
+test_expect_success '4a: Change on A, change on B subset of A, dirty mods present' '
 	test_setup_4a &&
 	(
 		cd 4a &&
t/t6428-merge-conflicts-sparse.sh +0 -2
index 9919c3fa7cd..c86e02c6e23 100755
@@ -23,8 +23,6 @@ test_description="merge cases"
 #                     files that might be renamed into each other's paths.)
 
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/lib-merge.sh
-
 
 # Testcase basic, conflicting changes in 'numerals'
 
t/t6430-merge-recursive.sh +1 -2
index c539a68fd07..e59560a5dd9 100755
@@ -6,7 +6,6 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/lib-merge.sh
 
 test_expect_success 'setup 1' '
 
@@ -645,7 +644,7 @@ test_expect_success 'merge-recursive copy vs. rename' '
 	test_cmp expected actual
 '
 
-test_expect_merge_algorithm failure success 'merge-recursive rename vs. rename/symlink' '
+test_expect_success 'merge-recursive rename vs. rename/symlink' '
 
 	git checkout -f rename &&
 	git merge rename-ln &&
t/t6436-merge-overwrite.sh +4 -13
index 4f4376421e7..70b5d2d6944 100755
@@ -101,19 +101,10 @@ test_expect_success 'will not overwrite unstaged changes in renamed file' '
 	git mv c1.c other.c &&
 	git commit -m rename &&
 	cp important other.c &&
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		test_must_fail git merge c1a >out 2>err &&
-		test_grep "would be overwritten by merge" err &&
-		test_cmp important other.c &&
-		test_path_is_missing .git/MERGE_HEAD
-	else
-		test_must_fail git merge c1a >out &&
-		test_grep "Refusing to lose dirty file at other.c" out &&
-		test_path_is_file other.c~HEAD &&
-		test $(git hash-object other.c~HEAD) = $(git rev-parse c1a:c1.c) &&
-		test_cmp important other.c
-	fi
+	test_must_fail git merge c1a >out 2>err &&
+	test_grep "would be overwritten by merge" err &&
+	test_cmp important other.c &&
+	test_path_is_missing .git/MERGE_HEAD
 '
 
 test_expect_success 'will not overwrite untracked subtree' '
t/t6437-submodule-merge.sh +20 -45

114 lines not on this page · raw message

t/t6438-submodule-directory-file-conflicts.sh +0 -5
index 8df67a0ef99..53d83c828a7 100755
@@ -12,11 +12,6 @@ test_submodule_switch "merge --ff"
 
 test_submodule_switch "merge --ff-only"
 
-if test "$GIT_TEST_MERGE_ALGORITHM" != ort
-then
-	KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1
-	KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1
-fi
 test_submodule_switch "merge --no-ff"
 
 test_done
t/t6439-merge-co-error-msgs.sh +1 -1
index 55bd744a3f4..643c9368e0e 100755
@@ -47,7 +47,7 @@ test_expect_success 'untracked files overwritten by merge (fast and non-fast for
 		export GIT_MERGE_VERBOSITY &&
 		test_must_fail git merge branch 2>out2
 	) &&
-	echo "Merge with strategy ${GIT_TEST_MERGE_ALGORITHM:-ort} failed." >>expect &&
+	echo "Merge with strategy ort failed." >>expect &&
 	test_cmp out2 expect &&
 	git reset --hard HEAD^
 '
t/t7402-submodule-rebase.sh +2 -5
index 25b33a1e875..06cee3432fe 100755
@@ -124,11 +124,8 @@ test_expect_success 'rebasing submodule that should conflict' '
 		echo "160000 $(git rev-parse HEAD) 3	submodule"
 	) >expect &&
 	test_cmp expect actual &&
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-    then
-		sub_expect="go to submodule (submodule), and either merge commit $(git -C submodule rev-parse --short HEAD^0)" &&
-		grep "$sub_expect" actual_output
-	fi
+	sub_expect="go to submodule (submodule), and either merge commit $(git -C submodule rev-parse --short HEAD^0)" &&
+	grep "$sub_expect" actual_output
 '
 
 test_done
t/t7602-merge-octopus-many.sh +3 -6
index ff085b086cc..42f675b7399 100755
@@ -77,12 +77,9 @@ Merge made by the 'recursive' strategy.
 EOF
 
 test_expect_success 'merge reduces irrelevant remote heads' '
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		mv expected expected.tmp &&
-		sed s/recursive/ort/ expected.tmp >expected &&
-		rm expected.tmp
-	fi &&
+	mv expected expected.tmp &&
+	sed s/recursive/ort/ expected.tmp >expected &&
+	rm expected.tmp &&
 	GIT_MERGE_VERBOSITY=0 git merge c4 c5 >actual &&
 	test_cmp expected actual
 '
t/t7610-mergetool.sh +11 -29
index c077aba7ced..957f8e20ba3 100755
@@ -535,14 +535,9 @@ test_expect_success 'file vs modified submodule' '
 	yes "" | git mergetool file1 file2 spaced\ name subdir/file3 &&
 	yes "" | git mergetool both &&
 	yes "d" | git mergetool file11 file12 &&
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		yes "c" | git mergetool submod~HEAD &&
-		git rm submod &&
-		git mv submod~HEAD submod
-	else
-		yes "l" | git mergetool submod
-	fi &&
+	yes "c" | git mergetool submod~HEAD &&
+	git rm submod &&
+	git mv submod~HEAD submod &&
 	git submodule update -N &&
 	echo "not a submodule" >expect &&
 	test_cmp expect submod &&
@@ -559,15 +554,10 @@ test_expect_success 'file vs modified submodule' '
 	yes "" | git mergetool file1 file2 spaced\ name subdir/file3 &&
 	yes "" | git mergetool both &&
 	yes "d" | git mergetool file11 file12 &&
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		mv submod submod.orig &&
-		git rm --cached submod &&
-		yes "c" | git mergetool submod~test19 &&
-		git mv submod~test19 submod
-	else
-		yes "r" | git mergetool submod
-	fi &&
+	mv submod submod.orig &&
+	git rm --cached submod &&
+	yes "c" | git mergetool submod~test19 &&
+	git mv submod~test19 submod &&
 	test -d submod.orig &&
 	git submodule update -N &&
 	echo "not a submodule" >expect &&
@@ -585,10 +575,7 @@ test_expect_success 'file vs modified submodule' '
 	yes "" | git mergetool both &&
 	yes "d" | git mergetool file11 file12 &&
 	yes "l" | git mergetool submod &&
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		yes "d" | git mergetool submod~test19
-	fi &&
+	yes "d" | git mergetool submod~test19 &&
 	echo "main submodule" >expect &&
 	test_cmp expect submod/bar &&
 	git submodule update -N &&
@@ -686,14 +673,9 @@ test_expect_success 'directory vs modified submodule' '
 	test_must_fail git merge main &&
 	test -n "$(git ls-files -u)" &&
 	test ! -e submod.orig &&
-	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
-	then
-		yes "r" | git mergetool submod~main &&
-		git mv submod submod.orig &&
-		git mv submod~main submod
-	else
-		yes "r" | git mergetool submod
-	fi &&
+	yes "r" | git mergetool submod~main &&
+	git mv submod submod.orig &&
+	git mv submod~main submod &&
 	test -d submod.orig &&
 	echo "not a submodule" >expect &&
 	test_cmp expect submod.orig/file16 &&
t/test-lib.sh +0 -2
index 9001ed3a647..5952eb56e27 100644
@@ -544,8 +544,6 @@ GIT_DEFAULT_HASH="${GIT_TEST_DEFAULT_HASH:-sha1}"
 export GIT_DEFAULT_HASH
 GIT_DEFAULT_REF_FORMAT="${GIT_TEST_DEFAULT_REF_FORMAT:-files}"
 export GIT_DEFAULT_REF_FORMAT
-GIT_TEST_MERGE_ALGORITHM="${GIT_TEST_MERGE_ALGORITHM:-ort}"
-export GIT_TEST_MERGE_ALGORITHM
 
 # Tests using GIT_TRACE typically don't want <timestamp> <file>:<line> output
 GIT_TRACE_BARE=1
Signature · 1 line
gitgitgadget
#30 lore raw

PATCH v3 8/8 builtin/{merge,rebase,revert}: remove GIT_TEST_MERGE_ALGORITHM

In reply to Elijah Newren via GitGitGadget

From: Elijah Newren <[email protected]>

This environment variable existed to allow the testsuite to reuse all the merge-related tests in the testsuite while easily flipping between the 'recursive' and the 'ort' backends. Now that we have removed merge-recursive and remapped 'recursive' to mean 'ort', we don't need this scaffolding anymore. Remove it from these three builtins.

---
 builtin/merge.c  | 14 +-------------
 builtin/rebase.c |  5 -----
 builtin/revert.c |  2 --
 3 files changed, 1 insertion(+), 20 deletions(-)
builtin/merge.c +1 -13
index c0bbdab7104..9efd585842f 100644
@@ -170,7 +170,7 @@ static struct strategy *get_strategy(const char *name)
 	struct strategy *ret;
 	static struct cmdnames main_cmds = {0}, other_cmds = {0};
 	static int loaded;
-	char *default_strategy = getenv("GIT_TEST_MERGE_ALGORITHM");
+	char *default_strategy = NULL;
 
 	if (!name)
 		return NULL;
@@ -1311,12 +1311,6 @@ int cmd_merge(int argc,
 	if (branch)
 		skip_prefix(branch, "refs/heads/", &branch);
 
-	if (!pull_twohead) {
-		char *default_strategy = getenv("GIT_TEST_MERGE_ALGORITHM");
-		if (default_strategy && !strcmp(default_strategy, "ort"))
-			pull_twohead = xstrdup("ort");
-	}
-
 	init_diff_ui_defaults();
 	git_config(git_merge_config, NULL);
 
@@ -1517,12 +1511,6 @@ int cmd_merge(int argc,
 			fast_forward = FF_NO;
 	}
 
-	if (!use_strategies && !pull_twohead &&
-	    remoteheads && !remoteheads->next) {
-		char *default_strategy = getenv("GIT_TEST_MERGE_ALGORITHM");
-		if (default_strategy)
-			append_strategy(get_strategy(default_strategy));
-	}
 	if (!use_strategies) {
 		if (!remoteheads)
 			; /* already up-to-date */
builtin/rebase.c +0 -5
index d4715ed35d7..e83193ac73e 100644
@@ -1575,11 +1575,6 @@ int cmd_rebase(int argc,
 			    options.default_backend);
 	}
 
-	if (options.type == REBASE_MERGE &&
-	    !options.strategy &&
-	    getenv("GIT_TEST_MERGE_ALGORITHM"))
-		options.strategy = xstrdup(getenv("GIT_TEST_MERGE_ALGORITHM"));
-
 	switch (options.type) {
 	case REBASE_MERGE:
 		options.state_dir = merge_dir();
builtin/revert.c +0 -2
index aca6c293cdf..2654f769a88 100644
@@ -252,8 +252,6 @@ static int run_sequencer(int argc, const char **argv, const char *prefix,
 		free(opts->strategy);
 		opts->strategy = xstrdup_or_null(strategy);
 	}
-	if (!opts->strategy && getenv("GIT_TEST_MERGE_ALGORITHM"))
-		opts->strategy = xstrdup(getenv("GIT_TEST_MERGE_ALGORITHM"));
 	free(options);
 
 	if (cmd == 'q') {
Signature · 1 line
gitgitgadget