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

From:
Omar Polo <op@omarpolo.com>
Subject:
applying patches loses the original perms
To:
gameoftrees@openbsd.org
Date:
Sat, 02 Jul 2022 22:47:29 +0200

Download raw body.

Thread
Stefan reported on IRC that applying the jrick' ssh key signing patch
with 'got patch' loses the executable bits in regress/cmdline/tag.sh.

The issue was introduced with the diff3 merge machinery.  To save an
fstat, I'm looking at the permission of the "old file" in patch_file.
The issue is that with the diff3 machinery in, patch_file is called with
a temp file and so we look at the wrong mode.

Diff below fixes the issue by looking at the mode of the old file
earlier in apply_patch right after opening it, and adds a regress for
it.

diff /home/op/w/got
commit - f5b0315f0e07bfd36a4eb37d91884fcd8614745a
path + /home/op/w/got
blob - 6b6915f9998d30a1ae5b1846365b299ba2c974fe
file + lib/patch.c
--- lib/patch.c
+++ lib/patch.c
@@ -469,7 +469,7 @@ apply_hunk(FILE *tmp, struct got_patch_hunk *h, int *l
 }
 
 static const struct got_error *
-patch_file(struct got_patch *p, FILE *orig, FILE *tmp, mode_t *mode)
+patch_file(struct got_patch *p, FILE *orig, FILE *tmp)
 {
 	const struct got_error *err = NULL;
 	struct got_patch_hunk *h;
@@ -489,7 +489,6 @@ patch_file(struct got_patch *p, FILE *orig, FILE *tmp,
 
 	if (fstat(fileno(orig), &sb) == -1)
 		return got_error_from_errno("fstat");
-	*mode = sb.st_mode;
 
 	copypos = 0;
 	STAILQ_FOREACH(h, &p->head, entries) {
@@ -651,6 +650,7 @@ apply_patch(int *overlapcnt, struct got_worktree *work
     struct patch_args *pa, got_cancel_cb cancel_cb, void *cancel_arg)
 {
 	const struct got_error *err = NULL;
+	struct stat sb;
 	int do_merge = 0, file_renamed = 0;
 	char *oldlabel = NULL, *newlabel = NULL, *anclabel = NULL;
 	char *oldpath = NULL, *newpath = NULL;
@@ -698,9 +698,16 @@ apply_patch(int *overlapcnt, struct got_worktree *work
 		goto done;
 	}
 
-	if (p->old != NULL && (oldfile = fopen(oldpath, "r")) == NULL) {
-		err = got_error_from_errno2("open", oldpath);
-		goto done;
+	if (p->old != NULL) {
+		if ((oldfile = fopen(oldpath, "r")) == NULL) {
+			err = got_error_from_errno2("open", oldpath);
+			goto done;
+		}
+		if (fstat(fileno(oldfile), &sb) == -1) {
+			err = got_error_from_errno2("fstat", oldpath);
+			goto done;
+		}
+		mode = sb.st_mode;
 	}
 
 	err = got_opentemp_named(&tmppath, &tmpfile, template);
@@ -708,7 +715,7 @@ apply_patch(int *overlapcnt, struct got_worktree *work
 		goto done;
 	outpath = tmppath;
 	outfd = fileno(tmpfile);
-	err = patch_file(p, afile != NULL ? afile : oldfile, tmpfile, &mode);
+	err = patch_file(p, afile != NULL ? afile : oldfile, tmpfile);
 	if (err)
 		goto done;
 
blob - a7ff83ea9847a8fa722fdbfda0cfc471da1d050d
file + regress/cmdline/patch.sh
--- regress/cmdline/patch.sh
+++ regress/cmdline/patch.sh
@@ -1458,6 +1458,7 @@ test_patch_merge_simple() {
 	fi
 
 	jot 10 > $testroot/wt/numbers
+	chmod +x $testroot/wt/numbers
 	(cd $testroot/wt && got add numbers && got commit -m +numbers) \
 		> /dev/null
 	ret=$?
@@ -1507,7 +1508,15 @@ test_patch_merge_simple() {
 	ret=$?
 	if [ $ret -ne 0 ]; then
 		diff -u $testroot/wt/numbers $testroot/wt/numbers.expected
+		test_done $testroot $ret
+		return 1
 	fi
+
+	[ -x $testroot/wt/numbers ]
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "numbers lost the executable bit" >&2
+	fi
 	test_done $testroot $ret
 }