Stefan Sperling <stsp@stsp.name> wrote:
> On Wed, Mar 16, 2022 at 07:30:12PM +0100, Omar Polo wrote:
> > 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);
> 
> Instead of running so many printf calls, can we build a string and
> do just one printf?
> 
> You do not have to omit the ,1 cases, I would just print them.
> It does not hurt to write ,1 explicitly.
> I doubt everyone will know that ,1 is implied if omitted.

(I for one didn't know ",1" is implied before taking a look at
usr.bin/patch.)

the rationale for hiding the ",1" anyway was to ease the grep-ability;
i'm expecting most (all?) programs that produce diffs to hide the ,1.

anyway, trimmed down to one printf for the hunk + the if in the updated
diff below.

> > +	}
> > +
> >  	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;
> > +};
> 
> If a callback messes with the STAILQ part of the struct, the code
> in lib/patch.c could crash.
> Are you sure you want to send this internal struct to the progress callback,
> instead of a smaller struct that has copies of the required fields?

agreed.  In the end i've found a way to provide a copy of the data that
i like so patch below does this instead of what we talked on irc.

> > +
> >  /*
> >   * 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");
> 
> Possible errors from getline() are EINVAL and EOVERFLOW, and various
> others including ENOMEM.
> Is it really fine to hide any such error behind GOT_ERR_PATCH_DONT_APPLY?

my bad.  i assumed that if getline fails then ferror() was set, which
now i see it's not implied.  I've dropped this bit in the patch below
and will fix this (and the other occurrence in patch_file) in a next
commit.

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

So, here's the updated diff.

I went ahead and merged also another diff I was working on because I
realized it didn't really made sense to commit those separatedly.

got_patch also recovers from some errors and provides the error to the
callback, but still proceeds with the rest of the patch.

I've s/GOT_ERR_PATCH_DONT_APPLY/GOT_ERR_HUNK_FAILED and adjusted the
errors, then introduced GOT_ERR_PATCH_FAILED to notify the caller when
got_patch has failed on one or more files.  (to do so I had to move some
stuff from apply_patch early in got_patch, otherwise I don't know the
paths and can't provide them to the callback)

Finally, there is a new test and some tweaks to the existing ones to
make advantage of this new got_patch behaviour.

-----------------------------------------------
commit 5aa8ae7dae3dacc1457632b90a8479eb21d430a4 (hunks)
from: Omar Polo <op@omarpolo.com>
date: Thu Mar 17 11:09:17 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 4de937bee2e445854c4f6e9b18a03fe5d13ccf63
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 + ddec93ca028fc4d8c5bb008669a486f80f9f972e
--- got/got.c
+++ got/got.c
@@ -7186,13 +7186,29 @@ 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, 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);
+	}
+
 	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 + 2064e2736a5d12d4a3c57c5bca31622df3ed1b22
--- include/got_patch.h
+++ include/got_patch.h
@@ -14,13 +14,23 @@
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
+struct got_hunk {
+	const struct got_error *error;
+	long	offset;
+	long	old_from;
+	long	old_lines;
+	long	new_from;
+	long	new_lines;
+};
+
 /*
  * A callback that gets invoked during the patch application.
  *
  * Receives the old and new path and a status code.
  */
 typedef const struct got_error *(*got_patch_progress_cb)(void *,
-    const char *, const char *, unsigned char);
+    const char *, const char *, unsigned char, struct got_hunk *, size_t,
+    const struct got_error *);
 
 /*
  * Apply the (already opened) patch to the repository and register the
blob - 58d4125553be6fd56d2408805b9c001d9c08c698
blob + f05b38d0a7bc311340040f101e343cdd9a9af59e
--- 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,74 @@ check_file_status(struct got_patch *p, int file_rename
 }
 
 static const struct got_error *
+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 = NULL;
+	struct got_patch_hunk *h;
+	struct got_hunk *hunks = NULL;
+	size_t n = 0;
+
+	STAILQ_FOREACH(h, pa->head, entries)
+		if (h->offset != 0 || h->err != NULL)
+			n++;
+
+	if (n > 0) {
+		if ((hunks = calloc(n, sizeof(*hunks))) == NULL)
+			return got_error_from_errno("calloc");
+
+		n = 0;
+		STAILQ_FOREACH(h, pa->head, entries) {
+			if (h->offset != 0 || h->err != NULL) {
+				hunks[n].old_from = h->old_from;
+				hunks[n].old_lines = h->old_lines;
+				hunks[n].new_from = h->new_from;
+				hunks[n].new_lines = h->new_lines;
+				hunks[n].offset = h->offset;
+				hunks[n].error = h->err;
+				n++;
+			}
+		}
+	}
+
+	err = pa->progress_cb(pa->progress_arg, old, new, status, hunks, n,
+	    orig_error);
+	free(hunks);
+	return err;
+}
+
+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 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 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,
+    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 +668,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 +699,16 @@ 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 = progress(pa, oldpath, newpath, GOT_STATUS_MODIFY, NULL);
 
 done:
 	if (err != NULL && newpath != NULL && (file_renamed || p->old == NULL))
@@ -690,28 +718,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 +800,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 = progress(&pa, p.old, p.new,
+				    GOT_STATUS_CANNOT_UPDATE, err);
+			else if (err->code == GOT_ERR_HUNK_FAILED)
+				err = 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 +848,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