Fix single-file ignore checks

To answer if a single given file should be ignored, the path to
that file has to be processed progressively checking that there
are no intermediate ignored directories in getting to the file
in question.  This enables that, fixing the broken old behavior,
and adds tests to exercise various ignore situations.

11 files changed, 215 insertions, 27 deletions

diff --git a/include/git2/ignore.h b/include/git2/ignore.h
index f7e04e881..964a108ce 100644
--- a/include/git2/ignore.h
+++ b/include/git2/ignore.h
@@ -54,9 +54,12 @@ GIT_EXTERN(int) git_ignore_clear_internal_rules(
 /**
  * Test if the ignore rules apply to a given path.
  *
- * This function simply checks the ignore rules to see if they would apply
- * to the given file.  This indicates if the file would be ignored regardless
- * of whether the file is already in the index or commited to the repository.
+ * This function checks the ignore rules to see if they would apply to the
+ * given file.  This indicates if the file would be ignored regardless of
+ * whether the file is already in the index or commited to the repository.
+ *
+ * One way to think of this is if you were to do "git add ." on the
+ * directory containing the file, would it be added or not?
  *
  * @param ignored boolean returning 0 if the file is not ignored, 1 if it is
  * @param repo a repository object
diff --git a/include/git2/status.h b/include/git2/status.h
index cc94d7680..0dd98596e 100644
--- a/include/git2/status.h
+++ b/include/git2/status.h
@@ -146,10 +146,12 @@ GIT_EXTERN(int) git_status_file(
 /**
  * Test if the ignore rules apply to a given file.
  *
- * This function simply checks the ignore rules to see if they would apply
- * to the given file.  Unlike git_status_file(), this indicates if the file
- * would be ignored regardless of whether the file is already in the index
- * or in the repository.
+ * This function checks the ignore rules to see if they would apply to the
+ * given file.  This indicates if the file would be ignored regardless of
+ * whether the file is already in the index or commited to the repository.
+ *
+ * One way to think of this is if you were to do "git add ." on the
+ * directory containing the file, would it be added or not?
  *
  * @param ignored boolean returning 0 if the file is not ignored, 1 if it is
  * @param repo a repository object
diff --git a/src/attr_file.h b/src/attr_file.h
index b28d8a02b..877daf306 100644
--- a/src/attr_file.h
+++ b/src/attr_file.h
@@ -71,10 +71,10 @@ typedef struct {
 } git_attr_file;
 
 typedef struct {
-	git_buf     full;
-	const char *path;
-	const char *basename;
-	int         is_dir;
+	git_buf  full;
+	char    *path;
+	char    *basename;
+	int      is_dir;
 } git_attr_path;
 
 typedef enum {
diff --git a/src/ignore.c b/src/ignore.c
index e711be206..6a377e60d 100644
--- a/src/ignore.c
+++ b/src/ignore.c
@@ -277,15 +277,76 @@ int git_ignore_clear_internal_rules(
 int git_ignore_path_is_ignored(
 	int *ignored,
 	git_repository *repo,
-	const char *path)
+	const char *pathname)
 {
 	int error;
+	const char *workdir;
+	git_attr_path path;
+	char *tail, *end;
+	bool full_is_dir;
 	git_ignores ignores;
+	unsigned int i;
+	git_attr_file *file;
 
-	if (git_ignore__for_path(repo, path, &ignores) < 0)
-		return -1;
+	assert(ignored && pathname);
+
+	workdir = repo ? git_repository_workdir(repo) : NULL;
+
+	if ((error = git_attr_path__init(&path, pathname, workdir)) < 0)
+		return error;
+
+	tail = path.path;
+	end  = &path.full.ptr[path.full.size];
+	full_is_dir = path.is_dir;
 
-	error = git_ignore__lookup(&ignores, path, ignored);
+	while (1) {
+		/* advance to next component of path */
+		path.basename = tail;
+
+		while (tail < end && *tail != '/') tail++;
+		*tail = '\0';
+
+		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)
+			break;
+
+		/* first process builtins - success means path was found */
+		if (ignore_lookup_in_rules(
+				&ignores.ign_internal->rules, &path, ignored))
+			goto cleanup;
+
+		/* next process files in the path */
+		git_vector_foreach(&ignores.ign_path, i, file) {
+			if (ignore_lookup_in_rules(&file->rules, &path, ignored))
+				goto cleanup;
+		}
+
+		/* last process global ignores */
+		git_vector_foreach(&ignores.ign_global, i, file) {
+			if (ignore_lookup_in_rules(&file->rules, &path, ignored))
+				goto cleanup;
+		}
+
+		/* if we found no rules before reaching the end, we're done */
+		if (tail == end)
+			break;
+
+		/* reinstate divider in path */
+		*tail = '/';
+		while (*tail == '/') tail++;
+	}
+
+	*ignored = 0;
+
+cleanup:
+	git_attr_path__free(&path);
 	git_ignore__free(&ignores);
 	return error;
 }
diff --git a/src/status.c b/src/status.c
index 3a0ed075f..a37653db4 100644
--- a/src/status.c
+++ b/src/status.c
@@ -86,6 +86,10 @@ int git_status_foreach_ext(
 
 	assert(show <= GIT_STATUS_SHOW_INDEX_THEN_WORKDIR);
 
+	if (show != GIT_STATUS_SHOW_INDEX_ONLY &&
+		(err = git_repository__ensure_not_bare(repo, "status")) < 0)
+		return err;
+
 	if ((err = git_repository_head_tree(&head, repo)) < 0)
 		return err;
 
@@ -245,9 +249,22 @@ int git_status_file(
 		error = GIT_EAMBIGUOUS;
 
 	if (!error && !sfi.count) {
-		giterr_set(GITERR_INVALID,
-			"Attempt to get status of nonexistent file '%s'", path);
-		error = GIT_ENOTFOUND;
+		git_buf full = GIT_BUF_INIT;
+
+		/* if the file actually exists and we still did not get a callback
+		 * for it, then it must be contained inside an ignored directory, so
+		 * mark it as such instead of generating an error.
+		 */
+		if (!git_buf_joinpath(&full, git_repository_workdir(repo), path) &&
+			git_path_exists(full.ptr))
+			sfi.status = GIT_STATUS_IGNORED;
+		else {
+			giterr_set(GITERR_INVALID,
+				"Attempt to get status of nonexistent file '%s'", path);
+			error = GIT_ENOTFOUND;
+		}
+
+		git_buf_free(&full);
 	}
 
 	*status_flags = sfi.status;
diff --git a/tests-clar/diff/iterator.c b/tests-clar/diff/iterator.c
index c27d3fa6c..cca6450d2 100644
--- a/tests-clar/diff/iterator.c
+++ b/tests-clar/diff/iterator.c
@@ -451,13 +451,13 @@ static void workdir_iterator_test(
 
 	git_iterator_free(i);
 
-	cl_assert(count == expected_count);
-	cl_assert(count_all == expected_count + expected_ignores);
+	cl_assert_equal_i(expected_count,count);
+	cl_assert_equal_i(expected_count + expected_ignores, count_all);
 }
 
 void test_diff_iterator__workdir_0(void)
 {
-	workdir_iterator_test("attr", NULL, NULL, 25, 2, NULL, "ign");
+	workdir_iterator_test("attr", NULL, NULL, 27, 1, NULL, "ign");
 }
 
 static const char *status_paths[] = {
diff --git a/tests-clar/resources/attr/gitignore b/tests-clar/resources/attr/gitignore
index 546d48f3a..192967012 100644
--- a/tests-clar/resources/attr/gitignore
+++ b/tests-clar/resources/attr/gitignore
@@ -1,3 +1,2 @@
-sub
 ign
 dir/
diff --git a/tests-clar/resources/attr/sub/ign b/tests-clar/resources/attr/sub/ign
deleted file mode 100644
index 592fd2594..000000000
--- a/tests-clar/resources/attr/sub/ign
+++ /dev/null
@@ -1 +0,0 @@
-ignore me
diff --git a/tests-clar/resources/attr/sub/ign/file b/tests-clar/resources/attr/sub/ign/file
new file mode 100644
index 000000000..4dcd992e1
--- /dev/null
+++ b/tests-clar/resources/attr/sub/ign/file
@@ -0,0 +1 @@
+in ignored dir
diff --git a/tests-clar/resources/attr/sub/ign/sub/file b/tests-clar/resources/attr/sub/ign/sub/file
new file mode 100644
index 000000000..88aca0164
--- /dev/null
+++ b/tests-clar/resources/attr/sub/ign/sub/file
@@ -0,0 +1 @@
+below ignored dir
diff --git a/tests-clar/status/ignore.c b/tests-clar/status/ignore.c
index 68dc652f5..bd74b9740 100644
--- a/tests-clar/status/ignore.c
+++ b/tests-clar/status/ignore.c
@@ -22,21 +22,25 @@ void test_status_ignore__0(void)
 		const char *path;
 		int expected;
 	} test_cases[] = {
-		/* patterns "sub" and "ign" from .gitignore */
+		/* pattern "ign" from .gitignore */
 		{ "file", 0 },
 		{ "ign", 1 },
-		{ "sub", 1 },
+		{ "sub", 0 },
 		{ "sub/file", 0 },
 		{ "sub/ign", 1 },
-		{ "sub/sub", 1 },
+		{ "sub/ign/file", 1 },
+		{ "sub/ign/sub", 1 },
+		{ "sub/ign/sub/file", 1 },
+		{ "sub/sub", 0 },
 		{ "sub/sub/file", 0 },
 		{ "sub/sub/ign", 1 },
-		{ "sub/sub/sub", 1 },
+		{ "sub/sub/sub", 0 },
 		/* pattern "dir/" from .gitignore */
 		{ "dir", 1 },
 		{ "dir/", 1 },
 		{ "sub/dir", 1 },
 		{ "sub/dir/", 1 },
+		{ "sub/dir/file", 1 }, /* contained in ignored parent */
 		{ "sub/sub/dir", 0 }, /* dir is not actually a dir, but a file */
 		{ NULL, 0 }
 	}, *one_test;
@@ -172,6 +176,61 @@ void test_status_ignore__ignore_pattern_ignorecase(void)
 	cl_assert(flags == ignore_case ? GIT_STATUS_IGNORED : GIT_STATUS_WT_NEW);
 }
 
+void test_status_ignore__subdirectories(void)
+{
+	status_entry_single st;
+	int ignored;
+	git_status_options opts;
+
+	g_repo = cl_git_sandbox_init("empty_standard_repo");
+
+	cl_git_mkfile(
+		"empty_standard_repo/ignore_me", "I'm going to be ignored!");
+
+	cl_git_rewritefile("empty_standard_repo/.gitignore", "ignore_me\n");
+
+	memset(&st, 0, sizeof(st));
+	cl_git_pass(git_status_foreach(g_repo, cb_status__single, &st));
+	cl_assert_equal_i(2, st.count);
+	cl_assert(st.status == GIT_STATUS_IGNORED);
+
+	cl_git_pass(git_status_file(&st.status, g_repo, "ignore_me"));
+	cl_assert(st.status == GIT_STATUS_IGNORED);
+
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "ignore_me"));
+	cl_assert(ignored);
+
+
+	/* So, interestingly, as per the comment in diff_from_iterators() the
+	 * following file is ignored, but in a way so that it does not show up
+	 * in status even if INCLUDE_IGNORED is used.  This actually matches
+	 * core git's behavior - if you follow these steps and try running "git
+	 * status -uall --ignored" then the following file and directory will
+	 * not show up in the output at all.
+	 */
+
+	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!");
+
+	opts.show = GIT_STATUS_SHOW_INDEX_AND_WORKDIR;
+	opts.flags = GIT_STATUS_OPT_INCLUDE_IGNORED |
+		GIT_STATUS_OPT_INCLUDE_UNTRACKED |
+		GIT_STATUS_OPT_RECURSE_UNTRACKED_DIRS;
+
+	memset(&st, 0, sizeof(st));
+	cl_git_pass(git_status_foreach(g_repo, cb_status__single, &st));
+	cl_assert_equal_i(2, st.count);
+
+	cl_git_pass(git_status_file(&st.status, g_repo, "test/ignore_me/file"));
+	cl_assert(st.status == GIT_STATUS_IGNORED);
+
+	cl_git_pass(
+		git_status_should_ignore(&ignored, g_repo, "test/ignore_me/file"));
+	cl_assert(ignored);
+}
+
 void test_status_ignore__adding_internal_ignores(void)
 {
 	int ignored;
@@ -234,3 +293,49 @@ void test_status_ignore__add_internal_as_first_thing(void)
 	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "two.bar"));
 	cl_assert(!ignored);
 }
+
+void test_status_ignore__internal_ignores_inside_deep_paths(void)
+{
+	int ignored;
+	const char *add_me = "Debug\nthis/is/deep\npatterned*/dir\n";
+
+	g_repo = cl_git_sandbox_init("empty_standard_repo");
+
+	cl_git_pass(git_ignore_add_rule(g_repo, add_me));
+
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "Debug"));
+	cl_assert(ignored);
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "and/Debug"));
+	cl_assert(ignored);
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "really/Debug/this/file"));
+	cl_assert(ignored);
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "Debug/what/I/say"));
+	cl_assert(ignored);
+
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "and/NoDebug"));
+	cl_assert(!ignored);
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "NoDebug/this"));
+	cl_assert(!ignored);
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "please/NoDebug/this"));
+	cl_assert(!ignored);
+
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "this/is/deep"));
+	cl_assert(ignored);
+	/* pattern containing slash gets FNM_PATHNAME so all slashes must match */
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "and/this/is/deep"));
+	cl_assert(!ignored);
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "this/is/deep/too"));
+	cl_assert(ignored);
+	/* pattern containing slash gets FNM_PATHNAME so all slashes must match */
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "but/this/is/deep/and/ignored"));
+	cl_assert(!ignored);
+
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "this/is/not/deep"));
+	cl_assert(!ignored);
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "is/this/not/as/deep"));
+	cl_assert(!ignored);
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "this/is/deepish"));
+	cl_assert(!ignored);
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "xthis/is/deep"));
+	cl_assert(!ignored);
+}