Commit 8f7bc6461b81499216b73881b07f5476c5085660

Russell Belfer 2014-04-10T16:33:39

Fix bug popping ignore files during wd iteration There were a couple bugs in popping ignore files during iteration that could result in incorrect decisions be made and thus ignore files below the root either not being loaded correctly or not being popped at the right time. One bug was an off-by-one in comparing the path of the gitignore file with the path being exited during iteration. The second bug was not correctly truncating the path being tracked during traversal if there were no ignores on the list (i.e. when you have no .gitignore at the root, but do have some in contained directories).

diff --git a/src/ignore.c b/src/ignore.c
index c79fe48..aef3e39 100644
--- a/src/ignore.c
+++ b/src/ignore.c
@@ -78,6 +78,8 @@ static int push_one_ignore(void *payload, git_buf *path)
 {
 	git_ignores *ign = payload;
 
+	ign->depth++;
+
 	return push_ignore_file(
 		ign->repo, ign, &ign->ign_path, path->ptr, GIT_IGNORE_FILE);
 }
@@ -108,6 +110,7 @@ int git_ignore__for_path(
 	ignores->repo = repo;
 	git_buf_init(&ignores->dir, 0);
 	ignores->ign_internal = NULL;
+	ignores->depth = 0;
 
 	/* Read the ignore_case flag */
 	if ((error = git_repository__cvar(
@@ -163,6 +166,8 @@ int git_ignore__push_dir(git_ignores *ign, const char *dir)
 	if (git_buf_joinpath(&ign->dir, ign->dir.ptr, dir) < 0)
 		return -1;
 
+	ign->depth++;
+
 	return push_ignore_file(
 		ign->repo, ign, &ign->ign_path, ign->dir.ptr, GIT_IGNORE_FILE);
 }
@@ -174,7 +179,7 @@ int git_ignore__pop_dir(git_ignores *ign)
 		const char *start, *end, *scan;
 		size_t keylen;
 
-		/* - ign->dir looks something like "a/b" (or "a/b/c/d")
+		/* - ign->dir looks something like "a/b/" (or "a/b/c/d/")
 		 * - file->key looks something like "0#a/b/.gitignore
 		 *
 		 * We are popping the last directory off ign->dir.  We also want to
@@ -191,9 +196,13 @@ int git_ignore__pop_dir(git_ignores *ign)
 		if (ign->dir.size >= keylen &&
 			!memcmp(ign->dir.ptr + ign->dir.size - keylen, start, keylen))
 			git_vector_pop(&ign->ign_path);
+	}
 
+	if (--ign->depth > 0) {
 		git_buf_rtruncate_at_char(&ign->dir, '/');
+		git_path_to_dir(&ign->dir);
 	}
+
 	return 0;
 }
 
diff --git a/src/ignore.h b/src/ignore.h
index 851c824..46172c7 100644
--- a/src/ignore.h
+++ b/src/ignore.h
@@ -29,6 +29,7 @@ typedef struct {
 	git_vector ign_path;
 	git_vector ign_global;
 	int ignore_case;
+	int depth;
 } git_ignores;
 
 extern int git_ignore__for_path(
diff --git a/tests/status/ignore.c b/tests/status/ignore.c
index acdc8fb..1fefcba 100644
--- a/tests/status/ignore.c
+++ b/tests/status/ignore.c
@@ -228,6 +228,32 @@ void test_status_ignore__subdirectories(void)
 	cl_assert(ignored);
 }
 
+static void make_test_data(void)
+{
+	static const char *files[] = {
+		"empty_standard_repo/dir/a/ignore_me",
+		"empty_standard_repo/dir/b/ignore_me",
+		"empty_standard_repo/dir/ignore_me",
+		"empty_standard_repo/ignore_also/file",
+		"empty_standard_repo/ignore_me",
+		"empty_standard_repo/test/ignore_me/file",
+		"empty_standard_repo/test/ignore_me/file2",
+		"empty_standard_repo/test/ignore_me/and_me/file",
+		NULL
+	};
+	static const char *repo = "empty_standard_repo";
+	const char **scan;
+	size_t repolen = strlen(repo) + 1;
+
+	g_repo = cl_git_sandbox_init(repo);
+
+	for (scan = files; *scan != NULL; ++scan) {
+		cl_git_pass(git_futils_mkdir(
+			*scan + repolen, repo, 0777, GIT_MKDIR_PATH | GIT_MKDIR_SKIP_LAST));
+		cl_git_mkfile(*scan, "contents");
+	}
+}
+
 void test_status_ignore__subdirectories_recursion(void)
 {
 	/* Let's try again with recursing into ignored dirs turned on */
@@ -235,6 +261,9 @@ void test_status_ignore__subdirectories_recursion(void)
 	status_entry_counts counts;
 	static const char *paths_r[] = {
 		".gitignore",
+		"dir/a/ignore_me",
+		"dir/b/ignore_me",
+		"dir/ignore_me",
 		"ignore_also/file",
 		"ignore_me",
 		"test/ignore_me/and_me/file",
@@ -242,49 +271,30 @@ void test_status_ignore__subdirectories_recursion(void)
 		"test/ignore_me/file2",
 	};
 	static const unsigned int statuses_r[] = {
-		GIT_STATUS_WT_NEW,
-		GIT_STATUS_IGNORED,
-		GIT_STATUS_IGNORED,
-		GIT_STATUS_IGNORED,
-		GIT_STATUS_IGNORED,
-		GIT_STATUS_IGNORED,
+		GIT_STATUS_WT_NEW,  GIT_STATUS_IGNORED, GIT_STATUS_IGNORED,
+		GIT_STATUS_IGNORED, GIT_STATUS_IGNORED, GIT_STATUS_IGNORED,
+		GIT_STATUS_IGNORED, GIT_STATUS_IGNORED, GIT_STATUS_IGNORED,
 	};
 	static const char *paths_nr[] = {
 		".gitignore",
+		"dir/a/ignore_me",
+		"dir/b/ignore_me",
+		"dir/ignore_me",
 		"ignore_also/",
 		"ignore_me",
 		"test/ignore_me/",
 	};
 	static const unsigned int statuses_nr[] = {
 		GIT_STATUS_WT_NEW,
-		GIT_STATUS_IGNORED,
-		GIT_STATUS_IGNORED,
-		GIT_STATUS_IGNORED,
+		GIT_STATUS_IGNORED, GIT_STATUS_IGNORED, GIT_STATUS_IGNORED,
+		GIT_STATUS_IGNORED, GIT_STATUS_IGNORED, GIT_STATUS_IGNORED,
 	};
 
-	g_repo = cl_git_sandbox_init("empty_standard_repo");
-
+	make_test_data();
 	cl_git_rewritefile("empty_standard_repo/.gitignore", "ignore_me\n/ignore_also\n");
 
-	cl_git_mkfile(
-		"empty_standard_repo/ignore_me", "I'm going to be ignored!");
-	cl_git_pass(git_futils_mkdir_r(
-		"empty_standard_repo/test/ignore_me", NULL, 0775));
-	cl_git_mkfile(
-		"empty_standard_repo/test/ignore_me/file", "I'm going to be ignored!");
-	cl_git_mkfile(
-		"empty_standard_repo/test/ignore_me/file2", "Me, too!");
-	cl_git_pass(git_futils_mkdir_r(
-		"empty_standard_repo/test/ignore_me/and_me", NULL, 0775));
-	cl_git_mkfile(
-		"empty_standard_repo/test/ignore_me/and_me/file", "Deeply ignored");
-	cl_git_pass(git_futils_mkdir_r(
-		"empty_standard_repo/ignore_also", NULL, 0775));
-	cl_git_mkfile(
-		"empty_standard_repo/ignore_also/file", "I'm going to be ignored!");
-
 	memset(&counts, 0x0, sizeof(status_entry_counts));
-	counts.expected_entry_count = 6;
+	counts.expected_entry_count = 9;
 	counts.expected_paths = paths_r;
 	counts.expected_statuses = statuses_r;
 
@@ -299,7 +309,7 @@ void test_status_ignore__subdirectories_recursion(void)
 
 
 	memset(&counts, 0x0, sizeof(status_entry_counts));
-	counts.expected_entry_count = 4;
+	counts.expected_entry_count = 7;
 	counts.expected_paths = paths_nr;
 	counts.expected_statuses = statuses_nr;
 
@@ -313,6 +323,47 @@ void test_status_ignore__subdirectories_recursion(void)
 	cl_assert_equal_i(0, counts.wrong_sorted_path);
 }
 
+void test_status_ignore__subdirectories_not_at_root(void)
+{
+	git_status_options opts = GIT_STATUS_OPTIONS_INIT;
+	status_entry_counts counts;
+	static const char *paths_1[] = {
+		"dir/.gitignore",
+		"dir/a/ignore_me",
+		"dir/b/ignore_me",
+		"dir/ignore_me",
+		"ignore_also/file",
+		"ignore_me",
+		"test/.gitignore",
+		"test/ignore_me/and_me/file",
+		"test/ignore_me/file",
+		"test/ignore_me/file2",
+	};
+	static const unsigned int statuses_1[] = {
+		GIT_STATUS_WT_NEW,  GIT_STATUS_IGNORED, GIT_STATUS_IGNORED,
+		GIT_STATUS_IGNORED, GIT_STATUS_WT_NEW, GIT_STATUS_WT_NEW,
+		GIT_STATUS_WT_NEW, GIT_STATUS_IGNORED, GIT_STATUS_WT_NEW, GIT_STATUS_WT_NEW,
+	};
+
+	make_test_data();
+	cl_git_rewritefile("empty_standard_repo/dir/.gitignore", "ignore_me\n/ignore_also\n");
+	cl_git_rewritefile("empty_standard_repo/test/.gitignore", "and_me\n");
+
+	memset(&counts, 0x0, sizeof(status_entry_counts));
+	counts.expected_entry_count = 10;
+	counts.expected_paths = paths_1;
+	counts.expected_statuses = statuses_1;
+
+	opts.flags = GIT_STATUS_OPT_DEFAULTS | GIT_STATUS_OPT_RECURSE_IGNORED_DIRS;
+
+	cl_git_pass(git_status_foreach_ext(
+		g_repo, &opts, cb_status__normal, &counts));
+
+	cl_assert_equal_i(counts.expected_entry_count, counts.entry_count);
+	cl_assert_equal_i(0, counts.wrong_status_flags_count);
+	cl_assert_equal_i(0, counts.wrong_sorted_path);
+}
+
 void test_status_ignore__adding_internal_ignores(void)
 {
 	int ignored;