From: Omar Polo Subject: got patch: resolve paths from $PWD To: gameoftrees@openbsd.org Date: Tue, 19 Apr 2022 22:40:11 +0200 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 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 < $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