Omar Polo <op@omarpolo.com> wrote:
> This extends the newly added -and just committed!- got_patch_progress_cb
> to deal with offsets and errros too.
> 
> 'got patch' was able to apply patches with offsets (i.e. at a different
> line then what indicated in the patch) since its addition, but never
> actually printed that information.
> 
> Another thing that's useful is to indicate which hunk failed to be
> applied instead of the currently "patch doesn't apply" message.  The
> format was suggested by stsp (thanks!) on irc a couple of days ago; see
> the bits in the regression suite.
> 
> To do so I'm moving the struct got_patch_hunk and the definition of its
> head in got_patch.h and passing to the callback the STAILQ of hunks.
> The struct got_patch_hunk is also augmented with the `offset' and `err'
> field which are filled before invoking the callback.
> 
> ok?
> 
> P.S.: i intend to change the "patch doesn't apply" error with "hunk
> failed" soon, but I have another patch to send before doing it ;)
> 
> P.P.S: in apply_patch I'm deliberating ignoring the error returned by
> the callback.  It won't stay like that for too long; next diff will deal
> with that.  (i wanted to avoid adding a conditional there only to drop
> it in the next commit)

updated diff after recent changes

(i'll get to the point where 'got patch' will be able to merge these
kinds of conflicts with diff3 eventually....)

-----------------------------------------------
commit 2f276013a6ef2116a3b2f7e49828e51653ba22f2 (pscb)
from: Omar Polo <op@omarpolo.com>
date: Wed Mar 16 18:26:02 2022 UTC
 
 got_patch_progress_cb: handle offsets and errors too
 
 This makes 'got patch' print the hunks applied with offset or that
 failed.
 
diff 03c350e0fd68967ea9bfeca0795fbd79de3dcb1b 0548581184e3531979d3a1d9a47af0df513730c0
blob - 1109e3c97f334540e101df641fdfc3423cedb552
blob + e45d19779f8e92372a5e4aa275cfc6837fc9b8e0
--- got/got.1
+++ got/got.1
@@ -1313,6 +1313,7 @@ Show the status of each affected file, using the follo
 .It M Ta file was modified
 .It D Ta file was deleted
 .It A Ta file was added
+.It # Ta failed to patch the file
 .El
 .Pp
 If a change does not match at its exact line number, attempt to
blob - 6e4f19aa3e9703b1e7a3b57f3d76fe41a2b801ac
blob + 453af5ad8978ba78acf573f3beb5d5f0d2092688
--- got/got.c
+++ got/got.c
@@ -7186,13 +7186,32 @@ patch_from_stdin(int *patchfd)
 }
 
 static const struct got_error *
-patch_progress(void *arg, const char *old, const char *new, unsigned char mode)
+patch_progress(void *arg, const char *old, const char *new,
+    unsigned char mode, const struct got_patch_hunk_head *head)
 {
+	struct got_patch_hunk *h;
 	const char *path = new == NULL ? old : new;
 
 	while (*path == '/')
 		path++;
 	printf("%c  %s\n", mode, path);
+
+	STAILQ_FOREACH(h, head, entries) {
+		if (h->offset != 0 || h->err != NULL) {
+			printf("@@ -%ld", h->old_from);
+			if (h->old_lines != 1)
+				printf(",%ld", h->old_lines);
+			printf(" +%ld", h->new_from);
+			if (h->new_lines != 1)
+				printf(",%ld", h->new_lines);
+			printf(" @@ ");
+		}
+		if (h->offset != 0)
+			printf("applied with offset %ld\n", h->offset);
+		if (h->err != NULL)
+			printf("%s\n", h->err->msg);
+	}
+
 	return NULL;
 }
 
blob - 391c0cf0a69178e759d03ba7c752062f5752dfc6
blob + 5b5f8d153e01e5595219bcda45f025ba7289bbbb
--- include/got_patch.h
+++ include/got_patch.h
@@ -14,13 +14,28 @@
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
+STAILQ_HEAD(got_patch_hunk_head, got_patch_hunk);
+struct got_patch_hunk {
+	STAILQ_ENTRY(got_patch_hunk) entries;
+	const struct got_error *err;
+	long	offset;
+	long	old_from;
+	long	old_lines;
+	long	new_from;
+	long	new_lines;
+	size_t	len;
+	size_t	cap;
+	char	**lines;
+};
+
 /*
  * A callback that gets invoked during the patch application.
  *
- * Receives the old and new path and a status code.
+ * Receives the old and new path, a status code and the hunks.
  */
 typedef const struct got_error *(*got_patch_progress_cb)(void *,
-    const char *, const char *, unsigned char);
+    const char *, const char *, unsigned char,
+    const struct got_patch_hunk_head *);
 
 /*
  * Apply the (already opened) patch to the repository and register the
blob - 58d4125553be6fd56d2408805b9c001d9c08c698
blob + 9eab8fa2fd1714e2fa85193a6843cc5480466f4d
--- lib/patch.c
+++ lib/patch.c
@@ -54,26 +54,16 @@
 
 #define MIN(a, b) ((a) < (b) ? (a) : (b))
 
-struct got_patch_hunk {
-	STAILQ_ENTRY(got_patch_hunk) entries;
-	long	old_from;
-	long	old_lines;
-	long	new_from;
-	long	new_lines;
-	size_t	len;
-	size_t	cap;
-	char	**lines;
-};
-
 struct got_patch {
 	char	*old;
 	char	*new;
-	STAILQ_HEAD(, got_patch_hunk) head;
+	struct got_patch_hunk_head head;
 };
 
 struct patch_args {
 	got_patch_progress_cb progress_cb;
 	void	*progress_arg;
+	struct got_patch_hunk_head *head;
 };
 
 static const struct got_error *
@@ -344,11 +334,8 @@ test_hunk(FILE *orig, struct got_patch_hunk *h)
 		case '-':
 			linelen = getline(&line, &linesize, orig);
 			if (linelen == -1) {
-				if (ferror(orig))
-					err = got_error_from_errno("getline");
-				else
-					err = got_error(
-					    GOT_ERR_PATCH_DONT_APPLY);
+				err = got_ferror(orig,
+				    GOT_ERR_PATCH_DONT_APPLY);
 				goto done;
 			}
 			if (strcmp(h->lines[i]+1, line)) {
@@ -433,6 +420,8 @@ patch_file(struct got_patch *p, const char *path, FILE
 
 	tryagain:
 		err = locate_hunk(orig, h, &pos, &lineno);
+		if (err != NULL && err->code == GOT_ERR_PATCH_DONT_APPLY)
+			h->err = err;
 		if (err != NULL)
 			goto done;
 		if (!nop)
@@ -453,7 +442,13 @@ patch_file(struct got_patch *p, const char *path, FILE
 			}
 			linelen = getline(&line, &linesize, orig);
 			if (linelen == -1) {
-				err = got_error_from_errno("getline");
+				if (ferror(orig))
+					err = got_error_from_errno("getline");
+				else {
+					err = got_error(
+					    GOT_ERR_PATCH_DONT_APPLY);
+					h->err = err;
+				}
 				goto done;
 			}
 			lineno++;
@@ -462,6 +457,9 @@ patch_file(struct got_patch *p, const char *path, FILE
 		if (err != NULL)
 			goto done;
 
+		if (lineno+1 != h->old_from)
+			h->offset = lineno+1 - h->old_from;
+
 		if (!nop)
 			err = apply_hunk(tmp, h, &lineno);
 		if (err != NULL)
@@ -474,9 +472,11 @@ patch_file(struct got_patch *p, const char *path, FILE
 		}
 	}
 
-	if (p->new == NULL && sb.st_size != copypos)
-		err = got_error(GOT_ERR_PATCH_DONT_APPLY);
-	else if (!nop && !feof(orig))
+	if (p->new == NULL && sb.st_size != copypos) {
+		h = STAILQ_FIRST(&p->head);
+		h->err = got_error(GOT_ERR_PATCH_DONT_APPLY);
+		err = h->err;
+	} else if (!nop && !feof(orig))
 		err = copy(tmp, orig, copypos, -1);
 
 done:
@@ -576,7 +576,8 @@ patch_delete(void *arg, unsigned char status, unsigned
 {
 	struct patch_args *pa = arg;
 
-	return pa->progress_cb(pa->progress_arg, path, NULL, status);
+	return pa->progress_cb(pa->progress_arg, path, NULL, status,
+	    pa->head);
 }
 
 static const struct got_error *
@@ -584,7 +585,8 @@ patch_add(void *arg, unsigned char status, const char 
 {
 	struct patch_args *pa = arg;
 
-	return pa->progress_cb(pa->progress_arg, NULL, path, status);
+	return pa->progress_cb(pa->progress_arg, NULL, path, status,
+	    pa->head);
 }
 
 static const struct got_error *
@@ -632,6 +634,9 @@ apply_patch(struct got_worktree *worktree, struct got_
 	if (err)
 		goto done;
 	err = patch_file(p, oldpath, tmp, nop, &mode);
+	if (err && err->code == GOT_ERR_PATCH_DONT_APPLY)
+		pa->progress_cb(pa->progress_arg, oldpath, newpath,
+		    GOT_STATUS_CANNOT_UPDATE, pa->head);
 	if (err)
 		goto done;
 
@@ -680,7 +685,7 @@ apply_patch(struct got_worktree *worktree, struct got_
 		    patch_add, pa, repo, 1);
 	else
 		err = pa->progress_cb(pa->progress_arg, oldpath, newpath,
-		    GOT_STATUS_MODIFY);
+		    GOT_STATUS_MODIFY, pa->head);
 
 done:
 	if (err != NULL && newpath != NULL && (file_renamed || p->old == NULL))
@@ -752,6 +757,7 @@ got_patch(int fd, struct got_worktree *worktree, struc
 		if (err || done)
 			break;
 
+		pa.head = &p.head;
 		err = apply_patch(worktree, repo, &p, nop, &pa,
 		    cancel_cb, cancel_arg);
 		patch_free(&p);
blob - 92f950fb6b4fb18338a3b9edcd4318445564b3e7
blob + bba237d06710fbca86b604cfa5f598c7226cc8cf
--- regress/cmdline/patch.sh
+++ regress/cmdline/patch.sh
@@ -411,12 +411,10 @@ test_patch_dont_apply() {
  alpha something
 EOF
 
-	echo -n > $testroot/stdout.expected
-	echo "got: patch doesn't apply" > $testroot/stderr.expected
+	echo "#  alpha" > $testroot/stdout.expected
+	echo "@@ -1 +1,2 @@ patch doesn't apply" >> $testroot/stdout.expected
 
-	(cd $testroot/wt && got patch patch) \
-		 > $testroot/stdout \
-		2> $testroot/stderr
+	(cd $testroot/wt && got patch patch) > $testroot/stdout 2>/dev/null
 	ret=$?
 	if [ $ret -eq 0 ]; then # should fail
 		test_done $testroot 1
@@ -431,14 +429,6 @@ EOF
 		return 1
 	fi
 
-	cmp -s $testroot/stderr.expected $testroot/stderr
-	ret=$?
-	if [ $ret -ne 0 ]; then
-		diff -u $testroot/stderr.expected $testroot/stderr
-		test_done $testroot $ret
-		return 1
-	fi
-
 	# try to delete a file with a patch that doesn't match
 	jot 100 > $testroot/wt/numbers
 	(cd $testroot/wt && got add numbers && got commit -m 'add numbers') \
@@ -464,18 +454,21 @@ EOF
 -9
 EOF
 
-	(cd $testroot/wt && got patch patch) > /dev/null 2> $testroot/stderr
+	(cd $testroot/wt && got patch patch) > $testroot/stdout 2> /dev/null
 	ret=$?
 	if [ $ret -eq 0 ]; then # should fail
 		test_done $testroot 1
 		return 1
 	fi
 
-	echo "got: patch doesn't apply" > $testroot/stderr.expected
-	cmp -s $testroot/stderr.expected $testroot/stderr
+	echo "#  numbers" > $testroot/stdout.expected
+	echo "@@ -1,9 +0,0 @@ patch doesn't apply" \
+		>> $testroot/stdout.expected
+
+	cmp -s $testroot/stdout.expected $testroot/stdout
 	ret=$?
 	if [ $ret -ne 0 ]; then
-		diff -u $testroot/stderr.expected $testroot/stderr
+		diff -u $testroot/stdout.expected $testroot/stdout
 	fi
 	test_done $testroot $ret
 }
@@ -1049,6 +1042,76 @@ EOF
 	test_done $testroot 0
 }
 
+test_patch_with_offset() {
+	local testroot=`test_init patch_with_offset`
+
+	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
+--- numbers
++++ numbers
+@@ -47,7 +47,7 @@
+ 47
+ 48
+ 49
+-50
++midway thru it!
+ 51
+ 52
+ 53
+@@ -87,7 +87,7 @@
+ 87
+ 88
+ 89
+-90
++almost there!
+ 91
+ 92
+ 93
+EOF
+
+	jot 100 > $testroot/wt/numbers
+	ed $testroot/wt/numbers <<EOF > /dev/null 2> /dev/null
+1,10d
+50r !jot 20
+w
+q
+EOF
+
+	(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
+
+	(cd $testroot/wt && got patch patch) > $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done $testroot $ret
+		return 1
+	fi
+
+	cat <<EOF > $testroot/stdout.expected
+M  numbers
+@@ -47,7 +47,7 @@ applied with offset -10
+@@ -87,7 +87,7 @@ applied with offset 10
+EOF
+
+	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 +1129,4 @@ run_test test_patch_illegal_status
 run_test test_patch_nop
 run_test test_patch_preserve_perm
 run_test test_patch_create_dirs
+run_test test_patch_with_offset