Re: Odd .gitignore behaviour
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:43:51
Bruce Stephens [off-list ref] writes:
So I think the output from git-ls-files is as expected (as I interpret the manpage and your explanation). So is git-add just using some different code?
No, you found one of the longstanding bugs in dir.c:read_directory(). The funny thing is that I just sent out a message pointing out bogus handling of per-directory exclude files in ls-files last night. Somehow people have a tendency to encounter the bugs in the same vicinity independently. The initial loop in read_directory() to push per-directory exclusion elements into the stack for directories above the given base forgets that push() does not make a copy of the path given as its parameter but stores the pointer to it instead, so multiple calls to push() need to use separate path buffers. Here is a tentative patch. I do not think the patch is broken but I call it tentative because: - It is ugly -- I never get this "walking path delimited by slashes" loop right; - It leaks the path buffer given to push(), but it is inherent in the design of "push/pop exclude per-directory" API. They were designed to be called from the recursive directory walking, and the path buffers are placed on the function call stack to be reclaimed automatically upon function return; --- dir.c | 55 ++++++++++++++++++++++++++++++++++--------------------- 1 files changed, 34 insertions(+), 21 deletions(-)
diff --git a/dir.c b/dir.c
index fa9f902..d32f437 100644
--- a/dir.c
+++ b/dir.c@@ -651,38 +651,51 @@ static void free_simplify(struct path_simplify *simplify) free(simplify); } +static int push_excludes(struct dir_struct *dir, const char *base, int len) +{ + /* + * base is like "a/b/c/" -- cause .gitignore, b/.gitignore and + * b/c/.gitignore to be read in this order, as if we recursed + * into it. + */ + int stk = -1; + int partlen = 0; + + if (!(dir->exclude_per_dir && len)) + return stk; + + while (1) { + char *part = xmalloc(partlen + 1); + memcpy(part, base, partlen); + part[partlen] = '\0'; + stk = push_exclude_per_directory(dir, part, partlen); + + if (len <= partlen++) + break; + + while (partlen < len && base[partlen] != '/') + partlen++; + partlen++; /* point at one past the found '/' */ + } + return stk; +} + int read_directory(struct dir_struct *dir, const char *path, const char *base, int baselen, const char **pathspec) { struct path_simplify *simplify = create_simplify(pathspec); + int stk; /* * Make sure to do the per-directory exclude for all the * directories leading up to our base. */ - if (baselen) { - if (dir->exclude_per_dir) { - char *p, *pp = xmalloc(baselen+1); - memcpy(pp, base, baselen+1); - p = pp; - while (1) { - char save = *p; - *p = 0; - push_exclude_per_directory(dir, pp, p-pp); - *p++ = save; - if (!save) - break; - p = strchr(p, '/'); - if (p) - p++; - else - p = pp + baselen; - } - free(pp); - } - } + stk = push_excludes(dir, base, baselen); read_directory_recursive(dir, path, base, baselen, 0, simplify); free_simplify(simplify); + if (0 <= stk) + pop_exclude_per_directory(dir, stk); + qsort(dir->entries, dir->nr, sizeof(struct dir_entry *), cmp_name); qsort(dir->ignored, dir->ignored_nr, sizeof(struct dir_entry *), cmp_name); return dir->nr;