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

From:
Omar Polo <op@omarpolo.com>
Subject:
Re: check file status before applying patches (second try)
To:
Stefan Sperling <stsp@stsp.name>
Cc:
gameoftrees@openbsd.org
Date:
Sun, 13 Mar 2022 14:49:00 +0100

Download raw body.

Thread
Stefan Sperling <stsp@stsp.name> wrote:
> On Sun, Mar 13, 2022 at 11:33:49AM +0100, Omar Polo wrote:
> > > If a patch file contains a series of patches which will be applied
> > > in sequence, then patches which pass pre-conditions will affect
> > > the work tree, and any later patch which does not pass pre-conditions
> > > will make 'got patch' error out, leaving any modifications made by
> > > previous patches behind.
> > > I think this behaviour is correct. The application of patch N to
> > > the work tree can affect pre-condition checks for patch N+1, so
> > > there is no way to check pre-conditions for patch N+1 before
> > > applying patch N to the work tree.
> > > Still, this might be worth documenting because users might otherwise
> > > expect that 'got patch' will not modify the work tree in any way
> > > unless *all* patches in the input pass pre-condition checks.
> > 
> > Yep, i tried to point this out in the diff below.
> > 
> > In theory it should be possible to apply a patch in a way that if
> > everything succeeds then the changes are persisted but otherwise the
> > worktree is unmodified, but it seems a bit difficult to do correctly.
> 
> This would require a multi-layer transactional system, where we could
> do things like 'revert' but go back to some intermediate modified state
> instead of the repository's base state.
> 
> I would suggest to keep such things as simple as possible.
> We'll produce too many bugs otherwise.

yeah :D

i agree with keeping this as easy as possible!

> > If we strengthen the preconditions so that only unmodified files are
> > allowed to be changed, than we could run an implicit 'got revert' but
> > i don't really like the idea.  Producing .rej files (which atm we still
> > don't do) or conflict markers on failures but still trying to continue
> > seems the most useful behavior to me, at least by default.
> 
> Allowing only M is not a restriction we would want to keep in the
> long-term. Having files in A and D status should be supported.

I meant that i'm only allowing M (or m, or A) when need to apply some
changes to a file.  How could it work for file scheduled for deletion
(D) ?

> > > > -	err = got_worktree_resolve_path(&oldpath, worktree,
> > > > -	    p->old != NULL ? p->old : p->new);
> > > > +	TAILQ_INIT(&oldpaths);
> > > > +	TAILQ_INIT(&newpaths);
> > > > +
> > > > +	err = build_pathlist(p->old != NULL ? p->old : p->new, &oldpath,
> > > > +	    &oldpaths, worktree);
> > > >  	if (err)
> > > >  		goto done;
> > > 
> > > Maybe worktree.c should provide a build_pathlist style funcion?
> > > This function returns a pathlist that corresponds to a single path
> > > in the work tree, and could be useful elsewhere, e.g. in got.c?
> > > In any case, we can refactor this later.
> > 
> > I'm not really sure which other parts of got works with only one file at
> > time...
> >
> > Furthermore, I'm expecting to handle some other paths operation there in
> > the future.  Atm we fail to work on `got checkout -p ...' worktrees and
> > with patches that have "non-worktree-absolute" paths.  I still haven't
> > solved these problems, but I'm guessing that I'll be able to solve them
> > in `build_pathlist'.
> 
> Absolute paths can be made relative via the strip option.
> 
> For checkout -p, we could strip away the work tree's path prefix from
> paths found in the patch, provided *all* such paths share this prefix.
> 
> Patches created from checkout -p are another problem. At present,
> 'got patch' expects paths in the patch to be relative to the work
> tree root. Perhaps it should treat paths as relative to the current
> working directory instead? (This directory would be expected to be
> somewhere within a work tree, of course.) Then we could apply a patch
> that was created from, say, a '-p sys' work tree, as follows:
>   cd /usr/src/sys
>   got patch < patchfile

yes, in the end it's probably the best behaviour.

I'll need to rebase a couple of diff that i have queued and then i'll
look at that.

> Great, looks good to me!
> 
> Some man page nits, with one behavioural change suggestion below:

thanks :)

> Missing means status !, correct?  I think 'got patch' should block an
> addition in this case. A missing file generally means that something
> went wrong (e.g. someone ran 'mv foo bar' but forgot to use 'got rm foo'
> and 'got add bar'). And the next 'got update' would usually try to restore
> any missing files, which after patching would no longer be missing, they
> would appear to be in modified status instead of added status after patching.
> 
> If you agree, please make this change as a follow-up in a later commit,
> or roll it into the current diff if you prefer.

I thought that if a patch wants to add a file that's already present,
the user could `rm file' and then apply the diff, turning a conflict
into an edit.  it's probably too convoluted and the user could just as
well commit the deletion of the file before applying the diff, and maybe
do an histedit after.  I've dropped this part.

Here's the full sequence of commit with the due corrections :)

-----------------------------------------------
commit 10ede3ed01bd27f334e0ef37327e17ac08fb4fb4
from: Omar Polo <op@omarpolo.com>
date: Sun Mar 13 13:43:00 2022 UTC
 
 check file status before applying the patch
 
 Don't allow `got patch' to delete file that are not known, or add files
 that are already known (but as an exception allow to re-add missing
 files) and only change file that are known and without conflicts or
 obstructions.
 
diff c0c2b3d1e2c80fe9ee36670cff033f948cca9fd6 692e6aaf0ae9d8ee0902b935bfc49a9c19ff1795
blob - 09c52c1e9bd95ceb4f5fa1f135b3fb570b7285b0
blob + 4bcea52af617043cda2fa50d87f21effce198199
--- got/got.c
+++ got/got.c
@@ -7245,7 +7245,7 @@ cmd_patch(int argc, char *argv[])
 #endif
 
 	error = got_patch(patchfd, worktree, repo, &print_remove_status,
-	    NULL, &add_progress, NULL);
+	    NULL, &add_progress, NULL, check_cancelled, NULL);
 
 done:
 	if (repo) {
blob - 04a23fc7f7da83078d23e8ec24820ce98aa43702
blob + 448bb17e65d75d8d5ba94bd577f5cde4fef566bf
--- include/got_patch.h
+++ include/got_patch.h
@@ -22,4 +22,5 @@
  */
 const struct got_error *
 got_patch(int, struct got_worktree *, struct got_repository *,
-    got_worktree_delete_cb, void *, got_worktree_checkout_cb, void *);
+    got_worktree_delete_cb, void *, got_worktree_checkout_cb, void *,
+    got_cancel_cb, void *);
blob - 8dc632ef6f52371e65674a21a73f8c9d2496afd5
blob + 478ade488151adafcb6e73d746f9a7ba2e84acdf
--- lib/patch.c
+++ lib/patch.c
@@ -28,6 +28,7 @@
 #include <sys/socket.h>
 #include <sys/uio.h>
 
+#include <errno.h>
 #include <limits.h>
 #include <sha1.h>
 #include <stdint.h>
@@ -381,44 +382,6 @@ apply_hunk(FILE *tmp, struct got_patch_hunk *h, long *
 }
 
 static const struct got_error *
-schedule_add(const char *path, struct got_worktree *worktree,
-    struct got_repository *repo, got_worktree_checkout_cb add_cb,
-    void *add_arg)
-{
-	static const struct got_error *err = NULL;
-	struct got_pathlist_head paths;
-	struct got_pathlist_entry *pe;
-
-	TAILQ_INIT(&paths);
-
-	err = got_pathlist_insert(&pe, &paths, path, NULL);
-	if (err == NULL)
-		err = got_worktree_schedule_add(worktree, &paths,
-		    add_cb, add_arg, repo, 1);
-	got_pathlist_free(&paths);
-	return err;
-}
-
-static const struct got_error *
-schedule_del(const char *path, struct got_worktree *worktree,
-    struct got_repository *repo, got_worktree_delete_cb delete_cb,
-    void *delete_arg)
-{
-	static const struct got_error *err = NULL;
-	struct got_pathlist_head paths;
-	struct got_pathlist_entry *pe;
-
-	TAILQ_INIT(&paths);
-
-	err = got_pathlist_insert(&pe, &paths, path, NULL);
-	if (err == NULL)
-		err = got_worktree_schedule_delete(worktree, &paths,
-		    0, NULL, delete_cb, delete_arg, repo, 0, 0);
-	got_pathlist_free(&paths);
-	return err;
-}
-
-static const struct got_error *
 patch_file(struct got_patch *p, const char *path, FILE *tmp)
 {
 	const struct got_error *err = NULL;
@@ -503,33 +466,131 @@ done:
 }
 
 static const struct got_error *
+build_pathlist(const char *p, char **path, struct got_pathlist_head *head,
+    struct got_worktree *worktree)
+{
+	const struct got_error *err;
+	struct got_pathlist_entry *pe;
+
+	err = got_worktree_resolve_path(path, worktree, p);
+	if (err == NULL)
+		err = got_pathlist_insert(&pe, head, *path, NULL);
+	return err;
+}
+
+static const struct got_error *
+can_rm(void *arg, unsigned char status, unsigned char staged_status,
+    const char *path, struct got_object_id *blob_id,
+    struct got_object_id *staged_blob_id, struct got_object_id *commit_id,
+    int dirfd, const char *de_name)
+{
+	if (status == GOT_STATUS_NONEXISTENT)
+		return got_error_set_errno(ENOENT, path);
+	if (status != GOT_STATUS_NO_CHANGE &&
+	    status != GOT_STATUS_ADD &&
+	    status != GOT_STATUS_MODIFY &&
+	    status != GOT_STATUS_MODE_CHANGE)
+		return got_error_path(path, GOT_ERR_FILE_STATUS);
+	if (staged_status == GOT_STATUS_DELETE)
+		return got_error_path(path, GOT_ERR_FILE_STATUS);
+	return NULL;
+}
+
+static const struct got_error *
+can_add(void *arg, unsigned char status, unsigned char staged_status,
+    const char *path, struct got_object_id *blob_id,
+    struct got_object_id *staged_blob_id, struct got_object_id *commit_id,
+    int dirfd, const char *de_name)
+{
+	if (status != GOT_STATUS_NONEXISTENT)
+		return got_error_path(path, GOT_ERR_FILE_STATUS);
+	return NULL;
+}
+
+static const struct got_error *
+can_edit(void *arg, unsigned char status, unsigned char staged_status,
+    const char *path, struct got_object_id *blob_id,
+    struct got_object_id *staged_blob_id, struct got_object_id *commit_id,
+    int dirfd, const char *de_name)
+{
+	if (status == GOT_STATUS_NONEXISTENT)
+		return got_error_set_errno(ENOENT, path);
+	if (status != GOT_STATUS_NO_CHANGE &&
+	    status != GOT_STATUS_ADD &&
+	    status != GOT_STATUS_MODIFY)
+		return got_error_path(path, GOT_ERR_FILE_STATUS);
+	if (staged_status == GOT_STATUS_DELETE)
+		return got_error_path(path, GOT_ERR_FILE_STATUS);
+	return NULL;
+}
+
+static const struct got_error *
+check_file_status(struct got_patch *p, int file_renamed,
+    struct got_worktree *worktree, struct got_repository *repo,
+    struct got_pathlist_head *old, struct got_pathlist_head *new,
+    got_cancel_cb cancel_cb, void *cancel_arg)
+{
+	static const struct got_error *err;
+
+	if (p->old != NULL && p->new == NULL)
+		return got_worktree_status(worktree, old, repo, 0,
+		    can_rm, NULL, cancel_cb, cancel_arg);
+	else if (file_renamed) {
+		err = got_worktree_status(worktree, old, repo, 0,
+		    can_rm, NULL, cancel_cb, cancel_arg);
+		if (err)
+			return err;
+		return got_worktree_status(worktree, new, repo, 0,
+		    can_add, NULL, cancel_cb, cancel_arg);
+	} else if (p->old == NULL)
+		return got_worktree_status(worktree, new, repo, 0,
+		    can_add, NULL, cancel_cb, cancel_arg);
+	else
+		return got_worktree_status(worktree, new, repo, 0,
+		    can_edit, NULL, cancel_cb, cancel_arg);
+}
+
+static const struct got_error *
 apply_patch(struct got_worktree *worktree, struct got_repository *repo,
     struct got_patch *p, got_worktree_delete_cb delete_cb, void *delete_arg,
-    got_worktree_checkout_cb add_cb, void *add_arg)
+    got_worktree_checkout_cb add_cb, void *add_arg, 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;
 	char *tmppath = NULL, *template = NULL;
 	FILE *tmp = NULL;
 
-	err = got_worktree_resolve_path(&oldpath, worktree,
-	    p->old != NULL ? p->old : p->new);
+	TAILQ_INIT(&oldpaths);
+	TAILQ_INIT(&newpaths);
+
+	err = build_pathlist(p->old != NULL ? p->old : p->new, &oldpath,
+	    &oldpaths, worktree);
 	if (err)
 		goto done;
 
-	err = got_worktree_resolve_path(&newpath, worktree,
-	    p->new != NULL ? p->new : p->old);
+	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 (p->old != NULL && p->new == NULL) {
 		/*
 		 * special case: delete a file.  don't try to match
 		 * the lines but just schedule the removal.
 		 */
-		err = schedule_del(p->old, worktree, repo, delete_cb,
-		    delete_arg);
+		err = got_worktree_schedule_delete(worktree, &oldpaths,
+		    0, NULL, delete_cb, delete_arg, repo, 0, 0);
 		goto done;
 	}
 
@@ -551,26 +612,27 @@ apply_patch(struct got_worktree *worktree, struct got_
 		goto done;
 	}
 
-	file_renamed = p->old != NULL && strcmp(p->old, p->new);
 	if (file_renamed) {
-		err = schedule_del(oldpath, worktree, repo, delete_cb,
-		    delete_arg);
+		err = got_worktree_schedule_delete(worktree, &oldpaths,
+		    0, NULL, delete_cb, delete_arg, repo, 0, 0);
 		if (err == NULL)
-			err = schedule_add(newpath, worktree, repo,
-			    add_cb, add_arg);
+			err = got_worktree_schedule_add(worktree, &newpaths,
+			    add_cb, add_arg, repo, 1);
 	} else if (p->old == NULL)
-		err = schedule_add(newpath, worktree, repo, add_cb,
-		    add_arg);
+		err = got_worktree_schedule_add(worktree, &newpaths,
+		    add_cb, add_arg, repo, 1);
 	else
 		printf("M  %s\n", oldpath); /* XXX */
 
 done:
-	if (err != NULL && (file_renamed || p->old == NULL))
+	if (err != NULL && newpath != NULL && (file_renamed || p->old == NULL))
 		unlink(newpath);
 	free(template);
 	if (tmppath != NULL)
 		unlink(tmppath);
 	free(tmppath);
+	got_pathlist_free(&oldpaths);
+	got_pathlist_free(&newpaths);
 	free(oldpath);
 	free(newpath);
 	return err;
@@ -579,7 +641,8 @@ done:
 const struct got_error *
 got_patch(int fd, struct got_worktree *worktree, struct got_repository *repo,
     got_worktree_delete_cb delete_cb, void *delete_arg,
-    got_worktree_checkout_cb add_cb, void *add_arg)
+    got_worktree_checkout_cb add_cb, void *add_arg, got_cancel_cb cancel_cb,
+    void *cancel_arg)
 {
 	const struct got_error *err = NULL;
 	struct imsgbuf *ibuf;
@@ -628,7 +691,7 @@ got_patch(int fd, struct got_worktree *worktree, struc
 			break;
 
 		err = apply_patch(worktree, repo, &p, delete_cb, delete_arg,
-		    add_cb, add_arg);
+		    add_cb, add_arg, cancel_cb, cancel_arg);
 		patch_free(&p);
 		if (err)
 			break;
blob - 5916921f260f5a0eaefdbf6036b813fbf3570dd5
blob + 36c9ef55d45f97cd15c3f787c35d5a7623def72a
--- regress/cmdline/patch.sh
+++ regress/cmdline/patch.sh
@@ -685,6 +685,7 @@ EOF
 		test_done $testroot $ret
 		return 1
 	fi
+	rm $testroot/wt/eta
 
 	cat <<EOF > $testroot/wt/patch
 --- alpha
@@ -730,6 +731,151 @@ EOF
 	test_done $testroot $ret
 }
 
+test_patch_illegal_status() {
+	local testroot=`test_init patch_illegal_status`
+
+	got checkout $testroot/repo $testroot/wt > /dev/null
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done $testroot $ret
+		return 1
+	fi
+
+	# edit an non-existent and unknown file
+	cat <<EOF > $testroot/wt/patch
+--- iota
++++ iota
+@@ -1 +1 @@
+- iota
++ IOTA
+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
+	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 \
+		2> $testroot/stderr
+	ret=$?
+	if [ $ret -eq 0 ]; then
+		echo "edited a missing file" >&2
+		test_done $testroot $ret
+		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
+
+	# delete an unknown file
+	cat <<EOF > $testroot/wt/patch
+--- iota
++++ /dev/null
+@@ -1 +0,0 @@
+-iota
+EOF
+
+	(cd $testroot/wt && got patch patch) > /dev/null \
+		2> $testroot/stderr
+	ret=$?
+	if [ $ret -eq 0 ]; then
+		echo "deleted 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 -eq 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
+}
+
 test_parseargs "$@"
 run_test test_patch_simple_add_file
 run_test test_patch_simple_rm_file
@@ -743,3 +889,4 @@ run_test test_patch_malformed
 run_test test_patch_no_patch
 run_test test_patch_equals_for_context
 run_test test_patch_rename
+run_test test_patch_illegal_status

-----------------------------------------------
commit 553293f1d33ebcf74c15d93a115883227327d187
from: Omar Polo <op@omarpolo.com>
date: Sun Mar 13 13:43:34 2022 UTC
 
 improve `got patch' section of the manpage
 
 Simplify some prasing, explain what preconditions `got patch' has and
 what happens to the work tree when an error occurs.
 
diff 692e6aaf0ae9d8ee0902b935bfc49a9c19ff1795 9d90d0d808ca3019fc2e5ec61a47171c395f56b5
blob - 61596d1cfc534598b2516ffa737754cfb667abb1
blob + 191ae2e5a9cee80ff3902ee897f67501ec3aad5e
--- got/got.1
+++ got/got.1
@@ -889,7 +889,6 @@ arguments are provided, only show differences for the 
 Cannot be used together with the
 .Fl P
 option.
-.Pp
 .It Fl C Ar number
 Set the number of context lines shown in the diff.
 By default, 3 lines of context are shown.
@@ -1291,18 +1290,19 @@ option)
 Apply changes from
 .Ar patchfile
 .Pq or standard input
-and record the state of the affected files afterwards.
-The content of
-.Ar patchfile
-must be an unified diff.
+that must be a unified diff.
 If
 .Ar patchfile
 contains more than one patch,
 .Nm
 .Cm patch
-will try to apply them all.
+will try to apply each of them in sequence.
+Files added or removed by the patch scheduled for addition or removal in
+the work tree.
 .Pp
-Show the status of each affected file, using the following status codes:
+While applying the
+.Ar patchfile ,
+show the status of each affected file, using the following status codes:
 .Bl -column XYZ description
 .It M Ta modified file
 .It D Ta deleted file
@@ -1314,6 +1314,25 @@ If a change does not match at its exact line number,
 .Cm patch
 applies it somewhere else in the file if it can find a good spot before
 giving up.
+.Pp
+.Nm
+.Cm patch
+will refuse to apply a patch if certain preconditions are not met.
+Files to be deleted must be present in the work tree and not have been
+scheduled for deletion already.
+Files to be added must be unknown and not present in the work tree.
+Files to be modified must be already present in the the work tree and
+not have conflicts or being obstructed.
+.Pp
+If an error occurs, the
+.Cm patch
+operation will be aborted.
+Any changes made to the work tree up to this point will be left behind.
+Such cahnges can be viewed with
+.Cm got diff
+and can be reverted with
+.Cm got revert
+if needed.
 .Tg rv
 .It Cm revert Oo Fl p Oc Oo Fl F Ar response-script Oc Oo Fl R Oc Ar path ...
 .Dl Pq alias: Cm rv

-----------------------------------------------
commit e554a12d8da3c5d7126f6eefb2f2c6319b2791d6 (patchcfs)
from: Omar Polo <op@omarpolo.com>
date: Sun Mar 13 13:43:34 2022 UTC
 
 got patch: require exact match when removing files
 
diff 9d90d0d808ca3019fc2e5ec61a47171c395f56b5 afa8df52030f9a8974254a77401b64d5733c7856
blob - 478ade488151adafcb6e73d746f9a7ba2e84acdf
blob + de9aba4bdf13e2f9f6948b69c132006c43aeb02a
--- lib/patch.c
+++ lib/patch.c
@@ -26,6 +26,7 @@
 #include <sys/types.h>
 #include <sys/queue.h>
 #include <sys/socket.h>
+#include <sys/stat.h>
 #include <sys/uio.h>
 
 #include <errno.h>
@@ -323,7 +324,7 @@ locate_hunk(FILE *orig, struct got_patch_hunk *h, off_
 }
 
 static const struct got_error *
-test_hunk(FILE *orig, struct got_patch_hunk *h)
+test_hunk(FILE *orig, struct got_patch_hunk *h, int fuzzy)
 {
 	const struct got_error *err = NULL;
 	char *line = NULL;
@@ -345,7 +346,7 @@ test_hunk(FILE *orig, struct got_patch_hunk *h)
 					    GOT_ERR_PATCH_DONT_APPLY);
 				goto done;
 			}
-			if (strcmp(h->lines[i]+1, line)) {
+			if (strcmp(h->lines[i]+1, line) && --fuzzy < 0) {
 				err = got_error(GOT_ERR_PATCH_DONT_APPLY);
 				goto done;
 			}
@@ -382,7 +383,7 @@ apply_hunk(FILE *tmp, struct got_patch_hunk *h, long *
 }
 
 static const struct got_error *
-patch_file(struct got_patch *p, const char *path, FILE *tmp)
+patch_file(struct got_patch *p, const char *path, FILE *tmp, int fuzzy)
 {
 	const struct got_error *err = NULL;
 	struct got_patch_hunk *h;
@@ -424,7 +425,7 @@ patch_file(struct got_patch *p, const char *path, FILE
 			goto done;
 		copypos = pos;
 
-		err = test_hunk(orig, h);
+		err = test_hunk(orig, h, fuzzy);
 		if (err != NULL && err->code == GOT_ERR_PATCH_DONT_APPLY) {
 			/*
 			 * try to apply the hunk again starting the search
@@ -456,7 +457,15 @@ patch_file(struct got_patch *p, const char *path, FILE
 		}
 	}
 
-	if (!feof(orig))
+	
+	if (p->new == NULL) {
+		struct stat sb;
+
+		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 (!feof(orig))
 		err = copy(tmp, orig, copypos, -1);
 
 done:
@@ -558,7 +567,7 @@ apply_patch(struct got_worktree *worktree, struct got_
 {
 	const struct got_error *err = NULL;
 	struct got_pathlist_head oldpaths, newpaths;
-	int file_renamed = 0;
+	int file_renamed = 0, fuzzy = 0;
 	char *oldpath = NULL, *newpath = NULL;
 	char *tmppath = NULL, *template = NULL;
 	FILE *tmp = NULL;
@@ -584,16 +593,6 @@ apply_patch(struct got_worktree *worktree, struct got_
 	if (err)
 		goto done;
 
-	if (p->old != NULL && p->new == NULL) {
-		/*
-		 * special case: delete a file.  don't try to match
-		 * the lines but just schedule the removal.
-		 */
-		err = got_worktree_schedule_delete(worktree, &oldpaths,
-		    0, NULL, delete_cb, delete_arg, repo, 0, 0);
-		goto done;
-	}
-
 	if (asprintf(&template, "%s/got-patch",
 	    got_worktree_get_root_path(worktree)) == -1) {
 		err = got_error_from_errno(template);
@@ -603,10 +602,18 @@ apply_patch(struct got_worktree *worktree, struct got_
 	err = got_opentemp_named(&tmppath, &tmp, template);
 	if (err)
 		goto done;
-	err = patch_file(p, oldpath, tmp);
+	if (p->old != NULL && p->new == NULL)
+		fuzzy = 2;
+	err = patch_file(p, oldpath, tmp, fuzzy);
 	if (err)
 		goto done;
 
+	if (p->old != NULL && p->new == NULL) {
+		err = got_worktree_schedule_delete(worktree, &oldpaths,
+		    0, NULL, delete_cb, delete_arg, repo, 0, 0);
+		goto done;
+	}
+
 	if (rename(tmppath, newpath) == -1) {
 		err = got_error_from_errno3("rename", tmppath, newpath);
 		goto done;
blob - 36c9ef55d45f97cd15c3f787c35d5a7623def72a
blob + 9a300a816a79b7eacef07263e7bd7cbce706ae25
--- regress/cmdline/patch.sh
+++ regress/cmdline/patch.sh
@@ -439,6 +439,44 @@ EOF
 		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
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done $testroot $ret
+		return 1
+	fi
+
+	cat <<EOF > $testroot/wt/patch
+--- numbers
++++ /dev/null
+@@ -1,9 +0,0 @@
+-1
+-2
+-3
+-4
+-5
+-6
+-7
+-8
+-9
+EOF
+
+	(cd $testroot/wt && got patch patch) > /dev/null 2> $testroot/stderr
+	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
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stderr.expected $testroot/stderr
+	fi
 	test_done $testroot $ret
 }