Commit 52032ae53689ac37350f6af3bf1834122e4b3cf0

Russell Belfer 2012-10-15T12:48:43

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.

diff --git a/include/git2/ignore.h b/include/git2/ignore.h
index f7e04e8..964a108 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 cc94d76..0dd9859 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 b28d8a0..877daf3 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 e711be2..6a377e6 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 3a0ed07..a37653d 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 c27d3fa..cca6450 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 546d48f..1929670 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 592fd25..0000000
--- 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 0000000..4dcd992
--- /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 0000000..88aca01
--- /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 68dc652..bd74b97 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);
+}