Merge branch 'jc/gitignore-precedence'
authorJunio C Hamano <gitster@pobox.com>
Tue, 19 May 2015 20:17:50 +0000 (13:17 -0700)
committerJunio C Hamano <gitster@pobox.com>
Tue, 19 May 2015 20:17:51 +0000 (13:17 -0700)
core.excludesfile (defaulting to $XDG_HOME/git/ignore) is supposed
to be overridden by repository-specific .git/info/exclude file, but
the order was swapped from the beginning. This belatedly fixes it.

* jc/gitignore-precedence:
ignore: info/exclude should trump core.excludesfile

1  2 
dir.c
t/t0008-ignores.sh
diff --combined dir.c
index 0c38d8601448053f94eca534833d47a975580856,e67b6f9b8bf95b284ee6976f4968eba80e281b8d..4183acc082671f135fe64cbcaa66ed3b17bc6364
--- 1/dir.c
--- 2/dir.c
+++ b/dir.c
@@@ -12,7 -12,6 +12,7 @@@
  #include "refs.h"
  #include "wildmatch.h"
  #include "pathspec.h"
 +#include "utf8.h"
  
  struct path_simplify {
        int len;
@@@ -50,18 -49,16 +50,18 @@@ int strncmp_icase(const char *a, const 
  
  int fnmatch_icase(const char *pattern, const char *string, int flags)
  {
 -      return fnmatch(pattern, string, flags | (ignore_case ? FNM_CASEFOLD : 0));
 +      return wildmatch(pattern, string,
 +                       flags | (ignore_case ? WM_CASEFOLD : 0),
 +                       NULL);
  }
  
 -inline int git_fnmatch(const struct pathspec_item *item,
 -                     const char *pattern, const char *string,
 -                     int prefix)
 +int git_fnmatch(const struct pathspec_item *item,
 +              const char *pattern, const char *string,
 +              int prefix)
  {
        if (prefix > 0) {
                if (ps_strncmp(item, pattern, string, prefix))
 -                      return FNM_NOMATCH;
 +                      return WM_NOMATCH;
                pattern += prefix;
                string += prefix;
        }
@@@ -79,9 -76,8 +79,9 @@@
                                 NULL);
        else
                /* wildmatch has not learned no FNM_PATHNAME mode yet */
 -              return fnmatch(pattern, string,
 -                             item->magic & PATHSPEC_ICASE ? FNM_CASEFOLD : 0);
 +              return wildmatch(pattern, string,
 +                               item->magic & PATHSPEC_ICASE ? WM_CASEFOLD : 0,
 +                               NULL);
  }
  
  static int fnmatch_icase_mem(const char *pattern, int patternlen,
@@@ -130,13 -126,10 +130,13 @@@ static size_t common_prefix_len(const s
                       PATHSPEC_MAXDEPTH |
                       PATHSPEC_LITERAL |
                       PATHSPEC_GLOB |
 -                     PATHSPEC_ICASE);
 +                     PATHSPEC_ICASE |
 +                     PATHSPEC_EXCLUDE);
  
        for (n = 0; n < pathspec->nr; n++) {
                size_t i = 0, len = 0, item_len;
 +              if (pathspec->items[n].magic & PATHSPEC_EXCLUDE)
 +                      continue;
                if (pathspec->items[n].magic & PATHSPEC_ICASE)
                        item_len = pathspec->items[n].prefix;
                else
@@@ -199,9 -192,6 +199,9 @@@ int within_depth(const char *name, int 
        return 1;
  }
  
 +#define DO_MATCH_EXCLUDE   1
 +#define DO_MATCH_DIRECTORY 2
 +
  /*
   * Does 'match' match the given name?
   * A match is found if
   * It returns 0 when there is no match.
   */
  static int match_pathspec_item(const struct pathspec_item *item, int prefix,
 -                             const char *name, int namelen)
 +                             const char *name, int namelen, unsigned flags)
  {
        /* name/namelen has prefix cut off by caller */
        const char *match = item->match + prefix;
         * The normal call pattern is:
         * 1. prefix = common_prefix_len(ps);
         * 2. prune something, or fill_directory
 -       * 3. match_pathspec_depth()
 +       * 3. match_pathspec()
         *
         * 'prefix' at #1 may be shorter than the command's prefix and
         * it's ok for #2 to match extra files. Those extras will be
  
                if (match[matchlen-1] == '/' || name[matchlen] == '/')
                        return MATCHED_RECURSIVELY;
 -      }
 +      } else if ((flags & DO_MATCH_DIRECTORY) &&
 +                 match[matchlen - 1] == '/' &&
 +                 namelen == matchlen - 1 &&
 +                 !ps_strncmp(item, match, name, namelen))
 +              return MATCHED_EXACTLY;
  
        if (item->nowildcard_len < item->len &&
            !git_fnmatch(item, match, name,
   * pathspec did not match any names, which could indicate that the
   * user mistyped the nth pathspec.
   */
 -int match_pathspec_depth(const struct pathspec *ps,
 -                       const char *name, int namelen,
 -                       int prefix, char *seen)
 +static int do_match_pathspec(const struct pathspec *ps,
 +                           const char *name, int namelen,
 +                           int prefix, char *seen,
 +                           unsigned flags)
  {
 -      int i, retval = 0;
 +      int i, retval = 0, exclude = flags & DO_MATCH_EXCLUDE;
  
        GUARD_PATHSPEC(ps,
                       PATHSPEC_FROMTOP |
                       PATHSPEC_MAXDEPTH |
                       PATHSPEC_LITERAL |
                       PATHSPEC_GLOB |
 -                     PATHSPEC_ICASE);
 +                     PATHSPEC_ICASE |
 +                     PATHSPEC_EXCLUDE);
  
        if (!ps->nr) {
                if (!ps->recursive ||
  
        for (i = ps->nr - 1; i >= 0; i--) {
                int how;
 +
 +              if ((!exclude &&   ps->items[i].magic & PATHSPEC_EXCLUDE) ||
 +                  ( exclude && !(ps->items[i].magic & PATHSPEC_EXCLUDE)))
 +                      continue;
 +
                if (seen && seen[i] == MATCHED_EXACTLY)
                        continue;
 -              how = match_pathspec_item(ps->items+i, prefix, name, namelen);
 +              /*
 +               * Make exclude patterns optional and never report
 +               * "pathspec ':(exclude)foo' matches no files"
 +               */
 +              if (seen && ps->items[i].magic & PATHSPEC_EXCLUDE)
 +                      seen[i] = MATCHED_FNMATCH;
 +              how = match_pathspec_item(ps->items+i, prefix, name,
 +                                        namelen, flags);
                if (ps->recursive &&
                    (ps->magic & PATHSPEC_MAXDEPTH) &&
                    ps->max_depth != -1 &&
        return retval;
  }
  
 +int match_pathspec(const struct pathspec *ps,
 +                 const char *name, int namelen,
 +                 int prefix, char *seen, int is_dir)
 +{
 +      int positive, negative;
 +      unsigned flags = is_dir ? DO_MATCH_DIRECTORY : 0;
 +      positive = do_match_pathspec(ps, name, namelen,
 +                                   prefix, seen, flags);
 +      if (!(ps->magic & PATHSPEC_EXCLUDE) || !positive)
 +              return positive;
 +      negative = do_match_pathspec(ps, name, namelen,
 +                                   prefix, seen,
 +                                   flags | DO_MATCH_EXCLUDE);
 +      return negative ? 0 : positive;
 +}
 +
 +int report_path_error(const char *ps_matched,
 +                    const struct pathspec *pathspec,
 +                    const char *prefix)
 +{
 +      /*
 +       * Make sure all pathspec matched; otherwise it is an error.
 +       */
 +      struct strbuf sb = STRBUF_INIT;
 +      int num, errors = 0;
 +      for (num = 0; num < pathspec->nr; num++) {
 +              int other, found_dup;
 +
 +              if (ps_matched[num])
 +                      continue;
 +              /*
 +               * The caller might have fed identical pathspec
 +               * twice.  Do not barf on such a mistake.
 +               * FIXME: parse_pathspec should have eliminated
 +               * duplicate pathspec.
 +               */
 +              for (found_dup = other = 0;
 +                   !found_dup && other < pathspec->nr;
 +                   other++) {
 +                      if (other == num || !ps_matched[other])
 +                              continue;
 +                      if (!strcmp(pathspec->items[other].original,
 +                                  pathspec->items[num].original))
 +                              /*
 +                               * Ok, we have a match already.
 +                               */
 +                              found_dup = 1;
 +              }
 +              if (found_dup)
 +                      continue;
 +
 +              error("pathspec '%s' did not match any file(s) known to git.",
 +                    pathspec->items[num].original);
 +              errors++;
 +      }
 +      strbuf_release(&sb);
 +      return errors;
 +}
 +
  /*
   * Return the length of the "simple" part of a path match limiter.
   */
@@@ -550,29 -463,6 +550,29 @@@ void clear_exclude_list(struct exclude_
        el->filebuf = NULL;
  }
  
 +static void trim_trailing_spaces(char *buf)
 +{
 +      char *p, *last_space = NULL;
 +
 +      for (p = buf; *p; p++)
 +              switch (*p) {
 +              case ' ':
 +                      if (!last_space)
 +                              last_space = p;
 +                      break;
 +              case '\\':
 +                      p++;
 +                      if (!*p)
 +                              return;
 +                      /* fallthrough */
 +              default:
 +                      last_space = NULL;
 +              }
 +
 +      if (last_space)
 +              *last_space = '\0';
 +}
 +
  int add_excludes_from_file_to_list(const char *fname,
                                   const char *base,
                                   int baselen,
                        buf = xrealloc(buf, size+1);
                        buf[size++] = '\n';
                }
 -      }
 -      else {
 +      } else {
                size = xsize_t(st.st_size);
                if (size == 0) {
                        close(fd);
        }
  
        el->filebuf = buf;
 +
 +      if (skip_utf8_bom(&buf, size))
 +              size -= buf - el->filebuf;
 +
        entry = buf;
 +
        for (i = 0; i < size; i++) {
                if (buf[i] == '\n') {
                        if (entry != buf + i && entry[0] != '#') {
                                buf[i - (i && buf[i-1] == '\r')] = 0;
 +                              trim_trailing_spaces(entry);
                                add_exclude(entry, base, baselen, el, lineno);
                        }
                        lineno++;
@@@ -841,19 -726,17 +841,19 @@@ static void prep_exclude(struct dir_str
  
        group = &dir->exclude_list_group[EXC_DIRS];
  
 -      /* Pop the exclude lists from the EXCL_DIRS exclude_list_group
 +      /*
 +       * Pop the exclude lists from the EXCL_DIRS exclude_list_group
         * which originate from directories not in the prefix of the
 -       * path being checked. */
 +       * path being checked.
 +       */
        while ((stk = dir->exclude_stack) != NULL) {
                if (stk->baselen <= baselen &&
 -                  !strncmp(dir->basebuf, base, stk->baselen))
 +                  !strncmp(dir->basebuf.buf, base, stk->baselen))
                        break;
                el = &group->el[dir->exclude_stack->exclude_ix];
                dir->exclude_stack = stk->prev;
                dir->exclude = NULL;
 -              free((char *)el->src); /* see strdup() below */
 +              free((char *)el->src); /* see strbuf_detach() below */
                clear_exclude_list(el);
                free(stk);
                group->nr--;
        if (dir->exclude)
                return;
  
 +      /*
 +       * Lazy initialization. All call sites currently just
 +       * memset(dir, 0, sizeof(*dir)) before use. Changing all of
 +       * them seems lots of work for little benefit.
 +       */
 +      if (!dir->basebuf.buf)
 +              strbuf_init(&dir->basebuf, PATH_MAX);
 +
        /* Read from the parent directories and push them down. */
        current = stk ? stk->baselen : -1;
 +      strbuf_setlen(&dir->basebuf, current < 0 ? 0 : current);
        while (current < baselen) {
 -              struct exclude_stack *stk = xcalloc(1, sizeof(*stk));
                const char *cp;
  
 +              stk = xcalloc(1, sizeof(*stk));
                if (current < 0) {
                        cp = base;
                        current = 0;
 -              }
 -              else {
 +              } else {
                        cp = strchr(base + current + 1, '/');
                        if (!cp)
                                die("oops in prep_exclude");
                stk->baselen = cp - base;
                stk->exclude_ix = group->nr;
                el = add_exclude_list(dir, EXC_DIRS, NULL);
 -              memcpy(dir->basebuf + current, base + current,
 -                     stk->baselen - current);
 +              strbuf_add(&dir->basebuf, base + current, stk->baselen - current);
 +              assert(stk->baselen == dir->basebuf.len);
  
                /* Abort if the directory is excluded */
                if (stk->baselen) {
                        int dt = DT_DIR;
 -                      dir->basebuf[stk->baselen - 1] = 0;
 +                      dir->basebuf.buf[stk->baselen - 1] = 0;
                        dir->exclude = last_exclude_matching_from_lists(dir,
 -                              dir->basebuf, stk->baselen - 1,
 -                              dir->basebuf + current, &dt);
 -                      dir->basebuf[stk->baselen - 1] = '/';
 +                              dir->basebuf.buf, stk->baselen - 1,
 +                              dir->basebuf.buf + current, &dt);
 +                      dir->basebuf.buf[stk->baselen - 1] = '/';
                        if (dir->exclude &&
                            dir->exclude->flags & EXC_FLAG_NEGATIVE)
                                dir->exclude = NULL;
                        if (dir->exclude) {
 -                              dir->basebuf[stk->baselen] = 0;
                                dir->exclude_stack = stk;
                                return;
                        }
                }
  
 -              /* Try to read per-directory file unless path is too long */
 -              if (dir->exclude_per_dir &&
 -                  stk->baselen + strlen(dir->exclude_per_dir) < PATH_MAX) {
 -                      strcpy(dir->basebuf + stk->baselen,
 -                                      dir->exclude_per_dir);
 +              /* Try to read per-directory file */
 +              if (dir->exclude_per_dir) {
                        /*
                         * dir->basebuf gets reused by the traversal, but we
                         * need fname to remain unchanged to ensure the src
                         * member of each struct exclude correctly
                         * back-references its source file.  Other invocations
                         * of add_exclude_list provide stable strings, so we
 -                       * strdup() and free() here in the caller.
 +                       * strbuf_detach() and free() here in the caller.
                         */
 -                      el->src = strdup(dir->basebuf);
 -                      add_excludes_from_file_to_list(dir->basebuf,
 -                                      dir->basebuf, stk->baselen, el, 1);
 +                      struct strbuf sb = STRBUF_INIT;
 +                      strbuf_addbuf(&sb, &dir->basebuf);
 +                      strbuf_addstr(&sb, dir->exclude_per_dir);
 +                      el->src = strbuf_detach(&sb, NULL);
 +                      add_excludes_from_file_to_list(el->src, el->src,
 +                                                     stk->baselen, el, 1);
                }
                dir->exclude_stack = stk;
                current = stk->baselen;
        }
 -      dir->basebuf[baselen] = '\0';
 +      strbuf_setlen(&dir->basebuf, baselen);
  }
  
  /*
@@@ -1411,7 -1287,8 +1411,7 @@@ static int cmp_name(const void *p1, con
        const struct dir_entry *e1 = *(const struct dir_entry **)p1;
        const struct dir_entry *e2 = *(const struct dir_entry **)p2;
  
 -      return cache_name_compare(e1->name, e1->len,
 -                                e2->name, e2->len);
 +      return name_compare(e1->name, e1->len, e2->name, e2->len);
  }
  
  static struct path_simplify *create_simplify(const char **pathspec)
  
        for (nr = 0 ; ; nr++) {
                const char *match;
 -              if (nr >= alloc) {
 -                      alloc = alloc_nr(alloc);
 -                      simplify = xrealloc(simplify, alloc * sizeof(*simplify));
 -              }
 +              ALLOC_GROW(simplify, nr + 1, alloc);
                match = *pathspec++;
                if (!match)
                        break;
@@@ -1495,18 -1375,11 +1495,18 @@@ int read_directory(struct dir_struct *d
                               PATHSPEC_MAXDEPTH |
                               PATHSPEC_LITERAL |
                               PATHSPEC_GLOB |
 -                             PATHSPEC_ICASE);
 +                             PATHSPEC_ICASE |
 +                             PATHSPEC_EXCLUDE);
  
        if (has_symlink_leading_path(path, len))
                return dir->nr;
  
 +      /*
 +       * exclude patterns are treated like positive ones in
 +       * create_simplify. Usually exclude patterns should be a
 +       * subset of positive ones, which has no impacts on
 +       * create_simplify().
 +       */
        simplify = create_simplify(pathspec ? pathspec->_raw : NULL);
        if (!len || treat_leading_path(dir, path, len, simplify))
                read_directory_recursive(dir, path, len, 0, simplify);
@@@ -1556,16 -1429,12 +1556,16 @@@ int dir_inside_of(const char *subdir, c
  
  int is_inside_dir(const char *dir)
  {
 -      char cwd[PATH_MAX];
 +      char *cwd;
 +      int rc;
 +
        if (!dir)
                return 0;
 -      if (!getcwd(cwd, sizeof(cwd)))
 -              die_errno("can't find the current directory");
 -      return dir_inside_of(cwd, dir) >= 0;
 +
 +      cwd = xgetcwd();
 +      rc = (dir_inside_of(cwd, dir) >= 0);
 +      free(cwd);
 +      return rc;
  }
  
  int is_empty_dir(const char *path)
@@@ -1607,13 -1476,8 +1607,13 @@@ static int remove_dir_recurse(struct st
        flag &= ~REMOVE_DIR_KEEP_TOPLEVEL;
        dir = opendir(path->buf);
        if (!dir) {
 -              /* an empty dir could be removed even if it is unreadble */
 -              if (!keep_toplevel)
 +              if (errno == ENOENT)
 +                      return keep_toplevel ? -1 : 0;
 +              else if (errno == EACCES && !keep_toplevel)
 +                      /*
 +                       * An empty dir could be removable even if it
 +                       * is unreadable:
 +                       */
                        return rmdir(path->buf);
                else
                        return -1;
  
                strbuf_setlen(path, len);
                strbuf_addstr(path, e->d_name);
 -              if (lstat(path->buf, &st))
 -                      ; /* fall thru */
 -              else if (S_ISDIR(st.st_mode)) {
 +              if (lstat(path->buf, &st)) {
 +                      if (errno == ENOENT)
 +                              /*
 +                               * file disappeared, which is what we
 +                               * wanted anyway
 +                               */
 +                              continue;
 +                      /* fall thru */
 +              } else if (S_ISDIR(st.st_mode)) {
                        if (!remove_dir_recurse(path, flag, &kept_down))
                                continue; /* happy */
 -              } else if (!only_empty && !unlink(path->buf))
 +              } else if (!only_empty &&
 +                         (!unlink(path->buf) || errno == ENOENT)) {
                        continue; /* happy, too */
 +              }
  
                /* path too long, stat fails, or non-directory still exists */
                ret = -1;
  
        strbuf_setlen(path, original_len);
        if (!ret && !keep_toplevel && !kept_down)
 -              ret = rmdir(path->buf);
 +              ret = (!rmdir(path->buf) || errno == ENOENT) ? 0 : -1;
        else if (kept_up)
                /*
                 * report the uplevel that it is not an error that we
@@@ -1671,15 -1527,22 +1671,19 @@@ int remove_dir_recursively(struct strbu
  void setup_standard_excludes(struct dir_struct *dir)
  {
        const char *path;
 -      char *xdg_path;
  
        dir->exclude_per_dir = ".gitignore";
-       path = git_path("info/exclude");
+       /* core.excludefile defaulting to $XDG_HOME/git/ignore */
 -      if (!excludes_file) {
 -              home_config_paths(NULL, &xdg_path, "ignore");
 -              excludes_file = xdg_path;
 -      }
 +      if (!excludes_file)
 +              excludes_file = xdg_config_home("ignore");
-       if (!access_or_warn(path, R_OK, 0))
-               add_excludes_from_file(dir, path);
        if (excludes_file && !access_or_warn(excludes_file, R_OK, 0))
                add_excludes_from_file(dir, excludes_file);
+       /* per repository user preference */
+       path = git_path("info/exclude");
+       if (!access_or_warn(path, R_OK, 0))
+               add_excludes_from_file(dir, path);
  }
  
  int remove_path(const char *name)
@@@ -1729,5 -1592,4 +1733,5 @@@ void clear_directory(struct dir_struct 
                free(stk);
                stk = prev;
        }
 +      strbuf_release(&dir->basebuf);
  }
diff --combined t/t0008-ignores.sh
index 8dc6939b9049baa32aca1200e36378e6e2296e9d,38405de17d6f9625a07080d4fc2e4b4541351cc5..4ef5ed484c12a4509b9bc66f2cc41118f7254431
@@@ -775,60 -775,14 +775,70 @@@ test_expect_success PIPE 'streaming sup
        echo "$response" | grep "^::    two"
  '
  
 +############################################################################
 +#
 +# test whitespace handling
 +
 +test_expect_success 'trailing whitespace is ignored' '
 +      mkdir whitespace &&
 +      >whitespace/trailing &&
 +      >whitespace/untracked &&
 +      echo "whitespace/trailing   " >ignore &&
 +      cat >expect <<EOF &&
 +whitespace/untracked
 +EOF
 +      : >err.expect &&
 +      git ls-files -o -X ignore whitespace >actual 2>err &&
 +      test_cmp expect actual &&
 +      test_cmp err.expect err
 +'
 +
 +test_expect_success !MINGW 'quoting allows trailing whitespace' '
 +      rm -rf whitespace &&
 +      mkdir whitespace &&
 +      >"whitespace/trailing  " &&
 +      >whitespace/untracked &&
 +      echo "whitespace/trailing\\ \\ " >ignore &&
 +      echo whitespace/untracked >expect &&
 +      : >err.expect &&
 +      git ls-files -o -X ignore whitespace >actual 2>err &&
 +      test_cmp expect actual &&
 +      test_cmp err.expect err
 +'
 +
 +test_expect_success !MINGW,!CYGWIN 'correct handling of backslashes' '
 +      rm -rf whitespace &&
 +      mkdir whitespace &&
 +      >"whitespace/trailing 1  " &&
 +      >"whitespace/trailing 2 \\\\" &&
 +      >"whitespace/trailing 3 \\\\" &&
 +      >"whitespace/trailing 4   \\ " &&
 +      >"whitespace/trailing 5 \\ \\ " &&
 +      >"whitespace/trailing 6 \\a\\" &&
 +      >whitespace/untracked &&
 +      sed -e "s/Z$//" >ignore <<-\EOF &&
 +      whitespace/trailing 1 \    Z
 +      whitespace/trailing 2 \\\\Z
 +      whitespace/trailing 3 \\\\ Z
 +      whitespace/trailing 4   \\\    Z
 +      whitespace/trailing 5 \\ \\\   Z
 +      whitespace/trailing 6 \\a\\Z
 +      EOF
 +      echo whitespace/untracked >expect &&
 +      >err.expect &&
 +      git ls-files -o -X ignore whitespace >actual 2>err &&
 +      test_cmp expect actual &&
 +      test_cmp err.expect err
 +'
 +
+ test_expect_success 'info/exclude trumps core.excludesfile' '
+       echo >>global-excludes usually-ignored &&
+       echo >>.git/info/exclude "!usually-ignored" &&
+       >usually-ignored &&
+       echo "?? usually-ignored" >expect &&
+       git status --porcelain usually-ignored >actual &&
+       test_cmp expect actual
+ '
  test_done