Commit bd4ca902 by Russell Belfer

Fix status for files under ignored dirs

There was a bug where tracked files inside directories that were
inside ignored directories where not being found by status.  To
make that a little clearer, if you have a .gitignore with:

    ignore/

And then have the following files:

    ignore/dir/tracked     <-- actually a tracked file
    ignore/dir/untracked   <-- should be ignored

Then we would show the tracked file as being removed (because
when we got the to contained item "dir/" inside the ignored
directory, we decided it was safe to skip -- bzzt, wrong!).

This update is much more careful about checking that we are
not skipping over any prefix of a tracked item, regardless of
whether it is ignored or not.

As documented in diff.c, this commit does create behavior that
still differs from core git with regards to the handling of
untracked files contained inside ignored directories.  With
libgit2, those files will just not show up in status or diff.
With core git, those files don't show up in status or diff
either *unless* they are explicitly ignored by a .gitignore
pattern in which case they show up as ignored files.

Needless to say, this is a local behavior difference only, so
it should not be important and (to me) the libgit2 behavior
seems more consistent.
parent 38f4f158
...@@ -551,29 +551,27 @@ static int diff_from_iterators( ...@@ -551,29 +551,27 @@ static int diff_from_iterators(
* matched in old (and/or descend into directories as needed) * matched in old (and/or descend into directories as needed)
*/ */
else if (nitem && (!oitem || strcmp(oitem->path, nitem->path) > 0)) { else if (nitem && (!oitem || strcmp(oitem->path, nitem->path) > 0)) {
int is_ignored; git_delta_t delta_type = GIT_DELTA_UNTRACKED;
git_delta_t delta_type = GIT_DELTA_ADDED;
/* contained in ignored parent directory, so this can be skipped. */ /* check if contained in ignored parent directory */
if (git_buf_len(&ignore_prefix) && if (git_buf_len(&ignore_prefix) &&
git__prefixcmp(nitem->path, git_buf_cstr(&ignore_prefix)) == 0) git__prefixcmp(nitem->path, git_buf_cstr(&ignore_prefix)) == 0)
{ delta_type = GIT_DELTA_IGNORED;
if (git_iterator_advance(new_iter, &nitem) < 0)
goto fail;
continue;
}
is_ignored = git_iterator_current_is_ignored(new_iter);
if (S_ISDIR(nitem->mode)) { if (S_ISDIR(nitem->mode)) {
/* recurse into directory if explicitly requested or /* recurse into directory only if there are tracked items in
* if there are tracked items inside the directory * it or if the user requested the contents of untracked
* directories and it is not under an ignored directory.
*/ */
if ((diff->opts.flags & GIT_DIFF_RECURSE_UNTRACKED_DIRS) || if ((oitem && git__prefixcmp(oitem->path, nitem->path) == 0) ||
(oitem && git__prefixcmp(oitem->path, nitem->path) == 0)) (delta_type == GIT_DELTA_UNTRACKED &&
(diff->opts.flags & GIT_DIFF_RECURSE_UNTRACKED_DIRS) != 0))
{ {
if (is_ignored) /* if this directory is ignored, remember it as the
* "ignore_prefix" for processing contained items
*/
if (delta_type == GIT_DELTA_UNTRACKED &&
git_iterator_current_is_ignored(new_iter))
git_buf_sets(&ignore_prefix, nitem->path); git_buf_sets(&ignore_prefix, nitem->path);
if (git_iterator_advance_into_directory(new_iter, &nitem) < 0) if (git_iterator_advance_into_directory(new_iter, &nitem) < 0)
...@@ -581,12 +579,34 @@ static int diff_from_iterators( ...@@ -581,12 +579,34 @@ static int diff_from_iterators(
continue; continue;
} }
delta_type = GIT_DELTA_UNTRACKED;
} }
else if (is_ignored)
/* In core git, the next two "else if" clauses are effectively
* reversed -- i.e. when an untracked file contained in an
* ignored directory is individually 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 is 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, just reverse the following
* two "else if" cases so that individual file ignores are
* checked before container directory exclusions are used to
* skip the file.
*/
else if (delta_type == GIT_DELTA_IGNORED) {
if (git_iterator_advance(new_iter, &nitem) < 0)
goto fail;
continue; /* ignored parent directory, so skip completely */
}
else if (git_iterator_current_is_ignored(new_iter))
delta_type = GIT_DELTA_IGNORED; delta_type = GIT_DELTA_IGNORED;
else if (new_iter->type == GIT_ITERATOR_WORKDIR)
delta_type = GIT_DELTA_UNTRACKED; else if (new_iter->type != GIT_ITERATOR_WORKDIR)
delta_type = GIT_DELTA_ADDED;
if (diff_delta__from_one(diff, delta_type, nitem) < 0 || if (diff_delta__from_one(diff, delta_type, nitem) < 0 ||
git_iterator_advance(new_iter, &nitem) < 0) git_iterator_advance(new_iter, &nitem) < 0)
......
[core]
repositoryformatversion = 0
filemode = true
bare = false
logallrefupdates = true
ignorecase = true
Unnamed repository; edit this file 'description' to name the repository.
#!/bin/sh
#
# An example hook script to prepare a packed repository for use over
# dumb transports.
#
# To enable this hook, rename this file to "post-update".
exec git update-server-info
# git ls-files --others --exclude-from=.git/info/exclude
# Lines that start with '#' are comments.
# For a project mostly in C, the following would be a good set of
# exclude patterns (uncomment them if you want to use them):
# *.[oa]
# *~
0000000000000000000000000000000000000000 3fbf1852f72fd268e36457b13a18cdd9a4c9ea35 Russell Belfer <rb@github.com> 1337205933 -0700 commit (initial): Initial commit
0000000000000000000000000000000000000000 3fbf1852f72fd268e36457b13a18cdd9a4c9ea35 Russell Belfer <rb@github.com> 1337205933 -0700 commit (initial): Initial commit
3fbf1852f72fd268e36457b13a18cdd9a4c9ea35
...@@ -45,9 +45,9 @@ void test_status_worktree__whole_repository(void) ...@@ -45,9 +45,9 @@ void test_status_worktree__whole_repository(void)
git_status_foreach(repo, cb_status__normal, &counts) git_status_foreach(repo, cb_status__normal, &counts)
); );
cl_assert(counts.entry_count == counts.expected_entry_count); cl_assert_equal_i(counts.expected_entry_count, counts.entry_count);
cl_assert(counts.wrong_status_flags_count == 0); cl_assert_equal_i(0, counts.wrong_status_flags_count);
cl_assert(counts.wrong_sorted_path == 0); cl_assert_equal_i(0, counts.wrong_sorted_path);
} }
/* this test is equivalent to t18-status.c:statuscb1 */ /* this test is equivalent to t18-status.c:statuscb1 */
...@@ -58,7 +58,7 @@ void test_status_worktree__empty_repository(void) ...@@ -58,7 +58,7 @@ void test_status_worktree__empty_repository(void)
cl_git_pass(git_status_foreach(repo, cb_status__count, &count)); cl_git_pass(git_status_foreach(repo, cb_status__count, &count));
cl_assert(count == 0); cl_assert_equal_i(0, count);
} }
static int remove_file_cb(void *data, git_buf *file) static int remove_file_cb(void *data, git_buf *file)
...@@ -100,9 +100,9 @@ void test_status_worktree__purged_worktree(void) ...@@ -100,9 +100,9 @@ void test_status_worktree__purged_worktree(void)
git_status_foreach(repo, cb_status__normal, &counts) git_status_foreach(repo, cb_status__normal, &counts)
); );
cl_assert(counts.entry_count == counts.expected_entry_count); cl_assert_equal_i(counts.expected_entry_count, counts.entry_count);
cl_assert(counts.wrong_status_flags_count == 0); cl_assert_equal_i(0, counts.wrong_status_flags_count);
cl_assert(counts.wrong_sorted_path == 0); cl_assert_equal_i(0, counts.wrong_sorted_path);
} }
/* this test is similar to t18-status.c:statuscb3 */ /* this test is similar to t18-status.c:statuscb3 */
...@@ -135,10 +135,9 @@ void test_status_worktree__swap_subdir_and_file(void) ...@@ -135,10 +135,9 @@ void test_status_worktree__swap_subdir_and_file(void)
git_status_foreach_ext(repo, &opts, cb_status__normal, &counts) git_status_foreach_ext(repo, &opts, cb_status__normal, &counts)
); );
cl_assert(counts.entry_count == counts.expected_entry_count); cl_assert_equal_i(counts.expected_entry_count, counts.entry_count);
cl_assert(counts.wrong_status_flags_count == 0); cl_assert_equal_i(0, counts.wrong_status_flags_count);
cl_assert(counts.wrong_sorted_path == 0); cl_assert_equal_i(0, counts.wrong_sorted_path);
} }
void test_status_worktree__swap_subdir_with_recurse_and_pathspec(void) void test_status_worktree__swap_subdir_with_recurse_and_pathspec(void)
...@@ -171,9 +170,9 @@ void test_status_worktree__swap_subdir_with_recurse_and_pathspec(void) ...@@ -171,9 +170,9 @@ void test_status_worktree__swap_subdir_with_recurse_and_pathspec(void)
git_status_foreach_ext(repo, &opts, cb_status__normal, &counts) git_status_foreach_ext(repo, &opts, cb_status__normal, &counts)
); );
cl_assert(counts.entry_count == counts.expected_entry_count); cl_assert_equal_i(counts.expected_entry_count, counts.entry_count);
cl_assert(counts.wrong_status_flags_count == 0); cl_assert_equal_i(0, counts.wrong_status_flags_count);
cl_assert(counts.wrong_sorted_path == 0); cl_assert_equal_i(0, counts.wrong_sorted_path);
} }
/* this test is equivalent to t18-status.c:singlestatus0 */ /* this test is equivalent to t18-status.c:singlestatus0 */
...@@ -347,6 +346,65 @@ void test_status_worktree__issue_592_5(void) ...@@ -347,6 +346,65 @@ void test_status_worktree__issue_592_5(void)
git_buf_free(&path); git_buf_free(&path);
} }
void test_status_worktree__issue_592_ignores_0(void)
{
int count = 0;
status_entry_single st;
git_repository *repo = cl_git_sandbox_init("issue_592");
cl_git_pass(git_status_foreach(repo, cb_status__count, &count));
cl_assert_equal_i(0, count);
cl_git_rewritefile("issue_592/.gitignore",
".gitignore\n*.txt\nc/\n[tT]*/\n");
cl_git_pass(git_status_foreach(repo, cb_status__count, &count));
cl_assert_equal_i(1, count);
/* This is a situation where the behavior of libgit2 is
* different from core git. Core git will show ignored.txt
* in the list of ignored files, even though the directory
* "t" is ignored and the file is untracked because we have
* the explicit "*.txt" ignore rule. Libgit2 just excludes
* all untracked files that are contained within ignored
* directories without explicitly listing them.
*/
cl_git_rewritefile("issue_592/t/ignored.txt", "ping");
memset(&st, 0, sizeof(st));
cl_git_pass(git_status_foreach(repo, cb_status__single, &st));
cl_assert_equal_i(1, st.count);
cl_assert(st.status == GIT_STATUS_IGNORED);
cl_git_rewritefile("issue_592/c/ignored_by_dir", "ping");
memset(&st, 0, sizeof(st));
cl_git_pass(git_status_foreach(repo, cb_status__single, &st));
cl_assert_equal_i(1, st.count);
cl_assert(st.status == GIT_STATUS_IGNORED);
cl_git_rewritefile("issue_592/t/ignored_by_dir_pattern", "ping");
memset(&st, 0, sizeof(st));
cl_git_pass(git_status_foreach(repo, cb_status__single, &st));
cl_assert_equal_i(1, st.count);
cl_assert(st.status == GIT_STATUS_IGNORED);
}
void test_status_worktree__issue_592_ignored_dirs_with_tracked_content(void)
{
int count = 0;
git_repository *repo = cl_git_sandbox_init("issue_592b");
cl_git_pass(git_status_foreach(repo, cb_status__count, &count));
cl_assert_equal_i(1, count);
/* if we are really mimicking core git, then only ignored1.txt
* at the top level will show up in the ignores list here.
* everything else will be unmodified or skipped completely.
*/
}
void test_status_worktree__cannot_retrieve_the_status_of_a_bare_repository(void) void test_status_worktree__cannot_retrieve_the_status_of_a_bare_repository(void)
{ {
git_repository *repo; git_repository *repo;
...@@ -374,7 +432,7 @@ void test_status_worktree__first_commit_in_progress(void) ...@@ -374,7 +432,7 @@ void test_status_worktree__first_commit_in_progress(void)
memset(&result, 0, sizeof(result)); memset(&result, 0, sizeof(result));
cl_git_pass(git_status_foreach(repo, cb_status__single, &result)); cl_git_pass(git_status_foreach(repo, cb_status__single, &result));
cl_assert(result.count == 1); cl_assert_equal_i(1, result.count);
cl_assert(result.status == GIT_STATUS_WT_NEW); cl_assert(result.status == GIT_STATUS_WT_NEW);
cl_git_pass(git_repository_index(&index, repo)); cl_git_pass(git_repository_index(&index, repo));
...@@ -383,7 +441,7 @@ void test_status_worktree__first_commit_in_progress(void) ...@@ -383,7 +441,7 @@ void test_status_worktree__first_commit_in_progress(void)
memset(&result, 0, sizeof(result)); memset(&result, 0, sizeof(result));
cl_git_pass(git_status_foreach(repo, cb_status__single, &result)); cl_git_pass(git_status_foreach(repo, cb_status__single, &result));
cl_assert(result.count == 1); cl_assert_equal_i(1, result.count);
cl_assert(result.status == GIT_STATUS_INDEX_NEW); cl_assert(result.status == GIT_STATUS_INDEX_NEW);
git_index_free(index); git_index_free(index);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment