Merge pull request #984 from arrbee/fix-fnmatch-and-ignore

Fix single file ignores

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

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

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 {

src/fnmatch.c

@@ -24,13 +24,16 @@
 
 static int rangematch(const char *, char, int, char **);
 
-int
-p_fnmatch(const char *pattern, const char *string, int flags)
+static int
+p_fnmatchx(const char *pattern, const char *string, int flags, size_t recurs)
 {
 		const char *stringstart;
 		char *newp;
 		char c, test;
 
+		if (recurs-- == 0)
+				return FNM_NORES;
+
 		for (stringstart = string;;)
 				switch (c = *pattern++) {
 				case EOS:
@@ -75,8 +78,11 @@ p_fnmatch(const char *pattern, const char *string, int flags)
 
 						/* General case, use recursion. */
 						while ((test = *string) != EOS) {
-								if (!p_fnmatch(pattern, string, flags & ~FNM_PERIOD))
-										return (0);
+								int e;
+
+								e = p_fnmatchx(pattern, string, flags & ~FNM_PERIOD, recurs);
+								if (e != FNM_NOMATCH)
+										return e;
 								if (test == '/' && (flags & FNM_PATHNAME))
 										break;
 								++string;
@@ -178,3 +184,9 @@ rangematch(const char *pattern, char test, int flags, char **newp)
 		return (ok == negate ? RANGE_NOMATCH : RANGE_MATCH);
 }
 
+int
+p_fnmatch(const char *pattern, const char *string, int flags)
+{
+		return p_fnmatchx(pattern, string, flags, 64);
+}
+

src/fnmatch.h

@@ -11,12 +11,13 @@
 
 #define FNM_NOMATCH		1		/* Match failed. */
 #define FNM_NOSYS		2		/* Function not supported (unused). */
+#define	FNM_NORES		3		/* Out of resources */
 
-#define FNM_NOESCAPE		0x01		/* Disable backslash escaping. */
-#define FNM_PATHNAME		0x02		/* Slash must be matched by slash. */
+#define FNM_NOESCAPE	0x01		/* Disable backslash escaping. */
+#define FNM_PATHNAME	0x02		/* Slash must be matched by slash. */
 #define FNM_PERIOD		0x04		/* Period must be matched by period. */
 #define FNM_LEADING_DIR 0x08		/* Ignore /<tail> after Imatch. */
-#define FNM_CASEFOLD		0x10		/* Case insensitive search. */
+#define FNM_CASEFOLD	0x10		/* Case insensitive search. */
 
 #define FNM_IGNORECASE	FNM_CASEFOLD
 #define FNM_FILE_NAME	FNM_PATHNAME

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);
 
-	error = git_ignore__lookup(&ignores, path, ignored);
+	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;
+
+	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;
 }

src/posix.h

@@ -10,6 +10,7 @@
 #include "common.h"
 #include <fcntl.h>
 #include <time.h>
+#include "fnmatch.h"
 
 #ifndef S_IFGITLINK
 #define S_IFGITLINK 0160000

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;

src/unix/posix.h

@@ -7,13 +7,6 @@
 #ifndef INCLUDE_posix__w32_h__
 #define INCLUDE_posix__w32_h__
 
-#if !defined(__sun) && !defined(__amigaos4__)
-#	include <fnmatch.h>
-#	define p_fnmatch(p, s, f) fnmatch(p, s, f)
-#else
-#	include "compat/fnmatch.h"
-#endif
-
 #include <stdio.h>
 
 #define p_lstat(p,b) lstat(p,b)

src/win32/posix.h

@@ -8,7 +8,6 @@
 #define INCLUDE_posix__w32_h__
 
 #include "common.h"
-#include "compat/fnmatch.h"
 #include "utf-conv.h"
 
 GIT_INLINE(int) p_link(const char *old, const char *new)

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[] = {

tests-clar/resources/attr/gitignore

@@ -1,3 +1,2 @@
-sub
 ign
 dir/

tests-clar/resources/attr/sub/ign

@@ -1 +0,0 @@
-ignore me

tests-clar/resources/attr/sub/ign/file

@@ -0,0 +1 @@
+in ignored dir

tests-clar/resources/attr/sub/ign/sub/file

@@ -0,0 +1 @@
+below ignored dir

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);
+}