hello,

as reminded by naddy@, 'got patch' resolve paths from the worktree root
and not from the current working directory.

let's fix it.

The attached diff changes `patch_check_path' to build the paths from the
current directory.  I'm also moving the pathlist handling from got_patch
to patch_check_path too, it makes more sense to fix the pathlist where
we're also doing the other path operations.

The only bit I don't really like is the changes at the end of
apply_patch.  I want to call report_progress with paths that are
absolute from the worktree root, and that's what happens when
report_progress is called via patch_add/delete (the callbacks for
got_worktree_schedule_add/delete.)  But for the "edited file" case I
have to peek into the tailq.  The `newpath' and `oldpath' paths are
relative to $PWD, so I can't use them.  It's not incredibly bad, I know
for sure that the pathlists have one and exactly one element in them,
but it doesn't read too well IMHO.  (but neither too bad maybe?)

With this I can successfully apply the "UPDATE: xterm 372" diff from
matthieu@:

	% cd app/xterm
	% mshow -r | qprint -d | got patch
	M  app/xterm/COPYING
	M  app/xterm/MANIFEST
	M  app/xterm/NEWS
	M  app/xterm/THANKS
	M  app/xterm/Tekproc.c
	M  app/xterm/VTPrsTbl.c
	...

Comments/ideas/oks?

-----------------------------------------------
commit 7424c48ea9bfe7d88cdb8c504dac9c776eacff20 (ppath)
from: Omar Polo <op@omarpolo.com>
date: Tue Apr 19 20:39:33 2022 UTC
 
 got patch: resolve paths from $PWD, not from worktree root
 
diff 765bfda82dee49bd1575eb87e64920ab3d5c3b16 ad7ade67de2cab1a5f118c6eed45186cb5b216f9
blob - 83c0f25c125f10f982a164a3a028f7554654e8c4
blob + bbacb527638afbfca56a221f23576afe6ea08de8
--- include/got_worktree.h
+++ include/got_worktree.h
@@ -556,7 +556,8 @@ got_worktree_patch_prepare(struct got_fileindex **, st
  * status.
  */
 const struct got_error *
-got_worktree_patch_check_path(const char *, const char *, char **, char **,
+got_worktree_patch_check_path(char **, char **, const char *, const char *,
+    const char *, struct got_pathlist_head *, struct got_pathlist_head *,
     struct got_worktree *, struct got_repository *, struct got_fileindex *);
 
 /* Complete the patch operation. */
blob - 9b49b4b1349c2d2994dfa6e667c9265975aea642
blob + 0de7a91f2a691f43c4298ed126ca334e09e685d6
--- lib/patch.c
+++ lib/patch.c
@@ -565,27 +565,17 @@ patch_add(void *arg, unsigned char status, const char 
 
 static const struct got_error *
 apply_patch(struct got_worktree *worktree, struct got_repository *repo,
-    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 char *oldpath, const char *newpath,
+    struct got_pathlist_head *newpaths, struct got_pathlist_head *oldpaths,
+    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;
-	struct got_pathlist_entry *pe;
 	int file_renamed = 0;
 	char *tmppath = NULL, *template = NULL, *parent = NULL;;
 	FILE *tmp = NULL;
 	mode_t mode = GOT_DEFAULT_FILE_MODE;
 
-	TAILQ_INIT(&oldpaths);
-	TAILQ_INIT(&newpaths);
-
-	err = got_pathlist_insert(&pe, &oldpaths, oldpath, NULL);
-	if (err)
-		goto done;
-	err = got_pathlist_insert(&pe, &newpaths, newpath, NULL);
-	if (err)
-		goto done;
-
 	file_renamed = strcmp(oldpath, newpath);
 
 	if (asprintf(&template, "%s/got-patch",
@@ -606,7 +596,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;
 	}
@@ -637,25 +627,23 @@ 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);
 		if (err)
 			unlink(newpath);
 	} else if (p->old == NULL) {
-		err = got_worktree_schedule_add(worktree, &newpaths,
+		err = got_worktree_schedule_add(worktree, newpaths,
 		    patch_add, pa, repo, 1);
 		if (err)
 			unlink(newpath);
 	} else
-		err = report_progress(pa, oldpath, newpath, GOT_STATUS_MODIFY,
-		    NULL);
+		err = report_progress(pa, TAILQ_FIRST(oldpaths)->path,
+		    TAILQ_FIRST(newpaths)->path, GOT_STATUS_MODIFY, NULL);
 
 done:
-	got_pathlist_free(&oldpaths);
-	got_pathlist_free(&newpaths);
 	free(parent);
 	free(template);
 	if (tmppath != NULL)
@@ -676,7 +664,11 @@ got_patch(int fd, struct got_worktree *worktree, struc
 	int imsg_fds[2] = {-1, -1};
 	int done = 0, failed = 0;
 	pid_t pid;
+	char *cwd;
 
+	if ((cwd = getcwd(NULL, 0)) == NULL)
+		return got_error_from_errno("getcwd");
+
 	ibuf = calloc(1, sizeof(*ibuf));
 	if (ibuf == NULL) {
 		err = got_error_from_errno("calloc");
@@ -717,7 +709,11 @@ got_patch(int fd, struct got_worktree *worktree, struc
 	while (!done && err == NULL) {
 		struct got_patch p;
 		struct patch_args pa;
+		struct got_pathlist_head oldpaths, newpaths;
 
+		TAILQ_INIT(&oldpaths);
+		TAILQ_INIT(&newpaths);
+
 		pa.progress_cb = progress_cb;
 		pa.progress_arg = progress_arg;
 		pa.head = &p.head;
@@ -726,11 +722,13 @@ got_patch(int fd, struct got_worktree *worktree, struc
 		if (err || done)
 			break;
 
-		err = got_worktree_patch_check_path(p.old, p.new, &oldpath,
-		    &newpath, worktree, repo, fileindex);
+		err = got_worktree_patch_check_path(&oldpath, &newpath,
+		    p.old, p.new, cwd, &oldpaths, &newpaths, worktree, repo,
+		    fileindex);
 		if (err == NULL)
 			err = apply_patch(worktree, repo, oldpath, newpath,
-			    &p, nop, &pa, cancel_cb, cancel_arg);
+			    &oldpaths, &newpaths, &p, nop, &pa, cancel_cb,
+			    cancel_arg);
 		if (err != NULL) {
 			failed = 1;
 			/* recoverable errors */
@@ -743,6 +741,9 @@ got_patch(int fd, struct got_worktree *worktree, struc
 				    GOT_STATUS_CANNOT_UPDATE, NULL);
 		}
 
+		got_pathlist_free(&oldpaths);
+		got_pathlist_free(&newpaths);
+
 		free(oldpath);
 		free(newpath);
 		patch_free(&p);
blob - bfebb46526cc0f3f0deda90d0868edad7c6946c9
blob + 2bbc28762fcb03fa1237b47f8da719ef5d8d2eb7
--- lib/worktree.c
+++ lib/worktree.c
@@ -8775,44 +8775,73 @@ done:
 }
 
 static const struct got_error *
-patch_check_path(const char *p, char **path, unsigned char *status,
-    unsigned char *staged_status, struct got_fileindex *fileindex,
+patch_check_path(char **resolved_path, unsigned char *status,
+    unsigned char *staged_status, const char *path, const char *cwd,
+    struct got_pathlist_head *head, struct got_fileindex *fileindex,
     struct got_worktree *worktree, struct got_repository *repo)
 {
 	const struct got_error *err;
+	struct got_pathlist_entry *pe;
 	struct got_fileindex_entry *ie;
 	struct stat sb;
-	char *ondisk_path = NULL;
+	char *prfx = NULL, *tmp = NULL, *abs = NULL, *wt_path = NULL;
 
-	err = got_worktree_resolve_path(path, worktree, p);
-	if (err)
+	err = got_path_skip_common_ancestor(&prfx, worktree->root_path, cwd);
+	if (err && err->code != GOT_ERR_BAD_PATH)
 		return err;
+	err = NULL;
 
-	if (asprintf(&ondisk_path, "%s%s%s", worktree->root_path,
-	    *path[0] ? "/" : "", *path) == -1)
-		return got_error_from_errno("asprintf");
+	if (asprintf(&tmp, "%s/%s/%s", worktree->root_path, prfx ? prfx : "",
+	    path) == -1) {
+		err = got_error_from_errno("asprintf");
+		goto done;
+	}
 
-	ie = got_fileindex_entry_get(fileindex, *path, strlen(*path));
+	if ((abs = strdup(tmp)) == NULL) {
+		err = got_error_from_errno("strdup");
+		goto done;
+	}
+
+	err = got_canonpath(tmp, abs, strlen(abs)+1);
+	if (err)
+		goto done;
+
+	err = got_path_skip_common_ancestor(resolved_path, cwd, abs);
+	if (err)
+		goto done;
+
+	err = got_path_skip_common_ancestor(&wt_path, worktree->root_path,
+	    abs);
+	if (err)
+		goto done;
+
+	ie = got_fileindex_entry_get(fileindex, wt_path, strlen(wt_path));
 	if (ie) {
 		*staged_status = get_staged_status(ie);
-		err = get_file_status(status, &sb, ie, *path, -1, NULL, repo);
+		err = get_file_status(status, &sb, ie, abs, -1, NULL,
+		    repo);
 		if (err)
 			goto done;
 	} else {
 		*staged_status = GOT_STATUS_NO_CHANGE;
 		*status = GOT_STATUS_UNVERSIONED;
-		if (lstat(ondisk_path, &sb) == -1) {
+		if (lstat(*resolved_path, &sb) == -1) {
 			if (errno != ENOENT) {
 				err = got_error_from_errno2("lstat",
-				    ondisk_path);
+				    *resolved_path);
 				goto done;
 			}
 			*status = GOT_STATUS_NONEXISTENT;
 		}
 	}
 
+	err = got_pathlist_insert(&pe, head, wt_path, NULL);
 done:
-	free(ondisk_path);
+	if (err)
+		free(wt_path);
+	free(prfx);
+	free(tmp);
+	free(abs);
 	return err;
 }
 
@@ -8868,8 +8897,9 @@ got_worktree_patch_prepare(struct got_fileindex **file
 }
 
 const struct got_error *
-got_worktree_patch_check_path(const char *old, const char *new,
-    char **oldpath, char **newpath, struct got_worktree *worktree,
+got_worktree_patch_check_path(char **oldpath, char **newpath, const char *old,
+    const char *new, const char *cwd, struct got_pathlist_head *newpaths,
+    struct got_pathlist_head *oldpaths, struct got_worktree *worktree,
     struct got_repository *repo, struct got_fileindex *fileindex)
 {
 	const struct got_error *err = NULL;
@@ -8880,13 +8910,13 @@ got_worktree_patch_check_path(const char *old, const c
 	*oldpath = NULL;
 	*newpath = NULL;
 
-	err = patch_check_path(old != NULL ? old : new, oldpath,
-	    &status_old, &staged_status_old, fileindex, worktree, repo);
+	err = patch_check_path(oldpath, &status_old, &staged_status_old,
+	    old != NULL ? old : new, cwd, oldpaths, fileindex, worktree, repo);
 	if (err)
 		goto done;
 
-	err = patch_check_path(new != NULL ? new : old, newpath,
-	    &status_new, &staged_status_new, fileindex, worktree, repo);
+	err = patch_check_path(newpath, &status_new, &staged_status_new,
+	    new != NULL ? new : old, cwd, newpaths, fileindex, worktree, repo);
 	if (err)
 		goto done;
 
blob - b3f10d2dc6e282f6548c695befab125a7f8f680e
blob + af1fd72e33001d46b8f822e0b0a7f00a5b1ad7b7
--- regress/cmdline/patch.sh
+++ regress/cmdline/patch.sh
@@ -1273,6 +1273,46 @@ EOF
 	test_done $testroot 0
 }
 
+test_patch_relative_paths() {
+	local testroot=`test_init patch_orig`
+
+	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/gamma/patch
+--- delta
++++ delta
+@@ -1 +1 @@
+-delta
++DELTA
+--- /dev/null
++++ eta
+@@ -0,0 +1 @@
++eta
+EOF
+
+	(cd $testroot/wt/gamma && got patch patch) > $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done $testroot $ret
+		return 1
+	fi
+
+	echo 'M  gamma/delta' > $testroot/stdout.expected
+	echo 'A  gamma/eta' >> $testroot/stdout.expected
+
+	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
@@ -1294,3 +1334,4 @@ run_test test_patch_with_offset
 run_test test_patch_prefer_new_path
 run_test test_patch_no_newline
 run_test test_patch_strip
+run_test test_patch_relative_paths