* [PATCH 0/3] apply: handle traditional patches with spaces in filename @ 2010-07-24 1:06 Jonathan Nieder 2010-07-24 1:09 ` [PATCH 1/3] apply: Split quoted filename handling into new function Jonathan Nieder ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: Jonathan Nieder @ 2010-07-24 1:06 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Guido Günther, Giuseppe Iuculano The goal: let ‘git apply’ handle such filenames as “b/debian/licenses/LICENSE.Apache (v2.0)” in patches produced by non-git tools without erroring out. When we last left our heroes[1] in April: > | The name and last modification time of each file shall be output in > | the following format: > | > | "---[space]%s %s%s%s", file1, <file1 timestamp>, <file1 frac>, <file1 zone> > | "+++[space]%s %s%s%s", file2, <file2 timestamp>, <file2 frac>, <file2 zone> [...] > If this is really describing the format of patches in the wild, that > means we should only look for a tab character to terminate the filename. [...] > A big downside: this does not cope with copy-and-pasted patches with > tabs transformed to spaces. The example [2] consists mostly of > file-creation patches, so we can’t look to the repository for hints. > Maybe the space-plus-date-plus-newline sequence should be used as a > delimiter. It turns out that is not so hard. Maybe it could be rewritten using regcomp() and regexec(); if someone wants to do that, I won’t stop them. ;-) Patch 1 factors out a function to handle "GNU-format" C-style quoted filenames in patches. The only tool I know of that produces this format is git; the discussion in [2] about what characters to escape seems to have come to no conclusion. Patch 2 adds some tests for all those weird characters that might appear in a filename. They abuse “diff” and “pr”; testing on weird platforms would be helpful. Patch 3 adds the logic to search for a date at the end of a filename line, for traditional (non --git) patches only. If no date is found at the end, we return to the previous heuristic, except that the only accepted filename terminator is a tab. Whitespace damage is only accepted if there is a timestamp at the end of the line. Thoughts, suggestions, improvements welcome. Jonathan Nieder (3): apply: Split quoted filename handling into new function tests: Test how well “git apply” copes with weird filenames apply: Handle traditional patches with space in filename builtin/apply.c | 251 ++++++++++++++++++++++++++++++++------ t/t4120-apply-popt.sh | 35 +++++- t/t4135-apply-weird-filenames.sh | 119 ++++++++++++++++++ 3 files changed, 363 insertions(+), 42 deletions(-) create mode 100755 t/t4135-apply-weird-filenames.sh [1] http://thread.gmane.org/gmane.linux.debian.devel.bugs.general/697969/focus=145543 [2] http://thread.gmane.org/gmane.comp.version-control.git/9813/focus=10046 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/3] apply: Split quoted filename handling into new function 2010-07-24 1:06 [PATCH 0/3] apply: handle traditional patches with spaces in filename Jonathan Nieder @ 2010-07-24 1:09 ` Jonathan Nieder 2010-07-24 1:11 ` [PATCH 2/3] tests: Test how well "git apply" copes with weird filenames Jonathan Nieder 2010-07-24 1:20 ` [PATCH 3/3] apply: Handle traditional patches with space in filename Jonathan Nieder 2 siblings, 0 replies; 7+ messages in thread From: Jonathan Nieder @ 2010-07-24 1:09 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Guido Günther, Giuseppe Iuculano The new find_name_gnu() function handles new-style ‘--- "a/foo"’ patch header lines, leaving find_name() itself a bit less daunting. Functional change: do not clobber the p-value when there are not enough path components in a quoted file name to honor it. Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- builtin/apply.c | 70 +++++++++++++++++++++++++++--------------------- t/t4120-apply-popt.sh | 35 ++++++++++++++++++++++-- 2 files changed, 71 insertions(+), 34 deletions(-) diff --git a/builtin/apply.c b/builtin/apply.c index 12ef9ea..efc109e 100644 --- a/builtin/apply.c +++ b/builtin/apply.c @@ -416,44 +416,52 @@ static char *squash_slash(char *name) return name; } +static char *find_name_gnu(const char *line, char *def, int p_value) +{ + struct strbuf name = STRBUF_INIT; + char *cp; + + /* + * Proposed "new-style" GNU patch/diff format; see + * http://marc.theaimsgroup.com/?l=git&m=112927316408690&w=2 + */ + if (unquote_c_style(&name, line, NULL)) { + strbuf_release(&name); + return NULL; + } + + for (cp = name.buf; p_value; p_value--) { + cp = strchr(cp, '/'); + if (!cp) { + strbuf_release(&name); + return NULL; + } + cp++; + } + + /* name can later be freed, so we need + * to memmove, not just return cp + */ + strbuf_remove(&name, 0, cp - name.buf); + free(def); + if (root) + strbuf_insert(&name, 0, root, root_len); + return squash_slash(strbuf_detach(&name, NULL)); +} + static char *find_name(const char *line, char *def, int p_value, int terminate) { int len; const char *start = NULL; + if (*line == '"') { + char *name = find_name_gnu(line, def, p_value); + if (name) + return name; + } + if (p_value == 0) start = line; - - if (*line == '"') { - struct strbuf name = STRBUF_INIT; - - /* - * Proposed "new-style" GNU patch/diff format; see - * http://marc.theaimsgroup.com/?l=git&m=112927316408690&w=2 - */ - if (!unquote_c_style(&name, line, NULL)) { - char *cp; - - for (cp = name.buf; p_value; p_value--) { - cp = strchr(cp, '/'); - if (!cp) - break; - cp++; - } - if (cp) { - /* name can later be freed, so we need - * to memmove, not just return cp - */ - strbuf_remove(&name, 0, cp - name.buf); - free(def); - if (root) - strbuf_insert(&name, 0, root, root_len); - return squash_slash(strbuf_detach(&name, NULL)); - } - } - strbuf_release(&name); - } - for (;;) { char c = *line; diff --git a/t/t4120-apply-popt.sh b/t/t4120-apply-popt.sh index b463b4f..2b2d00b 100755 --- a/t/t4120-apply-popt.sh +++ b/t/t4120-apply-popt.sh @@ -10,21 +10,50 @@ test_description='git apply -p handling.' test_expect_success setup ' mkdir sub && echo A >sub/file1 && - cp sub/file1 file1 && + cp sub/file1 file1.saved && git add sub/file1 && echo B >sub/file1 && git diff >patch.file && - rm sub/file1 && - rmdir sub + git checkout -- sub/file1 && + git mv sub süb && + echo B >süb/file1 && + git diff >patch.escaped && + grep "[\]" patch.escaped && + rm süb/file1 && + rmdir süb ' test_expect_success 'apply git diff with -p2' ' + cp file1.saved file1 && git apply -p2 patch.file ' test_expect_success 'apply with too large -p' ' + cp file1.saved file1 && test_must_fail git apply --stat -p3 patch.file 2>err && grep "removing 3 leading" err ' +test_expect_success 'apply (-p2) traditional diff with funny filenames' ' + cat >patch.quotes <<-\EOF && + diff -u "a/"sub/file1 "b/"sub/file1 + --- "a/"sub/file1 + +++ "b/"sub/file1 + @@ -1 +1 @@ + -A + +B + EOF + echo B >expected && + + cp file1.saved file1 && + git apply -p2 patch.quotes && + test_cmp expected file1 +' + +test_expect_success 'apply with too large -p and fancy filename' ' + cp file1.saved file1 && + test_must_fail git apply --stat -p3 patch.escaped 2>err && + grep "removing 3 leading" err +' + test_done -- 1.7.2.rc3 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/3] tests: Test how well "git apply" copes with weird filenames 2010-07-24 1:06 [PATCH 0/3] apply: handle traditional patches with spaces in filename Jonathan Nieder 2010-07-24 1:09 ` [PATCH 1/3] apply: Split quoted filename handling into new function Jonathan Nieder @ 2010-07-24 1:11 ` Jonathan Nieder 2010-07-24 8:03 ` Andreas Schwab 2010-07-24 1:20 ` [PATCH 3/3] apply: Handle traditional patches with space in filename Jonathan Nieder 2 siblings, 1 reply; 7+ messages in thread From: Jonathan Nieder @ 2010-07-24 1:11 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Guido Günther, Giuseppe Iuculano The tests assume a reasonably well-behaved “diff -u” and “pr” to produce the (possibly whitespace-damaged) patches. On platforms without those commands, skip the tests. Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- t/t4135-apply-weird-filenames.sh | 119 ++++++++++++++++++++++++++++++++++++++ 1 files changed, 119 insertions(+), 0 deletions(-) create mode 100755 t/t4135-apply-weird-filenames.sh diff --git a/t/t4135-apply-weird-filenames.sh b/t/t4135-apply-weird-filenames.sh new file mode 100755 index 0000000..2dcb040 --- /dev/null +++ b/t/t4135-apply-weird-filenames.sh @@ -0,0 +1,119 @@ +#!/bin/sh + +test_description='git apply with weird postimage filenames' + +. ./test-lib.sh + +test_expect_success 'setup: empty commit' ' + test_tick && + git commit --allow-empty -m preimage && + git tag preimage +' + +test_expect_success 'setup: clean-up functions' ' + reset_preimage() { + git checkout -f preimage^0 && + git read-tree -u --reset HEAD && + git update-index --refresh + } && + + reset_subdirs() { + rm -fr a b && + mkdir a b + } +' + +test_expect_success 'setup: test prerequisites' ' + echo one >1 && + echo one >2 && + if diff -u 1 2 + then + test_set_prereq UNIDIFF + fi && + + if diff -pruN 1 2 + then + test_set_prereq FULLDIFF + fi && + + echo "tab -> ." >expected && + echo "tab -> ." >with-tab && + + pr -tT -e8 with-tab >actual && + if test_cmp expected actual + then + test_set_prereq PR + fi +' + +try_filename() { + desc=$1 + postimage=$2 + exp1=${3:-success} + exp2=${4:-success} + exp3=${5:-success} + + test_expect_$exp1 "$desc, git-style file creation patch" " + reset_preimage && + echo postimage >'$postimage' && + git add -N '$postimage' && + git diff HEAD >'git-$desc.diff' && + + git rm -f --cached '$postimage' && + mv '$postimage' postimage.saved && + git apply -v 'git-$desc.diff' && + + test_cmp postimage.saved '$postimage' + " + + test_expect_$exp2 UNIDIFF "$desc, traditional patch" " + reset_preimage && + echo preimage >'$postimage.orig' && + echo postimage >'$postimage' && + ! diff -u '$postimage.orig' '$postimage' >'diff-$desc.diff' && + + mv '$postimage' postimage.saved && + mv '$postimage.orig' '$postimage' && + git apply -v 'diff-$desc.diff' && + + test_cmp postimage.saved '$postimage' + " + + test_expect_$exp3 FULLDIFF "$desc, traditional file creation patch" " + reset_preimage && + reset_subdirs && + echo postimage >b/'$postimage' && + ! diff -pruN a b >'add-$desc.diff' && + + rm -f '$postimage' && + mv b/'$postimage' postimage.saved && + git apply -v 'add-$desc.diff' && + + test_cmp postimage.saved '$postimage' + " +} + +try_filename 'plain' 'postimage.txt' +try_filename 'with spaces' 'post image.txt' success failure failure +try_filename 'with tab' 'post image.txt' success failure failure +try_filename 'with backslash' 'post\image.txt' +try_filename 'with quote' '"postimage".txt' success failure success + +if test_have_prereq FULLDIFF && test_have_prereq PR +then + test_set_prereq FULLDIFFPR +fi +test_expect_success FULLDIFFPR 'whitespace-damaged traditional patch' ' + reset_preimage && + reset_subdirs && + echo postimage >b/postimage.txt && + ! diff -pruN a b >diff-plain.txt && + pr -tT -e8 diff-plain.txt >damaged.diff && + + mv postimage.txt postimage.saved && + git apply -v damaged.diff && + + test_cmp postimage.saved postimage.txt +' + +test_done -- 1.7.2.rc3 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] tests: Test how well "git apply" copes with weird filenames 2010-07-24 1:11 ` [PATCH 2/3] tests: Test how well "git apply" copes with weird filenames Jonathan Nieder @ 2010-07-24 8:03 ` Andreas Schwab 2010-07-24 8:48 ` Jonathan Nieder 0 siblings, 1 reply; 7+ messages in thread From: Andreas Schwab @ 2010-07-24 8:03 UTC (permalink / raw) To: Jonathan Nieder Cc: git, Junio C Hamano, Guido Günther, Giuseppe Iuculano Jonathan Nieder <jrnieder@gmail.com> writes: > The tests assume a reasonably well-behaved “diff -u” and “pr” > to produce the (possibly whitespace-damaged) patches. On platforms > without those commands, skip the tests. pr -T is not portable. What's wrong with expand? Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] tests: Test how well "git apply" copes with weird filenames 2010-07-24 8:03 ` Andreas Schwab @ 2010-07-24 8:48 ` Jonathan Nieder 0 siblings, 0 replies; 7+ messages in thread From: Jonathan Nieder @ 2010-07-24 8:48 UTC (permalink / raw) To: Andreas Schwab; +Cc: git, Junio C Hamano, Guido Günther, Giuseppe Iuculano Andreas Schwab wrote: > pr -T is not portable. What's wrong with expand? Nothing at all. Here’s a patch for squashing in. -- 8< -- Subject: t4135 (apply): use expand instead of pr for portability expand is just the thing for expanding tabs into spaces, and unlike pr -T, it is portable. Use it. Noticed-by: Andreas Schwab <schwab@linux-m68k.org> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- Thank you. :) t/t4135-apply-weird-filenames.sh | 17 ++--------------- 1 files changed, 2 insertions(+), 15 deletions(-) diff --git i/t/t4135-apply-weird-filenames.sh w/t/t4135-apply-weird-filenames.sh index 2dcb040..dda554e 100755 --- i/t/t4135-apply-weird-filenames.sh +++ w/t/t4135-apply-weird-filenames.sh @@ -34,15 +34,6 @@ test_expect_success 'setup: test prerequisites' ' if diff -pruN 1 2 then test_set_prereq FULLDIFF - fi && - - echo "tab -> ." >expected && - echo "tab -> ." >with-tab && - - pr -tT -e8 with-tab >actual && - if test_cmp expected actual - then - test_set_prereq PR fi ' @@ -99,16 +90,12 @@ try_filename 'with tab' 'post image.txt' success failure failure try_filename 'with backslash' 'post\image.txt' try_filename 'with quote' '"postimage".txt' success failure success -if test_have_prereq FULLDIFF && test_have_prereq PR -then - test_set_prereq FULLDIFFPR -fi -test_expect_success FULLDIFFPR 'whitespace-damaged traditional patch' ' +test_expect_success FULLDIFF 'whitespace-damaged traditional patch' ' reset_preimage && reset_subdirs && echo postimage >b/postimage.txt && ! diff -pruN a b >diff-plain.txt && - pr -tT -e8 diff-plain.txt >damaged.diff && + expand diff-plain.txt >damaged.diff && mv postimage.txt postimage.saved && git apply -v damaged.diff && -- ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] apply: Handle traditional patches with space in filename 2010-07-24 1:06 [PATCH 0/3] apply: handle traditional patches with spaces in filename Jonathan Nieder 2010-07-24 1:09 ` [PATCH 1/3] apply: Split quoted filename handling into new function Jonathan Nieder 2010-07-24 1:11 ` [PATCH 2/3] tests: Test how well "git apply" copes with weird filenames Jonathan Nieder @ 2010-07-24 1:20 ` Jonathan Nieder 2 siblings, 0 replies; 7+ messages in thread From: Jonathan Nieder @ 2010-07-24 1:20 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Guido Günther, Giuseppe Iuculano To discover filenames from the --- and +++ lines in a traditional unified diff, currently ‘git apply’ scans forward for a whitespace character on each line and stops there. It can’t use the whole line because ‘diff -u’ likes to include timestamps, like so: --- foo 2000-07-12 16:56:50.020000414 -0500 +++ bar 2010-07-12 16:56:50.020000414 -0500 The whitespace-seeking heuristic works great, even when the tab has been converted to spaces by some email + copy-and-paste related corruption. Except for one problem: if the filename itself contains whitespace, the inferred filename will be too short. When Giuseppe ran into this problem, it was for a file creation patch (filename ‘debian/licenses/LICENSE.global BSD-style Chromium’). So one can’t use the list of files present in the index to deduce an appropriate filename (not to mention that way lies madness; see v0.99~402, 2005-05-31). Instead, look for a timestamp and use that if present to mark the end of the filename. If no timestamp is present, the old heuristic is used, with one exception: the space character \040 is not considered terminating whitespace any more unless it is followed by a timestamp. Reported-by: Giuseppe Iuculano <iuculano@debian.org> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Acked-by: Guido Günther <agx@sigxcpu.org> --- Guido, I have carried over your ack from <http://bugs.debian.org/578752>. I hope that is okay. And to both Guido and Giuseppe, sorry to have taken so long on this. That is the end of the series. I hope it was not too unpleasant a read. As always, thoughts, improvements, bugs welcome. Regards, Jonathan builtin/apply.c | 193 +++++++++++++++++++++++++++++++++++--- t/t4135-apply-weird-filenames.sh | 4 +- 2 files changed, 181 insertions(+), 16 deletions(-) diff --git a/builtin/apply.c b/builtin/apply.c index efc109e..b975c99 100644 --- a/builtin/apply.c +++ b/builtin/apply.c @@ -449,23 +449,157 @@ static char *find_name_gnu(const char *line, char *def, int p_value) return squash_slash(strbuf_detach(&name, NULL)); } -static char *find_name(const char *line, char *def, int p_value, int terminate) +static size_t tz_len(const char *line, size_t len) +{ + const char *tz, *p; + + if (len < strlen(" +0500") || line[len-strlen(" +0500")] != ' ') + return 0; + tz = line + len - strlen(" +0500"); + + if (tz[1] != '+' && tz[1] != '-') + return 0; + + for (p = tz + 2; p != line + len; p++) + if (!isdigit(*p)) + return 0; + + return line + len - tz; +} + +static size_t date_len(const char *line, size_t len) +{ + const char *date, *p; + + if (len < strlen("72-02-05") || line[len-strlen("-05")] != '-') + return 0; + p = date = line + len - strlen("72-02-05"); + + if (!isdigit(*p++) || !isdigit(*p++) || *p++ != '-' || + !isdigit(*p++) || !isdigit(*p++) || *p++ != '-' || + !isdigit(*p++) || !isdigit(*p++)) /* Not a date. */ + return 0; + + if (date - line >= strlen("19") && + isdigit(date[-1]) && isdigit(date[-2])) /* 4-digit year */ + date -= strlen("19"); + + return line + len - date; +} + +static size_t short_time_len(const char *line, size_t len) +{ + const char *time, *p; + + if (len < strlen(" 07:01:32") || line[len-strlen(":32")] != ':') + return 0; + p = time = line + len - strlen(" 07:01:32"); + + /* Permit 1-digit hours? */ + if (*p++ != ' ' || + !isdigit(*p++) || !isdigit(*p++) || *p++ != ':' || + !isdigit(*p++) || !isdigit(*p++) || *p++ != ':' || + !isdigit(*p++) || !isdigit(*p++)) /* Not a time. */ + return 0; + + return line + len - time; +} + +static size_t fractional_time_len(const char *line, size_t len) +{ + const char *p; + size_t n; + + /* Expected format: 19:41:17.620000023 */ + if (!len || !isdigit(line[len - 1])) + return 0; + p = line + len - 1; + + /* Fractional seconds. */ + while (p > line && isdigit(*p)) + p--; + if (*p != '.') + return 0; + + /* Hours, minutes, and whole seconds. */ + n = short_time_len(line, p - line); + if (!n) + return 0; + + return line + len - p + n; +} + +static size_t trailing_spaces_len(const char *line, size_t len) +{ + const char *p; + + /* Expected format: ' ' x (1 or more) */ + if (!len || line[len - 1] != ' ') + return 0; + + p = line + len; + while (p != line) { + p--; + if (*p != ' ') + return line + len - (p + 1); + } + + /* All spaces! */ + return len; +} + +static size_t diff_timestamp_len(const char *line, size_t len) +{ + const char *end = line + len; + size_t n; + + /* + * Posix: 2010-07-05 19:41:17 + * GNU: 2010-07-05 19:41:17.620000023 -0500 + */ + + if (!isdigit(end[-1])) + return 0; + + n = tz_len(line, end - line); + end -= n; + + n = short_time_len(line, end - line); + if (!n) + n = fractional_time_len(line, end - line); + end -= n; + + n = date_len(line, end - line); + if (!n) /* No date. Too bad. */ + return 0; + end -= n; + + if (end == line) /* No space before date. */ + return 0; + if (end[-1] == '\t') { /* Success! */ + end--; + return line + len - end; + } + if (end[-1] != ' ') /* No space before date. */ + return 0; + + /* Whitespace damage. */ + end -= trailing_spaces_len(line, end - line); + return line + len - end; +} + +static char *find_name_common(const char *line, char *def, int p_value, + const char *end, int terminate) { int len; const char *start = NULL; - if (*line == '"') { - char *name = find_name_gnu(line, def, p_value); - if (name) - return name; - } - if (p_value == 0) start = line; - for (;;) { + while (line != end) { char c = *line; - if (isspace(c)) { + if (!end && isspace(c)) { if (c == '\n') break; if (name_terminate(start, line-start, c, terminate)) @@ -505,6 +639,37 @@ static char *find_name(const char *line, char *def, int p_value, int terminate) return squash_slash(xmemdupz(start, len)); } +static char *find_name(const char *line, char *def, int p_value, int terminate) +{ + if (*line == '"') { + char *name = find_name_gnu(line, def, p_value); + if (name) + return name; + } + + return find_name_common(line, def, p_value, NULL, terminate); +} + +static char *find_name_traditional(const char *line, char *def, int p_value) +{ + size_t len = strlen(line); + size_t date_len; + + if (*line == '"') { + char *name = find_name_gnu(line, def, p_value); + if (name) + return name; + } + + len = strchrnul(line, '\n') - line; + date_len = diff_timestamp_len(line, len); + if (!date_len) + return find_name(line, def, p_value, TERM_TAB); + len -= date_len; + + return find_name_common(line, def, p_value, line + len, 0); +} + static int count_slashes(const char *cp) { int cnt = 0; @@ -527,7 +692,7 @@ static int guess_p_value(const char *nameline) if (is_dev_null(nameline)) return -1; - name = find_name(nameline, NULL, 0, TERM_SPACE | TERM_TAB); + name = find_name_traditional(nameline, NULL, 0); if (!name) return -1; cp = strchr(name, '/'); @@ -646,16 +811,16 @@ static void parse_traditional_patch(const char *first, const char *second, struc if (is_dev_null(first)) { patch->is_new = 1; patch->is_delete = 0; - name = find_name(second, NULL, p_value, TERM_SPACE | TERM_TAB); + name = find_name_traditional(second, NULL, p_value); patch->new_name = name; } else if (is_dev_null(second)) { patch->is_new = 0; patch->is_delete = 1; - name = find_name(first, NULL, p_value, TERM_SPACE | TERM_TAB); + name = find_name_traditional(first, NULL, p_value); patch->old_name = name; } else { - name = find_name(first, NULL, p_value, TERM_SPACE | TERM_TAB); - name = find_name(second, name, p_value, TERM_SPACE | TERM_TAB); + name = find_name_traditional(first, NULL, p_value); + name = find_name_traditional(second, name, p_value); if (has_epoch_timestamp(first)) { patch->is_new = 1; patch->is_delete = 0; diff --git a/t/t4135-apply-weird-filenames.sh b/t/t4135-apply-weird-filenames.sh index 2dcb040..52f9f5b 100755 --- a/t/t4135-apply-weird-filenames.sh +++ b/t/t4135-apply-weird-filenames.sh @@ -94,8 +94,8 @@ try_filename() { } try_filename 'plain' 'postimage.txt' -try_filename 'with spaces' 'post image.txt' success failure failure -try_filename 'with tab' 'post image.txt' success failure failure +try_filename 'with spaces' 'post image.txt' +try_filename 'with tab' 'post image.txt' try_filename 'with backslash' 'post\image.txt' try_filename 'with quote' '"postimage".txt' success failure success -- 1.7.2.rc3 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* What's cooking in git.git (Aug 2010, #02; Wed, 11) @ 2010-08-11 23:35 Junio C Hamano 2010-08-13 21:44 ` jn/apply-filename-with-sp (Re: What's cooking in git.git (Aug 2010, #02; Wed, 11)) Johannes Sixt 0 siblings, 1 reply; 7+ messages in thread From: Junio C Hamano @ 2010-08-11 23:35 UTC (permalink / raw) To: git Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' while commits prefixed with '+' are in 'next'. The ones marked with '.' do not appear in any of the integration branches, but I am still holding onto them. Has the rate of patch traffic increased markedly for the past few months? I am feeling perpetually behind these days... -------------------------------------------------- [New Topics] * ab/omit-aggregate-test-result-under-tap-harness (2010-08-11) 1 commit - test-lib: Don't write test-results when HARNESS_ACTIVE * ab/test-prereq (2010-08-11) 5 commits - test-lib: Multi-prereq support only checked the last prereq - tests: A SANITY test prereq for testing if we're root - t/README: Document the predefined test prerequisites - test-lib: Print missing prerequisites in test output - test-lib: Add support for multiple test prerequisites * ab/test-smoke (2010-08-08) 4 commits - t/README: Add SMOKE_{COMMENT,TAGS}= to smoke_report target - t/Makefile: Can't include GIT-BUILD-OPTIONS, it's a .sh - t/README: Document the Smoke testing - tests: Infrastructure for Git smoke testing Will merge the above three topics to 'next' together with other "test" topics from the same author shortly. * by/line-log (2010-08-11) 18 commits - line-log: minimum compilation fix - Document line history browser - Add tests for line history browser - Add --full-line-diff option - Add --graph prefix before line history output - Add parent rewriting to line history browser - Make graph_next_line external to other part of git - Make rewrite_parents public to other part of git - Hook line history into cmd_log, ensuring a topo-ordered walk - Print the line log - map/take range to the parent of commits - Add range clone functions - Export three functions from diff.c - Parse the -L options - Refactor parse_loc - Add the basic data structure for line level history - parse-options: add two helper functions - parse-options: enhance STOP_AT_NON_OPTION Does not seem to pass the self test for me... * dg/local-mod-error-messages (2010-08-11) 5 commits - t7609: test merge and checkout error messages - unpack_trees: group error messages by type - merge-recursive: distinguish "removed" and "overwritten" messages - merge-recursive: porcelain messages for checkout - Turn unpack_trees_options.msgs into an array + enum Looking good. * en/rebase-against-rebase-fix (2010-08-06) 2 commits - pull --rebase: Avoid spurious conflicts and reapplying unnecessary patches - t5520-pull: Add testcases showing spurious conflicts from git pull --rebase * gb/split-cmdline-errmsg (2010-08-07) 1 commit - split_cmdline: Allow caller to access error string * jc/builtin-binsearch (2010-08-05) 2 commits . git.c: binary search in the built-in command list . git.c: sort the list of built-in command names * jl/submodule-ignore-diff (2010-08-06) 4 commits - Add tests for the diff.ignoreSubmodules config option - Add the 'diff.ignoreSubmodules' config setting - Submodules: Use "ignore" settings from .gitmodules too for diff and status - Submodules: Add the new "ignore" config option for diff and status Looking good. * jn/commit-no-change-wo-status (2010-07-24) 9 commits . commit: suppress status summary when no changes staged . commit --dry-run: give advice on empty amend . commit: give empty-commit avoidance code its own function . t7508 (status): modernize style . commit: split off the piece that writes status . commit: split commit -s handling into its own function . commit: split off a function to fetch the default log message . wt-status: split off a function for printing submodule summary . wt-status: split wt_status_print into digestible pieces * jn/merge-renormalize (2010-08-05) 12 commits . merge-recursive --renormalize . rerere: never renormalize . rerere: migrate to parse-options API . t4200 (rerere): modernize style . ll-merge: let caller decide whether to renormalize . ll-merge: make flag easier to populate . Documentation/technical: document ll_merge . merge-trees: let caller decide whether to renormalize . merge-trees: push choice to renormalize away from low level . t6038 (merge.renormalize): check that it can be turned off . t6038 (merge.renormalize): try checkout -m and cherry-pick . t6038 (merge.renormalize): style nitpicks (this branch uses eb/double-convert-before-merge.) * jn/paginate-fix (2010-08-05) 13 commits . merge-file: run setup_git_directory_gently() sooner . var: run setup_git_directory_gently() sooner . ls-remote: run setup_git_directory_gently() sooner . index-pack: run setup_git_directory_gently() sooner . Merge branch 'jn/maint-setup-fix' (early part) into HEAD . config: run setup_git_directory_gently() sooner . bundle: run setup_git_directory_gently() sooner . apply: run setup_git_directory_gently() sooner . grep: run setup_git_directory_gently() sooner . shortlog: run setup_git_directory_gently() sooner . git wrapper: allow setup_git_directory_gently() be called earlier . setup: remember whether repository was found . git wrapper: introduce startup_info struct (this branch uses jn/maint-setup-fix.) I had trouble merging these three topics to 'pu' and ran out of time my git Wednesday this week. I think the "no-change-wo-status" one had some unintended consequences discovered on the list? * jn/maint-plug-leak (2010-08-09) 3 commits - write-tree: Avoid leak when index refers to an invalid object - read-tree: stop leaking tree objects - core: Stop leaking ondisk_cache_entrys Will fast-track merging to 'next', 'master' and 'maint' after giving it another look. * jn/svn-fe (2010-08-09) 10 commits - svn-fe manual: Clarify warning about deltas in dump files - Update svn-fe manual - SVN dump parser - Infrastructure to write revisions in fast-export format - Add stream helper library - Add string-specific memory pool - Add treap implementation - Add memory pool library - Introduce vcs-svn lib - Export parse_date_basic() to convert a date string to timestamp There was a screw-up on my part---I should have dropped the first one and branched this off of jn/parse-date-basic (I'll fix it up before merging this to 'next'). * js/detached-stash (2010-08-10) 9 commits - Documentation: git stash branch now tolerates non-stash references. - t3903-stash.sh: tests of git stash with stash-like arguments - stash: teach git stash show to always tolerate stash-like arguments. - stash: teach git stash branch to tolerate stash-like arguments - stash: teach git stash drop to fail early if the specified revision is not a stash reference - stash: teach git stash pop to fail early if the argument is not a stash ref - stash: introduce is_stash_ref and assert_stash_ref functions. - stash: extract stash-like check into its own function - stash: refactor - create pop_stash function Haven't looked this new round closely yet. * mm/rebase-i-exec (2010-08-10) 2 commits - test-lib: user-friendly alternatives to test [-d|-f|-e] - rebase -i: add exec command to launch a shell command Looking good. * so/http-user-agent (2010-08-11) 1 commit - Allow HTTP user agent string to be modified. Looking good. * sp/fix-smart-http-deadlock-on-error (2010-08-06) 1 commit (merged to 'next' on 2010-08-11 at 0a6369e) + smart-http: Don't deadlock on server failure Will fast-track merging to 'master' and 'maint' after giving it another look. -------------------------------------------------- [Ejected] * rr/svn-export (2010-07-29) 9 commits . vcs-svn: Remove stray calls to removed functions . Add SVN dump parser . Add infrastructure to write revisions in fast-export format . Add stream helper library . Add string-specific memory pool . vcs-svn: treap_search should return NULL for missing items . Add treap implementation . Add memory pool library . Introduce vcs-svn lib (this branch uses jn/parse-date-basic.) Replaced with Jonathan's reroll. -------------------------------------------------- [Stalled] * zl/mailinfo-recode-patch (2010-06-14) 2 commits - add --recode-patch option to git-am - add --recode-patch option to git-mailinfo I recall there was another round of re-roll planned for this one. * jk/tag-contains (2010-07-05) 4 commits - Why is "git tag --contains" so slow? - default core.clockskew variable to one day - limit "contains" traversals based on commit timestamp - tag: speed up --contains calculation -------------------------------------------------- [Cooking] * jc/sha1-name-find-fix (2010-08-02) 1 commit (merged to 'next' on 2010-08-11 at 51106a5) + sha1_name.c: fix parsing of ":/token" syntax Will merge to 'master' (and perhaps 'maint'). * jn/doc-pull (2010-08-02) 1 commit (merged to 'next' on 2010-08-11 at a223479) + Documentation: flesh out “git pull” description Will merge to 'master'. * jn/maint-gitweb-dynconf (2010-07-30) 1 commit (merged to 'next' on 2010-08-11 at a94ce85) + gitweb: allow configurations that change with each request Will merge to 'master'. * sr/local-config (2010-08-03) 1 commit - config: add --local option Looked Ok. Will merge to 'next'. * ab/test-coverage (2010-07-26) 8 commits - Makefile: make gcov invocation configurable - t/README: Add a note about the dangers of coverage chasing - t/README: A new section about test coverage - Makefile: Add cover_db_html target - Makefile: Add cover_db target - Makefile: Split out the untested functions target - Makefile: Include subdirectories in "make cover" reports - gitignore: Ignore files generated by "make coverage" * ab/test-no-skip (2010-08-10) 13 commits - git-notes: Run partial expensive test everywhere - t/t3300-funny-names: change from skip_all=* to prereq skip - t/t3902-quoted: change from skip_all=* to prereq skip - t/t4016-diff-quote: change from skip_all=* to prereq skip - t/t5503-tagfollow: change from skip_all=* to prereq skip - t/t7005-editor: change from skip_all=* to prereq skip - t/t5705-clone-2gb: change from skip_all=* to prereq skip - t/t1304-default-acl: change from skip_all=* to prereq skip - t/README: Update "Skipping tests" to align with best practices - t/t7800-difftool.sh: Skip with prereq on no PERL - t/t5800-remote-helpers.sh: Skip with prereq on python <2.4 - t/t4004-diff-rename-symlink.sh: use three-arg <prereq> - tests: implicitly skip SYMLINKS tests using <prereq> * bc/use-more-hardlinks-in-install (2010-07-23) 2 commits (merged to 'next' on 2010-08-11 at 59dd30e) + Makefile: make hard/symbolic links for non-builtins too + Makefile: link builtins residing in bin directory to main git binary too Will merge to 'master'. * cc/find-commit-subject (2010-07-22) 6 commits - blame: use find_commit_subject() instead of custom code - merge-recursive: use find_commit_subject() instead of custom code - bisect: use find_commit_subject() instead of custom code - revert: rename variables related to subject in get_message() - revert: refactor code to find commit subject in find_commit_subject() - revert: fix off by one read when searching the end of a commit subject Looked Ok. Will merge to 'next'. * gb/shell-ext (2010-07-28) 3 commits - Add sample commands for git-shell - Add interactive mode to git-shell for user-friendliness - Allow creation of arbitrary git-shell commands * jc/log-grep (2010-07-19) 1 commit - git log: add -G<regexp> that greps in the patch text This is broken, but haven't found any time to revisit it yet. * jh/clean-exclude (2010-07-20) 2 commits - Add test for git clean -e. - Add -e/--exclude to git-clean. Looked Ok. Will merge to 'next'. * jh/use-test-must-fail (2010-07-20) 1 commit - Convert "! git" to "test_must_fail git" Looked Ok. Will merge to 'next'. * jn/apply-filename-with-sp (2010-07-23) 4 commits - apply: Handle traditional patches with space in filename - t4135 (apply): use expand instead of pr for portability - tests: Test how well "git apply" copes with weird filenames - apply: Split quoted filename handling into new function Looked Ok. Will merge to 'next'. * jn/fix-abbrev (2010-07-27) 3 commits - examples/commit: use --abbrev for commit summary - checkout, commit: remove confusing assignments to rev.abbrev - archive: abbreviate substituted commit ids again Looked Ok. Will merge to 'next'. * jn/maint-setup-fix (2010-07-24) 11 commits - setup: split off a function to handle ordinary .git directories - Revert "rehabilitate 'git index-pack' inside the object store" - setup: do not forget working dir from subdir of gitdir - t4111 (apply): refresh index before applying patches to it - setup: split off get_device_or_die helper - setup: split off a function to handle hitting ceiling in repo search - setup: split off code to handle stumbling upon a repository - setup: split off a function to checks working dir for .git file - setup: split off $GIT_DIR-set case from setup_git_directory_gently - tests: try git apply from subdir of toplevel - t1501 (rev-parse): clarify (this branch is used by jn/paginate-fix.) * jn/rebase-rename-am (2008-11-10) 5 commits - rebase: protect against diff.renames configuration - t3400 (rebase): whitespace cleanup - Teach "apply --index-info" to handle rename patches - t4150 (am): futureproof against failing tests - t4150 (am): style fix Will merge to 'next'. * ml/rebase-x-strategy (2010-07-29) 1 commit - rebase: support -X to pass through strategy options Looked Ok. Will merge to 'next'. * mm/shortopt-detached (2010-08-05) 5 commits - log: parse separate option for --glob - log: parse separate options like git log --grep foo - diff: parse separate options --stat-width n, --stat-name-width n - diff: split off a function for --stat-* option parsing - diff: parse separate options like -S foo Looked Ok. Will merge to 'next'. * nd/fix-sparse-checkout (2010-07-31) 5 commits - unpack-trees: mark new entries skip-worktree appropriately - unpack-trees: do not check for conflict entries too early - unpack-trees: let read-tree -u remove index entries outside sparse area - unpack-trees: only clear CE_UPDATE|CE_REMOVE when skip-worktree is always set - t1011 (sparse checkout): style nitpicks Looked Ok. Will merge to 'next'. * tr/ab-i18n-fix (2010-07-25) 1 commit - tests: locate i18n lib&data correctly under --valgrind (this branch uses ab/i18n.) Looked Ok. Will merge to 'next'. * tr/maint-no-unquote-plus (2010-07-24) 1 commit - Do not unquote + into ' ' in URLs * tr/xsize-bits (2010-07-28) 1 commit (merged to 'next' on 2010-08-11 at bcc0271) + xsize_t: check whether we lose bits * vs/doc-spell (2010-07-20) 1 commit - Documentation: spelling fixes Looked Ok. Will merge to 'next'. * ab/report-corrupt-object-with-type (2010-06-10) 1 commit - sha1_file: Show the the type and path to corrupt objects Looked Ok. Will merge to 'next'. * cc/revert (2010-07-21) 5 commits - t3508: add check_head_differs_from() helper function and use it - revert: improve success message by adding abbreviated commit sha1 - revert: don't print "Finished one cherry-pick." if commit failed - revert: refactor commit code into a new run_git_commit() function - revert: report success when using option --strategy * en/fast-export-fix (2010-07-17) 2 commits - fast-export: Add a --full-tree option - fast-export: Fix dropping of files with --import-marks and path limiting * jn/parse-date-basic (2010-07-15) 1 commit (merged to 'next' on 2010-08-11 at ca9fef0) + Export parse_date_basic() to convert a date string to timestamp (this branch is used by rr/svn-export.) Will merge to 'master'. * kf/post-receive-sample-hook (2010-07-16) 1 commit - post-receive-email: optional message line count limit I do not particularly like the idea of counting number of lines in a shell loop but this is an opt-in feature to a contrib/ item so it should be ok. Will merge to 'next'. * tr/rfc-reset-doc (2010-07-18) 5 commits (merged to 'next' on 2010-08-11 at 8e7c8d1) + Documentation/reset: move "undo permanently" example behind "make topic" + Documentation/reset: reorder examples to match description + Documentation/reset: promote 'examples' one section up + Documentation/reset: separate options by mode + Documentation/git-reset: reorder modes for soft-mixed-hard progression Looked fine. Will merge to 'master'. * hv/autosquash-config (2010-07-14) 1 commit - add configuration variable for --autosquash option of interactive rebase * jh/graph-next-line (2010-07-13) 2 commits (merged to 'next' on 2010-08-11 at 333f9df) + Enable custom schemes for column colors in the graph API + Make graph_next_line() available in the graph.h API * ar/string-list-foreach (2010-07-03) 2 commits (merged to 'next' on 2010-08-11 at 993dc02) + Convert the users of for_each_string_list to for_each_string_list_item macro + Add a for_each_string_list_item macro (this branch is used by tf/string-list-init.) * il/rfc-remote-fd-ext (2010-07-31) 4 commits - Rewrite bidirectional traffic loop - gitignore: Ignore the new /git-remote-{ext,fd} helpers - New remote helper: git-remote-ext - New remote helper git-remote-fd * hv/submodule-find-ff-merge (2010-07-07) 3 commits (merged to 'next' on 2010-08-11 at 6900d2c) + Implement automatic fast-forward merge for submodules + setup_revisions(): Allow walking history in a submodule + Teach ref iteration module about submodules * jn/fast-import-subtree (2010-06-30) 1 commit (merged to 'next' on 2010-08-11 at 5e19de0) + Teach fast-import to import subtrees named by tree id * sg/rerere-gc-old-still-used (2010-07-13) 2 commits - rerere: fix overeager gc - mingw_utime(): handle NULL times parameter * tf/string-list-init (2010-07-04) 1 commit - string_list: Add STRING_LIST_INIT macro and make use of it. (this branch uses ar/string-list-foreach.) * en/d-f-conflict-fix (2010-07-27) 7 commits (merged to 'next' on 2010-08-03 at 7f78604) + t/t6035-merge-dir-to-symlink.sh: Remove TODO on passing test + fast-import: Improve robustness when D->F changes provided in wrong order + fast-export: Fix output order of D/F changes + merge_recursive: Fix renames across paths below D/F conflicts + merge-recursive: Fix D/F conflicts + Add a rename + D/F conflict testcase + Add additional testcases for D/F conflicts * ab/i18n (2010-07-19) 2 commits - tests: rename test to work around GNU gettext bug - Add infrastructure for translating Git with gettext (this branch is used by tr/ab-i18n-fix.) * tc/checkout-B (2010-08-10) 4 commits (merged to 'next' on 2010-08-11 at 5433b51) + builtin/checkout: handle -B from detached HEAD correctly + builtin/checkout: learn -B + builtin/checkout: reword hint for -b + add tests for checkout -b * eb/double-convert-before-merge (2010-07-02) 3 commits - Don't expand CRLFs when normalizing text during merge - Try normalizing files to avoid delete/modify conflicts when merging - Avoid conflicts when merging branches with mixed normalization (this branch is used by jn/merge-renormalize.) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: jn/apply-filename-with-sp (Re: What's cooking in git.git (Aug 2010, #02; Wed, 11)) @ 2010-08-13 21:44 ` Johannes Sixt 2010-08-14 2:27 ` Jonathan Nieder 0 siblings, 1 reply; 7+ messages in thread From: Johannes Sixt @ 2010-08-13 21:44 UTC (permalink / raw) To: Jonathan Nieder Cc: Johannes Sixt, Junio C Hamano, git, Greg Brockman, Ilari Liusvaara, Elijah Newren Am 13.08.2010 00:40, schrieb Jonathan Nieder: > Johannes Sixt wrote: >> Am 8/12/2010 1:35, schrieb Junio C Hamano: > >>> * jn/apply-filename-with-sp (2010-07-23) 4 commits >>> - apply: Handle traditional patches with space in filename >>> - t4135 (apply): use expand instead of pr for portability >>> - tests: Test how well "git apply" copes with weird filenames >>> - apply: Split quoted filename handling into new function >>> >>> Looked Ok. Will merge to 'next'. >> >> The test cases that work with files with tabs must have a prerequisite; >> see t3600-rm.sh. > > Would this work? > > -- 8< -- > Subject: t4135 (apply): filenames with tabs are not usable on NTFS > > Move the code setting up the FUNNYNAMES prerequisite from > v1.3.0-rc1~67 (workaround fat/ntfs deficiences for t3600-rm.sh, > 2006-03-03) to test-lib and use it. > > Reported-by: Johannes Sixt<j6t@kdbg.org> > Signed-off-by: Jonathan Nieder<jrnieder@gmail.com> No, it needs this squashed in (warning, whitespace damaged), except that the prerequisite EXPAND is nowhere defined - it is only a hint for you(?) that we need something else there because we don't have expand on Windows. I don't know why the test with quotes fails. It must be a bug in MSYS bash, because the file system does not have any problems with quotes in file names. diff --git a/t/t4135-apply-weird-filenames.sh b/t/t4135-apply-weird-filenames.sh index 5c7165e..6730a30 100755 --- a/t/t4135-apply-weird-filenames.sh +++ b/t/t4135-apply-weird-filenames.sh @@ -88,10 +88,10 @@ try_filename() { try_filename 'plain' 'postimage.txt' try_filename 'with spaces' 'post image.txt' try_filename 'with tab' 'post image.txt' FUNNYNAMES -try_filename 'with backslash' 'post\image.txt' -try_filename 'with quote' '"postimage".txt' '' success failure success +try_filename 'with backslash' 'post\image.txt' BSLASHPSPEC +try_filename 'with quote' '"postimage".txt' FUNNYNAMES success failure success -test_expect_success FULLDIFF 'whitespace-damaged traditional patch' ' +test_expect_success EXPAND,FULLDIFF 'whitespace-damaged traditional patch' ' reset_preimage && reset_subdirs && echo postimage >b/postimage.txt && diff --git a/t/test-lib.sh b/t/test-lib.sh index 1e5640b..d50e787 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -963,8 +963,8 @@ test -z "$NO_GETTEXT" && test_set_prereq GETTEXT f1='newline embedded' if - >"$f1" 2>/dev/null && - >'tab embedded' 2>/dev/null + { >"$f1"; } 2>/dev/null && + { >'tab embedded'; } 2>/dev/null then test_set_prereq FUNNYNAMES fi Without the braces in this check the error message is still printed. I didn't test whether just writing 2>/dev/null first would suppress it as well. BTW, I would prefer to keep the test in t3600 and t4135, that it doesn't happen for every other test script unnecessarily. -- Hannes ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: jn/apply-filename-with-sp (Re: What's cooking in git.git (Aug 2010, #02; Wed, 11)) 2010-08-13 21:44 ` jn/apply-filename-with-sp (Re: What's cooking in git.git (Aug 2010, #02; Wed, 11)) Johannes Sixt @ 2010-08-14 2:27 ` Jonathan Nieder 2010-08-14 18:37 ` Johannes Sixt 0 siblings, 1 reply; 7+ messages in thread From: Jonathan Nieder @ 2010-08-14 2:27 UTC (permalink / raw) To: Johannes Sixt Cc: Junio C Hamano, git, Greg Brockman, Ilari Liusvaara, Elijah Newren Johannes Sixt wrote: > Am 13.08.2010 00:40, schrieb Jonathan Nieder: >> Would this work? >> >> -- 8< -- >> Subject: t4135 (apply): filenames with tabs are not usable on NTFS [...] > No, it needs this squashed in Ah, makes sense. Here’s another rough patch. Patch is against jn/apply-filename-with-sp + ab/test. Not signed off because I am not sure about the licensing of expand.sed (and I would rather see some other fix instead). -- 8< -- Subject: t4135 (apply): tweaks for Windows Filenames with tabs are not usable on NTFS, so mimic the code setting up the FUNNYNAMES prerequisite from v1.3.0-rc1~67 (workaround fat/ntfs deficiences for t3600-rm.sh, 2006-03-03) and use it. The code is not shared in test-lib.sh to avoid wasting time on that check while running other tests. Backslashes is the path separator on Windows, so it cannot be used in filenames (see v1.6.3-rc0~93^2~6, t3700: Skip a test with backslashes in pathspec, 2009-03-13). Filenames starting with a quotation mark do not behave well in msys (see v1.7.0-rc0~94^2, t4030, t4031: work around bogus MSYS bash path conversion, 2010-01-01), so skip those tests on Windows. “expand” is not available in msysgit. Use a sed script by Greg Ubben to replace it. Reported-by: Johannes Sixt <j6t@kdbg.org> Helped-by: Johannes Sixt <j6t@kdbg.org> Not-signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- t/t4135-apply-weird-filenames.sh | 70 ++++++++++++++++++++++++++++++++------ 1 files changed, 59 insertions(+), 11 deletions(-) diff --git a/t/t4135-apply-weird-filenames.sh b/t/t4135-apply-weird-filenames.sh index f4c7e15..08f8fe2 100755 --- a/t/t4135-apply-weird-filenames.sh +++ b/t/t4135-apply-weird-filenames.sh @@ -10,7 +10,7 @@ test_expect_success 'setup: empty commit' ' git tag preimage ' -test_expect_success 'setup: clean-up functions' ' +test_expect_success 'setup: helper functions' ' reset_preimage() { git checkout -f preimage^0 && git read-tree -u --reset HEAD && @@ -20,6 +20,44 @@ test_expect_success 'setup: clean-up functions' ' reset_subdirs() { rm -fr a b && mkdir a b + } && + + cat <<-\EOF >expand.sed && + #! /bin/sed -f + # @(#)14apr89/31aug01 expand.sed by Greg Ubben + + / /!b + + # Change the text before a tab to + # text<MARKER>text<TAB><8 blanks><TAB> + # + s/\([^ ]*\) /\1Q& /g + + # Reduce the text between the marker and the tab to less + # than eight characters. We have to put in 8-(length MOD 8) + # blanks, and this effectively does the modulo operation. + :a + s/Q[^ ]\{8\}/Q/g + ta + + # The buffer is now: + # text<MARKER><(length MOD 8) characters><TAB><expansion><extra blanks><TAB> + # ----------------------------------------- + # Notice that the expansion is 8-(length MOD 8), so the + # underlined part is exactly nine characters. That is how + # we discard the extra blanks and the tabs. + # + s/\(Q.\{9\}\) * /\1/g + + # We have now: + # text<MARKER><(length MOD 8) characters><TAB><expansion> + # + # so we discard everything between the marker and the tab + # + s/Q[^ ]* //g + EOF + expand_tabs() { + sed -f "$TRASH_DIRECTORY/expand.sed" "$@" } ' @@ -34,17 +72,25 @@ test_expect_success 'setup: test prerequisites' ' if diff -pruN 1 2 then test_set_prereq FULLDIFF + fi && + + test_when_finished "rm -f \"tab embedded.txt\"" && + test_when_finished "rm -f '\''\"quoteembedded\".txt'\''" && + if touch -- "tab embedded.txt" '\''"quoteembedded".txt'\'' + then + test_set_prereq FUNNYNAMES fi ' try_filename() { desc=$1 postimage=$2 - exp1=${3:-success} - exp2=${4:-success} - exp3=${5:-success} + prereq=${3:-} + exp1=${4:-success} + exp2=${5:-success} + exp3=${6:-success} - test_expect_$exp1 "$desc, git-style file creation patch" " + test_expect_$exp1 $prereq "$desc, git-style file creation patch" " reset_preimage && echo postimage >'$postimage' && git add -N '$postimage' && @@ -57,7 +103,8 @@ try_filename() { test_cmp postimage.saved '$postimage' " - test_expect_$exp2 UNIDIFF "$desc, traditional patch" " + test_expect_$exp2 ${prereq:+$prereq,}UNIDIFF \ + "$desc, traditional patch" " reset_preimage && echo preimage >'$postimage.orig' && echo postimage >'$postimage' && @@ -70,7 +117,8 @@ try_filename() { test_cmp postimage.saved '$postimage' " - test_expect_$exp3 FULLDIFF "$desc, traditional file creation patch" " + test_expect_$exp3 ${prereq:+$prereq,}FULLDIFF \ + "$desc, traditional file creation patch" " reset_preimage && reset_subdirs && echo postimage >b/'$postimage' && @@ -86,16 +134,16 @@ try_filename() { try_filename 'plain' 'postimage.txt' try_filename 'with spaces' 'post image.txt' -try_filename 'with tab' 'post image.txt' -try_filename 'with backslash' 'post\image.txt' -try_filename 'with quote' '"postimage".txt' success failure success +try_filename 'with tab' 'post image.txt' FUNNYNAMES +try_filename 'with backslash' 'post\image.txt' BSLASHPSPEC +try_filename 'with quote' '"postimage".txt' FUNNYNAMES success failure success test_expect_success FULLDIFF 'whitespace-damaged traditional patch' ' reset_preimage && reset_subdirs && echo postimage >b/postimage.txt && ! diff -pruN a b >diff-plain.txt && - expand diff-plain.txt >damaged.diff && + expand_tabs diff-plain.txt >damaged.diff && mv postimage.txt postimage.saved && git apply -v damaged.diff && -- 1.7.2.1.544.ga752d.dirty ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: jn/apply-filename-with-sp (Re: What's cooking in git.git (Aug 2010, #02; Wed, 11)) 2010-08-14 2:27 ` Jonathan Nieder @ 2010-08-14 18:37 ` Johannes Sixt 2010-08-19 1:45 ` [PATCH v2 0/3] apply: handle traditional patches with space in filename Jonathan Nieder 0 siblings, 1 reply; 7+ messages in thread From: Johannes Sixt @ 2010-08-14 18:37 UTC (permalink / raw) To: Jonathan Nieder Cc: Junio C Hamano, git, Greg Brockman, Ilari Liusvaara, Elijah Newren On Samstag, 14. August 2010, Jonathan Nieder wrote: > @@ -20,6 +20,44 @@ test_expect_success 'setup: clean-up functions' ' > reset_subdirs() { > rm -fr a b && > mkdir a b > + } && > + > + cat <<-\EOF >expand.sed && ... Why not just write an explicit test vector? The result of expand will be constant, no? -- Hannes ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 0/3] apply: handle traditional patches with space in filename 2010-08-14 18:37 ` Johannes Sixt @ 2010-08-19 1:45 ` Jonathan Nieder 2010-08-19 1:50 ` [PATCH 3/3] " Jonathan Nieder 0 siblings, 1 reply; 7+ messages in thread From: Jonathan Nieder @ 2010-08-19 1:45 UTC (permalink / raw) To: Junio C Hamano Cc: Johannes Sixt, git, Greg Brockman, Ilari Liusvaara, Elijah Newren, Andreas Schwab Johannes Sixt wrote: > Why not just write an explicit test vector? The result of expand will be > constant, no? Sorry for the long delay. Here's a re-roll, meant to replace jn/apply-filename-with-sp from pu. Patch 1 fixes an edge case for git apply -p when traditional patches have quotation marks in file names (so: an edge case of an edge). But that is only a side-effect; the main purpose is to rearrange code to prepare for patch 3. Except for the commit message, it is unchanged from the version in pu. Patch 2 contains the funny-filenames tests. Testing on Windows would be welcome. The test vectors are included in the patch to avoid a source of non-determinism (for example, GNU diff could stop emiting traditional patches some day). The script used to generate them is provided to make the test easy to change. Patch 3 is basically as before, except that find_name_traditional() calls find_name_common() directly now instead of redundantly checking again for quotes at the beginning of filenames. As before, the purpose is to handle patches to files with whitespace in their names, by trying to use the timestamp instead of arbitrary whitespace as a name terminator (falling back to the old method if that fails). Thanks for your help so far. The code would be much worse without it. Jonathan Nieder (3): apply: split quoted filename handling into new function tests: exercise "git apply" with weird filenames apply: handle traditional patches with space in filename builtin/apply.c | 251 ++++++++++++++++++++++++++++++++------ t/t4120-apply-popt.sh | 35 +++++- t/t4135-apply-weird-filenames.sh | 75 +++++++++++ t/t4135/.gitignore | 3 + t/t4135/add-plain.diff | 5 + t/t4135/add-with backslash.diff | 5 + t/t4135/add-with quote.diff | 5 + t/t4135/add-with spaces.diff | 5 + t/t4135/add-with tab.diff | 5 + t/t4135/damaged.diff | 5 + t/t4135/diff-plain.diff | 5 + t/t4135/diff-with backslash.diff | 5 + t/t4135/diff-with quote.diff | 5 + t/t4135/diff-with spaces.diff | 5 + t/t4135/diff-with tab.diff | 5 + t/t4135/git-plain.diff | 7 + t/t4135/git-with backslash.diff | 7 + t/t4135/git-with quote.diff | 7 + t/t4135/git-with spaces.diff | 7 + t/t4135/git-with tab.diff | 7 + t/t4135/make-patches | 45 +++++++ 21 files changed, 457 insertions(+), 42 deletions(-) create mode 100755 t/t4135-apply-weird-filenames.sh create mode 100644 t/t4135/.gitignore create mode 100644 t/t4135/add-plain.diff create mode 100644 t/t4135/add-with backslash.diff create mode 100644 t/t4135/add-with quote.diff create mode 100644 t/t4135/add-with spaces.diff create mode 100644 t/t4135/add-with tab.diff create mode 100644 t/t4135/damaged.diff create mode 100644 t/t4135/diff-plain.diff create mode 100644 t/t4135/diff-with backslash.diff create mode 100644 t/t4135/diff-with quote.diff create mode 100644 t/t4135/diff-with spaces.diff create mode 100644 t/t4135/diff-with tab.diff create mode 100644 t/t4135/git-plain.diff create mode 100644 t/t4135/git-with backslash.diff create mode 100644 t/t4135/git-with quote.diff create mode 100644 t/t4135/git-with spaces.diff create mode 100644 t/t4135/git-with tab.diff create mode 100755 t/t4135/make-patches -- 1.7.2.1.544.ga752d.dirty ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 3/3] apply: handle traditional patches with space in filename 2010-08-19 1:45 ` [PATCH v2 0/3] apply: handle traditional patches with space in filename Jonathan Nieder @ 2010-08-19 1:50 ` Jonathan Nieder 0 siblings, 0 replies; 7+ messages in thread From: Jonathan Nieder @ 2010-08-19 1:50 UTC (permalink / raw) To: Junio C Hamano Cc: Johannes Sixt, git, Greg Brockman, Ilari Liusvaara, Elijah Newren, Andreas Schwab To discover filenames from the --- and +++ lines in a traditional unified diff, currently "git apply" scans forward for a whitespace character on each line and stops there. It can't use the whole line because "diff -u" likes to include timestamps, like so: --- foo 2000-07-12 16:56:50.020000414 -0500 +++ bar 2010-07-12 16:56:50.020000414 -0500 The whitespace-seeking heuristic works great, even when the tab has been converted to spaces by some email + copy-and-paste related corruption. Except for one problem: if the filename itself contains whitespace, the inferred filename will be too short. When Giuseppe ran into this problem, it was for a file creation patch (for debian/licenses/LICENSE.global BSD-style Chromium). So one can't use the list of files present in the index to deduce an appropriate filename (not to mention that way lies madness; see v0.99~402, 2005-05-31). Instead, look for a timestamp and use that if present to mark the end of the filename. If no timestamp is present, the old heuristic is used, with one exception: the space character \040 is not considered terminating whitespace any more unless it is followed by a timestamp. Reported-by: Giuseppe Iuculano <iuculano@debian.org> Acked-by: Guido Günther <agx@sigxcpu.org> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- Thanks for the helpful advice and patience at my use of it. builtin/apply.c | 193 +++++++++++++++++++++++++++++++++++--- t/t4135-apply-weird-filenames.sh | 4 +- 2 files changed, 181 insertions(+), 16 deletions(-) diff --git a/builtin/apply.c b/builtin/apply.c index efc109e..bd2fcb3 100644 --- a/builtin/apply.c +++ b/builtin/apply.c @@ -449,23 +449,157 @@ static char *find_name_gnu(const char *line, char *def, int p_value) return squash_slash(strbuf_detach(&name, NULL)); } -static char *find_name(const char *line, char *def, int p_value, int terminate) +static size_t tz_len(const char *line, size_t len) +{ + const char *tz, *p; + + if (len < strlen(" +0500") || line[len-strlen(" +0500")] != ' ') + return 0; + tz = line + len - strlen(" +0500"); + + if (tz[1] != '+' && tz[1] != '-') + return 0; + + for (p = tz + 2; p != line + len; p++) + if (!isdigit(*p)) + return 0; + + return line + len - tz; +} + +static size_t date_len(const char *line, size_t len) +{ + const char *date, *p; + + if (len < strlen("72-02-05") || line[len-strlen("-05")] != '-') + return 0; + p = date = line + len - strlen("72-02-05"); + + if (!isdigit(*p++) || !isdigit(*p++) || *p++ != '-' || + !isdigit(*p++) || !isdigit(*p++) || *p++ != '-' || + !isdigit(*p++) || !isdigit(*p++)) /* Not a date. */ + return 0; + + if (date - line >= strlen("19") && + isdigit(date[-1]) && isdigit(date[-2])) /* 4-digit year */ + date -= strlen("19"); + + return line + len - date; +} + +static size_t short_time_len(const char *line, size_t len) +{ + const char *time, *p; + + if (len < strlen(" 07:01:32") || line[len-strlen(":32")] != ':') + return 0; + p = time = line + len - strlen(" 07:01:32"); + + /* Permit 1-digit hours? */ + if (*p++ != ' ' || + !isdigit(*p++) || !isdigit(*p++) || *p++ != ':' || + !isdigit(*p++) || !isdigit(*p++) || *p++ != ':' || + !isdigit(*p++) || !isdigit(*p++)) /* Not a time. */ + return 0; + + return line + len - time; +} + +static size_t fractional_time_len(const char *line, size_t len) +{ + const char *p; + size_t n; + + /* Expected format: 19:41:17.620000023 */ + if (!len || !isdigit(line[len - 1])) + return 0; + p = line + len - 1; + + /* Fractional seconds. */ + while (p > line && isdigit(*p)) + p--; + if (*p != '.') + return 0; + + /* Hours, minutes, and whole seconds. */ + n = short_time_len(line, p - line); + if (!n) + return 0; + + return line + len - p + n; +} + +static size_t trailing_spaces_len(const char *line, size_t len) +{ + const char *p; + + /* Expected format: ' ' x (1 or more) */ + if (!len || line[len - 1] != ' ') + return 0; + + p = line + len; + while (p != line) { + p--; + if (*p != ' ') + return line + len - (p + 1); + } + + /* All spaces! */ + return len; +} + +static size_t diff_timestamp_len(const char *line, size_t len) +{ + const char *end = line + len; + size_t n; + + /* + * Posix: 2010-07-05 19:41:17 + * GNU: 2010-07-05 19:41:17.620000023 -0500 + */ + + if (!isdigit(end[-1])) + return 0; + + n = tz_len(line, end - line); + end -= n; + + n = short_time_len(line, end - line); + if (!n) + n = fractional_time_len(line, end - line); + end -= n; + + n = date_len(line, end - line); + if (!n) /* No date. Too bad. */ + return 0; + end -= n; + + if (end == line) /* No space before date. */ + return 0; + if (end[-1] == '\t') { /* Success! */ + end--; + return line + len - end; + } + if (end[-1] != ' ') /* No space before date. */ + return 0; + + /* Whitespace damage. */ + end -= trailing_spaces_len(line, end - line); + return line + len - end; +} + +static char *find_name_common(const char *line, char *def, int p_value, + const char *end, int terminate) { int len; const char *start = NULL; - if (*line == '"') { - char *name = find_name_gnu(line, def, p_value); - if (name) - return name; - } - if (p_value == 0) start = line; - for (;;) { + while (line != end) { char c = *line; - if (isspace(c)) { + if (!end && isspace(c)) { if (c == '\n') break; if (name_terminate(start, line-start, c, terminate)) @@ -505,6 +639,37 @@ static char *find_name(const char *line, char *def, int p_value, int terminate) return squash_slash(xmemdupz(start, len)); } +static char *find_name(const char *line, char *def, int p_value, int terminate) +{ + if (*line == '"') { + char *name = find_name_gnu(line, def, p_value); + if (name) + return name; + } + + return find_name_common(line, def, p_value, NULL, terminate); +} + +static char *find_name_traditional(const char *line, char *def, int p_value) +{ + size_t len = strlen(line); + size_t date_len; + + if (*line == '"') { + char *name = find_name_gnu(line, def, p_value); + if (name) + return name; + } + + len = strchrnul(line, '\n') - line; + date_len = diff_timestamp_len(line, len); + if (!date_len) + return find_name_common(line, def, p_value, NULL, TERM_TAB); + len -= date_len; + + return find_name_common(line, def, p_value, line + len, 0); +} + static int count_slashes(const char *cp) { int cnt = 0; @@ -527,7 +692,7 @@ static int guess_p_value(const char *nameline) if (is_dev_null(nameline)) return -1; - name = find_name(nameline, NULL, 0, TERM_SPACE | TERM_TAB); + name = find_name_traditional(nameline, NULL, 0); if (!name) return -1; cp = strchr(name, '/'); @@ -646,16 +811,16 @@ static void parse_traditional_patch(const char *first, const char *second, struc if (is_dev_null(first)) { patch->is_new = 1; patch->is_delete = 0; - name = find_name(second, NULL, p_value, TERM_SPACE | TERM_TAB); + name = find_name_traditional(second, NULL, p_value); patch->new_name = name; } else if (is_dev_null(second)) { patch->is_new = 0; patch->is_delete = 1; - name = find_name(first, NULL, p_value, TERM_SPACE | TERM_TAB); + name = find_name_traditional(first, NULL, p_value); patch->old_name = name; } else { - name = find_name(first, NULL, p_value, TERM_SPACE | TERM_TAB); - name = find_name(second, name, p_value, TERM_SPACE | TERM_TAB); + name = find_name_traditional(first, NULL, p_value); + name = find_name_traditional(second, name, p_value); if (has_epoch_timestamp(first)) { patch->is_new = 1; patch->is_delete = 0; diff --git a/t/t4135-apply-weird-filenames.sh b/t/t4135-apply-weird-filenames.sh index 9373f64..1e5aad5 100755 --- a/t/t4135-apply-weird-filenames.sh +++ b/t/t4135-apply-weird-filenames.sh @@ -59,8 +59,8 @@ try_filename() { } try_filename 'plain' 'postimage.txt' -try_filename 'with spaces' 'post image.txt' '' success failure failure -try_filename 'with tab' 'post image.txt' FUNNYNAMES success failure failure +try_filename 'with spaces' 'post image.txt' +try_filename 'with tab' 'post image.txt' FUNNYNAMES try_filename 'with backslash' 'post\image.txt' BSLASHPSPEC try_filename 'with quote' '"postimage".txt' FUNNYNAMES success failure success -- 1.7.2.1.544.ga752d.dirty ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2010-08-19 1:52 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-07-24 1:06 [PATCH 0/3] apply: handle traditional patches with spaces in filename Jonathan Nieder 2010-07-24 1:09 ` [PATCH 1/3] apply: Split quoted filename handling into new function Jonathan Nieder 2010-07-24 1:11 ` [PATCH 2/3] tests: Test how well "git apply" copes with weird filenames Jonathan Nieder 2010-07-24 8:03 ` Andreas Schwab 2010-07-24 8:48 ` Jonathan Nieder 2010-07-24 1:20 ` [PATCH 3/3] apply: Handle traditional patches with space in filename Jonathan Nieder -- strict thread matches above, loose matches on Subject: below -- 2010-08-11 23:35 What's cooking in git.git (Aug 2010, #02; Wed, 11) Junio C Hamano 2010-08-13 21:44 ` jn/apply-filename-with-sp (Re: What's cooking in git.git (Aug 2010, #02; Wed, 11)) Johannes Sixt 2010-08-14 2:27 ` Jonathan Nieder 2010-08-14 18:37 ` Johannes Sixt 2010-08-19 1:45 ` [PATCH v2 0/3] apply: handle traditional patches with space in filename Jonathan Nieder 2010-08-19 1:50 ` [PATCH 3/3] " Jonathan Nieder
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).