From: Omar Polo Subject: got patch vs git diff with renames To: gameoftrees@openbsd.org Date: Fri, 29 Apr 2022 11:08:41 +0200 A recent mail on ports@ (the net/znc diff) reminded me that there's still a corner case of git-style rename diffs that I forgot to handle: renames without edits. Such diffs looks like this > diff --git a/net/znc/pkg/DESCR b/net/znc/pkg/DESCR-main > similarity index 100% > rename from net/znc/pkg/DESCR > rename to net/znc/pkg/DESCR-main and don't have any hunks. I don't have any idea why they decided to avoid the +++/--- syntax and include a "@@ -0,0 +0,0 @@" since compatibility doesn't seem to be a concern... Here's a straightforward diff to handle this situation. I intend to rename that "ok" variable in read_patch in a follow-up diff (needs the same rename also in parse_hunk and parse_hdr.) I've also simplified the test_patch_rename function other than adding a test case for this. comments/ok? P.S.: i forgot that we supported the rename with an empty hunk "@@ -0,0 +0,0 @@". Since git doesn't seem to handle it, and neither patch(1), I intend to remove the support for it, but again, in a follow-up commit :) diff a8acad7d8eeb9aca78302e45eba5eb2211e6f291 /home/op/w/got blob - 879b77f8142d65729f38b9389c48cf5a78299027 file + libexec/got-read-patch/got-read-patch.c --- libexec/got-read-patch/got-read-patch.c +++ libexec/got-read-patch/got-read-patch.c @@ -123,14 +123,14 @@ filename(const char *at, char **name) } static const struct got_error * -find_patch(FILE *fp) +find_patch(int *empty, FILE *fp) { const struct got_error *err = NULL; char *old = NULL, *new = NULL; char *line = NULL; size_t linesize = 0; ssize_t linelen; - int create, git = 0; + int create, rename = 0, git = 0; while ((linelen = getline(&line, &linesize, fp)) != -1) { /* @@ -141,15 +141,34 @@ find_patch(FILE *fp) if (!strncmp(line, "--- ", 4)) { free(old); err = filename(line+4, &old); + } else if (rename && !strncmp(line, "rename from ", 12)) { + free(old); + err = filename(line+12, &old); } else if (!strncmp(line, "+++ ", 4)) { free(new); err = filename(line+4, &new); - } else if (!strncmp(line, "diff --git a/", 13)) + } 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)) git = 1; if (err) break; + /* + * Git-style diffs with "similarity index 100%" don't + * have any hunks and ends with the "rename to foobar" + * line. + */ + if (rename && old != NULL && new != NULL) { + *empty = 1; + err = send_patch(old, new, git); + break; + } + if (!strncmp(line, "@@ -", 4)) { create = !strncmp(line+4, "0,0", 3); if ((old == NULL && new == NULL) || @@ -412,7 +431,7 @@ read_patch(struct imsgbuf *ibuf, int fd) { const struct got_error *err = NULL; FILE *fp; - int ok, patch_found = 0; + int patch_found = 0; if ((fp = fdopen(fd, "r")) == NULL) { err = got_error_from_errno("fdopen"); @@ -421,16 +440,19 @@ read_patch(struct imsgbuf *ibuf, int fd) } while (!feof(fp)) { - err = find_patch(fp); + int empty = 0, ok = 1; + + err = find_patch(&empty, fp); if (err) goto done; patch_found = 1; for (;;) { - err = parse_hunk(fp, &ok); + if (!empty) + err = parse_hunk(fp, &ok); if (err) goto done; - if (!ok) { + if (!ok || empty) { err = send_patch_done(); if (err) goto done; blob - 43981599f51928fd929e8cc47538da750b875b97 file + regress/cmdline/patch.sh --- regress/cmdline/patch.sh +++ regress/cmdline/patch.sh @@ -647,14 +647,22 @@ test_patch_rename() { fi cat < $testroot/wt/patch +diff --git a/beta b/iota +similarity index 100% +rename from beta +rename to iota diff --git a/alpha b/eta --- a/alpha +++ b/eta -@@ -0,0 +0,0 @@ +@@ -1 +1 @@ +-alpha ++eta EOF - echo 'D alpha' > $testroot/stdout.expected - echo 'A eta' >> $testroot/stdout.expected + echo 'D beta' > $testroot/stdout.expected + echo 'A iota' >> $testroot/stdout.expected + echo 'D alpha' >> $testroot/stdout.expected + echo 'A eta' >> $testroot/stdout.expected (cd $testroot/wt && got patch patch) > $testroot/stdout ret=$? @@ -671,77 +679,35 @@ EOF return 1 fi - if [ -f $testroot/wt/alpha ]; then - echo "alpha was not removed" >&2 + if [ -f $testroot/wt/alpha -o -f $testroot/wt/beta ]; then + echo "alpha or beta were not removed" >&2 test_done $testroot 1 return 1 fi - if [ ! -f $testroot/wt/eta ]; then - echo "eta was not created" >&2 + if [ ! -f $testroot/wt/iota -o ! -f $testroot/wt/eta ]; then + echo "iota or eta were not created" >&2 test_done $testroot 1 return 1 fi - echo alpha > $testroot/wt/eta.expected - cmp -s $testroot/wt/eta.expected $testroot/wt/eta + echo beta > $testroot/wt/iota.expected + cmp -s $testroot/wt/iota.expected $testroot/wt/iota ret=$? if [ $ret -ne 0 ]; then - diff -u $testroot/wt/eta.expected $testroot/wt/eta + diff -u $testroot/wt/iota.expected $testroot/wt/iota test_done $testroot $ret return 1 fi - # revert the changes and try again with a rename + edit - (cd $testroot/wt && got revert alpha eta) > /dev/null + echo eta > $testroot/wt/eta.expected + cmp -s $testroot/wt/eta.expected $testroot/wt/eta ret=$? if [ $ret -ne 0 ]; then + diff -u $testroot/wt/eta.expected $testroot/wt/eta test_done $testroot $ret return 1 fi - rm $testroot/wt/eta - cat < $testroot/wt/patch -diff --git a/alpha b/eta ---- a/alpha -+++ b/eta -@@ -1 +1,2 @@ - alpha -+but now is eta -EOF - - (cd $testroot/wt && got patch patch) > $testroot/stdout - ret=$? - if [ $ret -ne 0 ]; then - test_done $testroot $ret - return 1 - fi - - 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/alpha ]; then - echo "alpha was not removed" >&2 - test_done $testroot 1 - return 1 - fi - if [ ! -f $testroot/wt/eta ]; then - echo "eta was not created" >&2 - test_done $testroot 1 - return 1 - fi - - echo alpha > $testroot/wt/eta.expected - echo 'but now is eta' >> $testroot/wt/eta.expected - cmp -s $testroot/wt/eta.expected $testroot/wt/eta - ret=$? - if [ $ret -ne 0 ]; then - diff -u $testroot/wt/eta.expected $testroot/wt/eta - fi test_done $testroot $ret }