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

From:
Omar Polo <op@omarpolo.com>
Subject:
check file status before applying a patch
To:
gameoftrees@openbsd.org
Date:
Tue, 08 Mar 2022 17:43:52 +0100

Download raw body.

Thread
  • Omar Polo:

    check file status before applying a patch

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 <op@omarpolo.com>
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 <<EOF > $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