diff options
author | Vicent Marti <vicent@github.com> | 2014-05-13 12:40:13 +0200 |
---|---|---|
committer | Vicent Marti <vicent@github.com> | 2014-05-13 12:40:13 +0200 |
commit | 03fcef188952184136539add2bd14a95ec0abacd (patch) | |
tree | eef6c1efce418c858950aec4b9420bb7cf0043fc | |
parent | bcf9792f08d25d7c9fe6a09f3c4d36c675c51290 (diff) | |
parent | f554611a274aafd702bd899f4663c16eb76ae8f0 (diff) | |
download | libgit2-03fcef188952184136539add2bd14a95ec0abacd.tar.gz |
Merge pull request #2328 from libgit2/rb/how-broken-can-ignores-be
Improve checks for ignore containment
-rw-r--r-- | src/attr_file.c | 25 | ||||
-rw-r--r-- | src/attr_file.h | 6 | ||||
-rw-r--r-- | src/diff.c | 40 | ||||
-rw-r--r-- | src/ignore.c | 69 | ||||
-rw-r--r-- | src/ignore.h | 9 | ||||
-rw-r--r-- | src/iterator.c | 66 | ||||
-rw-r--r-- | src/iterator.h | 2 | ||||
-rw-r--r-- | tests/diff/iterator.c | 2 | ||||
-rw-r--r-- | tests/status/ignore.c | 107 |
9 files changed, 220 insertions, 106 deletions
diff --git a/src/attr_file.c b/src/attr_file.c index 156a23d91..3e95a2134 100644 --- a/src/attr_file.c +++ b/src/attr_file.c @@ -281,7 +281,7 @@ uint32_t git_attr_file__name_hash(const char *name) int git_attr_file__lookup_one( git_attr_file *file, - const git_attr_path *path, + git_attr_path *path, const char *attr, const char **value) { @@ -342,14 +342,11 @@ int git_attr_file__load_standalone(git_attr_file **out, const char *path) bool git_attr_fnmatch__match( git_attr_fnmatch *match, - const git_attr_path *path) + git_attr_path *path) { const char *filename; int flags = 0; - if ((match->flags & GIT_ATTR_FNMATCH_DIRECTORY) && !path->is_dir) - return false; - if (match->flags & GIT_ATTR_FNMATCH_ICASE) flags |= FNM_CASEFOLD; if (match->flags & GIT_ATTR_FNMATCH_LEADINGDIR) @@ -365,12 +362,28 @@ bool git_attr_fnmatch__match( flags |= FNM_LEADING_DIR; } + if ((match->flags & GIT_ATTR_FNMATCH_DIRECTORY) && !path->is_dir) { + int matchval; + + /* for attribute checks or root ignore checks, fail match */ + if (!(match->flags & GIT_ATTR_FNMATCH_IGNORE) || + path->basename == path->path) + return false; + + /* for ignore checks, use container of current item for check */ + path->basename[-1] = '\0'; + flags |= FNM_LEADING_DIR; + matchval = p_fnmatch(match->pattern, path->path, flags); + path->basename[-1] = '/'; + return (matchval != FNM_NOMATCH); + } + return (p_fnmatch(match->pattern, filename, flags) != FNM_NOMATCH); } bool git_attr_rule__match( git_attr_rule *rule, - const git_attr_path *path) + git_attr_path *path) { bool matched = git_attr_fnmatch__match(&rule->match, path); diff --git a/src/attr_file.h b/src/attr_file.h index e50aec07c..87cde7e35 100644 --- a/src/attr_file.h +++ b/src/attr_file.h @@ -138,7 +138,7 @@ int git_attr_file__clear_rules( int git_attr_file__lookup_one( git_attr_file *file, - const git_attr_path *path, + git_attr_path *path, const char *attr, const char **value); @@ -162,13 +162,13 @@ extern int git_attr_fnmatch__parse( extern bool git_attr_fnmatch__match( git_attr_fnmatch *rule, - const git_attr_path *path); + git_attr_path *path); extern void git_attr_rule__free(git_attr_rule *rule); extern bool git_attr_rule__match( git_attr_rule *rule, - const git_attr_path *path); + git_attr_path *path); extern git_attr_assignment *git_attr_rule__lookup_assignment( git_attr_rule *rule, const char *name); diff --git a/src/diff.c b/src/diff.c index bc23e6b0d..b3e36101c 100644 --- a/src/diff.c +++ b/src/diff.c @@ -633,7 +633,6 @@ typedef struct { git_iterator *new_iter; const git_index_entry *oitem; const git_index_entry *nitem; - git_buf ignore_prefix; } diff_in_progress; #define MODE_BITS_MASK 0000777 @@ -844,24 +843,13 @@ static int handle_unmatched_new_item( /* check if this is a prefix of the other side */ contains_oitem = entry_is_prefixed(diff, info->oitem, nitem); - /* check if this is contained in an ignored parent directory */ - if (git_buf_len(&info->ignore_prefix)) { - if (diff->pfxcomp(nitem->path, git_buf_cstr(&info->ignore_prefix)) == 0) - delta_type = GIT_DELTA_IGNORED; - else - git_buf_clear(&info->ignore_prefix); - } + /* update delta_type if this item is ignored */ + if (git_iterator_current_is_ignored(info->new_iter)) + delta_type = GIT_DELTA_IGNORED; if (nitem->mode == GIT_FILEMODE_TREE) { bool recurse_into_dir = contains_oitem; - /* if not already inside an ignored dir, check if this is ignored */ - if (delta_type != GIT_DELTA_IGNORED && - git_iterator_current_is_ignored(info->new_iter)) { - delta_type = GIT_DELTA_IGNORED; - git_buf_sets(&info->ignore_prefix, nitem->path); - } - /* check if user requests recursion into this type of dir */ recurse_into_dir = contains_oitem || (delta_type == GIT_DELTA_UNTRACKED && @@ -938,27 +926,12 @@ static int handle_unmatched_new_item( } } - /* In core git, the next two checks are effectively reversed -- - * i.e. when an file contained in an ignored directory is explicitly - * ignored, it shows up as an ignored file in the diff list, even though - * other untracked files in the same directory are skipped completely. - * - * To me, this seems odd. If the directory is ignored and the file is - * untracked, we should skip it consistently, regardless of whether it - * happens to match a pattern in the ignore file. - * - * To match the core git behavior, reverse the following two if checks - * so that individual file ignores are checked before container - * directory exclusions are used to skip the file. - */ else if (delta_type == GIT_DELTA_IGNORED && - DIFF_FLAG_ISNT_SET(diff, GIT_DIFF_RECURSE_IGNORED_DIRS)) + DIFF_FLAG_ISNT_SET(diff, GIT_DIFF_RECURSE_IGNORED_DIRS) && + git_iterator_current_tree_is_ignored(info->new_iter)) /* item contained in ignored directory, so skip over it */ return git_iterator_advance(&info->nitem, info->new_iter); - else if (git_iterator_current_is_ignored(info->new_iter)) - delta_type = GIT_DELTA_IGNORED; - else if (info->new_iter->type != GIT_ITERATOR_TYPE_WORKDIR) delta_type = GIT_DELTA_ADDED; @@ -1068,7 +1041,6 @@ int git_diff__from_iterators( info.repo = repo; info.old_iter = old_iter; info.new_iter = new_iter; - git_buf_init(&info.ignore_prefix, 0); /* make iterators have matching icase behavior */ if (DIFF_FLAG_IS_SET(diff, GIT_DIFF_IGNORE_CASE)) { @@ -1123,8 +1095,6 @@ cleanup: else git_diff_free(diff); - git_buf_free(&info.ignore_prefix); - return error; } diff --git a/src/ignore.c b/src/ignore.c index f373c9482..78f01ac44 100644 --- a/src/ignore.c +++ b/src/ignore.c @@ -248,14 +248,15 @@ void git_ignore__free(git_ignores *ignores) } static bool ignore_lookup_in_rules( - git_attr_file *file, git_attr_path *path, int *ignored) + int *ignored, git_attr_file *file, git_attr_path *path) { size_t j; git_attr_fnmatch *match; git_vector_rforeach(&file->rules, j, match) { if (git_attr_fnmatch__match(match, path)) { - *ignored = ((match->flags & GIT_ATTR_FNMATCH_NEGATIVE) == 0); + *ignored = ((match->flags & GIT_ATTR_FNMATCH_NEGATIVE) == 0) ? + GIT_IGNORE_TRUE : GIT_IGNORE_FALSE; return true; } } @@ -264,34 +265,34 @@ static bool ignore_lookup_in_rules( } int git_ignore__lookup( - git_ignores *ignores, const char *pathname, int *ignored) + int *out, git_ignores *ignores, const char *pathname) { unsigned int i; git_attr_file *file; git_attr_path path; + *out = GIT_IGNORE_NOTFOUND; + if (git_attr_path__init( &path, pathname, git_repository_workdir(ignores->repo)) < 0) return -1; /* first process builtins - success means path was found */ - if (ignore_lookup_in_rules(ignores->ign_internal, &path, ignored)) + if (ignore_lookup_in_rules(out, ignores->ign_internal, &path)) goto cleanup; /* next process files in the path */ git_vector_foreach(&ignores->ign_path, i, file) { - if (ignore_lookup_in_rules(file, &path, ignored)) + if (ignore_lookup_in_rules(out, file, &path)) goto cleanup; } /* last process global ignores */ git_vector_foreach(&ignores->ign_global, i, file) { - if (ignore_lookup_in_rules(file, &path, ignored)) + if (ignore_lookup_in_rules(out, file, &path)) goto cleanup; } - *ignored = 0; - cleanup: git_attr_path__free(&path); return 0; @@ -335,8 +336,6 @@ int git_ignore_path_is_ignored( int error; const char *workdir; git_attr_path path; - char *tail, *end; - bool full_is_dir; git_ignores ignores; unsigned int i; git_attr_file *file; @@ -345,55 +344,42 @@ int git_ignore_path_is_ignored( workdir = repo ? git_repository_workdir(repo) : NULL; - if ((error = git_attr_path__init(&path, pathname, workdir)) < 0) - return error; + memset(&path, 0, sizeof(path)); + memset(&ignores, 0, sizeof(ignores)); - tail = path.path; - end = &path.full.ptr[path.full.size]; - full_is_dir = path.is_dir; + if ((error = git_attr_path__init(&path, pathname, workdir)) < 0 || + (error = git_ignore__for_path(repo, path.path, &ignores)) < 0) + goto cleanup; while (1) { - /* advance to next component of path */ - path.basename = tail; - - while (tail < end && *tail != '/') tail++; - *tail = '\0'; - - path.full.size = (tail - path.full.ptr); - path.is_dir = (tail == end) ? full_is_dir : true; - - /* initialize ignores the first time through */ - if (path.basename == path.path && - (error = git_ignore__for_path(repo, path.path, &ignores)) < 0) - break; - /* first process builtins - success means path was found */ - if (ignore_lookup_in_rules(ignores.ign_internal, &path, ignored)) + if (ignore_lookup_in_rules(ignored, ignores.ign_internal, &path)) goto cleanup; /* next process files in the path */ git_vector_foreach(&ignores.ign_path, i, file) { - if (ignore_lookup_in_rules(file, &path, ignored)) + if (ignore_lookup_in_rules(ignored, file, &path)) goto cleanup; } /* last process global ignores */ git_vector_foreach(&ignores.ign_global, i, file) { - if (ignore_lookup_in_rules(file, &path, ignored)) + if (ignore_lookup_in_rules(ignored, file, &path)) goto cleanup; } - /* if we found no rules before reaching the end, we're done */ - if (tail == end) + /* move up one directory */ + if (path.basename == path.path) break; - - /* now add this directory to list of ignores */ - if ((error = git_ignore__push_dir(&ignores, path.path)) < 0) + path.basename[-1] = '\0'; + while (path.basename > path.path && *path.basename != '/') + path.basename--; + if (path.basename > path.path) + path.basename++; + path.is_dir = 1; + + if ((error = git_ignore__pop_dir(&ignores)) < 0) break; - - /* reinstate divider in path */ - *tail = '/'; - while (*tail == '/') tail++; } *ignored = 0; @@ -404,7 +390,6 @@ cleanup: return error; } - int git_ignore__check_pathspec_for_exact_ignores( git_repository *repo, git_vector *vspec, diff --git a/src/ignore.h b/src/ignore.h index ff9369000..77668c661 100644 --- a/src/ignore.h +++ b/src/ignore.h @@ -42,7 +42,14 @@ extern int git_ignore__pop_dir(git_ignores *ign); extern void git_ignore__free(git_ignores *ign); -extern int git_ignore__lookup(git_ignores *ign, const char *path, int *ignored); +enum { + GIT_IGNORE_UNCHECKED = -2, + GIT_IGNORE_NOTFOUND = -1, + GIT_IGNORE_FALSE = 0, + GIT_IGNORE_TRUE = 1, +}; + +extern int git_ignore__lookup(int *out, git_ignores *ign, const char *path); /* command line Git sometimes generates an error message if given a * pathspec that contains an exact match to an ignored file (provided diff --git a/src/iterator.c b/src/iterator.c index 4f8087c8d..c664f17cd 100644 --- a/src/iterator.c +++ b/src/iterator.c @@ -897,6 +897,7 @@ struct fs_iterator_frame { fs_iterator_frame *next; git_vector entries; size_t index; + int is_ignored; }; typedef struct fs_iterator fs_iterator; @@ -1290,16 +1291,28 @@ GIT_INLINE(bool) workdir_path_is_dotgit(const git_buf *path) static int workdir_iterator__enter_dir(fs_iterator *fi) { + workdir_iterator *wi = (workdir_iterator *)fi; fs_iterator_frame *ff = fi->stack; size_t pos; git_path_with_stat *entry; bool found_submodules = false; - /* only push new ignores if this is not top level directory */ + /* check if this directory is ignored */ + if (git_ignore__lookup( + &ff->is_ignored, &wi->ignores, fi->path.ptr + fi->root_len) < 0) { + giterr_clear(); + ff->is_ignored = GIT_IGNORE_NOTFOUND; + } + + /* if this is not the top level directory... */ if (ff->next != NULL) { - workdir_iterator *wi = (workdir_iterator *)fi; ssize_t slash_pos = git_buf_rfind_next(&fi->path, '/'); + /* inherit ignored from parent if no rule specified */ + if (ff->is_ignored <= GIT_IGNORE_NOTFOUND) + ff->is_ignored = ff->next->is_ignored; + + /* push new ignores for files in this directory */ (void)git_ignore__push_dir(&wi->ignores, &fi->path.ptr[slash_pos + 1]); } @@ -1342,7 +1355,7 @@ static int workdir_iterator__update_entry(fs_iterator *fi) return GIT_ENOTFOUND; /* reset is_ignored since we haven't checked yet */ - wi->is_ignored = -1; + wi->is_ignored = GIT_IGNORE_UNCHECKED; return 0; } @@ -1487,6 +1500,19 @@ int git_iterator_current_parent_tree( return 0; } +static void workdir_iterator_update_is_ignored(workdir_iterator *wi) +{ + if (git_ignore__lookup( + &wi->is_ignored, &wi->ignores, wi->fi.entry.path) < 0) { + giterr_clear(); + wi->is_ignored = GIT_IGNORE_NOTFOUND; + } + + /* use ignore from containing frame stack */ + if (wi->is_ignored <= GIT_IGNORE_NOTFOUND) + wi->is_ignored = wi->fi.stack->is_ignored; +} + bool git_iterator_current_is_ignored(git_iterator *iter) { workdir_iterator *wi = (workdir_iterator *)iter; @@ -1494,14 +1520,22 @@ bool git_iterator_current_is_ignored(git_iterator *iter) if (iter->type != GIT_ITERATOR_TYPE_WORKDIR) return false; - if (wi->is_ignored != -1) - return (bool)(wi->is_ignored != 0); + if (wi->is_ignored != GIT_IGNORE_UNCHECKED) + return (bool)(wi->is_ignored == GIT_IGNORE_TRUE); - if (git_ignore__lookup( - &wi->ignores, wi->fi.entry.path, &wi->is_ignored) < 0) - wi->is_ignored = true; + workdir_iterator_update_is_ignored(wi); + + return (bool)(wi->is_ignored == GIT_IGNORE_TRUE); +} + +bool git_iterator_current_tree_is_ignored(git_iterator *iter) +{ + workdir_iterator *wi = (workdir_iterator *)iter; + + if (iter->type != GIT_ITERATOR_TYPE_WORKDIR) + return false; - return (bool)wi->is_ignored; + return (bool)(wi->fi.stack->is_ignored == GIT_IGNORE_TRUE); } int git_iterator_cmp(git_iterator *iter, const char *path_prefix) @@ -1549,10 +1583,8 @@ int git_iterator_advance_over_with_status( return error; if (!S_ISDIR(entry->mode)) { - if (git_ignore__lookup( - &wi->ignores, wi->fi.entry.path, &wi->is_ignored) < 0) - wi->is_ignored = true; - if (wi->is_ignored) + workdir_iterator_update_is_ignored(wi); + if (wi->is_ignored == GIT_IGNORE_TRUE) *status = GIT_ITERATOR_STATUS_IGNORED; return git_iterator_advance(entryptr, iter); } @@ -1564,14 +1596,12 @@ int git_iterator_advance_over_with_status( /* scan inside directory looking for a non-ignored item */ while (entry && !iter->prefixcomp(entry->path, base)) { - if (git_ignore__lookup( - &wi->ignores, wi->fi.entry.path, &wi->is_ignored) < 0) - wi->is_ignored = true; + workdir_iterator_update_is_ignored(wi); /* if we found an explicitly ignored item, then update from * EMPTY to IGNORED */ - if (wi->is_ignored) + if (wi->is_ignored == GIT_IGNORE_TRUE) *status = GIT_ITERATOR_STATUS_IGNORED; else if (S_ISDIR(entry->mode)) { error = git_iterator_advance_into(&entry, iter); @@ -1580,7 +1610,7 @@ int git_iterator_advance_over_with_status( continue; else if (error == GIT_ENOTFOUND) { error = 0; - wi->is_ignored = true; /* mark empty directories as ignored */ + wi->is_ignored = GIT_IGNORE_TRUE; /* mark empty dirs ignored */ } else break; /* real error, stop here */ } else { diff --git a/src/iterator.h b/src/iterator.h index f67830212..d88ad5191 100644 --- a/src/iterator.h +++ b/src/iterator.h @@ -245,6 +245,8 @@ extern int git_iterator_current_parent_tree( extern bool git_iterator_current_is_ignored(git_iterator *iter); +extern bool git_iterator_current_tree_is_ignored(git_iterator *iter); + extern int git_iterator_cmp( git_iterator *iter, const char *path_prefix); diff --git a/tests/diff/iterator.c b/tests/diff/iterator.c index cdc64eb1d..a2df1c7a7 100644 --- a/tests/diff/iterator.c +++ b/tests/diff/iterator.c @@ -647,7 +647,7 @@ static void workdir_iterator_test( void test_diff_iterator__workdir_0(void) { - workdir_iterator_test("attr", NULL, NULL, 27, 1, NULL, "ign"); + workdir_iterator_test("attr", NULL, NULL, 23, 5, NULL, "ign"); } static const char *status_paths[] = { diff --git a/tests/status/ignore.c b/tests/status/ignore.c index caa1f1927..a4e766fdf 100644 --- a/tests/status/ignore.c +++ b/tests/status/ignore.c @@ -681,3 +681,110 @@ void test_status_ignore__issue_1766_negated_ignores(void) } } +static void add_one_to_index(const char *file) +{ + git_index *index; + cl_git_pass(git_repository_index(&index, g_repo)); + cl_git_pass(git_index_add_bypath(index, file)); + git_index_free(index); +} + +/* Some further broken scenarios that have been reported */ +void test_status_ignore__more_breakage(void) +{ + static const char *test_files[] = { + "empty_standard_repo/d1/pfx-d2/d3/d4/d5/tracked", + "empty_standard_repo/d1/pfx-d2/d3/d4/d5/untracked", + "empty_standard_repo/d1/pfx-d2/d3/d4/untracked", + NULL + }; + + make_test_data("empty_standard_repo", test_files); + cl_git_mkfile( + "empty_standard_repo/.gitignore", + "/d1/pfx-*\n" + "!/d1/pfx-d2/\n" + "/d1/pfx-d2/*\n" + "!/d1/pfx-d2/d3/\n" + "/d1/pfx-d2/d3/*\n" + "!/d1/pfx-d2/d3/d4/\n"); + add_one_to_index("d1/pfx-d2/d3/d4/d5/tracked"); + + { + git_status_options opts = GIT_STATUS_OPTIONS_INIT; + status_entry_counts counts; + static const char *files[] = { + ".gitignore", + "d1/pfx-d2/d3/d4/d5/tracked", + "d1/pfx-d2/d3/d4/d5/untracked", + "d1/pfx-d2/d3/d4/untracked", + }; + static const unsigned int statuses[] = { + GIT_STATUS_WT_NEW, + GIT_STATUS_INDEX_NEW, + GIT_STATUS_WT_NEW, + GIT_STATUS_WT_NEW, + }; + + memset(&counts, 0x0, sizeof(status_entry_counts)); + counts.expected_entry_count = 4; + counts.expected_paths = files; + counts.expected_statuses = statuses; + opts.flags = GIT_STATUS_OPT_DEFAULTS | + GIT_STATUS_OPT_INCLUDE_IGNORED | + GIT_STATUS_OPT_RECURSE_IGNORED_DIRS; + cl_git_pass(git_status_foreach_ext( + g_repo, &opts, cb_status__normal, &counts)); + + cl_assert_equal_i(counts.expected_entry_count, counts.entry_count); + cl_assert_equal_i(0, counts.wrong_status_flags_count); + cl_assert_equal_i(0, counts.wrong_sorted_path); + } + + refute_is_ignored("d1/pfx-d2/d3/d4/d5/tracked"); + refute_is_ignored("d1/pfx-d2/d3/d4/d5/untracked"); + refute_is_ignored("d1/pfx-d2/d3/d4/untracked"); +} + +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", + NULL + }; + + make_test_data("empty_standard_repo", test_files); + cl_git_mkfile( + "empty_standard_repo/.gitignore", + "top\n!top/mid/btm\n"); + add_one_to_index("top/mid/btm/tracked"); + + { + git_status_options opts = GIT_STATUS_OPTIONS_INIT; + status_entry_counts counts; + static const char *files[] = { + ".gitignore", "top/mid/btm/tracked", "top/mid/btm/untracked", + }; + static const unsigned int statuses[] = { + GIT_STATUS_WT_NEW, GIT_STATUS_INDEX_NEW, GIT_STATUS_WT_NEW, + }; + + memset(&counts, 0x0, sizeof(status_entry_counts)); + counts.expected_entry_count = 3; + counts.expected_paths = files; + counts.expected_statuses = statuses; + opts.flags = GIT_STATUS_OPT_DEFAULTS | + GIT_STATUS_OPT_INCLUDE_IGNORED | + GIT_STATUS_OPT_RECURSE_IGNORED_DIRS; + cl_git_pass(git_status_foreach_ext( + g_repo, &opts, cb_status__normal, &counts)); + + cl_assert_equal_i(counts.expected_entry_count, counts.entry_count); + cl_assert_equal_i(0, counts.wrong_status_flags_count); + cl_assert_equal_i(0, counts.wrong_sorted_path); + } + + refute_is_ignored("top/mid/btm/tracked"); + refute_is_ignored("top/mid/btm/untracked"); +} |