Download raw body.
got patch: delete binary files too
Spotted by Stefan with the gotweb removal diff, `got patch' fails to delete binary files. What happens is that got-read-patch doesn't see any hunks and considers those lines noise. For reference, a got diff that deletes a binary file looks like this: blob - 460aa1299f8e9f37773618bcab2619794416fb49 file + /dev/null Binary files foobar.png and /dev/null differ git diffs are similar: diff --git a/foobar.png b/foobar.png deleted file mode 100644 index 460aa129..00000000 Binary files foobar.png and /dev/null differ diff and cvs diff looks similar but they don't provide the filename in a way that's easy to extract so they're left out. Here's an example for reference: % diff -u bin/got /dev/null Binary files bin/got and /dev/null differ % rm rpki-client.png % cvs rm rpki-client.png cvs remove: scheduling `rpki-client.png' for removal cvs remove: use 'cvs commit' to remove this file permanently % cvs diff cvs diff: Diffing . Index: rpki-client.png =================================================================== RCS file: rpki-client.png diff -N rpki-client.png Binary files /tmp/cvsq7uY1c and /dev/null differ The idea then is to parse the "file + /dev/null" or "deleted file mode" lines and extract the filename from the "Binary files foo and /dev/null differ." There's no problem with ambiguous spaces since we can treat "Binary files " as prefix and the " and /dev/null differ" string as suffix and strip them, what remains is the filename. (the patch file format has further restrictions on the bytes allowed on the filenames, for e.g. newlines and tabs are not permitted anyway.) ok? diff /home/op/w/got commit - cf536071bc57734308f29cda79d67c88abb3b9f0 path + /home/op/w/got blob - 6c31c4b5e3788850c30c78ed552e90f1eddd3982 file + lib/patch.c --- lib/patch.c +++ lib/patch.c @@ -587,6 +587,10 @@ patch_file(struct got_patch *p, FILE *orig, FILE *tmp) return apply_hunk(orig, tmp, h, &lineno, 0); } + /* When deleting binary files there are no hunks to apply. */ + if (p->new == NULL && STAILQ_EMPTY(&p->head)) + return NULL; + if (fstat(fileno(orig), &sb) == -1) return got_error_from_errno("fstat"); blob - becf27d3e00feb1fd2b32823931dc0d1b580cfb6 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 @@ -128,6 +128,26 @@ static int return NULL; } +static const struct got_error * +binaryfilename(const char *at, char **name) +{ + const char *trail = " and /dev/null differ\n"; + size_t len, d; + + len = strlen(at); + if (len <= strlen(trail)) + return NULL; + + d = len - strlen(trail); + if (strcmp(at + d, trail) != 0) + return NULL; + + *name = strndup(at, d); + if (*name == NULL) + return got_error_from_errno("strndup"); + return NULL; +} + static int filexbit(const char *line) { @@ -212,7 +232,7 @@ find_diff(int *done, int *next, FILE *fp, int git, con char *line = NULL; size_t linesize = 0; ssize_t linelen; - int create, rename = 0, xbit = 0; + int create, delete = 0, rename = 0, xbit = 0, binary = 0; *done = 0; *next = 0; @@ -231,13 +251,21 @@ find_diff(int *done, int *next, FILE *fp, int git, con } else if (!strncmp(line, "+++ ", 4)) { free(new); err = filename(line+4, &new); - } else if (!strncmp(line, "blob + ", 7) || - !strncmp(line, "file + ", 7)) { + } else if (!strncmp(line, "blob + ", 7)) { xbit = filexbit(line); - } else if (!git && !strncmp(line, "blob - ", 7)) { + delete = !strcmp(line + 7, "/dev/null\n"); + } else if (!strncmp(line, "file + ", 7)) + xbit = filexbit(line); + else if (!git && !strncmp(line, "blob - ", 7)) { free(blob); err = blobid(line + 7, &blob, git); - } else if (rename && !strncmp(line, "rename to ", 10)) { + } else if (!strncmp(line, "Binary files ", 13)) { + binary = 1; + free(old); + err = binaryfilename(line + 13, &old); + } else if (git && !strncmp(line, "deleted file mode ", 18)) + delete = 1; + else if (rename && !strncmp(line, "rename to ", 10)) { free(new); err = filename(line + 10, &new); } else if (git && !strncmp(line, "similarity index 100%", 21)) @@ -270,6 +298,16 @@ find_diff(int *done, int *next, FILE *fp, int git, con break; } + /* + * Diffs that remove binary files have no hunks. + */ + if (delete && binary) { + *done = 1; + err = send_patch(old, new, commitid, + blob, xbit, git); + break; + } + if (!strncmp(line, "@@ -", 4)) { create = !strncmp(line+4, "0,0", 3); if ((old == NULL && new == NULL) || blob - 9e3d4639d01241422b50bdddf316137ceb10d945 file + regress/cmdline/patch.sh --- regress/cmdline/patch.sh +++ regress/cmdline/patch.sh @@ -1881,6 +1881,61 @@ test_parseargs "$@" test_done "$testroot" 0 } +test_patch_remove_binary_file() { + local testroot=`test_init patch_remove_binary_file` + + if ! got checkout $testroot/repo $testroot/wt >/dev/null; then + test_done $testroot $ret + return 1 + fi + + dd if=/dev/zero of=$testroot/wt/x bs=1 count=16 2>/dev/null >&2 + (cd $testroot/wt && got add x && got commit -m +x) >/dev/null + + (cd $testroot/wt && \ + got branch demo && \ + got rm x && \ + got ci -m -x && + got log -l 1 -p >patch && + got up -b master) >/dev/null + + echo 'D x' > $testroot/stdout.expected + + (cd $testroot/wt && got patch <patch) > $testroot/stdout + if [ $? -ne 0 ]; then + echo 'patch failed' >&2 + test_done $testroot 1 + return 1 + fi + + if ! cmp -s $testroot/stdout.expected $testroot/stdout; then + diff -u $testroot/stdout.expected $testroot/stdout + test_done $testroot 1 + return 1 + fi + + # try again using a git produced diff + (cd $testroot/wt && got rv x) >/dev/null + + local commit_id=`git_show_branch_head $testroot/repo demo` + (cd $testroot/repo && git show $commit_id) >$testroot/wt/patch + + (cd $testroot/wt && got patch <patch) > $testroot/stdout + if [ $? -ne 0 ]; then + echo 'patch failed' >&2 + test_done $testroot 1 + return 1 + fi + + if ! cmp -s $testroot/stdout.expected $testroot/stdout; then + diff -u $testroot/stdout.expected $testroot/stdout + test_done $testroot 1 + return 1 + fi + + test_done $testroot 0 +} + test_parseargs "$@" run_test test_patch_basic run_test test_patch_dont_apply @@ -1910,3 +1965,4 @@ run_test test_patch_umask run_test test_patch_newfile_xbit_got_diff run_test test_patch_newfile_xbit_git_diff run_test test_patch_umask +run_test test_patch_remove_binary_file
got patch: delete binary files too