From: Omar Polo Subject: got patch: keep file permissions and create missing directories To: gameoftrees@openbsd.org Date: Wed, 16 Mar 2022 18:55:07 +0100 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 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 < $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 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 < $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