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

From:
Omar Polo <op@omarpolo.com>
Subject:
Re: got_patch_progress_cb: handle offsets and errors too
To:
Stefan Sperling <stsp@stsp.name>
Cc:
gameoftrees@openbsd.org
Date:
Thu, 17 Mar 2022 15:43:48 +0100

Download raw body.

Thread
Stefan Sperling <stsp@stsp.name> wrote:
> >  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, struct got_hunk *hunks, size_t n,
> > +    const struct got_error *error)
> >  {
> >  	const char *path = new == NULL ? old : new;
> > +	size_t i;
> >  
> >  	while (*path == '/')
> >  		path++;
> >  	printf("%c  %s\n", mode, path);
> > +
> > +	if (error != NULL)
> > +		fprintf(stderr, "%s: %s\n", getprogname(), error->msg);
> > +
> > +	for (i = 0; i < n; ++i) {
> > +		printf("@@ -%ld,%ld +%ld,%ld @@ ", hunks[i].old_from,
> > +		    hunks[i].old_lines, hunks[i].new_from, hunks[i].new_lines);
> > +		if (hunks[i].error != NULL)
> > +			printf("%s\n", hunks[i].error->msg);
> > +		else
> > +			printf("applied with offset %ld\n", hunks[i].offset);
> > +	}
> 
> If the progress() function of lib/patch.c was looping over hunks and
> called the patch_progress() with information about each hunk separately,
> you could save yourself the trouble of allocating a temporary array.
> This patch_progress() function could simply check which arguments
> are valid (non-NULL, non-zero), and print them if so.

I tried to allocate and copy the hunks as per original suggestion and to
avoid passing a lot of arguments to the callback.  i prefer to avoid the
extra allocation too and in the updated diff below I'm doing as you
suggest (that is passing the hunk info as parameters to the function and
calling it in a loop)

> > @@ -462,6 +468,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;
> 
> Spaces around operators with two operands, please: lineno + 1

wops, i forget about it.  Fixed.

(there are some other instances of `foo+1' in the code, i'll get rid of
them in a follow-up commit)

> >  static const struct got_error *
> > +progress(struct patch_args *pa, const char *old, const char *new,
> 
> I think report_progress() would be a better name for this function.

agreed!


-----------------------------------------------
commit d4570e469a3a1f0eb6cdd94c2cd8ed6e8fd4f03f (hunks)
from: Omar Polo <op@omarpolo.com>
date: Thu Mar 17 14:39:21 2022 UTC
 
 augment patch progress callback with hunks and errors, also recover from them
 
 Augment got_patch_progress_cb by providing the hunks that were applied
 with offset (or that failed) and the recoverable error encountered
 during the operation (bad status, missing file, ...)
 
 got_patch now proceeds when a file fails to be patched and exits with
 GOT_ERR_PATCH_FAILED if no other errors are encountered.
 
 While here, also add a test for the 'hunk applied with offset' case and
 shrink test_patch_dont_apply adn illegal_status by taking advantage that
 'got patch' doesn't stop at the first error.  (And add some other cases
 to illegal_status too.)
 
diff 03c350e0fd68967ea9bfeca0795fbd79de3dcb1b 19c5cd81d5723c15311cadac35c5179d3fe1212e
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 + 9036576976142f432ddf9a6b45be245db250f50e
--- got/got.c
+++ got/got.c
@@ -7186,13 +7186,31 @@ 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_error *error, long old_from,
+    long old_lines, long new_from, long new_lines, long offset,
+    const struct got_error *hunk_err)
 {
 	const char *path = new == NULL ? old : new;
 
 	while (*path == '/')
 		path++;
-	printf("%c  %s\n", mode, path);
+
+	if (mode != 0)
+		printf("%c  %s\n", mode, path);
+
+	if (error != NULL)
+		fprintf(stderr, "%s: %s\n", getprogname(), error->msg);
+
+	if (offset != 0 || hunk_err != NULL) {
+		printf("@@ -%ld,%ld +%ld,%ld @@ ", old_from,
+		    old_lines, new_from, new_lines);
+		if (hunk_err != NULL)
+			printf("%s\n", hunk_err->msg);
+		else
+			printf("applied with offset %ld\n", offset);
+	}
+
 	return NULL;
 }
 
blob - 11708ccf458de5506eccdad8dafcaaed715432bb
blob + 020d902fa5e38787f662f34a05d1a3b2472f3443
--- include/got_error.h
+++ include/got_error.h
@@ -164,8 +164,9 @@
 #define GOT_ERR_FILE_BINARY	146
 #define GOT_ERR_PATCH_MALFORMED	147
 #define GOT_ERR_PATCH_TRUNCATED	148
-#define GOT_ERR_PATCH_DONT_APPLY 149
-#define GOT_ERR_NO_PATCH	150
+#define GOT_ERR_NO_PATCH	149
+#define GOT_ERR_HUNK_FAILED	150
+#define GOT_ERR_PATCH_FAILED	151
 
 static const struct got_error {
 	int code;
@@ -344,8 +345,9 @@ static const struct got_error {
 	{ GOT_ERR_FILE_BINARY, "found a binary file instead of text" },
 	{ GOT_ERR_PATCH_MALFORMED, "malformed patch" },
 	{ GOT_ERR_PATCH_TRUNCATED, "patch truncated" },
-	{ GOT_ERR_PATCH_DONT_APPLY, "patch doesn't apply" },
 	{ GOT_ERR_NO_PATCH, "no patch found" },
+	{ GOT_ERR_HUNK_FAILED, "hunk failed to apply" },
+	{ GOT_ERR_PATCH_FAILED, "patch failed to apply" },
 };
 
 /*
blob - 391c0cf0a69178e759d03ba7c752062f5752dfc6
blob + 959a2129f32e5f561fc9ccfdc186d10507d45e00
--- include/got_patch.h
+++ include/got_patch.h
@@ -17,10 +17,12 @@
 /*
  * 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, if an error occurred while
+ * applying the patch, and a hunk applied with offset or its error.
  */
 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_error *,
+    long, long, long, long, long, const struct got_error *);
 
 /*
  * Apply the (already opened) patch to the repository and register the
blob - 58d4125553be6fd56d2408805b9c001d9c08c698
blob + acd793bdf71f41420eda39d2a713aaca2c159a43
--- lib/patch.c
+++ lib/patch.c
@@ -56,6 +56,8 @@
 
 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;
@@ -65,15 +67,17 @@ struct got_patch_hunk {
 	char	**lines;
 };
 
+STAILQ_HEAD(got_patch_hunk_head, got_patch_hunk);
 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 *
@@ -273,7 +277,7 @@ copy(FILE *tmp, FILE *orig, off_t copypos, off_t pos)
 		if (r != len && feof(orig)) {
 			if (pos == -1)
 				return NULL;
-			return got_error(GOT_ERR_PATCH_DONT_APPLY);
+			return got_error(GOT_ERR_HUNK_FAILED);
 		}
 	}
 	return NULL;
@@ -296,7 +300,7 @@ locate_hunk(FILE *orig, struct got_patch_hunk *h, off_
 			if (ferror(orig))
 				err = got_error_from_errno("getline");
 			else if (match == -1)
-				err = got_error(GOT_ERR_PATCH_DONT_APPLY);
+				err = got_error(GOT_ERR_HUNK_FAILED);
 			break;
 		}
 		(*lineno)++;
@@ -348,11 +352,11 @@ test_hunk(FILE *orig, struct got_patch_hunk *h)
 					err = got_error_from_errno("getline");
 				else
 					err = got_error(
-					    GOT_ERR_PATCH_DONT_APPLY);
+					    GOT_ERR_HUNK_FAILED);
 				goto done;
 			}
 			if (strcmp(h->lines[i]+1, line)) {
-				err = got_error(GOT_ERR_PATCH_DONT_APPLY);
+				err = got_error(GOT_ERR_HUNK_FAILED);
 				goto done;
 			}
 			break;
@@ -433,6 +437,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_HUNK_FAILED)
+			h->err = err;
 		if (err != NULL)
 			goto done;
 		if (!nop)
@@ -442,7 +448,7 @@ patch_file(struct got_patch *p, const char *path, FILE
 		copypos = pos;
 
 		err = test_hunk(orig, h);
-		if (err != NULL && err->code == GOT_ERR_PATCH_DONT_APPLY) {
+		if (err != NULL && err->code == GOT_ERR_HUNK_FAILED) {
 			/*
 			 * try to apply the hunk again starting the search
 			 * after the previous partial match.
@@ -462,6 +468,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 +483,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_HUNK_FAILED);
+		err = h->err;
+	} else if (!nop && !feof(orig))
 		err = copy(tmp, orig, copypos, -1);
 
 done:
@@ -571,56 +582,63 @@ check_file_status(struct got_patch *p, int file_rename
 }
 
 static const struct got_error *
+report_progress(struct patch_args *pa, const char *old, const char *new,
+    unsigned char status, const struct got_error *orig_error)
+{
+	const struct got_error *err;
+	struct got_patch_hunk *h;
+
+	err = pa->progress_cb(pa->progress_arg, old, new, status,
+	    orig_error, 0, 0, 0, 0, 0, NULL);
+	if (err)
+		return err;
+
+	STAILQ_FOREACH(h, pa->head, entries) {
+		if (h->offset == 0 && h->err == NULL)
+			continue;
+
+		err = pa->progress_cb(pa->progress_arg, old, new, 0, NULL,
+		    h->old_from, h->old_lines, h->new_from, h->new_lines,
+		    h->offset, h->err);
+		if (err)
+			return err;
+	}
+
+	return NULL;
+}
+
+static const struct got_error *
 patch_delete(void *arg, unsigned char status, unsigned char staged_status,
     const char *path)
 {
-	struct patch_args *pa = arg;
-
-	return pa->progress_cb(pa->progress_arg, path, NULL, status);
+	return report_progress(arg, path, NULL, status, NULL);
 }
 
 static const struct got_error *
 patch_add(void *arg, unsigned char status, const char *path)
 {
-	struct patch_args *pa = arg;
-
-	return pa->progress_cb(pa->progress_arg, NULL, path, status);
+	return report_progress(arg, NULL, path, status, NULL);
 }
 
 static const struct got_error *
 apply_patch(struct got_worktree *worktree, struct got_repository *repo,
-    struct got_patch *p, int nop, struct patch_args *pa,
-    got_cancel_cb cancel_cb, void *cancel_arg)
+    struct got_pathlist_head *oldpaths, struct got_pathlist_head *newpaths,
+    const char *oldpath, const char *newpath, struct got_patch *p,
+    int nop, struct patch_args *pa, got_cancel_cb cancel_cb, void *cancel_arg)
 {
 	const struct got_error *err = NULL;
-	struct got_pathlist_head oldpaths, newpaths;
 	int file_renamed = 0;
-	char *oldpath = NULL, *newpath = NULL, *parent = NULL;
-	char *tmppath = NULL, *template = NULL;
+	char *tmppath = NULL, *template = NULL, *parent = NULL;;
 	FILE *tmp = NULL;
 	mode_t mode = GOT_DEFAULT_FILE_MODE;
 
-	TAILQ_INIT(&oldpaths);
-	TAILQ_INIT(&newpaths);
+	file_renamed = strcmp(oldpath, newpath);
 
-	err = build_pathlist(p->old != NULL ? p->old : p->new, &oldpath,
-	    &oldpaths, worktree);
+	err = check_file_status(p, file_renamed, worktree, repo, oldpaths,
+	    newpaths, cancel_cb, cancel_arg);
 	if (err)
 		goto done;
 
-	err = build_pathlist(p->new != NULL ? p->new : p->old, &newpath,
-	    &newpaths, worktree);
-	if (err)
-		goto done;
-
-	if (p->old != NULL && p->new != NULL && strcmp(p->old, p->new))
-		file_renamed = 1;
-
-	err = check_file_status(p, file_renamed, worktree, repo, &oldpaths,
-	    &newpaths, cancel_cb, cancel_arg);
-	if (err)
-		goto done;
-
 	if (asprintf(&template, "%s/got-patch",
 	    got_worktree_get_root_path(worktree)) == -1) {
 		err = got_error_from_errno(template);
@@ -639,7 +657,7 @@ apply_patch(struct got_worktree *worktree, struct got_
 		goto done;
 
 	if (p->old != NULL && p->new == NULL) {
-		err = got_worktree_schedule_delete(worktree, &oldpaths,
+		err = got_worktree_schedule_delete(worktree, oldpaths,
 		    0, NULL, patch_delete, pa, repo, 0, 0);
 		goto done;
 	}
@@ -670,17 +688,17 @@ apply_patch(struct got_worktree *worktree, struct got_
 	}
 
 	if (file_renamed) {
-		err = got_worktree_schedule_delete(worktree, &oldpaths,
+		err = got_worktree_schedule_delete(worktree, oldpaths,
 		    0, NULL, patch_delete, pa, repo, 0, 0);
 		if (err == NULL)
-			err = got_worktree_schedule_add(worktree, &newpaths,
+			err = got_worktree_schedule_add(worktree, newpaths,
 			    patch_add, pa, repo, 1);
 	} else if (p->old == NULL)
-		err = got_worktree_schedule_add(worktree, &newpaths,
+		err = got_worktree_schedule_add(worktree, newpaths,
 		    patch_add, pa, repo, 1);
 	else
-		err = pa->progress_cb(pa->progress_arg, oldpath, newpath,
-		    GOT_STATUS_MODIFY);
+		err = report_progress(pa, oldpath, newpath, GOT_STATUS_MODIFY,
+		    NULL);
 
 done:
 	if (err != NULL && newpath != NULL && (file_renamed || p->old == NULL))
@@ -690,28 +708,53 @@ done:
 	if (tmppath != NULL)
 		unlink(tmppath);
 	free(tmppath);
-	got_pathlist_free(&oldpaths);
-	got_pathlist_free(&newpaths);
-	free(oldpath);
-	free(newpath);
 	return err;
 }
 
+static const struct got_error *
+resolve_paths(struct got_patch *p, struct got_worktree *worktree,
+    struct got_repository *repo, struct got_pathlist_head *oldpaths,
+    struct got_pathlist_head *newpaths, char **old, char **new)
+{
+	const struct got_error *err;
+
+	TAILQ_INIT(oldpaths);
+	TAILQ_INIT(newpaths);
+	*old = NULL;
+	*new = NULL;
+
+	err = build_pathlist(p->old != NULL ? p->old : p->new, old,
+	    oldpaths, worktree);
+	if (err)
+		goto err;
+
+	err = build_pathlist(p->new != NULL ? p->new : p->old, new,
+	    newpaths, worktree);
+	if (err)
+		goto err;
+	return NULL;
+
+err:
+	free(*old);
+	free(*new);
+	got_pathlist_free(oldpaths);
+	got_pathlist_free(newpaths);
+	return err;
+}
+
 const struct got_error *
 got_patch(int fd, struct got_worktree *worktree, struct got_repository *repo,
     int nop, got_patch_progress_cb progress_cb, void *progress_arg,
     got_cancel_cb cancel_cb, void *cancel_arg)
 {
 	const struct got_error *err = NULL;
-	struct patch_args pa;
+	struct got_pathlist_head oldpaths, newpaths;
+	char *oldpath, *newpath;
 	struct imsgbuf *ibuf;
 	int imsg_fds[2] = {-1, -1};
-	int done = 0;
+	int done = 0, failed = 0;
 	pid_t pid;
 
-	pa.progress_cb = progress_cb;
-	pa.progress_arg = progress_arg;
-
 	ibuf = calloc(1, sizeof(*ibuf));
 	if (ibuf == NULL) {
 		err = got_error_from_errno("calloc");
@@ -747,14 +790,41 @@ got_patch(int fd, struct got_worktree *worktree, struc
 
 	while (!done && err == NULL) {
 		struct got_patch p;
+		struct patch_args pa;
 
+		pa.progress_cb = progress_cb;
+		pa.progress_arg = progress_arg;
+		pa.head = &p.head;
+
 		err = recv_patch(ibuf, &done, &p);
 		if (err || done)
 			break;
 
-		err = apply_patch(worktree, repo, &p, nop, &pa,
-		    cancel_cb, cancel_arg);
+		err = resolve_paths(&p, worktree, repo, &oldpaths,
+		    &newpaths, &oldpath, &newpath);
+		if (err)
+			break;
+
+		err = apply_patch(worktree, repo, &oldpaths, &newpaths,
+		    oldpath, newpath, &p, nop, &pa, cancel_cb, cancel_arg);
+		if (err != NULL) {
+			failed = 1;
+			/* recoverable errors */
+			if (err->code == GOT_ERR_FILE_STATUS ||
+			    (err->code == GOT_ERR_ERRNO && errno == ENOENT))
+				err = report_progress(&pa, p.old, p.new,
+				    GOT_STATUS_CANNOT_UPDATE, err);
+			else if (err->code == GOT_ERR_HUNK_FAILED)
+				err = report_progress(&pa, p.old, p.new,
+				    GOT_STATUS_CANNOT_UPDATE, NULL);
+		}
+
+		free(oldpath);
+		free(newpath);
+		got_pathlist_free(&oldpaths);
+		got_pathlist_free(&newpaths);
 		patch_free(&p);
+
 		if (err)
 			break;
 	}
@@ -768,5 +838,7 @@ done:
 		err = got_error_from_errno("close");
 	if (imsg_fds[1] != -1 && close(imsg_fds[1]) == -1 && err == NULL)
 		err = got_error_from_errno("close");
+	if (err == NULL && failed)
+		err = got_error(GOT_ERR_PATCH_FAILED);
 	return err;
 }
blob - 92f950fb6b4fb18338a3b9edcd4318445564b3e7
blob + 65fb8dead18ae13233a8eb24e2f1f26a41d8906e
--- regress/cmdline/patch.sh
+++ regress/cmdline/patch.sh
@@ -403,43 +403,6 @@ test_patch_dont_apply() {
 		return 1
 	fi
 
-	cat <<EOF > $testroot/wt/patch
---- alpha
-+++ alpha
-@@ -1 +1,2 @@
-+hatsuseno
- alpha something
-EOF
-
-	echo -n > $testroot/stdout.expected
-	echo "got: patch doesn't apply" > $testroot/stderr.expected
-
-	(cd $testroot/wt && got patch patch) \
-		 > $testroot/stdout \
-		2> $testroot/stderr
-	ret=$?
-	if [ $ret -eq 0 ]; then # should fail
-		test_done $testroot 1
-		return 1
-	fi
-
-	cmp -s $testroot/stdout.expected $testroot/stdout
-	ret=$?
-	if [ $ret -ne 0 ]; then
-		diff -u $testroot/stdout.expected $testroot/stdout
-		test_done $testroot $ret
-		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') \
 		>/dev/null
@@ -450,6 +413,11 @@ EOF
 	fi
 
 	cat <<EOF > $testroot/wt/patch
+--- alpha
++++ alpha
+@@ -1 +1,2 @@
++hatsuseno
+ alpha something
 --- numbers
 +++ /dev/null
 @@ -1,9 +0,0 @@
@@ -464,18 +432,24 @@ 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
+	cat <<EOF > $testroot/stdout.expected
+#  alpha
+@@ -1,1 +1,2 @@ hunk failed to apply
+#  numbers
+@@ -1,9 +0,0 @@ hunk failed to apply
+EOF
+
+	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
 }
@@ -779,75 +753,40 @@ test_patch_illegal_status() {
 		return 1
 	fi
 
-	# edit an non-existent and unknown file
+	# try to patch an obstructed file, add a versioned one, edit a
+	# non existent file and an unversioned one, and remove a
+	# non existent file.
 	cat <<EOF > $testroot/wt/patch
+--- alpha
++++ alpha
+@@ -1 +1,2 @@
+ alpha
++was edited
+--- /dev/null
++++ beta
+@@ -0,0 +1 @@
++beta
 --- iota
 +++ iota
 @@ -1 +1 @@
-- iota
-+ IOTA
+-iota
++IOTA
+--- kappa
++++ kappa
+@@ -1 +1 @@
+-kappa
++KAPPA
+--- lambda
++++ /dev/null
+@@ -1 +0,0 @@
+-lambda
 EOF
 
-	(cd $testroot/wt && got patch patch) > /dev/null \
-		2> $testroot/stderr
-	ret=$?
-	if [ $ret -eq 0 ]; then
-		echo "edited a missing file" >&2
-		test_done $testroot $ret
-		return 1
-	fi
-
-	echo "got: iota: No such file or directory" \
-		> $testroot/stderr.expected
-	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
-
-	# create iota and re-try
-	echo iota > $testroot/wt/iota
-
-	(cd $testroot/wt && got patch patch) > /dev/null \
-		2> $testroot/stderr
-	ret=$?
-	if [ $ret -eq 0 ]; then
-		echo "patched an unknown file" >&2
-		test_done $testroot $ret
-		return 1
-	fi
-
-	echo "got: iota: file has unexpected status" \
-		> $testroot/stderr.expected
-	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
-	
-	rm $testroot/wt/iota
-	ret=$?
-	if [ $ret -ne 0 ]; then
-		test_done $testroot $ret
-		return 1
-	fi
-
-	# edit obstructed file
+	echo kappa > $testroot/wt/kappa
 	rm $testroot/wt/alpha
 	mkdir $testroot/wt/alpha
-	cat <<EOF > $testroot/wt/patch
---- alpha
-+++ alpha
-@@ -1 +1,2 @@
- alpha
-+was edited
-EOF
 
-	(cd $testroot/wt && got patch patch) > /dev/null \
+	(cd $testroot/wt && got patch patch) > $testroot/stdout \
 		2> $testroot/stderr
 	ret=$?
 	if [ $ret -eq 0 ]; then
@@ -856,61 +795,36 @@ EOF
 		return 1
 	fi
 
-	echo "got: alpha: file has unexpected status" \
-		> $testroot/stderr.expected
-	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
+	cat <<EOF > $testroot/stdout.expected
+#  alpha
+#  beta
+#  iota
+#  kappa
+#  lambda
+EOF
 
-	# delete an unknown file
-	cat <<EOF > $testroot/wt/patch
---- iota
-+++ /dev/null
-@@ -1 +0,0 @@
--iota
+	cat <<EOF > $testroot/stderr.expected
+got: alpha: file has unexpected status
+got: beta: file has unexpected status
+got: iota: No such file or directory
+got: kappa: file has unexpected status
+got: lambda: No such file or directory
+got: patch failed to apply
 EOF
 
-	(cd $testroot/wt && got patch patch) > /dev/null \
-		2> $testroot/stderr
+	cmp -s $testroot/stdout.expected $testroot/stdout
 	ret=$?
-	if [ $ret -eq 0 ]; then
-		echo "deleted a missing file?" >&2
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
 		test_done $testroot $ret
 		return 1
 	fi
 
-	echo "got: iota: No such file or directory" \
-		> $testroot/stderr.expected
 	cmp -s $testroot/stderr.expected $testroot/stderr
 	ret=$?
-	if [ $ret -eq 0 ]; then
+	if [ $ret -ne 0 ]; then
 		diff -u $testroot/stderr.expected $testroot/stderr
-		test_done $testroot $ret
-		return 1
 	fi
-
-	# try again with iota in place but still not registered
-	echo iota > $testroot/wt/iota
-	(cd $testroot/wt && got patch patch) > /dev/null \
-		2> $testroot/stderr
-	ret=$?
-	if [ $ret -eq 0 ]; then
-		echo "deleted an unversioned file?" >&2
-		test_done $testroot $ret
-		return 1
-	fi
-
-	echo "got: iota: file has unexpected status" \
-		> $testroot/stderr.expected
-	cmp -s $testroot/stderr.expected $testroot/stderr
-	ret=$?
-	if [ $ret -eq 0 ]; then
-		diff -u $testroot/stderr.expected $testroot/stderr
-	fi
 	test_done $testroot $ret
 }
 
@@ -1049,6 +963,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 tru 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/wt $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 +1050,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