commit 55e9459f41acc72438ed2c9f75fdaeae8f5c41d8 from: Omar Polo date: Sun Jun 19 11:51:32 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 - 05737d499ba523bccc36cbb584307259af88da88 commit + 55e9459f41acc72438ed2c9f75fdaeae8f5c41d8 blob - af9557d85c3c38603367d54310b5f2a59d7b3b11 blob + 8b9f4c059435abf6b351d2c9f180dd9926969528 --- 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 - 0a98afc1b334061cef12aac2a80ebe56c7b4a167 blob + 9fbe2170f84b59a8c01710ada69f683bbfc91f38 --- lib/patch.c +++ lib/patch.c @@ -42,12 +42,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)) @@ -70,6 +73,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; }; @@ -180,10 +184,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 && @@ -569,18 +576,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"); @@ -606,13 +670,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; @@ -622,14 +721,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; } @@ -640,8 +739,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; } @@ -661,21 +760,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; } @@ -719,7 +837,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)); @@ -778,8 +896,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 */ @@ -791,6 +910,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 - 23212abe02328d5837822225fecbf174e677f2bb blob + ba9f841f31f227976e27fe6dbd4cd56b5ae30699 --- libexec/got-read-patch/got-read-patch.c +++ libexec/got-read-patch/got-read-patch.c @@ -56,11 +56,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; @@ -72,9 +73,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; } @@ -123,10 +126,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; @@ -147,13 +171,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; @@ -165,7 +195,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; } @@ -175,7 +205,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; @@ -189,6 +219,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