From e0a97416d417895cb94d519ad0f16942051d302b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Fri, 5 Dec 2014 16:31:14 +0100 Subject: [PATCH 1/2] ignore: adjust test for negating inside a dir Given top !top/foo in an ignore file, we should not unignore top/foo. This is an implementation detail of the git code leaking, but that's the behaviour we should show. A negation rule can only negate an exact rule it has seen before. --- tests/status/ignore.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) 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) From e05b2ff127e57eaed9900ddd7b9ac199d43268f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Fri, 5 Dec 2014 18:25:38 +0100 Subject: [PATCH 2/2] ignore: match git's rule negation rules A rule can only negate something which was explicitly mentioned in the rules before it. Change our parsing to ignore a negative rule which does not negate something mentioned in the rules above it. While here, fix a wrong allocator usage. The memory for the match string comes from pool allocator. We must not free it with the general allocator. We can instead simply forget the string and it will be cleaned up. --- src/ignore.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 86 insertions(+), 3 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/ + * !foo/bar + * + * does unignore 'foo/bar', as it is contained within the 'foo/' 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/' 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)