Commit 20b4c1753ba5b1780ed7cdb079938b6bebf13634
2018-06-05T16:12:58
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.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86
diff --git a/src/attr_file.c b/src/attr_file.c
index f46cce3..6821587 100644
--- a/src/attr_file.c
+++ b/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++;
}
diff --git a/src/ignore.c b/src/ignore.c
index 76b9972..33db5bc 100644
--- a/src/ignore.c
+++ b/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;
diff --git a/tests/status/ignore.c b/tests/status/ignore.c
index 314067d..824475e 100644
--- a/tests/status/ignore.c
+++ b/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");
+}