From: James Cook <falsifian@falsifian.org>
Subject: [patch] Refuse to start a new merge if one's in progress.
To: gameoftrees@openbsd.org
Date: Thu, 1 Jun 2023 01:56:24 +0000

This patch makes "got merge" refuse to run if a merge is in progress and
the -a or -c option wasn't passed.

Normally, before this patch, it would fail anyway and say "work tree
contains local changes". (This patch changes the error to "a merge
operation is in progress", which I think is more helpful.)

However, before this patch the following sequence of actions would
bypass the "local changes" error:

1. Do an interrupted merge: "got merge -n branch", or "got merge branch"
   resulting in conflicts.

2. Manually restore all files to their contents on the current branch
   tip.

3. Run "got merge branch" again, or "got merge some_other_branch". It
   will happily try the merge again, but probably should instead say you
   need to deal with the current merge first.
 
That's not the end of the world, but I think it's better to be
consistent and remind the user they had a merge in progress. (Maybe
there's a practical argument that the current behaviour is actually
better, to avoid unnecessarily bothering the user?)

Minor decisions made:

1. I moved existing checks to be beside the new check; that's mostly
   aesthetic but does mean got_author is no longer called in the case
   (!merge_in_progress && continue_merge).

2. I didn't try to capture the above steps 1-3 in a new test case.
   Instead I just added it to test_merge_in_progress, which fails under
   the old behaviour because "got merge" outputs the wrong message to
   stderr.

   I could add a more specific test case if desired.

3. I incidentally made test_merge_in_progress check the exit code.
   That's not really related to the rest of this patch.


diff fb307946174c95e32d2048584c6ab1ce24f3ea00 refs/heads/falsifian/mg_cont
commit - fb307946174c95e32d2048584c6ab1ce24f3ea00
commit + faa397f9b139e246671a45d32b05fa40b9534dbf
blob - 123dedc51cae9b07a29394e9816496121697cf7a
blob + f4666b0aa2b87365b32db745d93c6feae807d6a4
--- got/got.c
+++ got/got.c
@@ -13241,11 +13241,17 @@ cmd_merge(int argc, char *argv[])
 	if (error)
 		goto done;
 
+	if (merge_in_progress && !(abort_merge || continue_merge)) {
+		error = got_error(GOT_ERR_MERGE_BUSY);
+		goto done;
+	}
+
+	if (!merge_in_progress && (abort_merge || continue_merge)) {
+		error = got_error(GOT_ERR_NOT_MERGING);
+		goto done;
+	}
+
 	if (abort_merge) {
-		if (!merge_in_progress) {
-			error = got_error(GOT_ERR_NOT_MERGING);
-			goto done;
-		}
 		error = got_worktree_merge_continue(&branch_name,
 		    &branch_tip, &fileindex, worktree, repo);
 		if (error)
@@ -13263,10 +13269,6 @@ cmd_merge(int argc, char *argv[])
 		goto done;
 
 	if (continue_merge) {
-		if (!merge_in_progress) {
-			error = got_error(GOT_ERR_NOT_MERGING);
-			goto done;
-		}
 		error = got_worktree_merge_continue(&branch_name,
 		    &branch_tip, &fileindex, worktree, repo);
 		if (error)
blob - 11ffa61f4a12efd380087a03aa5b186b9a745ae2
blob + df6f2f9edf54a23ff9b1e9f05267a77673a4fbcf
--- regress/cmdline/merge.sh
+++ regress/cmdline/merge.sh
@@ -870,9 +870,15 @@ test_merge_in_progress() {
 	fi
 
 	for cmd in update commit histedit "rebase newbranch" \
-		"integrate newbranch" "stage alpha"; do
+		"integrate newbranch" "merge newbranch" "stage alpha"; do
 		(cd $testroot/wt && got $cmd > $testroot/stdout \
 			2> $testroot/stderr)
+		ret=$?
+		if [ $ret -eq 0 ]; then
+			echo "got $cmd succeeded unexpectedly" >&2
+			test_done "$testroot" "1"
+			return 1
+		fi
 
 		echo -n > $testroot/stdout.expected
 		cmp -s $testroot/stdout.expected $testroot/stdout

-- 
James