"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Omar Polo <op@omarpolo.com>
Subject:
got_patch_progress_cb: handle offsets and errors too
To:
gameoftrees@openbsd.org
Date:
Wed, 16 Mar 2022 17:43:08 +0100

Download raw body.

Thread
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)

-----------------------------------------------
commit 6ad371bc6e8b073709e8a64cc5af6f5cbb8f48f7 (pscb)
from: Omar Polo <op@omarpolo.com>
date: Wed Mar 16 16:35:48 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 535daea402634388631e7318cfd30537e1530cc2 b9350c67630a6f6564236281d5da950e2123275e
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 - a56994ab50aea3ed8cbe907c4d8fa2cc65f6ae14
blob + 9bd5f7c573cffd1336ab46219c217f864e713f6f
--- 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 - 93a9570fa38f33af30ac5d909b03d23f7c63eda3
blob + 87da8b803d66c73d3e8cbf6156ee1a0f72299292
--- 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)) {
@@ -425,6 +412,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)
@@ -445,7 +434,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++;
@@ -454,6 +449,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)
@@ -472,8 +470,11 @@ patch_file(struct got_patch *p, const char *path, FILE
 
 		if (fstat(fileno(orig), &sb) == -1)
 			err = got_error_from_errno("fstat");
-		else if (sb.st_size != copypos)
-			err = got_error(GOT_ERR_PATCH_DONT_APPLY);
+		else if (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);
 
@@ -574,7 +575,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 *
@@ -582,7 +584,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 *
@@ -629,6 +632,9 @@ apply_patch(struct got_worktree *worktree, struct got_
 	if (err)
 		goto done;
 	err = patch_file(p, oldpath, tmp, nop);
+	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;
 
@@ -657,7 +663,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))
@@ -728,6 +734,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 - 41f290c529e09c20b73b0a6752c3351664a9559d
blob + 605e38fe907986317b2d535e316fea2b2957166a
--- 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
 }
@@ -967,6 +960,76 @@ EOF
 	test_done $testroot $ret
 }
 
+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
@@ -982,3 +1045,4 @@ run_test test_patch_equals_for_context
 run_test test_patch_rename
 run_test test_patch_illegal_status
 run_test test_patch_nop
+run_test test_patch_with_offset