From: Omar Polo Subject: check file status before applying a patch To: gameoftrees@openbsd.org Date: Tue, 08 Mar 2022 17:43:52 +0100 Hello, Currently `got patch' is happy to patch files not tracked by got, delete non existant files or "add" (overwriting) files that are already known. Diff below fixes it by checking the status of the file first: for additions the file must be unknown to got, while for removals and modifications are allowed only to files that are already being tracked. I'm still allowing to patch modified or added files. The new test_patch_unknown_files tries all these behaviours. Well, `got patch' bails out as soon as it see the first hunk (edits to untracked file `numbers'), but I have a follow-up diff that allows got_patch to recover from some errors and continue the patching so that test will soon be more useful :) I'm not 100% sure of the got_worktree_status usage. ----------------------------------------------- commit 5bdc399f7a45fb402fc07d56e4fd56b6f28a91fc (main) from: Omar Polo date: Tue Mar 8 16:04:31 2022 UTC check file status before trying to apply the diff i.e. don't delete non-existant files, add file that are already added or modify files that are not present in the repository, regardless of their status on the disk. diff a23dc9d0142fe7aaf6077e9865edb6dec334229e 5610bf8ff3a9bbddee69a9e572a03a47bddad694 blob - 5939e9366900091eb56fe2aa3cdbe21f4f7a63eb blob + aa60416f3adc4b4b2080c13808d0aa115c3a3d4e --- got/got.c +++ got/got.c @@ -7220,7 +7220,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 - 9bfe37463cbe64005617a3395ea55e551360f8d1 blob + 67957f3fc9583d84826de4bcce0555a56c5abc5e --- lib/patch.c +++ lib/patch.c @@ -376,13 +376,28 @@ apply_hunk(FILE *tmp, struct got_patch_hunk *h, long * } static const struct got_error * +check_status(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) +{ + int *u = arg; + + *u = status == GOT_STATUS_UNVERSIONED || + status == GOT_STATUS_NONEXISTENT; + return NULL; +} + +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 paths; struct got_pathlist_entry *pe; + int unknown = 0; char *path = NULL, *tmppath = NULL, *template = NULL; FILE *orig = NULL, *tmp = NULL; struct got_patch_hunk *h; @@ -406,19 +421,37 @@ apply_patch(struct got_worktree *worktree, struct got_ if (err) goto done; + err = got_worktree_status(worktree, &paths, repo, 0, check_status, + &unknown, 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. + * the lines but just schedule the removal; the file + * must exists and be known thought. */ - err = got_worktree_schedule_delete(worktree, &paths, - 0, NULL, delete_cb, delete_arg, repo, 0, 0); + if (unknown) + err = got_error_path(path, GOT_ERR_FILE_STATUS); + else + err = got_worktree_schedule_delete(worktree, &paths, + 0, NULL, delete_cb, delete_arg, repo, 0, 0); goto done; } else if (p->old != NULL && strcmp(p->old, p->new)) { err = got_error(GOT_ERR_PATCH_PATHS_DIFFER); goto done; } + /* can't add known files or modify unknown ones */ + if (p->old == NULL && !unknown) { + err = got_error_path(path, GOT_ERR_FILE_STATUS); + goto done; + } else if (p->old != NULL && p->new != NULL && unknown) { + err = got_error_path(path, GOT_ERR_FILE_STATUS); + goto done; + } + if (asprintf(&template, "%s/got-patch", got_worktree_get_root_path(worktree)) == -1) { err = got_error_from_errno(template); @@ -536,7 +569,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; @@ -585,7 +619,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 - 7a5ad3671fa62d02f74a21cc4f0edf7be252462a blob + 38371c263785ae7d9f385c58367f34bd5670b24f --- regress/cmdline/patch.sh +++ regress/cmdline/patch.sh @@ -624,6 +624,62 @@ EOF test_done $testroot $ret } +test_patch_unknown_files() { + local testroot=`test_init patch_edit_unknown_file` + + got checkout $testroot/repo $testroot/wt > /dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done $testroot $ret + return 1 + fi + + # try to edit an unknown file, add an already added one and remove + # one that doesn't exists. + cat < $testroot/wt/patch +--- numbers ++++ numbers +@@ -3,7 +3,7 @@ + 3 + 4 + 5 +-6 ++foo + 7 + 8 + 9 +--- /dev/null ++++ alpha +@@ -0,0 +1 @@ ++alpha +--- zeta ++++ /dev/null +@@ -1 +0,0 @@ +- zeta +EOF + + jot 10 > $testroot/wt/numbers + + echo "got: numbers: file has unexpected status" \ + > $testroot/stderr.expected + + (cd $testroot/wt && got patch patch) >/dev/null \ + 2> $testroot/stderr + ret=$? + if [ $ret -eq 0 ]; then + echo "applied a patch for an untracked file?" >&2 + 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 + fi + test_done $testroot $ret +} + test_parseargs "$@" run_test test_patch_simple_add_file run_test test_patch_simple_rm_file @@ -636,3 +692,4 @@ run_test test_patch_dont_apply run_test test_patch_malformed run_test test_patch_no_patch run_test test_patch_equals_for_context +run_test test_patch_unknown_files