From: James Cook <falsifian@falsifian.org>
Subject: Re: three bugs related to merge commits
To: Mark Jamsek <mark@jamsek.com>, gameoftrees@openbsd.org
Date: Mon, 17 Apr 2023 16:06:00 +0000

> Thanks!
> Below is a fix with your test adjusted to expect correct behaviour.
> 
> The bug is that the first parent's object ID was derived from the work
> tree's base commit. It should be derived from the branch tip commit.
> The comitted tree is already based on the branch tip's tree which is
> why the added file did not disappear.
> 
> It is unfortunate that the test suite didn't already catch such a basic
> problem, but here we are. All other tests are still passing for me.

Thanks, that fixes the first bug! The other two still seem to be present
though.

Here's a patch adding regress tests for the other two.
test_merge_same_change (for the third bug) seems to almost directly
contradict test_merge_no_op, though, so maybe the third bug is intended
behaviour? It would be nice to have a way to create a merge commit in
this situation. (I think it's actually come up for me, but now I forget
how).

test_send_merge_commit is for the second bug.

-- 
James


diff /home/falsifian/co/got
commit - 2b70695630ead3274d31727f6b477f3544dc9c9a
path + /home/falsifian/co/got
blob - 9093dd5b9fb524b14d2d17d4fe754da0ec8ef9f4
file + regress/cmdline/merge.sh
--- regress/cmdline/merge.sh
+++ regress/cmdline/merge.sh
@@ -1150,6 +1150,37 @@ test_merge_imported_branch() {
 	test_done "$testroot" "$ret"
 }
 
+test_merge_same_change() {
+	local testroot=`test_init merge_same_change`
+	local commit0=`git_show_head $testroot/repo`
+
+	echo 'modify alpha' > $testroot/repo/alpha
+	git_commit $testroot/repo -m 'change on master'
+
+	# the same change (but different commit id) on a branch
+	(cd $testroot/repo && git checkout -q -b branch $commit0)
+	echo 'modify alpha' > $testroot/repo/alpha
+	git_commit $testroot/repo -m 'change on branch'
+
+	got checkout -b master $testroot/repo $testroot/wt > /dev/null
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "got checkout failed unexpectedly" >&2
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	(cd $testroot/wt && got merge branch > /dev/null)
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "got merge failed unexpectedly" >&2
+		test_done "$testroot" "1"
+		return 1
+	fi
+
+	test_done "$testroot" "$ret"
+}
+
 test_merge_imported_branch() {
 	local testroot=`test_init merge_import`
 	local commit0=`git_show_head $testroot/repo`
@@ -1481,6 +1512,7 @@ run_test test_merge_imported_branch
 run_test test_merge_path_prefix
 run_test test_merge_missing_file
 run_test test_merge_no_op
+run_test test_merge_same_change
 run_test test_merge_imported_branch
 run_test test_merge_interrupt
 run_test test_merge_umask
blob - db8ed49392c78fe028db90d4d5c318b3d0fa8130
file + regress/cmdline/send.sh
--- regress/cmdline/send.sh
+++ regress/cmdline/send.sh
@@ -322,6 +322,39 @@ test_send_delete() {
 	test_done "$testroot" "$ret"
 }
 
+test_send_merge_commit() {
+	local testroot=`test_init send_merge_commit`
+	local testurl=ssh://127.0.0.1/$testroot
+
+	if ! got clone -q "$testurl/repo" "$testroot/repo-clone"; then
+		echo "got clone command failed unexpectedly" >&2
+		test_done "$testroot" 1
+		return 1
+	fi
+
+	echo 'upstream change' > $testroot/repo/alpha
+	git_commit $testroot/repo -m 'upstream change'
+
+	got checkout $testroot/repo-clone $testroot/wt-clone > /dev/null
+	echo 'downstream change' > $testroot/wt-clone/beta
+	(cd $testroot/wt-clone && got commit -m 'downstream change' > /dev/null)
+
+	got fetch -q -r $testroot/repo-clone
+	(cd $testroot/wt-clone && got up -b origin/master > /dev/null \
+		&& got merge master > /dev/null)
+
+	(cd $testroot/repo && git config receive.denyCurrentBranch ignore)
+
+	got send -r $testroot/repo-clone > /dev/null
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	test_done "$testroot" 0
+}
+
 test_send_delete() {
 	local testroot=`test_init send_delete`
 	local testurl=ssh://127.0.0.1/$testroot
@@ -1583,6 +1616,7 @@ run_test test_send_delete
 run_test test_send_basic
 run_test test_send_rebase_required
 run_test test_send_rebase_required_overwrite
+run_test test_send_merge_commit
 run_test test_send_delete
 run_test test_send_clone_and_send
 run_test test_send_tags