From: Stefan Sperling <stsp@stsp.name>
Subject: Re: three bugs related to merge commits
To: James Cook <falsifian@falsifian.org>
Cc: gameoftrees@openbsd.org
Date: Mon, 17 Apr 2023 17:55:14 +0200

On Mon, Mar 13, 2023 at 12:01:42AM +0000, James Cook wrote:
> ** Third bug: Can't merge a branch with identical content.
> 
> Create two branches A and B, where A includes all of B's changes, but as
> different commit objects. Then try to merge B into A. Got complains:
> 	got: merge of refs/heads/aux cannot proceed: no changes to commit
> 
> I do occasionally want to create merge commits like this. For example,
> it happened while I was cleaning up after I encountered the first bug
> above: I ended up with divergent commits making some of the same
> changes, and wanted to incorporate all of them into my commit graph for
> a historical record.
> 
> Here's a script to reproduce; for simplicity, the changes are not just a
> subset but exactly the same.

Thanks! There was already an existing test for this situation.
The patch below allows no-op merge commits to be created and adjusts the
existing test accordingly.

-----------------------------------------------
 allow no-op merge commits to be created
 
 Requested by James Cook
 
diff 2b70695630ead3274d31727f6b477f3544dc9c9a 35d2583f834a66f801dc229002d45e735882ba78
commit - 2b70695630ead3274d31727f6b477f3544dc9c9a
commit + 35d2583f834a66f801dc229002d45e735882ba78
blob - c924075eacdb3d7e5ebe3fc11b3c59262e7e120f
blob + 923e489c4fb37899470241b32f5235c9b6652c87
--- got/got.1
+++ got/got.1
@@ -2852,10 +2852,6 @@ If a merge conflict is resolved in a way which renders
 Alternatively, the merge operation may be aborted which will leave
 the work tree's current branch unmodified.
 .Pp
-If a merge conflict is resolved in a way which renders all merged
-changes into no-op changes, the merge operation cannot continue
-and must be aborted.
-.Pp
 .Cm got merge
 will refuse to run if certain preconditions are not met.
 If history of the
blob - 2bc8caf5426fa97b5db0fe63e371b0cbd9347786
blob + 622c0ce1ca669ed14ca0d26259483e9beea95863
--- lib/worktree.c
+++ lib/worktree.c
@@ -8390,12 +8390,6 @@ got_worktree_merge_commit(struct got_object_id **new_c
 	if (err)
 		goto done;
 
-	if (TAILQ_EMPTY(&commitable_paths)) {
-		err = got_error_fmt(GOT_ERR_COMMIT_NO_CHANGES,
-		    "merge of %s cannot proceed", branch_name);
-		goto done;
-	}
-
 	TAILQ_FOREACH(pe, &commitable_paths, entry) {
 		struct got_commitable *ct = pe->data;
 		const char *ct_path = ct->in_repo_path;
blob - 9093dd5b9fb524b14d2d17d4fe754da0ec8ef9f4
blob + 11ffa61f4a12efd380087a03aa5b186b9a745ae2
--- regress/cmdline/merge.sh
+++ regress/cmdline/merge.sh
@@ -1045,7 +1045,7 @@ test_merge_no_op() {
 	(cd $testroot/repo && git checkout -q -b newbranch)
 	echo "modified alpha on branch" > $testroot/repo/alpha
 	git_commit $testroot/repo -m "committing to alpha on newbranch"
-	local branch_commitk=`git_show_branch_head $testroot/repo newbranch`
+	local branch_commit=`git_show_branch_head $testroot/repo newbranch`
 
 	got checkout -b master $testroot/repo $testroot/wt > /dev/null
 	ret=$?
@@ -1122,15 +1122,13 @@ test_merge_no_op() {
 	(cd $testroot/wt && got merge -c > $testroot/stdout \
 		2> $testroot/stderr)
 	ret=$?
-	if [ $ret -eq 0 ]; then
-		echo "got merge succeeded unexpectedly" >&2
+	if [ $ret -ne 0 ]; then
+		echo "got merge failed unexpectedly" >&2
 		test_done "$testroot" "$ret"
 		return 1
 	fi
 
-	echo -n "got: merge of refs/heads/newbranch cannot proceed: " \
-		> $testroot/stderr.expected
-	echo "no changes to commit" >> $testroot/stderr.expected
+	echo -n '' > $testroot/stderr.expected
 	cmp -s $testroot/stderr.expected $testroot/stderr
 	ret=$?
 	if [ $ret -ne 0 ]; then
@@ -1139,6 +1137,19 @@ test_merge_no_op() {
 		return 1
 	fi
 
+	local merge_commit=`git_show_head $testroot/repo`
+	echo -n "Merged refs/heads/newbranch into refs/heads/master: " \
+		> $testroot/stdout.expected
+	echo $merge_commit >> $testroot/stdout.expected
+
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
 	(cd $testroot/wt && got status > $testroot/stdout)
 
 	echo -n "" > $testroot/stdout.expected
@@ -1146,7 +1157,20 @@ test_merge_no_op() {
 	ret=$?
 	if [ $ret -ne 0 ]; then
 		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
 	fi
+
+	# We should have created a merge commit with two parents.
+	got log -r $testroot/repo -l1 -c $merge_commit | grep ^parent \
+		> $testroot/stdout
+	echo "parent 1: $master_commit" > $testroot/stdout.expected
+	echo "parent 2: $branch_commit" >> $testroot/stdout.expected
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+	fi
 	test_done "$testroot" "$ret"
 }