ignore: fix negative leading directory rules unignoring subdirectory files

When computing whether a file is ignored, we simply search for the first
matching rule and return whether it is a positive ignore rule (the file
is really ignored) or whether it is a negative ignore rule (the file is
being unignored). Each rule has a set of flags which are being passed to
`fnmatch`, depending on what kind of rule it is. E.g. in case it is a
negative ignore we add a flag `GIT_ATTR_FNMATCH_NEGATIVE`, in case it
contains a glob we set the `GIT_ATTR_FNMATCH_HASGLOB` flag.

One of these flags is the `GIT_ATTR_FNMATCH_LEADINGDIR` flag, which is
always set in case the pattern has a trailing "/*" or in case the
pattern is negative. The flag causes the `fnmatch` function to return a
match in case a string is a leading directory of another, e.g. "dir/"
matches "dir/foo/bar.c". In case of negative patterns, this is wrong in
certain cases.

Take the following simple example of a gitignore:

    dir/
    !dir/

The `LEADINGDIR` flag causes "!dir/" to match "dir/foo/bar.c", and we
correctly unignore the directory. But take this example:

    *.test
    !dir/*

We expect everything in "dir/" to be unignored, but e.g. a file in a
subdirectory of dir should be ignored, as the "*" does not cross
directory hierarchies. With `LEADINGDIR`, though, we would just see that
"dir/" matches and return that the file is unignored, even if it is
contained in a subdirectory. Instead, we want to ignore leading
directories here and check "*.test". Afterwards, we have to iterate up
to the parent directory and do the same checks.

To fix the issue, disallow matching against leading directories in
gitignore files. This can be trivially done by just adding the
`GIT_ATTR_FNMATCH_NOLEADINGDIR` to the spec passed to
`git_attr_fnmatch__parse`. Due to a bug in that function, though, this
flag is being ignored for negative patterns, which is fixed in this
commit, as well. As a last fix, we need to ignore rules that are
supposed to match a directory when our path itself is a file.

All together, these changes fix the described error case.

Author: pks-t

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

@@ -203,7 +203,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 +448,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

@@ -1177,3 +1177,39 @@ void test_status_ignore__deeper(void)
 	refute_is_ignored("dont_ignore/foo.data");
 	refute_is_ignored("dont_ignore/bar.data");
 }
+
+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
+	};
+
+	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");
+}