From: Omar Polo Subject: Re: got patch; got patch -R To: Stefan Sperling Cc: Christian Weisgerber , gameoftrees@openbsd.org Date: Thu, 21 Jul 2022 11:14:57 +0200 Stefan Sperling wrote: > On Wed, Jul 20, 2022 at 11:51:56PM +0200, Omar Polo wrote: > > A quick solution may be to disable the diff3 merge in -R mode: > > > However, I think we can do better. > > Yes, swapping the diff3 tmpfile/oldfile file arguments (and labels) > in patch.c will do a reverse-merge for -R mode. > > The old blob will still be used as the merge base. I did something similar to what you suggest, but instead I'm swapping tmpfile (which is the "new" file) and the ancestor: this way the merge3 is done with the patched file against the old blob (left) and the current revision (right). (yeah, it gets confusing easily) To do this, i'm postponing reverse_patch in the diff3 case: we don't need to reverse it unless we don't find the blob and fall back to the search/replace. The diff is a bit longer because i had to move reverse_patch early for apply_patch to see it, and i've added a (simple) regress. (would a more complex example help there? I'm already ensuring we're using the merge.) ok? P.S.: yeah, there's an error in the order of the label passed to got_merge_diff3; it should be (newlabel, anclabel, oldlabel) -- or alternatively swap the order of tmpfile and oldfile -- will fix this in a follow-up diff. diff /home/op/w/got commit - fced5a66069199024aaf413a06bcf544b959f6a8 path + /home/op/w/got blob - 703c22b84de200f2a0ec9fc0f7bdc70dc725ab20 file + lib/patch.c --- lib/patch.c +++ lib/patch.c @@ -314,6 +314,35 @@ done: return err; } +static void +reverse_patch(struct got_patch *p) +{ + struct got_patch_hunk *h; + size_t i; + int tmp; + + STAILQ_FOREACH(h, &p->head, entries) { + tmp = h->old_from; + h->old_from = h->new_from; + h->new_from = tmp; + + tmp = h->old_lines; + h->old_lines = h->new_lines; + h->new_lines = tmp; + + tmp = h->old_nonl; + h->old_nonl = h->new_nonl; + h->new_nonl = tmp; + + for (i = 0; i < h->len; ++i) { + if (*h->lines[i] == '+') + *h->lines[i] = '-'; + else if (*h->lines[i] == '-') + *h->lines[i] = '+'; + } + } +} + /* * Copy data from orig starting at copypos until pos into tmp. * If pos is -1, copy until EOF. @@ -700,7 +729,8 @@ 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) + int reverse, struct patch_args *pa, + got_cancel_cb cancel_cb, void *cancel_arg) { const struct got_error *err = NULL; struct stat sb; @@ -728,6 +758,8 @@ apply_patch(int *overlapcnt, struct got_worktree *work return err; else if (err == NULL) do_merge = 1; + else if (reverse) + reverse_patch(p); err = NULL; } @@ -808,6 +840,19 @@ apply_patch(int *overlapcnt, struct got_worktree *work goto done; } + if (reverse) { + char *s; + FILE *t; + + s = anclabel; + anclabel = newlabel; + newlabel = s; + + t = afile; + afile = tmpfile; + tmpfile = t; + } + err = got_opentemp_named(&mergepath, &mergefile, template); if (err) goto done; @@ -909,35 +954,6 @@ done: return err; } -static void -reverse_patch(struct got_patch *p) -{ - struct got_patch_hunk *h; - size_t i; - int tmp; - - STAILQ_FOREACH(h, &p->head, entries) { - tmp = h->old_from; - h->old_from = h->new_from; - h->new_from = tmp; - - tmp = h->old_lines; - h->old_lines = h->new_lines; - h->new_lines = tmp; - - tmp = h->old_nonl; - h->old_nonl = h->new_nonl; - h->new_nonl = tmp; - - for (i = 0; i < h->len; ++i) { - if (*h->lines[i] == '+') - *h->lines[i] = '-'; - else if (*h->lines[i] == '-') - *h->lines[i] = '+'; - } - } -} - const struct got_error * got_patch(int fd, struct got_worktree *worktree, struct got_repository *repo, int nop, int strip, int reverse, got_patch_progress_cb progress_cb, @@ -1002,15 +1018,16 @@ got_patch(int fd, struct got_worktree *worktree, struc if (err || done) break; - if (reverse) + /* reversal application with merge base is done differently */ + if (reverse && *p.blob == '\0') reverse_patch(&p); err = got_worktree_patch_check_path(p.old, p.new, &oldpath, &newpath, worktree, repo, fileindex); if (err == NULL) err = apply_patch(&overlapcnt, worktree, repo, - fileindex, oldpath, newpath, &p, nop, &pa, - cancel_cb, cancel_arg); + fileindex, oldpath, newpath, &p, nop, reverse, + &pa, cancel_cb, cancel_arg); if (err != NULL) { failed = 1; /* recoverable errors */ blob - 7901777201377549e2dce265cf173b09d716ee4d file + regress/cmdline/patch.sh --- regress/cmdline/patch.sh +++ regress/cmdline/patch.sh @@ -1875,6 +1875,68 @@ EOF test_done $testroot $ret } +test_patch_merge_reverse() { + 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/wt/patch \ + && got commit -m 'edit numbers') > /dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done $testroot $ret + return 1 + fi + + jot 10 | sed -e s/4/four/g -e s/6/six/g > $testroot/wt/numbers + (cd $testroot/wt && got commit -m 'edit numbers again') >/dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done $testroot $ret + return 1 + fi + + (cd $testroot/wt && got patch -R patch) > $testroot/stdout + ret=$? + if [ $ret -ne 0 ]; then + echo "failed to revert the patch" >&2 + test_done $testroot $ret + return 1 + fi + + echo 'G numbers' > $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 + + jot 10 | sed s/6/six/g > $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_parseargs "$@" run_test test_patch_add_file run_test test_patch_rm_file @@ -1905,3 +1967,4 @@ run_test test_patch_merge_simple run_test test_patch_merge_gitdiff run_test test_patch_merge_conflict run_test test_patch_merge_unknown_blob +run_test test_patch_merge_reverse