commit 0f76ab831bc589bb34a4953cf60c8a05a4ed708c from: Omar Polo via: Thomas Adam date: Thu Jun 23 14:09:35 2022 UTC got patch: use diff3 to merge the changes Parse the "blob -" metadata in diffs produced by 'got diff' and use the original file for patching. Then, use the diff3 with the current version of the file to merge the differences. This solves many failures automagically or at least turns them into a conflict. ok/improvements stsp@ commit - 2d9b683740176cd3223b23ca8bed498f87c0ecbc commit + 0f76ab831bc589bb34a4953cf60c8a05a4ed708c blob - 242b4340503cbbfe02b3dd739802ae8cfaf898e7 blob + 7418f687e34fddd7bc47a309d75e671e26709308 --- lib/got_lib_privsep.h +++ lib/got_lib_privsep.h @@ -613,6 +613,7 @@ struct got_imsg_patch { int git; char old[PATH_MAX]; char new[PATH_MAX]; + char blob[41]; }; /* blob - d9307b5cc40fda8f07811c3d35ad2515dd64992d blob + d713307ee278f076b3b46464b40dd6050c58a459 --- lib/patch.c +++ lib/patch.c @@ -39,12 +39,15 @@ #include "got_reference.h" #include "got_cancel.h" #include "got_worktree.h" +#include "got_repository.h" #include "got_opentemp.h" #include "got_patch.h" #include "got_lib_delta.h" +#include "got_lib_diff.h" #include "got_lib_object.h" #include "got_lib_privsep.h" +#include "got_lib_sha1.h" #define MIN(a, b) ((a) < (b) ? (a) : (b)) @@ -67,6 +70,7 @@ STAILQ_HEAD(got_patch_hunk_head, got_patch_hunk); struct got_patch { char *old; char *new; + char blob[41]; struct got_patch_hunk_head head; }; @@ -177,10 +181,13 @@ recv_patch(struct imsgbuf *ibuf, int *done, struct got memcpy(&patch, imsg.data, sizeof(patch)); if (patch.old[sizeof(patch.old)-1] != '\0' || - patch.new[sizeof(patch.new)-1] != '\0') { + patch.new[sizeof(patch.new)-1] != '\0' || + patch.blob[sizeof(patch.blob)-1] != '\0') { err = got_error(GOT_ERR_PRIVSEP_LEN); goto done; } + + strlcpy(p->blob, patch.blob, sizeof(p->blob)); /* automatically set strip=1 for git-style diffs */ if (strip == -1 && patch.git && @@ -566,18 +573,75 @@ patch_add(void *arg, unsigned char status, const char } static const struct got_error * -apply_patch(struct got_worktree *worktree, struct got_repository *repo, - struct got_fileindex *fileindex, const char *old, const char *new, - struct got_patch *p, int nop, struct patch_args *pa, - got_cancel_cb cancel_cb, void *cancel_arg) +open_blob(char **path, FILE **fp, const char *blobid, + struct got_repository *repo) { const struct got_error *err = NULL; - int file_renamed = 0; + struct got_blob_object *blob = NULL; + struct got_object_id id; + + *fp = NULL; + *path = NULL; + + if (!got_parse_sha1_digest(id.sha1, blobid)) + return got_error(GOT_ERR_BAD_OBJ_ID_STR); + + err = got_object_open_as_blob(&blob, repo, &id, 8192); + if (err) + goto done; + + err = got_opentemp_named(path, fp, GOT_TMPDIR_STR "/got-patch-blob"); + if (err) + goto done; + + err = got_object_blob_dump_to_file(NULL, NULL, NULL, *fp, blob); + if (err) + goto done; + +done: + if (blob) + got_object_blob_close(blob); + if (err) { + if (*fp != NULL) + fclose(*fp); + if (*path != NULL) + unlink(*path); + free(*path); + *fp = NULL; + *path = NULL; + } + return err; +} + +static const struct got_error * +apply_patch(int *overlapcnt, struct got_worktree *worktree, + struct got_repository *repo, struct got_fileindex *fileindex, + const char *old, const char *new, struct got_patch *p, int nop, + struct patch_args *pa, got_cancel_cb cancel_cb, void *cancel_arg) +{ + const struct got_error *err = NULL; + int do_merge = 0, file_renamed = 0; + char *oldlabel = NULL, *newlabel = NULL; char *oldpath = NULL, *newpath = NULL; char *tmppath = NULL, *template = NULL, *parent = NULL; - FILE *oldfile = NULL, *tmp = NULL; + char *apath = NULL, *mergepath = NULL; + FILE *oldfile = NULL, *tmpfile = NULL, *afile = NULL, *mergefile = NULL; + int outfd; + const char *outpath; mode_t mode = GOT_DEFAULT_FILE_MODE; + *overlapcnt = 0; + + /* don't run the diff3 merge on creations/deletions */ + if (*p->blob != '\0' && p->old != NULL && p->new != NULL) { + err = open_blob(&apath, &afile, p->blob, repo); + if (err && err->code != GOT_ERR_NOT_REF) + return err; + else if (err == NULL) + do_merge = 1; + err = NULL; + } + if (asprintf(&oldpath, "%s/%s", got_worktree_get_root_path(worktree), old) == -1) { err = got_error_from_errno("asprintf"); @@ -603,13 +667,48 @@ apply_patch(struct got_worktree *worktree, struct got_ goto done; } - err = got_opentemp_named(&tmppath, &tmp, template); + err = got_opentemp_named(&tmppath, &tmpfile, template); if (err) goto done; - err = patch_file(p, oldfile, tmp, &mode); + outpath = tmppath; + outfd = fileno(tmpfile); + err = patch_file(p, afile != NULL ? afile : oldfile, tmpfile, &mode); if (err) goto done; + if (do_merge) { + if (fseeko(afile, 0, SEEK_SET) == -1 || + fseeko(oldfile, 0, SEEK_SET) == -1 || + fseeko(tmpfile, 0, SEEK_SET) == -1) { + err = got_error_from_errno("fseeko"); + goto done; + } + + if (asprintf(&oldlabel, "--- %s", p->old) == -1) { + err = got_error_from_errno("asprintf"); + oldlabel = NULL; + goto done; + } + + if (asprintf(&newlabel, "+++ %s", p->new) == -1) { + err = got_error_from_errno("asprintf"); + newlabel = NULL; + goto done; + } + + err = got_opentemp_named(&mergepath, &mergefile, template); + if (err) + goto done; + outpath = mergepath; + outfd = fileno(mergefile); + + err = got_merge_diff3(overlapcnt, outfd, tmpfile, afile, + oldfile, oldlabel, p->blob, newlabel, + GOT_DIFF_ALGORITHM_PATIENCE); + if (err) + goto done; + } + if (nop) goto done; @@ -619,14 +718,14 @@ apply_patch(struct got_worktree *worktree, struct got_ goto done; } - if (fchmod(fileno(tmp), mode) == -1) { + if (fchmod(outfd, mode) == -1) { err = got_error_from_errno2("chmod", tmppath); goto done; } - if (rename(tmppath, newpath) == -1) { + if (rename(outpath, newpath) == -1) { if (errno != ENOENT) { - err = got_error_from_errno3("rename", tmppath, + err = got_error_from_errno3("rename", outpath, newpath); goto done; } @@ -637,8 +736,8 @@ apply_patch(struct got_worktree *worktree, struct got_ err = got_path_mkdir(parent); if (err != NULL) goto done; - if (rename(tmppath, newpath) == -1) { - err = got_error_from_errno3("rename", tmppath, + if (rename(outpath, newpath) == -1) { + err = got_error_from_errno3("rename", outpath, newpath); goto done; } @@ -658,21 +757,40 @@ apply_patch(struct got_worktree *worktree, struct got_ fileindex, patch_add, pa); if (err) unlink(newpath); - } else + } else if (*overlapcnt != 0) + err = report_progress(pa, old, new, GOT_STATUS_CONFLICT, NULL); + else err = report_progress(pa, old, new, GOT_STATUS_MODIFY, NULL); done: free(parent); free(template); + if (tmppath != NULL) unlink(tmppath); - if (tmp != NULL && fclose(tmp) == EOF && err == NULL) + if (tmpfile != NULL && fclose(tmpfile) == EOF && err == NULL) err = got_error_from_errno("fclose"); free(tmppath); + free(oldpath); if (oldfile != NULL && fclose(oldfile) == EOF && err == NULL) err = got_error_from_errno("fclose"); + + if (apath != NULL) + unlink(apath); + if (afile != NULL && fclose(afile) == EOF && err == NULL) + err = got_error_from_errno("fclose"); + free(apath); + + if (mergepath != NULL) + unlink(mergepath); + if (mergefile != NULL && fclose(mergefile) == EOF && err == NULL) + err = got_error_from_errno("fclose"); + free(mergepath); + free(newpath); + free(oldlabel); + free(newlabel); return err; } @@ -716,7 +834,7 @@ got_patch(int fd, struct got_worktree *worktree, struc char *oldpath, *newpath; struct imsgbuf *ibuf; int imsg_fds[2] = {-1, -1}; - int done = 0, failed = 0; + int overlapcnt, done = 0, failed = 0; pid_t pid; ibuf = calloc(1, sizeof(*ibuf)); @@ -775,8 +893,9 @@ got_patch(int fd, struct got_worktree *worktree, struc err = got_worktree_patch_check_path(p.old, p.new, &oldpath, &newpath, worktree, repo, fileindex); if (err == NULL) - err = apply_patch(worktree, repo, fileindex, oldpath, - newpath, &p, nop, &pa, cancel_cb, cancel_arg); + err = apply_patch(&overlapcnt, worktree, repo, + fileindex, oldpath, newpath, &p, nop, &pa, + cancel_cb, cancel_arg); if (err != NULL) { failed = 1; /* recoverable errors */ @@ -788,6 +907,8 @@ got_patch(int fd, struct got_worktree *worktree, struc err = report_progress(&pa, p.old, p.new, GOT_STATUS_CANNOT_UPDATE, NULL); } + if (overlapcnt != 0) + failed = 1; free(oldpath); free(newpath); blob - e3fb875dfb206b0f0ca027d9645c0b7bbfafd410 blob + 841db30ae083c7116c5d261ae998294883ff2b52 --- libexec/got-read-patch/got-read-patch.c +++ libexec/got-read-patch/got-read-patch.c @@ -55,11 +55,12 @@ #include "got_lib_delta.h" #include "got_lib_object.h" #include "got_lib_privsep.h" +#include "got_lib_sha1.h" struct imsgbuf ibuf; static const struct got_error * -send_patch(const char *oldname, const char *newname, int git) +send_patch(const char *oldname, const char *newname, const char *blob, int git) { struct got_imsg_patch p; @@ -71,9 +72,11 @@ send_patch(const char *oldname, const char *newname, i if (newname != NULL) strlcpy(p.new, newname, sizeof(p.new)); + if (blob != NULL) + strlcpy(p.blob, blob, sizeof(p.blob)); + p.git = git; - if (imsg_compose(&ibuf, GOT_IMSG_PATCH, 0, 0, -1, - &p, sizeof(p)) == -1) + if (imsg_compose(&ibuf, GOT_IMSG_PATCH, 0, 0, -1, &p, sizeof(p)) == -1) return got_error_from_errno("imsg_compose GOT_IMSG_PATCH"); return NULL; } @@ -122,10 +125,31 @@ filename(const char *at, char **name) } static const struct got_error * +blobid(const char *line, char **blob) +{ + uint8_t digest[SHA1_DIGEST_LENGTH]; + size_t len; + + *blob = NULL; + + len = strspn(line, "0123456789abcdefABCDEF"); + if ((*blob = strndup(line, len)) == NULL) + return got_error_from_errno("strndup"); + + if (!got_parse_sha1_digest(digest, *blob)) { + /* silently ignore invalid blob ids */ + free(*blob); + *blob = NULL; + } + return NULL; +} + +static const struct got_error * find_patch(int *done, FILE *fp) { const struct got_error *err = NULL; char *old = NULL, *new = NULL; + char *blob = NULL; char *line = NULL; size_t linesize = 0; ssize_t linelen; @@ -146,13 +170,19 @@ find_patch(int *done, FILE *fp) } else if (!strncmp(line, "+++ ", 4)) { free(new); err = filename(line+4, &new); + } else if (!git && !strncmp(line, "blob - ", 7)) { + free(blob); + err = blobid(line + 7, &blob); } else if (rename && !strncmp(line, "rename to ", 10)) { free(new); err = filename(line + 10, &new); } else if (git && !strncmp(line, "similarity index 100%", 21)) rename = 1; - else if (!strncmp(line, "diff --git a/", 13)) + else if (!strncmp(line, "diff --git a/", 13)) { git = 1; + free(blob); + blob = NULL; + } if (err) break; @@ -164,7 +194,7 @@ find_patch(int *done, FILE *fp) */ if (rename && old != NULL && new != NULL) { *done = 1; - err = send_patch(old, new, git); + err = send_patch(old, new, blob, git); break; } @@ -174,7 +204,7 @@ find_patch(int *done, FILE *fp) (!create && old == NULL)) err = got_error(GOT_ERR_PATCH_MALFORMED); else - err = send_patch(old, new, git); + err = send_patch(old, new, blob, git); if (err) break; @@ -188,6 +218,7 @@ find_patch(int *done, FILE *fp) free(old); free(new); + free(blob); free(line); if (ferror(fp) && err == NULL) err = got_error_from_errno("getline"); blob - c1e703a5e36a9140b460df532dc84b36583e5884 blob + 6c9ee03971f22c84e0e5a023a0b18209a73bbdd8 --- regress/cmdline/patch.sh +++ regress/cmdline/patch.sh @@ -1439,10 +1439,181 @@ EOF ret=$? if [ $ret -ne 0 ]; then diff -u $testroot/wt/alpha.expected $testroot/wt/alpha + fi + test_done $testroot $ret +} + +test_patch_merge_simple() { + local testroot=`test_init patch_merge_simple` + + got checkout $testroot/repo $testroot/wt > /dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done $testroot $ret + return 1 + fi + + jot 10 > $testroot/wt/numbers + (cd $testroot/wt && got add numbers && got commit -m +numbers) \ + > /dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done $testroot $ret + return 1 + fi + + jot 10 | sed 's/4/four/g' > $testroot/wt/numbers + + (cd $testroot/wt && got diff > $testroot/old.diff \ + && got revert numbers) >/dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done $testroot $ret + return 1 + fi + + jot 10 | sed 's/6/six/g' > $testroot/wt/numbers + (cd $testroot/wt && got commit -m 'edit numbers') \ + > /dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done $testroot $ret + return 1 + fi + + (cd $testroot/wt && got patch $testroot/old.diff) \ + 2>&1 > /dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done $testroot $ret + return 1 + fi + + jot 10 | sed -e s/4/four/ -e s/6/six/ > $testroot/wt/numbers.expected + cmp -s $testroot/wt/numbers $testroot/wt/numbers.expected + ret=$? + if [ $ret -ne 0 ]; then + diff -u $testroot/wt/numbers $testroot/wt/numbers.expected + fi + test_done $testroot $ret +} + +test_patch_merge_conflict() { + local testroot=`test_init patch_merge_conflict` + + got checkout $testroot/repo $testroot/wt > /dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done $testroot $ret + return 1 + fi + + jot 10 > $testroot/wt/numbers + (cd $testroot/wt && got add numbers && got commit -m +numbers) \ + > /dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done $testroot $ret + return 1 + fi + + jot 10 | sed 's/6/six/g' > $testroot/wt/numbers + + (cd $testroot/wt && got diff > $testroot/old.diff \ + && got revert numbers) >/dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done $testroot $ret + return 1 + fi + + jot 10 | sed 's/6/3+3/g' > $testroot/wt/numbers + (cd $testroot/wt && got commit -m 'edit numbers') \ + > /dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done $testroot $ret + return 1 fi + + (cd $testroot/wt && got patch $testroot/old.diff) \ + >/dev/null 2>&1 + ret=$? + if [ $ret -eq 0 ]; then + echo "got patch merged a diff that should conflict" >&2 + test_done $testroot 0 + return 1 + fi + + # XXX: prefixing every line with a tab otherwise got thinks + # the file has conflicts in it. + cat <<-EOF > $testroot/wt/numbers.expected + 1 + 2 + 3 + 4 + 5 + <<<<<<< --- numbers + six + ||||||| f00c965d8307308469e537302baa73048488f162 + 6 + ======= + 3+3 + >>>>>>> +++ numbers + 7 + 8 + 9 + 10 +EOF + + cmp -s $testroot/wt/numbers $testroot/wt/numbers.expected + ret=$? + if [ $ret -ne 0 ]; then + diff -u $testroot/wt/numbers $testroot/wt/numbers.expected + fi test_done $testroot $ret } +test_patch_merge_unknown_blob() { + local testroot=`test_init patch_merge_unknown_blob` + + 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 +I've got a +blob - aaaabbbbccccddddeeeeffff0000111122223333 +and also a +blob + 0000111122223333444455556666888899990000 +for this dummy diff +--- alpha ++++ alpha +@@ -1 +1 @@ +-alpha ++ALPHA +will it work? +EOF + + (cd $testroot/wt/ && got patch patch) > $testroot/stdout + ret=$? + if [ $ret -ne 0 ]; then + test_done $testroot $ret + return 1 + fi + + echo 'M alpha' > $testroot/stdout.expected + cmp -s $testroot/stdout.expected $testroot/stdout + ret=$? + if [ $ret -ne 0 ]; then + diff -u $testroot/stdout.expected $testroot/stdout + fi + test_done $testroot $ret +} + test_parseargs "$@" run_test test_patch_simple_add_file run_test test_patch_simple_rm_file @@ -1468,3 +1639,6 @@ run_test test_patch_relative_paths run_test test_patch_with_path_prefix run_test test_patch_relpath_with_path_prefix run_test test_patch_reverse +run_test test_patch_merge_simple +run_test test_patch_merge_conflict +run_test test_patch_merge_unknown_blob