From: Omar Polo <op@omarpolo.com>
Subject: got patch and got diff patches
To: gameoftrees@openbsd.org
Date: Mon, 20 Jun 2022 23:26:15 +0200

Stefan reminded me that the output the first line of `got diff' doesn't
always have a commit ID, it may just as well have reference names.

the issue is that 'got patch' is expecting a commit id, otherwise it
won't even try to do diff3 (it will still apply the patch as always,
just not with the merge magic.)

diff below tries to fix this.  It makes got-read-patch and recv_patch
slightly more permissive: we can have a blob id alone without a commit
id, and changes the conflict markers to use the blob id if we don't have
the commit id.

it also fixes another issue, of which i'm not really sure of the way i'm
addressing it.  open_blob may fails if we read some gibberish and
thought it was a blob id.  i'm not really keen on inspecting errno in
this case, should i always ignore open_blob failures regardless of the
returned error?

diff 6131ff18e81056001a823f913094a92c10580cba /home/op/w/got
blob - 9209e54ce7111b54b69980a44a48a92a333e5888
file + lib/patch.c
--- lib/patch.c
+++ lib/patch.c
@@ -192,10 +192,11 @@ recv_patch(struct imsgbuf *ibuf, int *done, struct got
 		goto done;
 	}
 
-	if (*patch.cid != '\0' && *patch.blob != '\0') {
+	if (*patch.cid != '\0')
 		strlcpy(p->cid, patch.cid, sizeof(p->cid));
+
+	if (*patch.blob != '\0')
 		strlcpy(p->blob, patch.blob, sizeof(p->blob));
-	}
 
 	/* automatically set strip=1 for git-style diffs */
 	if (strip == -1 && patch.git &&
@@ -643,7 +644,7 @@ apply_patch(int *overlapcnt, struct got_worktree *work
 	/* don't run the diff3 merge on creations/deletions */
 	if (*p->blob != '\0' && p->old != NULL && p->new != NULL) {
 		err = open_blob(&apath, &afile, p->blob, repo);
-		if (err && err->code != GOT_ERR_NOT_REF)
+		if (err && err->code != GOT_ERR_ERRNO && errno == ENOENT)
 			return err;
 		else if (err == NULL)
 			do_merge = 1;
@@ -685,6 +686,8 @@ apply_patch(int *overlapcnt, struct got_worktree *work
 		goto done;
 
 	if (do_merge) {
+		const char *type, *idstr;
+
 		if (fseeko(afile, 0, SEEK_SET) == -1 ||
 		    fseeko(oldfile, 0, SEEK_SET) == -1 ||
 		    fseeko(tmpfile, 0, SEEK_SET) == -1) {
@@ -704,7 +707,15 @@ apply_patch(int *overlapcnt, struct got_worktree *work
 			goto done;
 		}
 
-		if (asprintf(&anclabel, "commit %s", p->cid) == -1) {
+		if (*p->cid != '\0') {
+			type = "commit";
+			idstr = p->cid;
+		} else {
+			type = "blob";
+			idstr = p->blob;
+		}
+
+		if (asprintf(&anclabel, "%s %s", type, idstr) == -1) {
 			err = got_error_from_errno("asprintf");
 			anclabel = NULL;
 			goto done;
blob - 5f1a2842034c7b2f4296aa5b45ca262a927c4a7a
file + libexec/got-read-patch/got-read-patch.c
--- libexec/got-read-patch/got-read-patch.c
+++ libexec/got-read-patch/got-read-patch.c
@@ -74,10 +74,11 @@ send_patch(const char *oldname, const char *newname, c
 	if (newname != NULL)
 		strlcpy(p.new, newname, sizeof(p.new));
 
-	if (commitid != NULL && blob != NULL) {
+	if (commitid != NULL)
 		strlcpy(p.cid, commitid, sizeof(p.cid));
+
+	if (blob != NULL)
 		strlcpy(p.blob, blob, sizeof(p.blob));
-	}
 
 	p.git = git;
 	if (imsg_compose(&ibuf, GOT_IMSG_PATCH, 0, 0, -1, &p, sizeof(p)) == -1)
blob - 20e899eab3a352dc9e8636b15e894c186595d20a
file + regress/cmdline/patch.sh
--- regress/cmdline/patch.sh
+++ regress/cmdline/patch.sh
@@ -1586,11 +1586,15 @@ test_patch_merge_unknown_blob() {
 		return 1
 	fi
 
+	local commit_id=`git_show_head $testroot/repo`
+
 	cat <<EOF > $testroot/wt/patch
 I've got a
+diff $commit_id foo/bar
+with a
 blob - aaaabbbbccccddddeeeeffff0000111122223333
 and also a
-blob + 0000111122223333444455556666888899990000
+blob + 0000111122223333444455556666777788889999
 for this dummy diff
 --- alpha
 +++ alpha
@@ -1612,7 +1616,40 @@ EOF
 	ret=$?
 	if [ $ret -ne 0 ]; then
 		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done $testroot $ret
+		return 1
 	fi
+
+	# try again without a `diff $commit_id' header
+
+	cat <<EOF > $testroot/wt/patch
+I've got a
+blob - aaaabbbbccccddddeeeeffff0000111122223333
+and also a
+blob + 0000111122223333444455556666777788889999
+for this dummy diff
+--- alpha
++++ alpha
+@@ -1 +1 @@
+-alpha
++ALPHA
+will it work?
+EOF
+
+	(cd $testroot/wt && got revert alpha > /dev/null && 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
 }