Merge pull request #1768 from arrbee/issue-1766-gitignore-weirdness

Fix issue 1766 - bugs in managing ignore file lists

5 files changed, 122 insertions, 14 deletions

diff --git a/src/ignore.c b/src/ignore.c
index 3c23158c2..0c35d0431 100644
--- a/src/ignore.c
+++ b/src/ignore.c
@@ -159,17 +159,36 @@ int git_ignore__push_dir(git_ignores *ign, const char *dir)
 {
 	if (git_buf_joinpath(&ign->dir, ign->dir.ptr, dir) < 0)
 		return -1;
-	else
-		return push_ignore_file(
-			ign->repo, ign, &ign->ign_path, ign->dir.ptr, GIT_IGNORE_FILE);
+
+	return push_ignore_file(
+		ign->repo, ign, &ign->ign_path, ign->dir.ptr, GIT_IGNORE_FILE);
 }
 
 int git_ignore__pop_dir(git_ignores *ign)
 {
 	if (ign->ign_path.length > 0) {
 		git_attr_file *file = git_vector_last(&ign->ign_path);
-		if (git__suffixcmp(ign->dir.ptr, file->key + 2) == 0)
+		const char *start, *end, *scan;
+		size_t keylen;
+
+		/* - 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
+		 * remove the file from the vector if the directory part of the key
+		 * matches the ign->dir path.  We need to test if the "a/b" part of
+		 * the file key matches the path we are about to pop.
+		 */
+
+		for (start = end = scan = &file->key[2]; *scan; ++scan)
+			if (*scan == '/')
+				end = scan; /* point 'end' to last '/' in key */
+		keylen = (end - start) + 1;
+
+		if (ign->dir.size >= keylen &&
+			!memcmp(ign->dir.ptr + ign->dir.size - keylen, start, keylen))
 			git_vector_pop(&ign->ign_path);
+
 		git_buf_rtruncate_at_char(&ign->dir, '/');
 	}
 	return 0;
@@ -298,12 +317,9 @@ int git_ignore_path_is_ignored(
 		path.full.size = (tail - path.full.ptr);
 		path.is_dir = (tail == end) ? full_is_dir : true;
 
-		/* update ignores for new path fragment */
-		if (path.basename == path.path)
-			error = git_ignore__for_path(repo, path.path, &ignores);
-		else
-			error = git_ignore__push_dir(&ignores, path.basename);
-		if (error < 0)
+		/* initialize ignores the first time through */
+		if (path.basename == path.path &&
+			(error = git_ignore__for_path(repo, path.path, &ignores)) < 0)
 			break;
 
 		/* first process builtins - success means path was found */
@@ -327,6 +343,10 @@ int git_ignore_path_is_ignored(
 		if (tail == end)
 			break;
 
+		/* now add this directory to list of ignores */
+		if ((error = git_ignore__push_dir(&ignores, path.path)) < 0)
+			break;
+
 		/* reinstate divider in path */
 		*tail = '/';
 		while (*tail == '/') tail++;
diff --git a/src/ignore.h b/src/ignore.h
index cc114b001..851c824bf 100644
--- a/src/ignore.h
+++ b/src/ignore.h
@@ -24,14 +24,15 @@
  */
 typedef struct {
 	git_repository *repo;
-	git_buf dir;
+	git_buf dir; /* current directory reflected in ign_path */
 	git_attr_file *ign_internal;
 	git_vector ign_path;
 	git_vector ign_global;
 	int ignore_case;
 } git_ignores;
 
-extern int git_ignore__for_path(git_repository *repo, const char *path, git_ignores *ign);
+extern int git_ignore__for_path(
+	git_repository *repo, const char *path, git_ignores *ign);
 
 extern int git_ignore__push_dir(git_ignores *ign, const char *dir);
 
diff --git a/src/iterator.c b/src/iterator.c
index 5917f63fd..bdc98d22b 100644
--- a/src/iterator.c
+++ b/src/iterator.c
@@ -1350,7 +1350,7 @@ int git_iterator_for_workdir_ext(
 	wi->fi.update_entry_cb = workdir_iterator__update_entry;
 
 	if ((error = iterator__update_ignore_case((git_iterator *)wi, flags)) < 0 ||
-		(error = git_ignore__for_path(repo, "", &wi->ignores)) < 0)
+		(error = git_ignore__for_path(repo, ".gitignore", &wi->ignores)) < 0)
 	{
 		git_iterator_free((git_iterator *)wi);
 		return error;
diff --git a/src/path.c b/src/path.c
index 6437979d5..b81675b49 100644
--- a/src/path.c
+++ b/src/path.c
@@ -603,7 +603,7 @@ int git_path_find_dir(git_buf *dir, const char *path, const char *base)
 	}
 
 	/* call dirname if this is not a directory */
-	if (!error && git_path_isdir(dir->ptr) == false)
+	if (!error) /* && git_path_isdir(dir->ptr) == false) */
 		error = git_path_dirname_r(dir, dir->ptr);
 
 	if (!error)
diff --git a/tests-clar/status/ignore.c b/tests-clar/status/ignore.c
index 1b41fed1a..acdc8fb58 100644
--- a/tests-clar/status/ignore.c
+++ b/tests-clar/status/ignore.c
@@ -493,3 +493,90 @@ void test_status_ignore__filenames_with_special_prefixes_do_not_interfere_with_s
 		git_buf_free(&file);
 	}
 }
+
+void test_status_ignore__issue_1766_negated_ignores(void)
+{
+	int ignored = 0;
+	unsigned int status;
+
+	g_repo = cl_git_sandbox_init("empty_standard_repo");
+
+	cl_git_pass(git_futils_mkdir_r(
+		"empty_standard_repo/a", NULL, 0775));
+	cl_git_mkfile(
+		"empty_standard_repo/a/.gitignore", "*\n!.gitignore\n");
+	cl_git_mkfile(
+		"empty_standard_repo/a/ignoreme", "I should be ignored\n");
+
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "a/.gitignore"));
+	cl_assert(!ignored);
+
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "a/ignoreme"));
+	cl_assert(ignored);
+
+	cl_git_pass(git_futils_mkdir_r(
+		"empty_standard_repo/b", NULL, 0775));
+	cl_git_mkfile(
+		"empty_standard_repo/b/.gitignore", "*\n!.gitignore\n");
+	cl_git_mkfile(
+		"empty_standard_repo/b/ignoreme", "I should be ignored\n");
+
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "b/.gitignore"));
+	cl_assert(!ignored);
+
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "b/ignoreme"));
+	cl_assert(ignored);
+
+	/* shouldn't have changed results from first couple either */
+
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "a/.gitignore"));
+	cl_assert(!ignored);
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "a/ignoreme"));
+	cl_assert(ignored);
+
+	/* status should find the two ignore files and nothing else */
+
+	cl_git_pass(git_status_file(&status, g_repo, "a/.gitignore"));
+	cl_assert_equal_i(GIT_STATUS_WT_NEW, (int)status);
+
+	cl_git_pass(git_status_file(&status, g_repo, "a/ignoreme"));
+	cl_assert_equal_i(GIT_STATUS_IGNORED, (int)status);
+
+	cl_git_pass(git_status_file(&status, g_repo, "b/.gitignore"));
+	cl_assert_equal_i(GIT_STATUS_WT_NEW, (int)status);
+
+	cl_git_pass(git_status_file(&status, g_repo, "b/ignoreme"));
+	cl_assert_equal_i(GIT_STATUS_IGNORED, (int)status);
+
+	{
+		git_status_options opts = GIT_STATUS_OPTIONS_INIT;
+		status_entry_counts counts;
+		static const char *paths[] = {
+			"a/.gitignore",
+			"a/ignoreme",
+			"b/.gitignore",
+			"b/ignoreme",
+		};
+		static const unsigned int statuses[] = {
+			GIT_STATUS_WT_NEW,
+			GIT_STATUS_IGNORED,
+			GIT_STATUS_WT_NEW,
+			GIT_STATUS_IGNORED,
+		};
+
+		memset(&counts, 0x0, sizeof(status_entry_counts));
+		counts.expected_entry_count = 4;
+		counts.expected_paths = paths;
+		counts.expected_statuses = statuses;
+
+		opts.flags = GIT_STATUS_OPT_DEFAULTS;
+
+		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);
+	}
+}
+