Merge pull request #2746 from libgit2/cmn/neg-ignore-dir

Fix negative ignores withing ignored dirs

2 files changed, 100 insertions, 8 deletions

diff --git a/src/ignore.c b/src/ignore.c
index 520bcfe41..6e00f7db5 100644
--- a/src/ignore.c
+++ b/src/ignore.c
@@ -4,11 +4,87 @@
 #include "attrcache.h"
 #include "path.h"
 #include "config.h"
+#include "fnmatch.h"
 
 #define GIT_IGNORE_INTERNAL		"[internal]exclude"
 
 #define GIT_IGNORE_DEFAULT_RULES ".\n..\n.git\n"
 
+/**
+ * A negative ignore can only unignore a file which is given explicitly before, thus
+ *
+ *    foo
+ *    !foo/bar
+ *
+ * does not unignore 'foo/bar' as it's not in the list. However
+ *
+ *    foo/<star>
+ *    !foo/bar
+ *
+ * does unignore 'foo/bar', as it is contained within the 'foo/<star>' rule.
+ */
+static int does_negate_rule(int *out, git_vector *rules, git_attr_fnmatch *match)
+{
+	int error = 0;
+	size_t i;
+	git_attr_fnmatch *rule;
+	char *path;
+	git_buf buf = GIT_BUF_INIT;
+
+	/* path of the file relative to the workdir, so we match the rules in subdirs */
+	if (match->containing_dir) {
+		git_buf_puts(&buf, match->containing_dir);
+	}
+	if (git_buf_puts(&buf, match->pattern) < 0)
+		return -1;
+
+	path = git_buf_detach(&buf);
+
+	git_vector_foreach(rules, i, rule) {
+		/* no chance of matching w/o a wilcard */
+		if (!(rule->flags & GIT_ATTR_FNMATCH_HASWILD))
+			continue;
+
+	/*
+	 * If we're dealing with a directory (which we know via the
+	 * strchr() check) we want to use 'dirname/<star>' as the
+	 * pattern so p_fnmatch() honours FNM_PATHNAME
+	 */
+		git_buf_clear(&buf);
+		if (rule->containing_dir) {
+			git_buf_puts(&buf, rule->containing_dir);
+		}
+		if (!strchr(rule->pattern, '*'))
+			error = git_buf_printf(&buf, "%s/*", rule->pattern);
+		else
+			error = git_buf_puts(&buf, rule->pattern);
+
+		if (error < 0)
+			goto out;
+
+
+		if ((error = p_fnmatch(git_buf_cstr(&buf), path, FNM_PATHNAME)) < 0) {
+			giterr_set(GITERR_INVALID, "error matching pattern");
+			goto out;
+		}
+
+		/* if we found a match, we want to keep this rule */
+		if (error != FNM_NOMATCH) {
+			*out = 1;
+			error = 0;
+			goto out;
+		}
+	}
+
+	*out = 0;
+	error = 0;
+
+out:
+	git__free(path);
+	git_buf_free(&buf);
+	return error;
+}
+
 static int parse_ignore_file(
 	git_repository *repo, git_attr_file *attrs, const char *data)
 {
@@ -32,6 +108,8 @@ static int parse_ignore_file(
 	}
 
 	while (!error && *scan) {
+		int valid_rule = 1;
+
 		if (!match && !(match = git__calloc(1, sizeof(*match)))) {
 			error = -1;
 			break;
@@ -48,11 +126,16 @@ static int parse_ignore_file(
 				match->flags |= GIT_ATTR_FNMATCH_ICASE;
 
 			scan = git__next_line(scan);
-			error = git_vector_insert(&attrs->rules, match);
+
+			/* if a negative match doesn't actually do anything, throw it away */
+			if (match->flags & GIT_ATTR_FNMATCH_NEGATIVE)
+				error = does_negate_rule(&valid_rule, &attrs->rules, match);
+
+			if (!error && valid_rule)
+				error = git_vector_insert(&attrs->rules, match);
 		}
 
-		if (error != 0) {
-			git__free(match->pattern);
+		if (error != 0 || !valid_rule) {
 			match->pattern = NULL;
 
 			if (error == GIT_ENOTFOUND)
diff --git a/tests/status/ignore.c b/tests/status/ignore.c
index ea4376c1e..a15b11d1d 100644
--- a/tests/status/ignore.c
+++ b/tests/status/ignore.c
@@ -751,13 +751,19 @@ void test_status_ignore__negative_ignores_inside_ignores(void)
 	static const char *test_files[] = {
 		"empty_standard_repo/top/mid/btm/tracked",
 		"empty_standard_repo/top/mid/btm/untracked",
+		"empty_standard_repo/zoo/bar",
+		"empty_standard_repo/zoo/foo/bar",
 		NULL
 	};
 
 	make_test_data("empty_standard_repo", test_files);
 	cl_git_mkfile(
 		"empty_standard_repo/.gitignore",
-		"top\n!top/mid/btm\n");
+		"top\n"
+		"!top/mid/btm\n"
+		"zoo/*\n"
+		"!zoo/bar\n"
+		"!zoo/foo/bar\n");
 	add_one_to_index("top/mid/btm/tracked");
 
 	{
@@ -765,13 +771,15 @@ void test_status_ignore__negative_ignores_inside_ignores(void)
 		status_entry_counts counts;
 		static const char *files[] = {
 			".gitignore", "top/mid/btm/tracked", "top/mid/btm/untracked",
+			"zoo/bar", "zoo/foo/bar",
 		};
 		static const unsigned int statuses[] = {
-			GIT_STATUS_WT_NEW, GIT_STATUS_INDEX_NEW, GIT_STATUS_WT_NEW,
+			GIT_STATUS_WT_NEW, GIT_STATUS_INDEX_NEW, GIT_STATUS_IGNORED,
+			GIT_STATUS_WT_NEW, GIT_STATUS_IGNORED,
 		};
 
 		memset(&counts, 0x0, sizeof(status_entry_counts));
-		counts.expected_entry_count = 3;
+		counts.expected_entry_count = 5;
 		counts.expected_paths = files;
 		counts.expected_statuses = statuses;
 		opts.flags = GIT_STATUS_OPT_DEFAULTS |
@@ -785,8 +793,9 @@ void test_status_ignore__negative_ignores_inside_ignores(void)
 		cl_assert_equal_i(0, counts.wrong_sorted_path);
 	}
 
-	refute_is_ignored("top/mid/btm/tracked");
-	refute_is_ignored("top/mid/btm/untracked");
+	assert_is_ignored("top/mid/btm/tracked");
+	assert_is_ignored("top/mid/btm/untracked");
+	refute_is_ignored("foo/bar");
 }
 
 void test_status_ignore__negative_ignores_in_slash_star(void)