Merge pull request #2279 from libgit2/rb/moar-eegnöre-fîxés

Fix several ignore and attribute file behavior bugs

11 files changed, 325 insertions, 229 deletions

diff --git a/src/attr.c b/src/attr.c
index 622874348..05b0c1b3c 100644
--- a/src/attr.c
+++ b/src/attr.c
@@ -217,6 +217,74 @@ cleanup:
 	return error;
 }
 
+static int preload_attr_file(
+	git_repository *repo,
+	git_attr_file_source source,
+	const char *base,
+	const char *file)
+{
+	int error;
+	git_attr_file *preload = NULL;
+
+	if (!file)
+		return 0;
+	if (!(error = git_attr_cache__get(
+			&preload, repo, source, base, file, git_attr_file__parse_buffer)))
+		git_attr_file__free(preload);
+
+	return error;
+}
+
+static int attr_setup(git_repository *repo)
+{
+	int error = 0;
+	const char *workdir = git_repository_workdir(repo);
+	git_index *idx = NULL;
+	git_buf sys = GIT_BUF_INIT;
+
+	if ((error = git_attr_cache__init(repo)) < 0)
+		return error;
+
+	/* preload attribute files that could contain macros so the
+	 * definitions will be available for later file parsing
+	 */
+
+	if (!(error = git_sysdir_find_system_file(&sys, GIT_ATTR_FILE_SYSTEM))) {
+		error = preload_attr_file(
+			repo, GIT_ATTR_FILE__FROM_FILE, NULL, sys.ptr);
+		git_buf_free(&sys);
+	}
+	if (error < 0) {
+		if (error == GIT_ENOTFOUND) {
+			giterr_clear();
+			error = 0;
+		} else
+			return error;
+	}
+
+	if ((error = preload_attr_file(
+			repo, GIT_ATTR_FILE__FROM_FILE,
+			NULL, git_repository_attr_cache(repo)->cfg_attr_file)) < 0)
+		return error;
+
+	if ((error = preload_attr_file(
+			repo, GIT_ATTR_FILE__FROM_FILE,
+			git_repository_path(repo), GIT_ATTR_FILE_INREPO)) < 0)
+		return error;
+
+	if (workdir != NULL &&
+		(error = preload_attr_file(
+			repo, GIT_ATTR_FILE__FROM_FILE, workdir, GIT_ATTR_FILE)) < 0)
+		return error;
+
+	if ((error = git_repository_index__weakptr(&idx, repo)) < 0 ||
+		(error = preload_attr_file(
+			repo, GIT_ATTR_FILE__FROM_INDEX, NULL, GIT_ATTR_FILE)) < 0)
+		return error;
+
+	return error;
+}
+
 int git_attr_add_macro(
 	git_repository *repo,
 	const char *name,
@@ -226,8 +294,8 @@ int git_attr_add_macro(
 	git_attr_rule *macro = NULL;
 	git_pool *pool;
 
-	if (git_attr_cache__init(repo) < 0)
-		return -1;
+	if ((error = git_attr_cache__init(repo)) < 0)
+		return error;
 
 	macro = git__calloc(1, sizeof(git_attr_rule));
 	GITERR_CHECK_ALLOC(macro);
@@ -348,7 +416,7 @@ static int collect_attr_files(
 	const char *workdir = git_repository_workdir(repo);
 	attr_walk_up_info info = { NULL };
 
-	if ((error = git_attr_cache__init(repo)) < 0)
+	if ((error = attr_setup(repo)) < 0)
 		return error;
 
 	/* Resolve path in a non-bare repo */
diff --git a/src/attr_file.c b/src/attr_file.c
index 65bbf78e8..d107b5ab0 100644
--- a/src/attr_file.c
+++ b/src/attr_file.c
@@ -248,9 +248,7 @@ int git_attr_file__parse_buffer(
 				repo, &attrs->pool, &rule->assigns, &scan)))
 		{
 			if (rule->match.flags & GIT_ATTR_FNMATCH_MACRO)
-				/* should generate error/warning if this is coming from any
-				 * file other than .gitattributes at repo root.
-				 */
+				/* TODO: warning if macro found in file below repo root */
 				error = git_attr_cache__insert_macro(repo, rule);
 			else
 				error = git_vector_insert(&attrs->rules, rule);
@@ -355,6 +353,8 @@ bool git_attr_fnmatch__match(
 
 	if (match->flags & GIT_ATTR_FNMATCH_ICASE)
 		flags |= FNM_CASEFOLD;
+	if (match->flags & GIT_ATTR_FNMATCH_LEADINGDIR)
+		flags |= FNM_LEADING_DIR;
 
 	if (match->flags & GIT_ATTR_FNMATCH_FULLPATH) {
 		filename = path->path;
@@ -545,6 +545,14 @@ int git_attr_fnmatch__parse(
 		if (--slash_count <= 0)
 			spec->flags = spec->flags & ~GIT_ATTR_FNMATCH_FULLPATH;
 	}
+	if ((spec->flags & GIT_ATTR_FNMATCH_NOLEADINGDIR) == 0 &&
+		spec->length >= 2 &&
+		pattern[spec->length - 1] == '*' &&
+		pattern[spec->length - 2] == '/') {
+		spec->length -= 2;
+		spec->flags = spec->flags | GIT_ATTR_FNMATCH_LEADINGDIR;
+		/* leave FULLPATH match on, however */
+	}
 
 	if ((spec->flags & GIT_ATTR_FNMATCH_FULLPATH) != 0 &&
 		context != NULL && git_path_root(pattern) < 0)
diff --git a/src/attr_file.h b/src/attr_file.h
index c906be44d..e50aec07c 100644
--- a/src/attr_file.h
+++ b/src/attr_file.h
@@ -30,10 +30,12 @@
 #define GIT_ATTR_FNMATCH_MATCH_ALL	(1U << 8)
 #define GIT_ATTR_FNMATCH_ALLOWNEG   (1U << 9)
 #define GIT_ATTR_FNMATCH_ALLOWMACRO (1U << 10)
+#define GIT_ATTR_FNMATCH_LEADINGDIR (1U << 11)
+#define GIT_ATTR_FNMATCH_NOLEADINGDIR (1U << 12)
 
 #define GIT_ATTR_FNMATCH__INCOMING \
-	(GIT_ATTR_FNMATCH_ALLOWSPACE | \
-	 GIT_ATTR_FNMATCH_ALLOWNEG | GIT_ATTR_FNMATCH_ALLOWMACRO)
+	(GIT_ATTR_FNMATCH_ALLOWSPACE | GIT_ATTR_FNMATCH_ALLOWNEG | \
+	 GIT_ATTR_FNMATCH_ALLOWMACRO | GIT_ATTR_FNMATCH_NOLEADINGDIR)
 
 typedef enum {
 	GIT_ATTR_FILE__IN_MEMORY  = 0,
diff --git a/src/ignore.c b/src/ignore.c
index deae204f8..b08ff2200 100644
--- a/src/ignore.c
+++ b/src/ignore.c
@@ -123,7 +123,7 @@ int git_ignore__for_path(
 	int error = 0;
 	const char *workdir = git_repository_workdir(repo);
 
-	assert(ignores);
+	assert(ignores && path);
 
 	memset(ignores, 0, sizeof(*ignores));
 	ignores->repo = repo;
@@ -140,10 +140,13 @@ int git_ignore__for_path(
 	if (workdir && git_path_root(path) < 0)
 		error = git_path_find_dir(&ignores->dir, path, workdir);
 	else
-		error = git_buf_sets(&ignores->dir, path);
+		error = git_buf_joinpath(&ignores->dir, path, "");
 	if (error < 0)
 		goto cleanup;
 
+	if (workdir && !git__prefixcmp(ignores->dir.ptr, workdir))
+		ignores->dir_root = strlen(workdir);
+
 	/* set up internals */
 	if ((error = get_internal_ignores(&ignores->ign_internal, repo)) < 0)
 		goto cleanup;
@@ -204,10 +207,10 @@ int git_ignore__pop_dir(git_ignores *ign)
 
 		if ((end = strrchr(start, '/')) != NULL) {
 			size_t dirlen = (end - start) + 1;
+			const char *relpath = ign->dir.ptr + ign->dir_root;
+			size_t pathlen = ign->dir.size - ign->dir_root;
 
-			if (ign->dir.size >= dirlen &&
-				!memcmp(ign->dir.ptr + ign->dir.size - dirlen, start, dirlen))
-			{
+			if (pathlen == dirlen && !memcmp(relpath, start, dirlen)) {
 				git_vector_pop(&ign->ign_path);
 				git_attr_file__free(file);
 			}
diff --git a/src/ignore.h b/src/ignore.h
index 46172c72f..ff9369000 100644
--- a/src/ignore.h
+++ b/src/ignore.h
@@ -28,6 +28,7 @@ typedef struct {
 	git_attr_file *ign_internal;
 	git_vector ign_path;
 	git_vector ign_global;
+	size_t dir_root; /* offset in dir to repo root */
 	int ignore_case;
 	int depth;
 } git_ignores;
diff --git a/src/path.c b/src/path.c
index 7cad28d45..a990b005f 100644
--- a/src/path.c
+++ b/src/path.c
@@ -624,7 +624,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) */
-		error = git_path_dirname_r(dir, dir->ptr);
+		error = (git_path_dirname_r(dir, dir->ptr) < 0) ? -1 : 0;
 
 	if (!error)
 		error = git_path_to_dir(dir);
diff --git a/src/pathspec.c b/src/pathspec.c
index 09650de7c..a01d74f07 100644
--- a/src/pathspec.c
+++ b/src/pathspec.c
@@ -83,7 +83,8 @@ int git_pathspec__vinit(
 		if (!match)
 			return -1;
 
-		match->flags = GIT_ATTR_FNMATCH_ALLOWSPACE | GIT_ATTR_FNMATCH_ALLOWNEG;
+		match->flags = GIT_ATTR_FNMATCH_ALLOWSPACE |
+			GIT_ATTR_FNMATCH_ALLOWNEG | GIT_ATTR_FNMATCH_NOLEADINGDIR;
 
 		ret = git_attr_fnmatch__parse(match, strpool, NULL, &pattern);
 		if (ret == GIT_ENOTFOUND) {
diff --git a/tests/attr/ignore.c b/tests/attr/ignore.c
index a83c5bd74..68875194d 100644
--- a/tests/attr/ignore.c
+++ b/tests/attr/ignore.c
@@ -16,7 +16,7 @@ void test_attr_ignore__cleanup(void)
 	g_repo = NULL;
 }
 
-void assert_is_ignored_(
+static void assert_is_ignored_(
 	bool expected, const char *filepath, const char *file, int line)
 {
 	int is_ignored = 0;
diff --git a/tests/attr/repo.c b/tests/attr/repo.c
index 71dc7a5b5..9aab7ed96 100644
--- a/tests/attr/repo.c
+++ b/tests/attr/repo.c
@@ -23,49 +23,74 @@ void test_attr_repo__cleanup(void)
 	g_repo = NULL;
 }
 
+static struct attr_expected get_one_test_cases[] = {
+	{ "root_test1", "repoattr", EXPECT_TRUE, NULL },
+	{ "root_test1", "rootattr", EXPECT_TRUE, NULL },
+	{ "root_test1", "missingattr", EXPECT_UNDEFINED, NULL },
+	{ "root_test1", "subattr", EXPECT_UNDEFINED, NULL },
+	{ "root_test1", "negattr", EXPECT_UNDEFINED, NULL },
+	{ "root_test2", "repoattr", EXPECT_TRUE, NULL },
+	{ "root_test2", "rootattr", EXPECT_FALSE, NULL },
+	{ "root_test2", "missingattr", EXPECT_UNDEFINED, NULL },
+	{ "root_test2", "multiattr", EXPECT_FALSE, NULL },
+	{ "root_test3", "repoattr", EXPECT_TRUE, NULL },
+	{ "root_test3", "rootattr", EXPECT_UNDEFINED, NULL },
+	{ "root_test3", "multiattr", EXPECT_STRING, "3" },
+	{ "root_test3", "multi2", EXPECT_UNDEFINED, NULL },
+	{ "sub/subdir_test1", "repoattr", EXPECT_TRUE, NULL },
+	{ "sub/subdir_test1", "rootattr", EXPECT_TRUE, NULL },
+	{ "sub/subdir_test1", "missingattr", EXPECT_UNDEFINED, NULL },
+	{ "sub/subdir_test1", "subattr", EXPECT_STRING, "yes" },
+	{ "sub/subdir_test1", "negattr", EXPECT_FALSE, NULL },
+	{ "sub/subdir_test1", "another", EXPECT_UNDEFINED, NULL },
+	{ "sub/subdir_test2.txt", "repoattr", EXPECT_TRUE, NULL },
+	{ "sub/subdir_test2.txt", "rootattr", EXPECT_TRUE, NULL },
+	{ "sub/subdir_test2.txt", "missingattr", EXPECT_UNDEFINED, NULL },
+	{ "sub/subdir_test2.txt", "subattr", EXPECT_STRING, "yes" },
+	{ "sub/subdir_test2.txt", "negattr", EXPECT_FALSE, NULL },
+	{ "sub/subdir_test2.txt", "another", EXPECT_STRING, "zero" },
+	{ "sub/subdir_test2.txt", "reposub", EXPECT_TRUE, NULL },
+	{ "sub/sub/subdir.txt", "another", EXPECT_STRING, "one" },
+	{ "sub/sub/subdir.txt", "reposubsub", EXPECT_TRUE, NULL },
+	{ "sub/sub/subdir.txt", "reposub", EXPECT_UNDEFINED, NULL },
+	{ "does-not-exist", "foo", EXPECT_STRING, "yes" },
+	{ "sub/deep/file", "deepdeep", EXPECT_TRUE, NULL },
+	{ "sub/sub/d/no", "test", EXPECT_STRING, "a/b/d/*" },
+	{ "sub/sub/d/yes", "test", EXPECT_UNDEFINED, NULL },
+};
+
 void test_attr_repo__get_one(void)
 {
-	struct attr_expected test_cases[] = {
-		{ "root_test1", "repoattr", EXPECT_TRUE, NULL },
-		{ "root_test1", "rootattr", EXPECT_TRUE, NULL },
-		{ "root_test1", "missingattr", EXPECT_UNDEFINED, NULL },
-		{ "root_test1", "subattr", EXPECT_UNDEFINED, NULL },
-		{ "root_test1", "negattr", EXPECT_UNDEFINED, NULL },
-		{ "root_test2", "repoattr", EXPECT_TRUE, NULL },
-		{ "root_test2", "rootattr", EXPECT_FALSE, NULL },
-		{ "root_test2", "missingattr", EXPECT_UNDEFINED, NULL },
-		{ "root_test2", "multiattr", EXPECT_FALSE, NULL },
-		{ "root_test3", "repoattr", EXPECT_TRUE, NULL },
-		{ "root_test3", "rootattr", EXPECT_UNDEFINED, NULL },
-		{ "root_test3", "multiattr", EXPECT_STRING, "3" },
-		{ "root_test3", "multi2", EXPECT_UNDEFINED, NULL },
-		{ "sub/subdir_test1", "repoattr", EXPECT_TRUE, NULL },
-		{ "sub/subdir_test1", "rootattr", EXPECT_TRUE, NULL },
-		{ "sub/subdir_test1", "missingattr", EXPECT_UNDEFINED, NULL },
-		{ "sub/subdir_test1", "subattr", EXPECT_STRING, "yes" },
-		{ "sub/subdir_test1", "negattr", EXPECT_FALSE, NULL },
-		{ "sub/subdir_test1", "another", EXPECT_UNDEFINED, NULL },
-		{ "sub/subdir_test2.txt", "repoattr", EXPECT_TRUE, NULL },
-		{ "sub/subdir_test2.txt", "rootattr", EXPECT_TRUE, NULL },
-		{ "sub/subdir_test2.txt", "missingattr", EXPECT_UNDEFINED, NULL },
-		{ "sub/subdir_test2.txt", "subattr", EXPECT_STRING, "yes" },
-		{ "sub/subdir_test2.txt", "negattr", EXPECT_FALSE, NULL },
-		{ "sub/subdir_test2.txt", "another", EXPECT_STRING, "zero" },
-		{ "sub/subdir_test2.txt", "reposub", EXPECT_TRUE, NULL },
-		{ "sub/sub/subdir.txt", "another", EXPECT_STRING, "one" },
-		{ "sub/sub/subdir.txt", "reposubsub", EXPECT_TRUE, NULL },
-		{ "sub/sub/subdir.txt", "reposub", EXPECT_UNDEFINED, NULL },
-		{ "does-not-exist", "foo", EXPECT_STRING, "yes" },
-		{ "sub/deep/file", "deepdeep", EXPECT_TRUE, NULL },
-		{ "sub/sub/d/no", "test", EXPECT_STRING, "a/b/d/*" },
-		{ "sub/sub/d/yes", "test", EXPECT_UNDEFINED, NULL },
-		{ NULL, NULL, 0, NULL }
-	}, *scan;
-
-	for (scan = test_cases; scan->path != NULL; scan++) {
+	int i;
+
+	for (i = 0; i < (int)ARRAY_SIZE(get_one_test_cases); ++i) {
+		struct attr_expected *scan = &get_one_test_cases[i];
+		const char *value;
+
+		cl_git_pass(git_attr_get(&value, g_repo, 0, scan->path, scan->attr));
+		attr_check_expected(
+			scan->expected, scan->expected_str, scan->attr, value);
+	}
+
+	cl_assert(git_attr_cache__is_cached(
+		g_repo, GIT_ATTR_FILE__FROM_FILE, ".git/info/attributes"));
+	cl_assert(git_attr_cache__is_cached(
+		g_repo, GIT_ATTR_FILE__FROM_FILE, ".gitattributes"));
+	cl_assert(git_attr_cache__is_cached(
+		g_repo, GIT_ATTR_FILE__FROM_FILE, "sub/.gitattributes"));
+}
+
+void test_attr_repo__get_one_start_deep(void)
+{
+	int i;
+
+	for (i = (int)ARRAY_SIZE(get_one_test_cases) - 1; i >= 0; --i) {
+		struct attr_expected *scan = &get_one_test_cases[i];
 		const char *value;
+
 		cl_git_pass(git_attr_get(&value, g_repo, 0, scan->path, scan->attr));
-		attr_check_expected(scan->expected, scan->expected_str, scan->attr, value);
+		attr_check_expected(
+			scan->expected, scan->expected_str, scan->attr, value);
 	}
 
 	cl_assert(git_attr_cache__is_cached(
diff --git a/tests/status/ignore.c b/tests/status/ignore.c
index 052a8eae8..d88b2eb6b 100644
--- a/tests/status/ignore.c
+++ b/tests/status/ignore.c
@@ -16,6 +16,23 @@ void test_status_ignore__cleanup(void)
 	cl_git_sandbox_cleanup();
 }
 
+static void assert_ignored_(
+	bool expected, const char *filepath, const char *file, int line)
+{
+	int is_ignored = 0;
+	cl_git_pass_(
+		git_status_should_ignore(&is_ignored, g_repo, filepath), file, line);
+	clar__assert(
+		(expected != 0) == (is_ignored != 0),
+		file, line, "expected != is_ignored", filepath, 1);
+}
+#define assert_ignored(expected, filepath) \
+	assert_ignored_(expected, filepath, __FILE__, __LINE__)
+#define assert_is_ignored(filepath) \
+	assert_ignored_(true, filepath, __FILE__, __LINE__)
+#define refute_is_ignored(filepath) \
+	assert_ignored_(false, filepath, __FILE__, __LINE__)
+
 void test_status_ignore__0(void)
 {
 	struct {
@@ -47,11 +64,8 @@ void test_status_ignore__0(void)
 
 	g_repo = cl_git_sandbox_init("attr");
 
-	for (one_test = test_cases; one_test->path != NULL; one_test++) {
-		int ignored;
-		cl_git_pass(git_status_should_ignore(&ignored, g_repo, one_test->path));
-		cl_assert_(ignored == one_test->expected, one_test->path);
-	}
+	for (one_test = test_cases; one_test->path != NULL; one_test++)
+		assert_ignored(one_test->expected, one_test->path);
 
 	/* confirm that ignore files were cached */
 	cl_assert(git_attr_cache__is_cached(
@@ -63,37 +77,22 @@ void test_status_ignore__0(void)
 
 void test_status_ignore__1(void)
 {
-	int ignored;
-
 	g_repo = cl_git_sandbox_init("attr");
 
 	cl_git_rewritefile("attr/.gitignore", "/*.txt\n/dir/\n");
 	git_attr_cache_flush(g_repo);
 
-	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "root_test4.txt"));
-	cl_assert(ignored);
-
-	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "sub/subdir_test2.txt"));
-	cl_assert(!ignored);
-
-	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "dir"));
-	cl_assert(ignored);
-
-	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "dir/"));
-	cl_assert(ignored);
-
-	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "sub/dir"));
-	cl_assert(!ignored);
-
-	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "sub/dir/"));
-	cl_assert(!ignored);
+	assert_is_ignored("root_test4.txt");
+	refute_is_ignored("sub/subdir_test2.txt");
+	assert_is_ignored("dir");
+	assert_is_ignored("dir/");
+	refute_is_ignored("sub/dir");
+	refute_is_ignored("sub/dir/");
 }
 
-
 void test_status_ignore__empty_repo_with_gitignore_rewrite(void)
 {
 	status_entry_single st;
-	int ignored;
 
 	g_repo = cl_git_sandbox_init("empty_standard_repo");
 
@@ -108,8 +107,7 @@ void test_status_ignore__empty_repo_with_gitignore_rewrite(void)
 	cl_git_pass(git_status_file(&st.status, g_repo, "look-ma.txt"));
 	cl_assert(st.status == GIT_STATUS_WT_NEW);
 
-	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "look-ma.txt"));
-	cl_assert(!ignored);
+	refute_is_ignored("look-ma.txt");
 
 	cl_git_rewritefile("empty_standard_repo/.gitignore", "*.nomatch\n");
 
@@ -121,8 +119,7 @@ void test_status_ignore__empty_repo_with_gitignore_rewrite(void)
 	cl_git_pass(git_status_file(&st.status, g_repo, "look-ma.txt"));
 	cl_assert(st.status == GIT_STATUS_WT_NEW);
 
-	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "look-ma.txt"));
-	cl_assert(!ignored);
+	refute_is_ignored("look-ma.txt");
 
 	cl_git_rewritefile("empty_standard_repo/.gitignore", "*.txt\n");
 
@@ -134,8 +131,7 @@ void test_status_ignore__empty_repo_with_gitignore_rewrite(void)
 	cl_git_pass(git_status_file(&st.status, g_repo, "look-ma.txt"));
 	cl_assert(st.status == GIT_STATUS_IGNORED);
 
-	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "look-ma.txt"));
-	cl_assert(ignored);
+	assert_is_ignored("look-ma.txt");
 }
 
 void test_status_ignore__ignore_pattern_contains_space(void)
@@ -181,7 +177,6 @@ void test_status_ignore__ignore_pattern_ignorecase(void)
 void test_status_ignore__subdirectories(void)
 {
 	status_entry_single st;
-	int ignored;
 
 	g_repo = cl_git_sandbox_init("empty_standard_repo");
 
@@ -198,8 +193,7 @@ void test_status_ignore__subdirectories(void)
 	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);
+	assert_is_ignored("ignore_me");
 
 	/* I've changed libgit2 so that the behavior here now differs from
 	 * core git but seems to make more sense.  In core git, the following
@@ -225,37 +219,37 @@ void test_status_ignore__subdirectories(void)
 	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);
+	assert_is_ignored("test/ignore_me/file");
 }
 
-static void make_test_data(void)
+static void make_test_data(const char *reponame, const char **files)
 {
-	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;
+	size_t repolen = strlen(reponame) + 1;
 
-	g_repo = cl_git_sandbox_init(repo);
+	g_repo = cl_git_sandbox_init(reponame);
 
 	for (scan = files; *scan != NULL; ++scan) {
 		cl_git_pass(git_futils_mkdir(
-			*scan + repolen, repo, 0777, GIT_MKDIR_PATH | GIT_MKDIR_SKIP_LAST));
+			*scan + repolen, reponame,
+			0777, GIT_MKDIR_PATH | GIT_MKDIR_SKIP_LAST));
 		cl_git_mkfile(*scan, "contents");
 	}
 }
 
+static const char *test_repo_1 = "empty_standard_repo";
+static const char *test_files_1[] = {
+	"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
+};
+
 void test_status_ignore__subdirectories_recursion(void)
 {
 	/* Let's try again with recursing into ignored dirs turned on */
@@ -292,7 +286,7 @@ void test_status_ignore__subdirectories_recursion(void)
 		GIT_STATUS_IGNORED, GIT_STATUS_IGNORED, GIT_STATUS_IGNORED,
 	};
 
-	make_test_data();
+	make_test_data(test_repo_1, test_files_1);
 	cl_git_rewritefile("empty_standard_repo/.gitignore", "ignore_me\n/ignore_also\n");
 
 	memset(&counts, 0x0, sizeof(status_entry_counts));
@@ -347,7 +341,7 @@ void test_status_ignore__subdirectories_not_at_root(void)
 		GIT_STATUS_WT_NEW, GIT_STATUS_IGNORED, GIT_STATUS_WT_NEW, GIT_STATUS_WT_NEW,
 	};
 
-	make_test_data();
+	make_test_data(test_repo_1, test_files_1);
 	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");
 
@@ -389,7 +383,7 @@ void test_status_ignore__leading_slash_ignores(void)
 		GIT_STATUS_WT_NEW, GIT_STATUS_WT_NEW, GIT_STATUS_WT_NEW, GIT_STATUS_WT_NEW,
 	};
 
-	make_test_data();
+	make_test_data(test_repo_1, test_files_1);
 
 	cl_fake_home(&home);
 	cl_git_mkfile("home/.gitignore", "/ignore_me\n");
@@ -422,151 +416,162 @@ void test_status_ignore__leading_slash_ignores(void)
 	cl_fake_home_cleanup(&home);
 }
 
-void test_status_ignore__adding_internal_ignores(void)
+void test_status_ignore__contained_dir_with_matching_name(void)
 {
-	int ignored;
+	static const char *test_files[] = {
+		"empty_standard_repo/subdir_match/aaa/subdir_match/file",
+		"empty_standard_repo/subdir_match/zzz_ignoreme",
+		NULL
+	};
+	static const char *expected_paths[] = {
+		"subdir_match/.gitignore",
+		"subdir_match/aaa/subdir_match/file",
+		"subdir_match/zzz_ignoreme",
+	};
+	static const unsigned int expected_statuses[] = {
+		GIT_STATUS_WT_NEW,  GIT_STATUS_WT_NEW,  GIT_STATUS_IGNORED
+	};
+	git_status_options opts = GIT_STATUS_OPTIONS_INIT;
+	status_entry_counts counts;
 
+	make_test_data("empty_standard_repo", test_files);
+	cl_git_mkfile(
+		"empty_standard_repo/subdir_match/.gitignore", "*_ignoreme\n");
+
+	refute_is_ignored("subdir_match/aaa/subdir_match/file");
+	assert_is_ignored("subdir_match/zzz_ignoreme");
+
+	memset(&counts, 0x0, sizeof(status_entry_counts));
+	counts.expected_entry_count = 3;
+	counts.expected_paths = expected_paths;
+	counts.expected_statuses = expected_statuses;
+
+	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__trailing_slash_star(void)
+{
+	static const char *test_files[] = {
+		"empty_standard_repo/file",
+		"empty_standard_repo/subdir/file",
+		"empty_standard_repo/subdir/sub2/sub3/file",
+		NULL
+	};
+
+	make_test_data("empty_standard_repo", test_files);
+	cl_git_mkfile(
+		"empty_standard_repo/subdir/.gitignore", "/**/*\n");
+
+	refute_is_ignored("file");
+	assert_is_ignored("subdir/sub2/sub3/file");
+	assert_is_ignored("subdir/file");
+}
+
+void test_status_ignore__adding_internal_ignores(void)
+{
 	g_repo = cl_git_sandbox_init("empty_standard_repo");
 
-	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "one.txt"));
-	cl_assert(!ignored);
-	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "two.bar"));
-	cl_assert(!ignored);
+	refute_is_ignored("one.txt");
+	refute_is_ignored("two.bar");
 
 	cl_git_pass(git_ignore_add_rule(g_repo, "*.nomatch\n"));
 
-	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "one.txt"));
-	cl_assert(!ignored);
-	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "two.bar"));
-	cl_assert(!ignored);
+	refute_is_ignored("one.txt");
+	refute_is_ignored("two.bar");
 
 	cl_git_pass(git_ignore_add_rule(g_repo, "*.txt\n"));
 
-	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "one.txt"));
-	cl_assert(ignored);
-	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "two.bar"));
-	cl_assert(!ignored);
+	assert_is_ignored("one.txt");
+	refute_is_ignored("two.bar");
 
 	cl_git_pass(git_ignore_add_rule(g_repo, "*.bar\n"));
 
-	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "one.txt"));
-	cl_assert(ignored);
-	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "two.bar"));
-	cl_assert(ignored);
+	assert_is_ignored("one.txt");
+	assert_is_ignored("two.bar");
 
 	cl_git_pass(git_ignore_clear_internal_rules(g_repo));
 
-	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "one.txt"));
-	cl_assert(!ignored);
-	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "two.bar"));
-	cl_assert(!ignored);
+	refute_is_ignored("one.txt");
+	refute_is_ignored("two.bar");
 
 	cl_git_pass(git_ignore_add_rule(
 		g_repo, "multiple\n*.rules\n# comment line\n*.bar\n"));
 
-	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "one.txt"));
-	cl_assert(!ignored);
-	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "two.bar"));
-	cl_assert(ignored);
+	refute_is_ignored("one.txt");
+	assert_is_ignored("two.bar");
 }
 
 void test_status_ignore__add_internal_as_first_thing(void)
 {
-	int ignored;
 	const char *add_me = "\n#################\n## Eclipse\n#################\n\n*.pydevproject\n.project\n.metadata\nbin/\ntmp/\n*.tmp\n\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, "one.tmp"));
-	cl_assert(ignored);
-	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "two.bar"));
-	cl_assert(!ignored);
+	assert_is_ignored("one.tmp");
+	refute_is_ignored("two.bar");
 }
 
 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);
+	assert_is_ignored("Debug");
+	assert_is_ignored("and/Debug");
+	assert_is_ignored("really/Debug/this/file");
+	assert_is_ignored("Debug/what/I/say");
+
+	refute_is_ignored("and/NoDebug");
+	refute_is_ignored("NoDebug/this");
+	refute_is_ignored("please/NoDebug/this");
+
+	assert_is_ignored("this/is/deep");
 	/* 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);
+	refute_is_ignored("and/this/is/deep");
+	assert_is_ignored("this/is/deep/too");
 	/* 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);
+	refute_is_ignored("but/this/is/deep/and/ignored");
+
+	refute_is_ignored("this/is/not/deep");
+	refute_is_ignored("is/this/not/as/deep");
+	refute_is_ignored("this/is/deepish");
+	refute_is_ignored("xthis/is/deep");
 }
 
 void test_status_ignore__automatically_ignore_bad_files(void)
 {
-	int ignored;
-
 	g_repo = cl_git_sandbox_init("empty_standard_repo");
 
-	cl_git_pass(git_status_should_ignore(&ignored, g_repo, ".git"));
-	cl_assert(ignored);
-	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "this/file/."));
-	cl_assert(ignored);
-	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "path/../funky"));
-	cl_assert(ignored);
-	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "path/whatever.c"));
-	cl_assert(!ignored);
+	assert_is_ignored(".git");
+	assert_is_ignored("this/file/.");
+	assert_is_ignored("path/../funky");
+	refute_is_ignored("path/whatever.c");
 
 	cl_git_pass(git_ignore_add_rule(g_repo, "*.c\n"));
 
-	cl_git_pass(git_status_should_ignore(&ignored, g_repo, ".git"));
-	cl_assert(ignored);
-	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "this/file/."));
-	cl_assert(ignored);
-	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "path/../funky"));
-	cl_assert(ignored);
-	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "path/whatever.c"));
-	cl_assert(ignored);
+	assert_is_ignored(".git");
+	assert_is_ignored("this/file/.");
+	assert_is_ignored("path/../funky");
+	assert_is_ignored("path/whatever.c");
 
 	cl_git_pass(git_ignore_clear_internal_rules(g_repo));
 
-	cl_git_pass(git_status_should_ignore(&ignored, g_repo, ".git"));
-	cl_assert(ignored);
-	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "this/file/."));
-	cl_assert(ignored);
-	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "path/../funky"));
-	cl_assert(ignored);
-	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "path/whatever.c"));
-	cl_assert(!ignored);
+	assert_is_ignored(".git");
+	assert_is_ignored("this/file/.");
+	assert_is_ignored("path/../funky");
+	refute_is_ignored("path/whatever.c");
 }
 
 void test_status_ignore__filenames_with_special_prefixes_do_not_interfere_with_status_retrieval(void)
@@ -605,7 +610,6 @@ void test_status_ignore__filenames_with_special_prefixes_do_not_interfere_with_s
 
 void test_status_ignore__issue_1766_negated_ignores(void)
 {
-	int ignored = 0;
 	unsigned int status;
 
 	g_repo = cl_git_sandbox_init("empty_standard_repo");
@@ -617,11 +621,8 @@ void test_status_ignore__issue_1766_negated_ignores(void)
 	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);
+	refute_is_ignored("a/.gitignore");
+	assert_is_ignored("a/ignoreme");
 
 	cl_git_pass(git_futils_mkdir_r(
 		"empty_standard_repo/b", NULL, 0775));
@@ -630,18 +631,12 @@ void test_status_ignore__issue_1766_negated_ignores(void)
 	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);
+	refute_is_ignored("b/.gitignore");
+	assert_is_ignored("b/ignoreme");
 
 	/* 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);
+	refute_is_ignored("a/.gitignore");
+	assert_is_ignored("a/ignoreme");
 
 	/* status should find the two ignore files and nothing else */
 
diff --git a/tests/status/status_helpers.c b/tests/status/status_helpers.c
index 902b65c4f..088279252 100644
--- a/tests/status/status_helpers.c
+++ b/tests/status/status_helpers.c
@@ -9,20 +9,13 @@ int cb_status__normal(
 	if (counts->debug)
 		cb_status__print(path, status_flags, NULL);
 
-	if (counts->entry_count >= counts->expected_entry_count) {
+	if (counts->entry_count >= counts->expected_entry_count)
 		counts->wrong_status_flags_count++;
-		goto exit;
-	}
-
-	if (strcmp(path, counts->expected_paths[counts->entry_count])) {
+	else if (strcmp(path, counts->expected_paths[counts->entry_count]))
 		counts->wrong_sorted_path++;
-		goto exit;
-	}
-
-	if (status_flags != counts->expected_statuses[counts->entry_count])
+	else if (status_flags != counts->expected_statuses[counts->entry_count])
 		counts->wrong_status_flags_count++;
 
-exit:
 	counts->entry_count++;
 	return 0;
 }