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

From:
Omar Polo <op@omarpolo.com>
Subject:
got patch: keep file permissions and create missing directories
To:
gameoftrees@openbsd.org
Date:
Wed, 16 Mar 2022 18:55:07 +0100

Download raw body.

Thread
Two great omissions that got patch has is that is does not preserve file
permissions and doesn't create missing directories.

File permissions are not preserved because it creates a temp file that is
then rename(2)d, so we loose all the info.  The following patch solves
this by inspecting the permissions of the file and restoring them just
before the rename.

The second diff handles ENOENT from rename and tries to create the
missing directory tree (got_path_mkdir creates the full directory
structure.)

ok for both?

-----------------------------------------------
commit eefa9219abab5b1fa28a2ce666b67662e37d92df
from: Omar Polo <op@omarpolo.com>
date: Wed Mar 16 17:38:21 2022 UTC
 
 got patch: keep permissions after patching a file
 
diff 535daea402634388631e7318cfd30537e1530cc2 c8a54ed8b33fdd341c3a8fe4fd60fa5303d989a0
blob - a56994ab50aea3ed8cbe907c4d8fa2cc65f6ae14
blob + 6e4f19aa3e9703b1e7a3b57f3d76fe41a2b801ac
--- got/got.c
+++ got/got.c
@@ -7253,7 +7253,7 @@ cmd_patch(int argc, char *argv[])
 		goto done;
 
 #ifndef PROFILE
-	if (pledge("stdio rpath wpath cpath proc exec sendfd flock",
+	if (pledge("stdio rpath wpath cpath fattr proc exec sendfd flock",
 	    NULL) == -1)
 		err(1, "pledge");
 #endif
blob - 93a9570fa38f33af30ac5d909b03d23f7c63eda3
blob + 8fc9edf2163e5db80ebfc2641f66fd8d288fa8bd
--- lib/patch.c
+++ lib/patch.c
@@ -388,10 +388,12 @@ apply_hunk(FILE *tmp, struct got_patch_hunk *h, long *
 }
 
 static const struct got_error *
-patch_file(struct got_patch *p, const char *path, FILE *tmp, int nop)
+patch_file(struct got_patch *p, const char *path, FILE *tmp, int nop,
+    mode_t *mode)
 {
 	const struct got_error *err = NULL;
 	struct got_patch_hunk *h;
+	struct stat sb;
 	size_t i;
 	long lineno = 0;
 	FILE *orig;
@@ -418,6 +420,12 @@ patch_file(struct got_patch *p, const char *path, FILE
 		goto done;
 	}
 
+	if (fstat(fileno(orig), &sb) == -1) {
+		err = got_error_from_errno("fstat");
+		goto done;
+	}
+	*mode = sb.st_mode;
+
 	copypos = 0;
 	STAILQ_FOREACH(h, &p->head, entries) {
 		if (h->lines == NULL)
@@ -466,15 +474,9 @@ patch_file(struct got_patch *p, const char *path, FILE
 		}
 	}
 
-	
-	if (p->new == NULL) {
-		struct stat sb;
-
-		if (fstat(fileno(orig), &sb) == -1)
-			err = got_error_from_errno("fstat");
-		else if (sb.st_size != copypos)
-			err = got_error(GOT_ERR_PATCH_DONT_APPLY);
-	} else if (!nop && !feof(orig))
+	if (p->new == NULL && sb.st_size != copypos)
+		err = got_error(GOT_ERR_PATCH_DONT_APPLY);
+	else if (!nop && !feof(orig))
 		err = copy(tmp, orig, copypos, -1);
 
 done:
@@ -596,6 +598,7 @@ apply_patch(struct got_worktree *worktree, struct got_
 	char *oldpath = NULL, *newpath = NULL;
 	char *tmppath = NULL, *template = NULL;
 	FILE *tmp = NULL;
+	mode_t mode = GOT_DEFAULT_FILE_MODE;
 
 	TAILQ_INIT(&oldpaths);
 	TAILQ_INIT(&newpaths);
@@ -628,7 +631,7 @@ apply_patch(struct got_worktree *worktree, struct got_
 		err = got_opentemp_named(&tmppath, &tmp, template);
 	if (err)
 		goto done;
-	err = patch_file(p, oldpath, tmp, nop);
+	err = patch_file(p, oldpath, tmp, nop, &mode);
 	if (err)
 		goto done;
 
@@ -641,6 +644,11 @@ apply_patch(struct got_worktree *worktree, struct got_
 		goto done;
 	}
 
+	if (fchmod(fileno(tmp), mode) == -1) {
+		err = got_error_from_errno2("chmod", newpath);
+		goto done;
+	}
+
 	if (rename(tmppath, newpath) == -1) {
 		err = got_error_from_errno3("rename", tmppath, newpath);
 		goto done;
blob - 41f290c529e09c20b73b0a6752c3351664a9559d
blob + 6d468d0334318dc22a5552f720bf777276c909d8
--- regress/cmdline/patch.sh
+++ regress/cmdline/patch.sh
@@ -967,6 +967,47 @@ EOF
 	test_done $testroot $ret
 }
 
+test_patch_preserve_perm() {
+	local testroot=`test_init patch_preserve_perm`
+
+	got checkout $testroot/repo $testroot/wt > /dev/null
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done $testroot $ret
+		return 1
+	fi
+
+	chmod +x $testroot/wt/alpha
+	(cd $testroot/wt && got commit -m 'alpha executable') > /dev/null
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done $testroot $ret
+		return 1
+	fi
+
+	cat <<EOF > $testroot/wt/patch
+--- alpha
++++ alpha
+@@ -1 +1,2 @@
+ alpha
++was edited
+EOF
+
+	(cd $testroot/wt && got patch patch) > /dev/null
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done $testroot $ret
+		return 1
+	fi
+
+	if [ ! -x $testroot/wt/alpha ]; then
+		echo "alpha is no more executable!" >&2
+		test_done $testroot 1
+		return 1
+	fi
+	test_done $testroot 0
+}
+
 test_parseargs "$@"
 run_test test_patch_simple_add_file
 run_test test_patch_simple_rm_file
@@ -982,3 +1023,4 @@ run_test test_patch_equals_for_context
 run_test test_patch_rename
 run_test test_patch_illegal_status
 run_test test_patch_nop
+run_test test_patch_preserve_perm

-----------------------------------------------
commit 6425f004589bac38dac5922036493bebda09d5a8 (paf)
from: Omar Polo <op@omarpolo.com>
date: Wed Mar 16 17:50:08 2022 UTC
 
 got patch: create missing directories when adding files
 
diff c8a54ed8b33fdd341c3a8fe4fd60fa5303d989a0 03c350e0fd68967ea9bfeca0795fbd79de3dcb1b
blob - 8fc9edf2163e5db80ebfc2641f66fd8d288fa8bd
blob + 58d4125553be6fd56d2408805b9c001d9c08c698
--- lib/patch.c
+++ lib/patch.c
@@ -595,7 +595,7 @@ apply_patch(struct got_worktree *worktree, struct got_
 	const struct got_error *err = NULL;
 	struct got_pathlist_head oldpaths, newpaths;
 	int file_renamed = 0;
-	char *oldpath = NULL, *newpath = NULL;
+	char *oldpath = NULL, *newpath = NULL, *parent = NULL;
 	char *tmppath = NULL, *template = NULL;
 	FILE *tmp = NULL;
 	mode_t mode = GOT_DEFAULT_FILE_MODE;
@@ -650,8 +650,23 @@ apply_patch(struct got_worktree *worktree, struct got_
 	}
 
 	if (rename(tmppath, newpath) == -1) {
-		err = got_error_from_errno3("rename", tmppath, newpath);
-		goto done;
+		if (errno != ENOENT) {
+			err = got_error_from_errno3("rename", tmppath,
+			    newpath);
+			goto done;
+		}
+
+		err = got_path_dirname(&parent, newpath);
+		if (err != NULL)
+			goto done;
+		err = got_path_mkdir(parent);
+		if (err != NULL)
+			goto done;
+		if (rename(tmppath, newpath) == -1) {
+			err = got_error_from_errno3("rename", tmppath,
+			    newpath);
+			goto done;
+		}
 	}
 
 	if (file_renamed) {
@@ -670,6 +685,7 @@ apply_patch(struct got_worktree *worktree, struct got_
 done:
 	if (err != NULL && newpath != NULL && (file_renamed || p->old == NULL))
 		unlink(newpath);
+	free(parent);
 	free(template);
 	if (tmppath != NULL)
 		unlink(tmppath);
blob - 6d468d0334318dc22a5552f720bf777276c909d8
blob + 92f950fb6b4fb18338a3b9edcd4318445564b3e7
--- regress/cmdline/patch.sh
+++ regress/cmdline/patch.sh
@@ -1008,6 +1008,47 @@ EOF
 	test_done $testroot 0
 }
 
+test_patch_create_dirs() {
+	local testroot=`test_init patch_create_dirs`
+
+	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/patch
+--- /dev/null
++++ iota/kappa/lambda
+@@ -0,0 +1 @@
++lambda
+EOF
+
+	(cd $testroot/wt && got patch patch) > $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done $testroot $ret
+		return 1
+	fi
+
+	echo 'A  iota/kappa/lambda' >> $testroot/stdout.expected
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done $testroot $ret
+		return 1
+	fi
+
+	if [ ! -f $testroot/wt/iota/kappa/lambda ]; then
+		echo "file not created!" >&2
+		test_done $testroot $ret
+		return 1
+	fi
+	test_done $testroot 0
+}
+
 test_parseargs "$@"
 run_test test_patch_simple_add_file
 run_test test_patch_simple_rm_file
@@ -1024,3 +1065,4 @@ run_test test_patch_rename
 run_test test_patch_illegal_status
 run_test test_patch_nop
 run_test test_patch_preserve_perm
+run_test test_patch_create_dirs