Merge pull request libgit2#4670 from pks-t/pks/ignore-leadingdir

Fix negative gitignore rules with leading directories

Author: ethomson

src/attr_file.c

@@ -594,8 +594,9 @@ int git_attr_fnmatch__parse(
 	}
 
 	if (*pattern == '!' && (spec->flags & GIT_ATTR_FNMATCH_ALLOWNEG) != 0) {
-		spec->flags = spec->flags |
-			GIT_ATTR_FNMATCH_NEGATIVE | GIT_ATTR_FNMATCH_LEADINGDIR;
+		spec->flags = spec->flags | GIT_ATTR_FNMATCH_NEGATIVE;
+		if ((spec->flags & GIT_ATTR_FNMATCH_NOLEADINGDIR) == 0)
+			spec->flags |= GIT_ATTR_FNMATCH_LEADINGDIR;
 		pattern++;
 	}
 

src/ignore.c

@@ -133,23 +133,12 @@ static int does_negate_rule(int *out, git_vector *rules, git_attr_fnmatch *match
 				continue;
 		}
 
-		/*
-		 * When dealing with a directory, we add '/<star>' so
-		 * p_fnmatch() honours FNM_PATHNAME. Checking for LEADINGDIR
-		 * alone isn't enough as that's also set for nagations, so we
-		 * need to check that NEGATIVE is off.
-		 */
 		git_buf_clear(&buf);
-		if (rule->containing_dir) {
+		if (rule->containing_dir)
 			git_buf_puts(&buf, rule->containing_dir);
-		}
-
-		error = git_buf_puts(&buf, rule->pattern);
+		git_buf_puts(&buf, rule->pattern);
 
-		if ((rule->flags & (GIT_ATTR_FNMATCH_LEADINGDIR | GIT_ATTR_FNMATCH_NEGATIVE)) == GIT_ATTR_FNMATCH_LEADINGDIR)
-			error = git_buf_PUTS(&buf, "/*");
-
-		if (error < 0)
+		if (git_buf_oom(&buf))
 			goto out;
 
 		if ((error = p_fnmatch(git_buf_cstr(&buf), path, fnflags)) < 0) {
@@ -203,7 +192,10 @@ static int parse_ignore_file(
 			break;
 		}
 
-		match->flags = GIT_ATTR_FNMATCH_ALLOWSPACE | GIT_ATTR_FNMATCH_ALLOWNEG;
+		match->flags =
+		    GIT_ATTR_FNMATCH_ALLOWSPACE |
+		    GIT_ATTR_FNMATCH_ALLOWNEG |
+		    GIT_ATTR_FNMATCH_NOLEADINGDIR;
 
 		if (!(error = git_attr_fnmatch__parse(
 			match, &attrs->pool, context, &scan)))
@@ -445,6 +437,9 @@ static bool ignore_lookup_in_rules(
 	git_attr_fnmatch *match;
 
 	git_vector_rforeach(&file->rules, j, match) {
+		if (match->flags & GIT_ATTR_FNMATCH_DIRECTORY &&
+		    path->is_dir == GIT_DIR_FLAG_FALSE)
+			continue;
 		if (git_attr_fnmatch__match(match, path)) {
 			*ignored = ((match->flags & GIT_ATTR_FNMATCH_NEGATIVE) == 0) ?
 				GIT_IGNORE_TRUE : GIT_IGNORE_FALSE;

tests/status/ignore.c

@@ -1158,27 +1158,58 @@ void test_status_ignore__subdir_ignore_everything_except_certain_files(void)
 
 void test_status_ignore__deeper(void)
 {
-   int ignored;
+	const char *test_files[] = {
+		"empty_standard_repo/foo.data",
+		"empty_standard_repo/bar.data",
+		"empty_standard_repo/dont_ignore/foo.data",
+		"empty_standard_repo/dont_ignore/bar.data",
+		NULL
+	};
 
-    g_repo = cl_git_sandbox_init("empty_standard_repo");
+	make_test_data("empty_standard_repo", test_files);
+	cl_git_mkfile("empty_standard_repo/.gitignore",
+		"*.data\n"
+		"!dont_ignore/*.data\n");
 
-    cl_git_mkfile("empty_standard_repo/.gitignore",
-          "*.data\n"
-          "!dont_ignore/*.data\n");
+	assert_is_ignored("foo.data");
+	assert_is_ignored("bar.data");
 
-    cl_git_pass(p_mkdir("empty_standard_repo/dont_ignore", 0777));
-    cl_git_mkfile("empty_standard_repo/foo.data", "");
-    cl_git_mkfile("empty_standard_repo/bar.data", "");
-    cl_git_mkfile("empty_standard_repo/dont_ignore/foo.data", "");
-    cl_git_mkfile("empty_standard_repo/dont_ignore/bar.data", "");
+	refute_is_ignored("dont_ignore/foo.data");
+	refute_is_ignored("dont_ignore/bar.data");
+}
 
-    cl_git_pass(git_ignore_path_is_ignored(&ignored, g_repo, "foo.data"));
-    cl_assert_equal_i(1, ignored);
-    cl_git_pass(git_ignore_path_is_ignored(&ignored, g_repo, "bar.data"));
-    cl_assert_equal_i(1, ignored);
+void test_status_ignore__unignored_dir_with_ignored_contents(void)
+{
+	static const char *test_files[] = {
+		"empty_standard_repo/dir/a.test",
+		"empty_standard_repo/dir/subdir/a.test",
+		NULL
+	};
 
-    cl_git_pass(git_ignore_path_is_ignored(&ignored, g_repo, "dont_ignore/foo.data"));
-    cl_assert_equal_i(0, ignored);
-    cl_git_pass(git_ignore_path_is_ignored(&ignored, g_repo, "dont_ignore/bar.data"));
-    cl_assert_equal_i(0, ignored);
+	make_test_data("empty_standard_repo", test_files);
+	cl_git_mkfile(
+		"empty_standard_repo/.gitignore",
+		"*.test\n"
+		"!dir/*\n");
+
+	refute_is_ignored("dir/a.test");
+	assert_is_ignored("dir/subdir/a.test");
+}
+
+void test_status_ignore__unignored_subdirs(void)
+{
+	static const char *test_files[] = {
+		"empty_standard_repo/dir/a.test",
+		"empty_standard_repo/dir/subdir/a.test",
+		NULL
+	};
+
+	make_test_data("empty_standard_repo", test_files);
+	cl_git_mkfile(
+		"empty_standard_repo/.gitignore",
+		"dir/*\n"
+		"!dir/*/\n");
+
+	assert_is_ignored("dir/a.test");
+	refute_is_ignored("dir/subdir/a.test");
 }