Merge branch 'as/tree-walk-fix-aggressive-short-cut'
authorJunio C Hamano <gitster@pobox.com>
Mon, 27 Jan 2014 18:48:32 +0000 (10:48 -0800)
committerJunio C Hamano <gitster@pobox.com>
Mon, 27 Jan 2014 18:48:32 +0000 (10:48 -0800)
* as/tree-walk-fix-aggressive-short-cut:
tree_entry_interesting: match against all pathspecs

1  2 
t/t4010-diff-pathspec.sh
tree-walk.c
diff --combined t/t4010-diff-pathspec.sh
index 15a491295ed3b85cb4a66d46e720e352787a8344,589bc6bfb4543324dcf6f166c4db482958f26e63..9f5659f7fe4df23d805e6c38264a1ebf38fc2538
@@@ -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 c29b6a3a566fe911271c9833e511190822aed723,d5914f598dcacc681f747bb27df508a7f37f7f52..79dba1d0f4e729b5e62825d347c7e6a9b1455f1d
@@@ -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.
        if (matchlen > pathlen) {
                if (match[pathlen] != '/')
                        return 0;
 -              if (!S_ISDIR(entry->mode))
 +              if (!S_ISDIR(entry->mode) && !S_ISGITLINK(entry->mode))
                        return 0;
        }
  
                 * 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,
         * 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;
                 * 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
   * 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),
                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,
                }
  
                /* 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
  
                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;
        }
        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 */
 +}