From: Junio C Hamano Date: Mon, 27 Jan 2014 18:48:32 +0000 (-0800) Subject: Merge branch 'as/tree-walk-fix-aggressive-short-cut' X-Git-Tag: v1.9-rc1~1 X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/8bba7206b503b63a8208355f66bc09c981072d51?ds=inline;hp=-c Merge branch 'as/tree-walk-fix-aggressive-short-cut' * as/tree-walk-fix-aggressive-short-cut: tree_entry_interesting: match against all pathspecs --- 8bba7206b503b63a8208355f66bc09c981072d51 diff --combined t/t4010-diff-pathspec.sh index 15a491295e,589bc6bfb4..9f5659f7fe --- a/t/t4010-diff-pathspec.sh +++ b/t/t4010-diff-pathspec.sh @@@ -110,21 -110,17 +110,34 @@@ test_expect_success 'diff-tree -r with test_cmp expected result ' +test_expect_success 'setup submodules' ' + test_tick && + git init submod && + ( cd submod && test_commit first; ) && + git add submod && + git commit -m first && + ( cd submod && test_commit second; ) && + git add submod && + git commit -m second +' + +test_expect_success 'diff-tree ignores trailing slash on submodule path' ' + git diff --name-only HEAD^ HEAD submod >expect && + git diff --name-only HEAD^ HEAD submod/ >actual && + test_cmp expect actual +' + + test_expect_success 'diff multiple wildcard pathspecs' ' + mkdir path2 && + echo rezrov >path2/file1 && + git update-index --add path2/file1 && + tree3=`git write-tree` && + git diff --name-only $tree $tree3 -- "path2*1" "path1*1" >actual && + cat <<-\EOF >expect && + path1/file1 + path2/file1 + EOF + test_cmp expect actual + ' + test_done diff --combined tree-walk.c index c29b6a3a56,d5914f598d..79dba1d0f4 --- a/tree-walk.c +++ b/tree-walk.c @@@ -3,7 -3,6 +3,7 @@@ #include "unpack-trees.h" #include "dir.h" #include "tree.h" +#include "pathspec.h" static const char *get_mode(const char *str, unsigned int *modep) { @@@ -488,25 -487,13 +488,25 @@@ int get_tree_entry(const unsigned char return retval; } -static int match_entry(const struct name_entry *entry, int pathlen, +static int match_entry(const struct pathspec_item *item, + const struct name_entry *entry, int pathlen, const char *match, int matchlen, enum interesting *never_interesting) { int m = -1; /* signals that we haven't called strncmp() */ - if (*never_interesting != entry_not_interesting) { + if (item->magic & PATHSPEC_ICASE) + /* + * "Never interesting" trick requires exact + * matching. We could do something clever with inexact + * matching, but it's trickier (and not to forget that + * strcasecmp is locale-dependent, at least in + * glibc). Just disable it for now. It can't be worse + * than the wildcard's codepath of '[Tt][Hi][Is][Ss]' + * pattern. + */ + *never_interesting = entry_not_interesting; + else if (*never_interesting != entry_not_interesting) { /* * We have not seen any match that sorts later * than the current path. @@@ -543,7 -530,7 +543,7 @@@ if (matchlen > pathlen) { if (match[pathlen] != '/') return 0; - if (!S_ISDIR(entry->mode)) + if (!S_ISDIR(entry->mode) && !S_ISGITLINK(entry->mode)) return 0; } @@@ -552,7 -539,7 +552,7 @@@ * we cheated and did not do strncmp(), so we do * that here. */ - m = strncmp(match, entry->path, pathlen); + m = ps_strncmp(item, match, entry->path, pathlen); /* * If common part matched earlier then it is a hit, @@@ -560,39 -547,15 +560,39 @@@ * leading directory and is shorter than match. */ if (!m) + /* + * match_entry does not check if the prefix part is + * matched case-sensitively. If the entry is a + * directory and part of prefix, it'll be rematched + * eventually by basecmp with special treatment for + * the prefix. + */ return 1; return 0; } -static int match_dir_prefix(const char *base, +/* :(icase)-aware string compare */ +static int basecmp(const struct pathspec_item *item, + const char *base, const char *match, int len) +{ + if (item->magic & PATHSPEC_ICASE) { + int ret, n = len > item->prefix ? item->prefix : len; + ret = strncmp(base, match, n); + if (ret) + return ret; + base += n; + match += n; + len -= n; + } + return ps_strncmp(item, base, match, len); +} + +static int match_dir_prefix(const struct pathspec_item *item, + const char *base, const char *match, int matchlen) { - if (strncmp(base, match, matchlen)) + if (basecmp(item, base, match, matchlen)) return 0; /* @@@ -629,7 -592,7 +629,7 @@@ static int match_wildcard_base(const st */ if (baselen >= matchlen) { *matched = matchlen; - return !strncmp(base, match, matchlen); + return !basecmp(item, base, match, matchlen); } dirlen = matchlen; @@@ -642,7 -605,7 +642,7 @@@ * base ends with '/' so we are sure it really matches * directory */ - if (strncmp(base, match, baselen)) + if (basecmp(item, base, match, baselen)) return 0; *matched = baselen; } else @@@ -662,28 -625,17 +662,28 @@@ * Pre-condition: either baselen == base_offset (i.e. empty path) * or base[baselen-1] == '/' (i.e. with trailing slash). */ -enum interesting tree_entry_interesting(const struct name_entry *entry, - struct strbuf *base, int base_offset, - const struct pathspec *ps) +static enum interesting do_match(const struct name_entry *entry, + struct strbuf *base, int base_offset, + const struct pathspec *ps, + int exclude) { int i; int pathlen, baselen = base->len - base_offset; enum interesting never_interesting = ps->has_wildcard ? entry_not_interesting : all_entries_not_interesting; + GUARD_PATHSPEC(ps, + PATHSPEC_FROMTOP | + PATHSPEC_MAXDEPTH | + PATHSPEC_LITERAL | + PATHSPEC_GLOB | + PATHSPEC_ICASE | + PATHSPEC_EXCLUDE); + if (!ps->nr) { - if (!ps->recursive || ps->max_depth == -1) + if (!ps->recursive || + !(ps->magic & PATHSPEC_MAXDEPTH) || + ps->max_depth == -1) return all_entries_interesting; return within_depth(base->buf + base_offset, baselen, !!S_ISDIR(entry->mode), @@@ -699,18 -651,12 +699,18 @@@ const char *base_str = base->buf + base_offset; int matchlen = item->len, matched = 0; + if ((!exclude && item->magic & PATHSPEC_EXCLUDE) || + ( exclude && !(item->magic & PATHSPEC_EXCLUDE))) + continue; + if (baselen >= matchlen) { /* If it doesn't match, move along... */ - if (!match_dir_prefix(base_str, match, matchlen)) + if (!match_dir_prefix(item, base_str, match, matchlen)) goto match_wildcards; - if (!ps->recursive || ps->max_depth == -1) + if (!ps->recursive || + !(ps->magic & PATHSPEC_MAXDEPTH) || + ps->max_depth == -1) return all_entries_interesting; return within_depth(base_str + matchlen + 1, @@@ -721,14 -667,15 +721,14 @@@ } /* Either there must be no base, or the base must match. */ - if (baselen == 0 || !strncmp(base_str, match, baselen)) { - if (match_entry(entry, pathlen, + if (baselen == 0 || !basecmp(item, base_str, match, baselen)) { + if (match_entry(item, entry, pathlen, match + baselen, matchlen - baselen, &never_interesting)) return entry_interesting; if (item->nowildcard_len < item->len) { - if (!git_fnmatch(match + baselen, entry->path, - item->flags & PATHSPEC_ONESTAR ? GFNM_ONESTAR : 0, + if (!git_fnmatch(item, match + baselen, entry->path, item->nowildcard_len - baselen)) return entry_interesting; @@@ -749,7 -696,7 +749,7 @@@ match_wildcards if (item->nowildcard_len && !match_wildcard_base(item, base_str, baselen, &matched)) - return entry_not_interesting; + continue; /* * Concatenate base and entry->path into one and do @@@ -769,7 -716,8 +769,7 @@@ strbuf_add(base, entry->path, pathlen); - if (!git_fnmatch(match, base->buf + base_offset, - item->flags & PATHSPEC_ONESTAR ? GFNM_ONESTAR : 0, + if (!git_fnmatch(item, match, base->buf + base_offset, item->nowildcard_len)) { strbuf_setlen(base, base_offset + baselen); return entry_interesting; @@@ -788,72 -736,3 +788,72 @@@ } return never_interesting; /* No matches */ } + +/* + * Is a tree entry interesting given the pathspec we have? + * + * Pre-condition: either baselen == base_offset (i.e. empty path) + * or base[baselen-1] == '/' (i.e. with trailing slash). + */ +enum interesting tree_entry_interesting(const struct name_entry *entry, + struct strbuf *base, int base_offset, + const struct pathspec *ps) +{ + enum interesting positive, negative; + positive = do_match(entry, base, base_offset, ps, 0); + + /* + * case | entry | positive | negative | result + * -----+-------+----------+----------+------- + * 1 | file | -1 | -1..2 | -1 + * 2 | file | 0 | -1..2 | 0 + * 3 | file | 1 | -1 | 1 + * 4 | file | 1 | 0 | 1 + * 5 | file | 1 | 1 | 0 + * 6 | file | 1 | 2 | 0 + * 7 | file | 2 | -1 | 2 + * 8 | file | 2 | 0 | 2 + * 9 | file | 2 | 1 | 0 + * 10 | file | 2 | 2 | -1 + * -----+-------+----------+----------+------- + * 11 | dir | -1 | -1..2 | -1 + * 12 | dir | 0 | -1..2 | 0 + * 13 | dir | 1 | -1 | 1 + * 14 | dir | 1 | 0 | 1 + * 15 | dir | 1 | 1 | 1 (*) + * 16 | dir | 1 | 2 | 0 + * 17 | dir | 2 | -1 | 2 + * 18 | dir | 2 | 0 | 2 + * 19 | dir | 2 | 1 | 1 (*) + * 20 | dir | 2 | 2 | -1 + * + * (*) An exclude pattern interested in a directory does not + * necessarily mean it will exclude all of the directory. In + * wildcard case, it can't decide until looking at individual + * files inside. So don't write such directories off yet. + */ + + if (!(ps->magic & PATHSPEC_EXCLUDE) || + positive <= entry_not_interesting) /* #1, #2, #11, #12 */ + return positive; + + negative = do_match(entry, base, base_offset, ps, 1); + + /* #3, #4, #7, #8, #13, #14, #17, #18 */ + if (negative <= entry_not_interesting) + return positive; + + /* #15, #19 */ + if (S_ISDIR(entry->mode) && + positive >= entry_interesting && + negative == entry_interesting) + return entry_interesting; + + if ((positive == entry_interesting && + negative >= entry_interesting) || /* #5, #6, #16 */ + (positive == all_entries_interesting && + negative == entry_interesting)) /* #9 */ + return entry_not_interesting; + + return all_entries_not_interesting; /* #10, #20 */ +}