commit 38d61ead4d6e9ff3f44b8d6e0e7ff3d02ffcec26
from: Omar Polo <op@omarpolo.com>
date: Sat Jul 23 13:15:55 2022 UTC

fix `got patch -R' when using diff3 merge

`got patch -R' fails spectacularly when applied on a diff that contains
the info of the original blob for the diff3 merge machinery since it
tries to apply the reverse of the patch to the old blob.

change it to run the patch (_not_ reversed) on the old blob and then
swap the arguments to got_diff3_merge which gives us the correct reverse
merge of the diff.  while here add a test case too.

reported by naddy, discussed with and ok stsp@

commit - 65858f9a993c163be8f4af482e7b400535d61d17
commit + 38d61ead4d6e9ff3f44b8d6e0e7ff3d02ffcec26
blob - 703c22b84de200f2a0ec9fc0f7bdc70dc725ab20
blob + ab334cf0239cc3dcf0653836d82b513a55e0f1b6
--- lib/patch.c
+++ lib/patch.c
@@ -313,7 +313,36 @@ done:
 	imsg_free(&imsg);
 	return err;
 }
+
+static void
+reverse_patch(struct got_patch *p)
+{
+	struct got_patch_hunk *h;
+	size_t i;
+	int tmp;
+
+	STAILQ_FOREACH(h, &p->head, entries) {
+		tmp = h->old_from;
+		h->old_from = h->new_from;
+		h->new_from = tmp;
 
+		tmp = h->old_lines;
+		h->old_lines = h->new_lines;
+		h->new_lines = tmp;
+
+		tmp = h->old_nonl;
+		h->old_nonl = h->new_nonl;
+		h->new_nonl = tmp;
+
+		for (i = 0; i < h->len; ++i) {
+			if (*h->lines[i] == '+')
+				*h->lines[i] = '-';
+			else if (*h->lines[i] == '-')
+				*h->lines[i] = '+';
+		}
+	}
+}
+
 /*
  * Copy data from orig starting at copypos until pos into tmp.
  * If pos is -1, copy until EOF.
@@ -700,7 +729,8 @@ static const struct got_error *
 apply_patch(int *overlapcnt, struct got_worktree *worktree,
     struct got_repository *repo, struct got_fileindex *fileindex,
     const char *old, const char *new, struct got_patch *p, int nop,
-    struct patch_args *pa, got_cancel_cb cancel_cb, void *cancel_arg)
+    int reverse, struct patch_args *pa,
+    got_cancel_cb cancel_cb, void *cancel_arg)
 {
 	const struct got_error *err = NULL;
 	struct stat sb;
@@ -728,6 +758,8 @@ apply_patch(int *overlapcnt, struct got_worktree *work
 			return err;
 		else if (err == NULL)
 			do_merge = 1;
+		else if (reverse)
+			reverse_patch(p);
 		err = NULL;
 	}
 
@@ -808,6 +840,19 @@ apply_patch(int *overlapcnt, struct got_worktree *work
 			goto done;
 		}
 
+		if (reverse) {
+			char *s;
+			FILE *t;
+
+			s = anclabel;
+			anclabel = newlabel;
+			newlabel = s;
+
+			t = afile;
+			afile = tmpfile;
+			tmpfile = t;
+		}
+
 		err = got_opentemp_named(&mergepath, &mergefile, template);
 		if (err)
 			goto done;
@@ -909,35 +954,6 @@ done:
 	return err;
 }
 
-static void
-reverse_patch(struct got_patch *p)
-{
-	struct got_patch_hunk *h;
-	size_t i;
-	int tmp;
-
-	STAILQ_FOREACH(h, &p->head, entries) {
-		tmp = h->old_from;
-		h->old_from = h->new_from;
-		h->new_from = tmp;
-
-		tmp = h->old_lines;
-		h->old_lines = h->new_lines;
-		h->new_lines = tmp;
-
-		tmp = h->old_nonl;
-		h->old_nonl = h->new_nonl;
-		h->new_nonl = tmp;
-
-		for (i = 0; i < h->len; ++i) {
-			if (*h->lines[i] == '+')
-				*h->lines[i] = '-';
-			else if (*h->lines[i] == '-')
-				*h->lines[i] = '+';
-		}
-	}
-}
-
 const struct got_error *
 got_patch(int fd, struct got_worktree *worktree, struct got_repository *repo,
     int nop, int strip, int reverse, got_patch_progress_cb progress_cb,
@@ -1002,15 +1018,16 @@ got_patch(int fd, struct got_worktree *worktree, struc
 		if (err || done)
 			break;
 
-		if (reverse)
+		/* reversal application with merge base is done differently */
+		if (reverse && *p.blob == '\0')
 			reverse_patch(&p);
 
 		err = got_worktree_patch_check_path(p.old, p.new, &oldpath,
 		    &newpath, worktree, repo, fileindex);
 		if (err == NULL)
 			err = apply_patch(&overlapcnt, worktree, repo,
-			    fileindex, oldpath, newpath, &p, nop, &pa,
-			    cancel_cb, cancel_arg);
+			    fileindex, oldpath, newpath, &p, nop, reverse,
+			    &pa, cancel_cb, cancel_arg);
 		if (err != NULL) {
 			failed = 1;
 			/* recoverable errors */
blob - 7901777201377549e2dce265cf173b09d716ee4d
blob + 5b3df4e132acb5f7a05acfcdaca7e9d1eb7ee25a
--- regress/cmdline/patch.sh
+++ regress/cmdline/patch.sh
@@ -1874,7 +1874,81 @@ EOF
 	fi
 	test_done $testroot $ret
 }
+
+test_patch_merge_reverse() {
+	local testroot=`test_init patch_merge_simple`
+
+	got checkout $testroot/repo $testroot/wt > /dev/null
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done $testroot $ret
+		return 1
+	fi
 
+	jot 10 > $testroot/wt/numbers
+	(cd $testroot/wt && got add numbers && got commit -m +numbers) \
+		> /dev/null
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done $testroot $ret
+		return 1
+	fi
+
+	local commit_id=`git_show_head $testroot/repo`
+
+	jot 10 | sed s/5/five/g > $testroot/wt/numbers
+	(cd $testroot/wt && got diff > $testroot/wt/patch \
+		&& got commit -m 'edit numbers') > /dev/null
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done $testroot $ret
+		return 1
+	fi
+
+	jot 10 | sed -e s/5/five/g -e s/6/six/g > $testroot/wt/numbers
+	(cd $testroot/wt && got commit -m 'edit numbers again') >/dev/null
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done $testroot $ret
+		return 1
+	fi
+
+	(cd $testroot/wt && got patch -R patch) >/dev/null 2>&1
+	ret=$?
+	if [ $ret -eq 0 ]; then
+		echo "unexpectedly reverted the patch" >&2
+		test_done $testroot 1
+		return 1
+	fi
+
+	cat <<-EOF > $testroot/wt/numbers.expected
+	1
+	2
+	3
+	4
+	<<<<<<< --- numbers
+	5
+	6
+	||||||| +++ numbers
+	five
+	=======
+	five
+	six
+	>>>>>>> commit $commit_id
+	7
+	8
+	9
+	10
+EOF
+
+	cmp -s $testroot/wt/numbers $testroot/wt/numbers.expected
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/wt/numbers $testroot/wt/numbers.expected
+	fi
+	test_done $testroot $ret
+}
+
 test_parseargs "$@"
 run_test test_patch_add_file
 run_test test_patch_rm_file
@@ -1905,3 +1979,4 @@ run_test test_patch_merge_simple
 run_test test_patch_merge_gitdiff
 run_test test_patch_merge_conflict
 run_test test_patch_merge_unknown_blob
+run_test test_patch_merge_reverse