From 8f7bc6461b81499216b73881b07f5476c5085660 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 10 Apr 2014 16:33:39 -0700 Subject: [PATCH] Fix bug popping ignore files during wd iteration There were a couple bugs in popping ignore files during iteration that could result in incorrect decisions be made and thus ignore files below the root either not being loaded correctly or not being popped at the right time. One bug was an off-by-one in comparing the path of the gitignore file with the path being exited during iteration. The second bug was not correctly truncating the path being tracked during traversal if there were no ignores on the list (i.e. when you have no .gitignore at the root, but do have some in contained directories). --- src/ignore.c | 15 ++++-- src/ignore.h | 1 + tests/status/ignore.c | 111 ++++++++++++++++++++++++++++++------------ 3 files changed, 94 insertions(+), 33 deletions(-) diff --git a/src/ignore.c b/src/ignore.c index c79fe4871..aef3e39b4 100644 --- a/src/ignore.c +++ b/src/ignore.c @@ -78,6 +78,8 @@ static int push_one_ignore(void *payload, git_buf *path) { git_ignores *ign = payload; + ign->depth++; + return push_ignore_file( ign->repo, ign, &ign->ign_path, path->ptr, GIT_IGNORE_FILE); } @@ -108,6 +110,7 @@ int git_ignore__for_path( ignores->repo = repo; git_buf_init(&ignores->dir, 0); ignores->ign_internal = NULL; + ignores->depth = 0; /* Read the ignore_case flag */ if ((error = git_repository__cvar( @@ -163,6 +166,8 @@ int git_ignore__push_dir(git_ignores *ign, const char *dir) if (git_buf_joinpath(&ign->dir, ign->dir.ptr, dir) < 0) return -1; + ign->depth++; + return push_ignore_file( ign->repo, ign, &ign->ign_path, ign->dir.ptr, GIT_IGNORE_FILE); } @@ -174,7 +179,7 @@ int git_ignore__pop_dir(git_ignores *ign) const char *start, *end, *scan; size_t keylen; - /* - ign->dir looks something like "a/b" (or "a/b/c/d") + /* - ign->dir looks something like "a/b/" (or "a/b/c/d/") * - file->key looks something like "0#a/b/.gitignore * * We are popping the last directory off ign->dir. We also want to @@ -191,9 +196,13 @@ int git_ignore__pop_dir(git_ignores *ign) if (ign->dir.size >= keylen && !memcmp(ign->dir.ptr + ign->dir.size - keylen, start, keylen)) git_vector_pop(&ign->ign_path); - - git_buf_rtruncate_at_char(&ign->dir, '/'); } + + if (--ign->depth > 0) { + git_buf_rtruncate_at_char(&ign->dir, '/'); + git_path_to_dir(&ign->dir); + } + return 0; } diff --git a/src/ignore.h b/src/ignore.h index 851c824bf..46172c72f 100644 --- a/src/ignore.h +++ b/src/ignore.h @@ -29,6 +29,7 @@ typedef struct { git_vector ign_path; git_vector ign_global; int ignore_case; + int depth; } git_ignores; extern int git_ignore__for_path( diff --git a/tests/status/ignore.c b/tests/status/ignore.c index acdc8fb58..1fefcba5f 100644 --- a/tests/status/ignore.c +++ b/tests/status/ignore.c @@ -228,6 +228,32 @@ void test_status_ignore__subdirectories(void) cl_assert(ignored); } +static void make_test_data(void) +{ + static const char *files[] = { + "empty_standard_repo/dir/a/ignore_me", + "empty_standard_repo/dir/b/ignore_me", + "empty_standard_repo/dir/ignore_me", + "empty_standard_repo/ignore_also/file", + "empty_standard_repo/ignore_me", + "empty_standard_repo/test/ignore_me/file", + "empty_standard_repo/test/ignore_me/file2", + "empty_standard_repo/test/ignore_me/and_me/file", + NULL + }; + static const char *repo = "empty_standard_repo"; + const char **scan; + size_t repolen = strlen(repo) + 1; + + g_repo = cl_git_sandbox_init(repo); + + for (scan = files; *scan != NULL; ++scan) { + cl_git_pass(git_futils_mkdir( + *scan + repolen, repo, 0777, GIT_MKDIR_PATH | GIT_MKDIR_SKIP_LAST)); + cl_git_mkfile(*scan, "contents"); + } +} + void test_status_ignore__subdirectories_recursion(void) { /* Let's try again with recursing into ignored dirs turned on */ @@ -235,6 +261,9 @@ void test_status_ignore__subdirectories_recursion(void) status_entry_counts counts; static const char *paths_r[] = { ".gitignore", + "dir/a/ignore_me", + "dir/b/ignore_me", + "dir/ignore_me", "ignore_also/file", "ignore_me", "test/ignore_me/and_me/file", @@ -242,49 +271,30 @@ void test_status_ignore__subdirectories_recursion(void) "test/ignore_me/file2", }; static const unsigned int statuses_r[] = { - GIT_STATUS_WT_NEW, - GIT_STATUS_IGNORED, - GIT_STATUS_IGNORED, - GIT_STATUS_IGNORED, - GIT_STATUS_IGNORED, - GIT_STATUS_IGNORED, + GIT_STATUS_WT_NEW, GIT_STATUS_IGNORED, GIT_STATUS_IGNORED, + GIT_STATUS_IGNORED, GIT_STATUS_IGNORED, GIT_STATUS_IGNORED, + GIT_STATUS_IGNORED, GIT_STATUS_IGNORED, GIT_STATUS_IGNORED, }; static const char *paths_nr[] = { ".gitignore", + "dir/a/ignore_me", + "dir/b/ignore_me", + "dir/ignore_me", "ignore_also/", "ignore_me", "test/ignore_me/", }; static const unsigned int statuses_nr[] = { GIT_STATUS_WT_NEW, - GIT_STATUS_IGNORED, - GIT_STATUS_IGNORED, - GIT_STATUS_IGNORED, + GIT_STATUS_IGNORED, GIT_STATUS_IGNORED, GIT_STATUS_IGNORED, + GIT_STATUS_IGNORED, GIT_STATUS_IGNORED, GIT_STATUS_IGNORED, }; - g_repo = cl_git_sandbox_init("empty_standard_repo"); - + make_test_data(); cl_git_rewritefile("empty_standard_repo/.gitignore", "ignore_me\n/ignore_also\n"); - cl_git_mkfile( - "empty_standard_repo/ignore_me", "I'm going to be ignored!"); - cl_git_pass(git_futils_mkdir_r( - "empty_standard_repo/test/ignore_me", NULL, 0775)); - cl_git_mkfile( - "empty_standard_repo/test/ignore_me/file", "I'm going to be ignored!"); - cl_git_mkfile( - "empty_standard_repo/test/ignore_me/file2", "Me, too!"); - cl_git_pass(git_futils_mkdir_r( - "empty_standard_repo/test/ignore_me/and_me", NULL, 0775)); - cl_git_mkfile( - "empty_standard_repo/test/ignore_me/and_me/file", "Deeply ignored"); - cl_git_pass(git_futils_mkdir_r( - "empty_standard_repo/ignore_also", NULL, 0775)); - cl_git_mkfile( - "empty_standard_repo/ignore_also/file", "I'm going to be ignored!"); - memset(&counts, 0x0, sizeof(status_entry_counts)); - counts.expected_entry_count = 6; + counts.expected_entry_count = 9; counts.expected_paths = paths_r; counts.expected_statuses = statuses_r; @@ -299,7 +309,7 @@ void test_status_ignore__subdirectories_recursion(void) memset(&counts, 0x0, sizeof(status_entry_counts)); - counts.expected_entry_count = 4; + counts.expected_entry_count = 7; counts.expected_paths = paths_nr; counts.expected_statuses = statuses_nr; @@ -313,6 +323,47 @@ void test_status_ignore__subdirectories_recursion(void) cl_assert_equal_i(0, counts.wrong_sorted_path); } +void test_status_ignore__subdirectories_not_at_root(void) +{ + git_status_options opts = GIT_STATUS_OPTIONS_INIT; + status_entry_counts counts; + static const char *paths_1[] = { + "dir/.gitignore", + "dir/a/ignore_me", + "dir/b/ignore_me", + "dir/ignore_me", + "ignore_also/file", + "ignore_me", + "test/.gitignore", + "test/ignore_me/and_me/file", + "test/ignore_me/file", + "test/ignore_me/file2", + }; + static const unsigned int statuses_1[] = { + GIT_STATUS_WT_NEW, GIT_STATUS_IGNORED, GIT_STATUS_IGNORED, + GIT_STATUS_IGNORED, GIT_STATUS_WT_NEW, GIT_STATUS_WT_NEW, + GIT_STATUS_WT_NEW, GIT_STATUS_IGNORED, GIT_STATUS_WT_NEW, GIT_STATUS_WT_NEW, + }; + + make_test_data(); + cl_git_rewritefile("empty_standard_repo/dir/.gitignore", "ignore_me\n/ignore_also\n"); + cl_git_rewritefile("empty_standard_repo/test/.gitignore", "and_me\n"); + + memset(&counts, 0x0, sizeof(status_entry_counts)); + counts.expected_entry_count = 10; + counts.expected_paths = paths_1; + counts.expected_statuses = statuses_1; + + opts.flags = GIT_STATUS_OPT_DEFAULTS | 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); +} + void test_status_ignore__adding_internal_ignores(void) { int ignored;