Pop ignore only if whole relative path matches

When traversing the directory structure, the iterator pushes and
pops ignore files using a vector.  Some directories don't have
ignore files, so it uses a path comparison to decide when it is
right to actually pop the last ignore file.  This was only
comparing directory suffixes, though, so a subdirectory with the
same name as a parent could result in the parent's .gitignore
being popped off the list ignores too early.  This changes the
logic to compare the entire relative path of the ignore file.

diff --git a/src/ignore.c b/src/ignore.c
index deae204f85b6d81596febc6d71200938b81520a3..b08ff2200467b449596a9e74f6bce1a6b18dbabe 100644 (file)
--- a/src/ignore.c
+++ b/src/ignore.c
@@ -123,7 +123,7 @@ int git_ignore__for_path(
        int error = 0;
        const char *workdir = git_repository_workdir(repo);
 
-       assert(ignores);
+       assert(ignores && path);
 
        memset(ignores, 0, sizeof(*ignores));
        ignores->repo = repo;
@@ -140,10 +140,13 @@ int git_ignore__for_path(
        if (workdir && git_path_root(path) < 0)
                error = git_path_find_dir(&ignores->dir, path, workdir);
        else
-               error = git_buf_sets(&ignores->dir, path);
+               error = git_buf_joinpath(&ignores->dir, path, "");
        if (error < 0)
                goto cleanup;
 
+       if (workdir && !git__prefixcmp(ignores->dir.ptr, workdir))
+               ignores->dir_root = strlen(workdir);
+
        /* set up internals */
        if ((error = get_internal_ignores(&ignores->ign_internal, repo)) < 0)
                goto cleanup;
@@ -204,10 +207,10 @@ int git_ignore__pop_dir(git_ignores *ign)
 
                if ((end = strrchr(start, '/')) != NULL) {
                        size_t dirlen = (end - start) + 1;
+                       const char *relpath = ign->dir.ptr + ign->dir_root;
+                       size_t pathlen = ign->dir.size - ign->dir_root;
 
-                       if (ign->dir.size >= dirlen &&
-                               !memcmp(ign->dir.ptr + ign->dir.size - dirlen, start, dirlen))
-                       {
+                       if (pathlen == dirlen && !memcmp(relpath, start, dirlen)) {
                                git_vector_pop(&ign->ign_path);
                                git_attr_file__free(file);
                        }

diff --git a/src/ignore.h b/src/ignore.h
index 46172c72f5aa9bd7551841d1cbc22ba039af0223..ff9369000817badda11afcc1aa0d62f76d54c590 100644 (file)
--- a/src/ignore.h
+++ b/src/ignore.h
@@ -28,6 +28,7 @@ typedef struct {
        git_attr_file *ign_internal;
        git_vector ign_path;
        git_vector ign_global;
+       size_t dir_root; /* offset in dir to repo root */
        int ignore_case;
        int depth;
 } git_ignores;

diff --git a/src/path.c b/src/path.c
index 7cad28d4553cc1136631146f0a8e8788cba3c1ad..a990b005fe7cca6c01cd53e67188d499ad557f25 100644 (file)
--- a/src/path.c
+++ b/src/path.c
@@ -624,7 +624,7 @@ int git_path_find_dir(git_buf *dir, const char *path, const char *base)
 
        /* call dirname if this is not a directory */
        if (!error) /* && git_path_isdir(dir->ptr) == false) */
-               error = git_path_dirname_r(dir, dir->ptr);
+               error = (git_path_dirname_r(dir, dir->ptr) < 0) ? -1 : 0;
 
        if (!error)
                error = git_path_to_dir(dir);

diff --git a/tests/status/ignore.c b/tests/status/ignore.c
index 052a8eae8beec01cdbe0372a047e8775626255ee..eb171deb6bd3edfe9258338078d267f552c6e72a 100644 (file)
--- a/tests/status/ignore.c
+++ b/tests/status/ignore.c
@@ -230,32 +230,34 @@ void test_status_ignore__subdirectories(void)
        cl_assert(ignored);
 }
 
-static void make_test_data(void)
+static void make_test_data(const char *reponame, const char **files)
 {
-       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;
+       size_t repolen = strlen(reponame) + 1;
 
-       g_repo = cl_git_sandbox_init(repo);
+       g_repo = cl_git_sandbox_init(reponame);
 
        for (scan = files; *scan != NULL; ++scan) {
                cl_git_pass(git_futils_mkdir(
-                       *scan + repolen, repo, 0777, GIT_MKDIR_PATH | GIT_MKDIR_SKIP_LAST));
+                       *scan + repolen, reponame,
+                       0777, GIT_MKDIR_PATH | GIT_MKDIR_SKIP_LAST));
                cl_git_mkfile(*scan, "contents");
        }
 }
 
+static const char *test_repo_1 = "empty_standard_repo";
+static const char *test_files_1[] = {
+       "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
+};
+
 void test_status_ignore__subdirectories_recursion(void)
 {
        /* Let's try again with recursing into ignored dirs turned on */
@@ -292,7 +294,7 @@ void test_status_ignore__subdirectories_recursion(void)
                GIT_STATUS_IGNORED, GIT_STATUS_IGNORED, GIT_STATUS_IGNORED,
        };
 
-       make_test_data();
+       make_test_data(test_repo_1, test_files_1);
        cl_git_rewritefile("empty_standard_repo/.gitignore", "ignore_me\n/ignore_also\n");
 
        memset(&counts, 0x0, sizeof(status_entry_counts));
@@ -347,7 +349,7 @@ void test_status_ignore__subdirectories_not_at_root(void)
                GIT_STATUS_WT_NEW, GIT_STATUS_IGNORED, GIT_STATUS_WT_NEW, GIT_STATUS_WT_NEW,
        };
 
-       make_test_data();
+       make_test_data(test_repo_1, test_files_1);
        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");
 
@@ -389,7 +391,7 @@ void test_status_ignore__leading_slash_ignores(void)
                GIT_STATUS_WT_NEW, GIT_STATUS_WT_NEW, GIT_STATUS_WT_NEW, GIT_STATUS_WT_NEW,
        };
 
-       make_test_data();
+       make_test_data(test_repo_1, test_files_1);
 
        cl_fake_home(&home);
        cl_git_mkfile("home/.gitignore", "/ignore_me\n");
@@ -422,6 +424,52 @@ void test_status_ignore__leading_slash_ignores(void)
        cl_fake_home_cleanup(&home);
 }
 
+void test_status_ignore__contained_dir_with_matching_name(void)
+{
+       static const char *test_files[] = {
+               "empty_standard_repo/subdir_match/aaa/subdir_match/file",
+               "empty_standard_repo/subdir_match/zzz_ignoreme",
+               NULL
+       };
+       static const char *expected_paths[] = {
+               "subdir_match/.gitignore",
+               "subdir_match/aaa/subdir_match/file",
+               "subdir_match/zzz_ignoreme",
+       };
+       static const unsigned int expected_statuses[] = {
+               GIT_STATUS_WT_NEW,  GIT_STATUS_WT_NEW,  GIT_STATUS_IGNORED
+       };
+       int ignored;
+       git_status_options opts = GIT_STATUS_OPTIONS_INIT;
+       status_entry_counts counts;
+
+       make_test_data("empty_standard_repo", test_files);
+       cl_git_mkfile(
+               "empty_standard_repo/subdir_match/.gitignore", "*_ignoreme\n");
+
+       cl_git_pass(git_status_should_ignore(
+               &ignored, g_repo, "subdir_match/aaa/subdir_match/file"));
+       cl_assert(!ignored);
+
+       cl_git_pass(git_status_should_ignore(
+               &ignored, g_repo, "subdir_match/zzz_ignoreme"));
+       cl_assert(ignored);
+
+       memset(&counts, 0x0, sizeof(status_entry_counts));
+       counts.expected_entry_count = 3;
+       counts.expected_paths = expected_paths;
+       counts.expected_statuses = expected_statuses;
+
+       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;

diff --git a/tests/status/status_helpers.c b/tests/status/status_helpers.c
index 902b65c4f3e69d50816c1b16847dd2a159f2f4df..08827925270189e4236a591e2b87bc99f97d14c7 100644 (file)
--- a/tests/status/status_helpers.c
+++ b/tests/status/status_helpers.c
@@ -9,20 +9,13 @@ int cb_status__normal(
        if (counts->debug)
                cb_status__print(path, status_flags, NULL);
 
-       if (counts->entry_count >= counts->expected_entry_count) {
+       if (counts->entry_count >= counts->expected_entry_count)
                counts->wrong_status_flags_count++;
-               goto exit;
-       }
-
-       if (strcmp(path, counts->expected_paths[counts->entry_count])) {
+       else if (strcmp(path, counts->expected_paths[counts->entry_count]))
                counts->wrong_sorted_path++;
-               goto exit;
-       }
-
-       if (status_flags != counts->expected_statuses[counts->entry_count])
+       else if (status_flags != counts->expected_statuses[counts->entry_count])
                counts->wrong_status_flags_count++;
 
-exit:
        counts->entry_count++;
        return 0;
 }