While testing a diff for an unrelated thing I noticed that `got patch'
fails to handle the case of a patch that tries to add a file already
versioned.

Long story short, I'm using got_worktree_status to check the status of a
file before applying the patch.  got_worktree_status calls
worktree_status with report_unchanged=0, so the `can_add' callback in
lib/patch.c doesn't get called and check_file_status assumes that it's
possible to add the file.

Patch below adds another argument to got_worktree_status so the caller
can decide to have also unchanged files reported to the callback.  If we
don't want to change got_worktree_status I have another idea to solve
this issue locally in patch.c, but I thought this was the easiest way.

---

Why the regression suite didn't found this earlier?  There's a test for
it, test_patch_illegal_status, which seems to be OK, fails as expected
with 'beta: file has unexpected status', but for the wrong reason :)

Since check_file_status returns NULL, patch_file is happy to create the
file but then got_worktree_schedule_add fails because the file is
already present in the repository.  What's worse is that at the end of
apply_patch I call unlink(newpath) in a tentative to roll back the
creation of a file when aborting the operation which removes the file
from the worktree.  The patch below fixes this too by moving the unlink
call up a bit so the file is deleted only when got_worktree_schedule_*
fails.  (i guess that we can also completely drop the unlink call, was
there only to avoid leaving an extraneous file in the worktree if their
scheduling failed.)

---

diff 46ebad135d9dc52eb84d6985393298465fa7b3ff /home/op/w/got
blob - fb5211d500574e714e2964dd6a9ab1ac8433e57d
file + got/got.c
--- got/got.c
+++ got/got.c
@@ -4690,7 +4690,7 @@ cmd_diff(int argc, char *argv[])
 		arg.ignore_whitespace = ignore_whitespace;
 		arg.force_text_diff = force_text_diff;
 
-		error = got_worktree_status(worktree, &paths, repo, 0,
+		error = got_worktree_status(worktree, &paths, repo, 0, 0,
 		    print_diff, &arg, check_cancelled, NULL);
 		free(id_str);
 		goto done;
@@ -5619,7 +5619,7 @@ cmd_status(int argc, char *argv[])
 	if (error)
 		goto done;
 
-	error = got_worktree_status(worktree, &paths, repo, no_ignores,
+	error = got_worktree_status(worktree, &paths, repo, no_ignores, 0,
 	    print_status, &st, check_cancelled, NULL);
 done:
 	TAILQ_FOREACH(pe, &paths, entry)
@@ -10816,8 +10816,8 @@ cmd_histedit(int argc, char *argv[])
 				if (error)
 					goto done;
 				error = got_worktree_status(worktree, &paths,
-				    repo, 0, check_local_changes, &have_changes,
-				    check_cancelled, NULL);
+				    repo, 0, 0, check_local_changes,
+				    &have_changes, check_cancelled, NULL);
 				got_pathlist_free(&paths);
 				if (error) {
 					if (error->code != GOT_ERR_CANCELLED)
@@ -11495,7 +11495,7 @@ cmd_stage(int argc, char *argv[])
 		goto done;
 
 	if (list_stage)
-		error = got_worktree_status(worktree, &paths, repo, 0,
+		error = got_worktree_status(worktree, &paths, repo, 0, 0,
 		    print_stage, NULL, check_cancelled, NULL);
 	else {
 		cpa.patch_script_file = patch_script_file;
blob - 2a33834a903f96a6f0206be4636193177a5f443d
file + include/got_worktree.h
--- include/got_worktree.h
+++ include/got_worktree.h
@@ -171,7 +171,7 @@ typedef const struct got_error *(*got_worktree_status_
  * a path, and a corresponding status code.
  */
 const struct got_error *got_worktree_status(struct got_worktree *,
-    struct got_pathlist_head *, struct got_repository *, int no_ignores,
+    struct got_pathlist_head *, struct got_repository *, int, int,
     got_worktree_status_cb, void *, got_cancel_cb cancel_cb, void *);
 
 /*
blob - 4aabb46d0d6d31262d423862cce84ad3cf0b11f6
file + lib/patch.c
--- lib/patch.c
+++ lib/patch.c
@@ -564,20 +564,20 @@ check_file_status(struct got_patch *p, int file_rename
 	static const struct got_error *err;
 
 	if (p->old != NULL && p->new == NULL)
-		return got_worktree_status(worktree, old, repo, 0,
+		return got_worktree_status(worktree, old, repo, 0, 1,
 		    can_rm, NULL, cancel_cb, cancel_arg);
 	else if (file_renamed) {
-		err = got_worktree_status(worktree, old, repo, 0,
+		err = got_worktree_status(worktree, old, repo, 0, 1,
 		    can_rm, NULL, cancel_cb, cancel_arg);
 		if (err)
 			return err;
-		return got_worktree_status(worktree, new, repo, 0,
+		return got_worktree_status(worktree, new, repo, 0, 1,
 		    can_add, NULL, cancel_cb, cancel_arg);
 	} else if (p->old == NULL)
-		return got_worktree_status(worktree, new, repo, 0,
+		return got_worktree_status(worktree, new, repo, 0, 1,
 		    can_add, NULL, cancel_cb, cancel_arg);
 	else
-		return got_worktree_status(worktree, new, repo, 0,
+		return got_worktree_status(worktree, new, repo, 0, 1,
 		    can_edit, NULL, cancel_cb, cancel_arg);
 }
 
@@ -693,16 +693,18 @@ apply_patch(struct got_worktree *worktree, struct got_
 		if (err == NULL)
 			err = got_worktree_schedule_add(worktree, newpaths,
 			    patch_add, pa, repo, 1);
-	} else if (p->old == NULL)
+		if (err)
+			unlink(newpath);
+	} else if (p->old == NULL) {
 		err = got_worktree_schedule_add(worktree, newpaths,
 		    patch_add, pa, repo, 1);
-	else
+		if (err)
+			unlink(newpath);
+	} else
 		err = report_progress(pa, oldpath, newpath, GOT_STATUS_MODIFY,
 		    NULL);
 
 done:
-	if (err != NULL && newpath != NULL && (file_renamed || p->old == NULL))
-		unlink(newpath);
 	free(parent);
 	free(template);
 	if (tmppath != NULL)
blob - 26ce9d0c775728d7d4b6a3450c3704c51f29672a
file + lib/worktree.c
--- lib/worktree.c
+++ lib/worktree.c
@@ -3714,8 +3714,8 @@ done:
 const struct got_error *
 got_worktree_status(struct got_worktree *worktree,
     struct got_pathlist_head *paths, struct got_repository *repo,
-    int no_ignores, got_worktree_status_cb status_cb, void *status_arg,
-    got_cancel_cb cancel_cb, void *cancel_arg)
+    int no_ignores, int report_unchanged, got_worktree_status_cb status_cb,
+    void *status_arg, got_cancel_cb cancel_cb, void *cancel_arg)
 {
 	const struct got_error *err = NULL;
 	char *fileindex_path = NULL;
@@ -3729,7 +3729,7 @@ got_worktree_status(struct got_worktree *worktree,
 	TAILQ_FOREACH(pe, paths, entry) {
 		err = worktree_status(worktree, pe->path, fileindex, repo,
 			status_cb, status_arg, cancel_cb, cancel_arg,
-			no_ignores, 0);
+			no_ignores, report_unchanged);
 		if (err)
 			break;
 	}
blob - 65fb8dead18ae13233a8eb24e2f1f26a41d8906e
file + regress/cmdline/patch.sh
--- regress/cmdline/patch.sh
+++ regress/cmdline/patch.sh
@@ -824,7 +824,22 @@ EOF
 	ret=$?
 	if [ $ret -ne 0 ]; then
 		diff -u $testroot/stderr.expected $testroot/stderr
+		test_done $testroot $ret
+		return 1
 	fi
+
+	(cd $testroot/wt && got status) > $testroot/stdout
+	cat <<EOF > $testroot/stdout.expected
+~  alpha
+?  kappa
+?  patch
+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
 }