Re: Odd .gitignore behaviour

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 15, 2007, 21:51 UTC
Message-ID: <7vir4341ok.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <80ir43e04o.fsf@tiny.isode.net>

Bruce Stephens <bruce.stephens@isode.com> 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;