From: Omar Polo <op@omarpolo.com>
Subject: Re: "got patch" format?
To: Christian Weisgerber <naddy@mips.inka.de>
Cc: gameoftrees@openbsd.org
Date: Sat, 19 Mar 2022 18:15:23 +0100

Christian Weisgerber <naddy@mips.inka.de> wrote:
> I just tried to use "got patch" and promptly failed because I had
> generated the patch file with "diff -u foo.orig foo", and "got patch"
> choked because foo.orig didn't exist in the worktree.  Is this a bug,
> work in progress, ...?

patch(1) (and in most cases 'git apply' too) prefer the new path over
the old path, so in your case both pick up 'foo' as path and it works.

in 'got patch' i tried to handle the rename case too, and in fact at the
moment it handles "git-style rename diffs" well.  This means however
that both the "old" path and the "new" path must exists in the worktree.
(see test_patch_rename)

(The only case where 'git diff' produces a diff for a rename that's not
currently handled by 'got patch' is when a file is renamed without any
change.  in that case the diff has only the header and no hunks in it.)

'git apply' solves this by requiring some git-specific "annotations" to
the diff for it to interpret the paths as rename.  ("headers" like
similarity index, rename from/to and the leading "diff --git".)  In
practice thought seems that "diff --git" and paths wit a/ and b/ are
enough for it to do the rename.  I haven't checked the sources and the
git-apply manpage doesn't mention anything (judging from a quick grep.)

We have multiple way to fix this:

 - don't support patches that rename files (can always be achieved by a
   remove folled by the addition of the file like 'got diff' already
   does.)

 - add some heuristics in got_worktree_patch_check_path.  We could, for
   instance, ignore the old path and just use the new when the old file
   is nonexistent or unversioned.  Diff below is an example of this.

 - invent our own annotations?  "diff --got" anyone? :D


(yes, test_patch_orig is an ugly name but on the spot I couldn't came up
with a more descriptive name)

diff 78f5ac2436c8d17d1dd687d69e51354707275988 /home/op/w/got
blob - 16eebc519859d846026d968e3e953c502035de48
file + lib/worktree.c
--- lib/worktree.c
+++ lib/worktree.c
@@ -8829,7 +8829,17 @@ got_worktree_patch_check_path(const char *old, const c
 	if (err)
 		goto done;
 
-	if (old != NULL && new != NULL && strcmp(old, new) != 0)
+	if ((status_old == GOT_STATUS_NONEXISTENT ||
+	    status_old == GOT_STATUS_UNVERSIONED) &&
+	    new != NULL) {
+		free(*oldpath);
+		if ((*oldpath = strdup(*newpath)) == NULL) {
+			err = got_error_from_errno("strdup");
+			goto done;
+		}
+	}
+
+	if (old != NULL && new != NULL && strcmp(*oldpath, *newpath) != 0)
 		file_renamed = 1;
 
 	if (old != NULL && new == NULL)
blob - 32c852932a5cc8f51426acc413933a80b95f41de
file + regress/cmdline/patch.sh
--- regress/cmdline/patch.sh
+++ regress/cmdline/patch.sh
@@ -1048,6 +1048,40 @@ EOF
 	test_done $testroot $ret
 }
 
+test_patch_orig() {
+	local testroot=`test_init patch_orig`
+
+	got checkout $testroot/repo $testroot/wt > /dev/null
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done $testroot $ret
+		return 1
+	fi
+
+	cat <<EOF > $testroot/wt/patch
+--- alpha.orig
++++ alpha
+@@ -1 +1,2 @@
+ alpha
++was edited
+EOF
+
+	(cd $testroot/wt && got patch patch) > $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done $testroot $ret
+		return 1
+	fi
+
+	echo 'M  alpha' > $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_parseargs "$@"
 run_test test_patch_simple_add_file
 run_test test_patch_simple_rm_file
@@ -1066,3 +1100,4 @@ run_test test_patch_nop
 run_test test_patch_preserve_perm
 run_test test_patch_create_dirs
 run_test test_patch_with_offset
+run_test test_patch_orig