"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
James Cook <falsifian@falsifian.org>
Subject:
add -M option: tell got merge not to fast-forward
To:
gameoftrees@openbsd.org
Date:
Fri, 30 Jun 2023 22:37:34 +0000

Download raw body.

Thread
The below patch adds an -M option to got merge which makes it create a
merge commit even if fast-forwarding is possible. (The merge can still
be interrupted by a conflict or the -n option.)

Notes:

- Is M the right letter for this option? I think the answer is "yes" but
  this is sort of the last chance to change our minds.

  Devil's advocate: if we decide to make -n the default, it's possible
  we'd want to make "-M" might the opposite of -n, i.e. create the merge
  commit without interrupting. (But there are plenty of other letters.
  It's too bad -C ("commit!") is already used. Maybe "-c" could serve
  two purposes, continuing and opposite of -n; not sure if that's a good
  idea.)

- Option conflicts for "got merge" could use work. "c" + "n" isn't
  detected, and the man page is wrong that "no other command-line
  arguments are allowed" with "-c", since "-C" is permitted.

-- 
James


diff 13d9dc7e2e880cfe93cf23a39c48427cda7ff636 refs/heads/main
commit - 13d9dc7e2e880cfe93cf23a39c48427cda7ff636
commit + f5547efe8089114ee0383d65d9b42ada004016e6
blob - 71c0950fc006ad8faab300bfdc70c29496c8493d
blob + 234d94604d5296c784792c130bb88dd64b8393bb
--- got/got.1
+++ got/got.1
@@ -2811,7 +2811,7 @@ or reverted with
 .Tg mg
 .It Xo
 .Cm merge
-.Op Fl aCcn
+.Op Fl aCcMn
 .Op Ar branch
 .Xc
 .Dl Pq alias: Cm mg
@@ -2940,6 +2940,8 @@ If this option is used, no other command-line argument
 .It Fl c
 Continue an interrupted merge operation.
 If this option is used, no other command-line arguments are allowed.
+.It Fl M
+Create a merge commit even if the branches have not diverged.
 .It Fl n
 Merge changes into the work tree as usual but do not create a merge
 commit immediately.
blob - 5348f3e02299f72518fb1fae98ac8eb68f013b85
blob + f0f73cb11cc2c22294fb3716a2eba2a4a2ba8be9
--- got/got.c
+++ got/got.c
@@ -13144,7 +13144,7 @@ cmd_merge(int argc, char *argv[])
 	struct got_object_id *branch_tip = NULL, *yca_id = NULL;
 	struct got_object_id *wt_branch_tip = NULL;
 	int ch, merge_in_progress = 0, abort_merge = 0, continue_merge = 0;
-	int allow_conflict = 0, interrupt_merge = 0;
+	int allow_conflict = 0, prefer_fast_forward = 1, interrupt_merge = 0;
 	struct got_update_progress_arg upa;
 	struct got_object_id *merge_commit_id = NULL;
 	char *branch_name = NULL;
@@ -13158,7 +13158,7 @@ cmd_merge(int argc, char *argv[])
 		err(1, "pledge");
 #endif
 
-	while ((ch = getopt(argc, argv, "aCcn")) != -1) {
+	while ((ch = getopt(argc, argv, "aCcMn")) != -1) {
 		switch (ch) {
 		case 'a':
 			abort_merge = 1;
@@ -13168,6 +13168,9 @@ cmd_merge(int argc, char *argv[])
 		case 'c':
 			continue_merge = 1;
 			break;
+		case 'M':
+			prefer_fast_forward = 0;
+			break;
 		case 'n':
 			interrupt_merge = 1;
 			break;
@@ -13188,6 +13191,10 @@ cmd_merge(int argc, char *argv[])
 	}
 	if (abort_merge && continue_merge)
 		option_conflict('a', 'c');
+	if (abort_merge && !prefer_fast_forward)
+		option_conflict('a', 'M');
+	if (continue_merge && !prefer_fast_forward)
+		option_conflict('c', 'M');
 	if (abort_merge || continue_merge) {
 		if (argc != 0)
 			usage_merge();
@@ -13326,7 +13333,8 @@ cmd_merge(int argc, char *argv[])
 		error = got_worktree_merge_prepare(&fileindex, worktree, repo);
 		if (error)
 			goto done;
-		if (yca_id && got_object_id_cmp(wt_branch_tip, yca_id) == 0) {
+		if (prefer_fast_forward && yca_id &&
+		    got_object_id_cmp(wt_branch_tip, yca_id) == 0) {
 			struct got_pathlist_head paths;
 			if (interrupt_merge) {
 				error = got_error_fmt(GOT_ERR_BAD_OPTION,
blob - b1980ee2c0e36130e8066d6897401ba97642d4a4
blob + a7908e3851266c3fe538eb4321c3fab455fc0d8d
--- regress/cmdline/merge.sh
+++ regress/cmdline/merge.sh
@@ -470,6 +470,134 @@ test_merge_backward() {
 	test_done "$testroot" "$ret"
 }
 
+test_merge_forward_commit() {
+	local testroot=`test_init merge_forward_commit`
+	local commit0=`git_show_head $testroot/repo`
+
+	(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 commit1=`git_show_head $testroot/repo`
+
+	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 -M newbranch > $testroot/stdout)
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "got merge failed unexpectedly" >&2
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	local merge_commit=`git_show_branch_head $testroot/repo master`
+
+	echo "G  alpha" >> $testroot/stdout.expected
+	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
+
+	# We should have created a merge commit with two parents.
+	(cd $testroot/wt && got log -l1 | grep ^parent > $testroot/stdout)
+	echo "parent 1: $commit0" > $testroot/stdout.expected
+	echo "parent 2: $commit1" >> $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
+
+	test_done "$testroot" "$ret"
+}
+
+test_merge_forward_interrupt() {
+	# Test -M and -n options together.
+	local testroot=`test_init merge_forward_commit`
+	local commit0=`git_show_head $testroot/repo`
+
+	(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 commit1=`git_show_head $testroot/repo`
+
+	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 -M -n newbranch > $testroot/stdout)
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "got merge failed unexpectedly" >&2
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	echo "G  alpha" > $testroot/stdout.expected
+	echo "Merge of refs/heads/newbranch interrupted on request" \
+		>> $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
+
+	# Continue the merge.
+	(cd $testroot/wt && got merge -c > $testroot/stdout)
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "got merge failed unexpectedly" >&2
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	local merge_commit=`git_show_branch_head $testroot/repo master`
+
+	echo "M  alpha" > $testroot/stdout.expected
+	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
+
+	# We should have created a merge commit with two parents.
+	(cd $testroot/wt && got log -l1 | grep ^parent > $testroot/stdout)
+	echo "parent 1: $commit0" > $testroot/stdout.expected
+	echo "parent 2: $commit1" >> $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"
+}
+
 test_merge_backward() {
 	local testroot=`test_init merge_backward`
 	local commit0=`git_show_head $testroot/repo`
@@ -1867,6 +1995,8 @@ run_test test_merge_backward
 test_parseargs "$@"
 run_test test_merge_basic
 run_test test_merge_forward
+run_test test_merge_forward_commit
+run_test test_merge_forward_interrupt
 run_test test_merge_backward
 run_test test_merge_continue
 run_test test_merge_continue_new_commit