Merge branch 'as/tree-walk-fix-aggressive-short-cut' into maint
authorJunio C Hamano <gitster@pobox.com>
Thu, 13 Feb 2014 21:37:53 +0000 (13:37 -0800)
committerJunio C Hamano <gitster@pobox.com>
Thu, 13 Feb 2014 21:37:53 +0000 (13:37 -0800)
The pathspec matching code, while comparing two trees (e.g. "git
diff A B -- path1 path2") was too aggressive and failed to match
some paths when multiple pathspecs were involved.

* as/tree-walk-fix-aggressive-short-cut:
tree_entry_interesting: match against all pathspecs

1  2 
tree-walk.c
diff --combined tree-walk.c
index 5ece8c3477b11f3bf0c54196924ac24568309225,d5914f598dcacc681f747bb27df508a7f37f7f52..e06f240c7515ab7b08c4441c3cb48fce1602d0e2
@@@ -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.
                 * 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
@@@ -671,17 -634,8 +671,17 @@@ enum interesting tree_entry_interesting
        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);
 +
        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),
  
                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;
  
@@@ -743,7 -696,7 +743,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;