On Wed, Aug 17 2022, Junio C Hamano wrote:

Elijah Newren [off-list ref] writes:

quoted

There's also another concern you tried to address in your other email;
let me quote from that email here:

quoted

If you want to have an easy example of a custom merge strategy, then let's
have that easy example. `git-merge-resolve.sh` ain't that example.

It would be a different matter if you had commented about
`git-merge-ours.sh`:
https://github.com/git/git/blob/v2.17.0/contrib/examples/git-merge-ours.sh
That _was_ a simple and easy example.

...and it was _utterly useless_ as an example.  It only checked that
the user hadn't modified the index since HEAD.  It doesn't demonstrate
anything about how to merge differing entries, since that merge
strategy specifically ignores changes made on the other side.  Since
merging differing entries is the whole point of writing a strategy, I
see no educational value in that particular script.

`git-merge-resolve.sh` may be an imperfect example, but it's certainly
far superior to that.
...
If someone makes a better example (which I agree could be done,
especially if it added lots of comments about what was required and
why), and ensures we keep useful test coverage (maybe using Junio's
c-resolve suggestion in another email), then my concerns about
reimplementing git-merge-resolve.sh in C go away.

If that happens, then I still think it's a useless exercise to do the
reimplementation -- unless someone can provide evidence of `-s
resolve` being in use -- but it's not a harmful exercise and wouldn't
concern me.

If the better example and mechanism to retain good test coverage
aren't provided, then I worry that reimplementing is a bunch of work
for an at best theoretical benefit, coupled with a double whammy
practical regression.

Ah, you said many things I wanted to say already.  Thanks.

I may have missed something in this thread, but wouldn't an acceptable
way to please everyone here be to:

 1. Have git's behavior be that of the end of this series...
 2. Add a GIT_TEST_* mode where we'll optionally invoke these "built-in"
    merge strategies as commands, i.e. have them fall back to
    "try_merge_command()".

So something like this on top of this series (assume my SOB etc. if this
is acceptable). I only tested this locally, but it seems to do the right
thing for me:

diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh
index 8ebff425967..9d0f68b8147 100755
--- a/ci/run-build-and-tests.sh
+++ b/ci/run-build-and-tests.sh

@@ -30,6 +30,7 @@ linux-TEST-vars)
 	export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master
 	export GIT_TEST_WRITE_REV_INDEX=1
 	export GIT_TEST_CHECKOUT_WORKERS=2
+	export GIT_TEST_MERGE_COMMANDS=true
 	;;
 linux-clang)
 	export GIT_TEST_DEFAULT_HASH=sha1

diff --git a/sequencer.c b/sequencer.c
index 00a36205848..91d651f9b12 100644
--- a/sequencer.c
+++ b/sequencer.c

@@ -2309,6 +2309,7 @@ static int do_pick_commit(struct repository *r,
 	} else {
 		struct commit_list *common = NULL;
 		struct commit_list *remotes = NULL;
+		const int test_commands = git_env_bool("GIT_TEST_MERGE_COMMANDS", 0);
 
 		res = write_message(msgbuf.buf, msgbuf.len,
 				    git_path_merge_msg(r), 0);

@@ -2316,10 +2317,10 @@ static int do_pick_commit(struct repository *r,
 		commit_list_insert(base, &common);
 		commit_list_insert(next, &remotes);
 
-		if (!strcmp(opts->strategy, "resolve")) {
+		if (!test_commands && !strcmp(opts->strategy, "resolve")) {
 			repo_read_index(r);
 			res |= merge_strategies_resolve(r, common, oid_to_hex(&head), remotes);
-		} else if (!strcmp(opts->strategy, "octopus")) {
+		} else if (!test_commands && !strcmp(opts->strategy, "octopus")) {
 			repo_read_index(r);
 			res |= merge_strategies_octopus(r, common, oid_to_hex(&head), remotes);
 		} else {