Fix single-file ignore checks To answer if a single given file should be ignored, the path to that file has to be processed progressively checking that there are no intermediate ignored directories in getting to the file in question. This enables that, fixing the broken old behavior, and adds tests to exercise various ignore situations.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 215 216 217 218 219 220 221 222 223 224 225 226 227 228 229 230 231 232 233 234 235 236 237 238 239 240 241 242 243 244 245 246 247 248 249 250 251 252 253 254 255 256 257 258 259 260 261 262 263 264 265 266 267 268 269 270 271 272 273 274 275 276 277 278 279 280 281 282 283 284 285 286 287 288 289 290 291 292 293 294 295 296 297 298 299 300 301 302 303 304 305 306 307 308 309 310 311 312 313 314 315 316 317 318 319 320 321 322 323 324 325 326 327 328 329 330 331 332 333 334 335 336 337 338 339 340 341 342 343 344 345 346 347 348 349 350 351 352 353 354 355 356 357 358 359 360 361 362 363 364 365 366 367 368 369 370 371 372 373 374 375 376 377 378 379 380 381 382
diff --git a/include/git2/ignore.h b/include/git2/ignore.h
index f7e04e8..964a108 100644
--- a/include/git2/ignore.h
+++ b/include/git2/ignore.h
@@ -54,9 +54,12 @@ GIT_EXTERN(int) git_ignore_clear_internal_rules(
/**
* Test if the ignore rules apply to a given path.
*
- * This function simply checks the ignore rules to see if they would apply
- * to the given file. This indicates if the file would be ignored regardless
- * of whether the file is already in the index or commited to the repository.
+ * This function checks the ignore rules to see if they would apply to the
+ * given file. This indicates if the file would be ignored regardless of
+ * whether the file is already in the index or commited to the repository.
+ *
+ * One way to think of this is if you were to do "git add ." on the
+ * directory containing the file, would it be added or not?
*
* @param ignored boolean returning 0 if the file is not ignored, 1 if it is
* @param repo a repository object
diff --git a/include/git2/status.h b/include/git2/status.h
index cc94d76..0dd9859 100644
--- a/include/git2/status.h
+++ b/include/git2/status.h
@@ -146,10 +146,12 @@ GIT_EXTERN(int) git_status_file(
/**
* Test if the ignore rules apply to a given file.
*
- * This function simply checks the ignore rules to see if they would apply
- * to the given file. Unlike git_status_file(), this indicates if the file
- * would be ignored regardless of whether the file is already in the index
- * or in the repository.
+ * This function checks the ignore rules to see if they would apply to the
+ * given file. This indicates if the file would be ignored regardless of
+ * whether the file is already in the index or commited to the repository.
+ *
+ * One way to think of this is if you were to do "git add ." on the
+ * directory containing the file, would it be added or not?
*
* @param ignored boolean returning 0 if the file is not ignored, 1 if it is
* @param repo a repository object
diff --git a/src/attr_file.h b/src/attr_file.h
index b28d8a0..877daf3 100644
--- a/src/attr_file.h
+++ b/src/attr_file.h
@@ -71,10 +71,10 @@ typedef struct {
} git_attr_file;
typedef struct {
- git_buf full;
- const char *path;
- const char *basename;
- int is_dir;
+ git_buf full;
+ char *path;
+ char *basename;
+ int is_dir;
} git_attr_path;
typedef enum {
diff --git a/src/ignore.c b/src/ignore.c
index e711be2..6a377e6 100644
--- a/src/ignore.c
+++ b/src/ignore.c
@@ -277,15 +277,76 @@ int git_ignore_clear_internal_rules(
int git_ignore_path_is_ignored(
int *ignored,
git_repository *repo,
- const char *path)
+ const char *pathname)
{
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;
- if (git_ignore__for_path(repo, path, &ignores) < 0)
- return -1;
+ assert(ignored && pathname);
+
+ workdir = repo ? git_repository_workdir(repo) : NULL;
+
+ if ((error = git_attr_path__init(&path, pathname, workdir)) < 0)
+ return error;
+
+ tail = path.path;
+ end = &path.full.ptr[path.full.size];
+ full_is_dir = path.is_dir;
- error = git_ignore__lookup(&ignores, path, ignored);
+ 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;
+
+ /* update ignores for new path fragment */
+ if (path.basename == path.path)
+ error = git_ignore__for_path(repo, path.path, &ignores);
+ else
+ error = git_ignore__push_dir(&ignores, path.basename);
+ if (error < 0)
+ break;
+
+ /* first process builtins - success means path was found */
+ if (ignore_lookup_in_rules(
+ &ignores.ign_internal->rules, &path, ignored))
+ goto cleanup;
+
+ /* next process files in the path */
+ git_vector_foreach(&ignores.ign_path, i, file) {
+ if (ignore_lookup_in_rules(&file->rules, &path, ignored))
+ goto cleanup;
+ }
+
+ /* last process global ignores */
+ git_vector_foreach(&ignores.ign_global, i, file) {
+ if (ignore_lookup_in_rules(&file->rules, &path, ignored))
+ goto cleanup;
+ }
+
+ /* if we found no rules before reaching the end, we're done */
+ if (tail == end)
+ break;
+
+ /* reinstate divider in path */
+ *tail = '/';
+ while (*tail == '/') tail++;
+ }
+
+ *ignored = 0;
+
+cleanup:
+ git_attr_path__free(&path);
git_ignore__free(&ignores);
return error;
}
diff --git a/src/status.c b/src/status.c
index 3a0ed07..a37653d 100644
--- a/src/status.c
+++ b/src/status.c
@@ -86,6 +86,10 @@ int git_status_foreach_ext(
assert(show <= GIT_STATUS_SHOW_INDEX_THEN_WORKDIR);
+ if (show != GIT_STATUS_SHOW_INDEX_ONLY &&
+ (err = git_repository__ensure_not_bare(repo, "status")) < 0)
+ return err;
+
if ((err = git_repository_head_tree(&head, repo)) < 0)
return err;
@@ -245,9 +249,22 @@ int git_status_file(
error = GIT_EAMBIGUOUS;
if (!error && !sfi.count) {
- giterr_set(GITERR_INVALID,
- "Attempt to get status of nonexistent file '%s'", path);
- error = GIT_ENOTFOUND;
+ git_buf full = GIT_BUF_INIT;
+
+ /* if the file actually exists and we still did not get a callback
+ * for it, then it must be contained inside an ignored directory, so
+ * mark it as such instead of generating an error.
+ */
+ if (!git_buf_joinpath(&full, git_repository_workdir(repo), path) &&
+ git_path_exists(full.ptr))
+ sfi.status = GIT_STATUS_IGNORED;
+ else {
+ giterr_set(GITERR_INVALID,
+ "Attempt to get status of nonexistent file '%s'", path);
+ error = GIT_ENOTFOUND;
+ }
+
+ git_buf_free(&full);
}
*status_flags = sfi.status;
diff --git a/tests-clar/diff/iterator.c b/tests-clar/diff/iterator.c
index c27d3fa..cca6450 100644
--- a/tests-clar/diff/iterator.c
+++ b/tests-clar/diff/iterator.c
@@ -451,13 +451,13 @@ static void workdir_iterator_test(
git_iterator_free(i);
- cl_assert(count == expected_count);
- cl_assert(count_all == expected_count + expected_ignores);
+ cl_assert_equal_i(expected_count,count);
+ cl_assert_equal_i(expected_count + expected_ignores, count_all);
}
void test_diff_iterator__workdir_0(void)
{
- workdir_iterator_test("attr", NULL, NULL, 25, 2, NULL, "ign");
+ workdir_iterator_test("attr", NULL, NULL, 27, 1, NULL, "ign");
}
static const char *status_paths[] = {
diff --git a/tests-clar/resources/attr/gitignore b/tests-clar/resources/attr/gitignore
index 546d48f..1929670 100644
--- a/tests-clar/resources/attr/gitignore
+++ b/tests-clar/resources/attr/gitignore
@@ -1,3 +1,2 @@
-sub
ign
dir/
diff --git a/tests-clar/resources/attr/sub/ign b/tests-clar/resources/attr/sub/ign
deleted file mode 100644
index 592fd25..0000000
--- a/tests-clar/resources/attr/sub/ign
+++ /dev/null
@@ -1 +0,0 @@
-ignore me
diff --git a/tests-clar/resources/attr/sub/ign/file b/tests-clar/resources/attr/sub/ign/file
new file mode 100644
index 0000000..4dcd992
--- /dev/null
+++ b/tests-clar/resources/attr/sub/ign/file
@@ -0,0 +1 @@
+in ignored dir
diff --git a/tests-clar/resources/attr/sub/ign/sub/file b/tests-clar/resources/attr/sub/ign/sub/file
new file mode 100644
index 0000000..88aca01
--- /dev/null
+++ b/tests-clar/resources/attr/sub/ign/sub/file
@@ -0,0 +1 @@
+below ignored dir
diff --git a/tests-clar/status/ignore.c b/tests-clar/status/ignore.c
index 68dc652..bd74b97 100644
--- a/tests-clar/status/ignore.c
+++ b/tests-clar/status/ignore.c
@@ -22,21 +22,25 @@ void test_status_ignore__0(void)
const char *path;
int expected;
} test_cases[] = {
- /* patterns "sub" and "ign" from .gitignore */
+ /* pattern "ign" from .gitignore */
{ "file", 0 },
{ "ign", 1 },
- { "sub", 1 },
+ { "sub", 0 },
{ "sub/file", 0 },
{ "sub/ign", 1 },
- { "sub/sub", 1 },
+ { "sub/ign/file", 1 },
+ { "sub/ign/sub", 1 },
+ { "sub/ign/sub/file", 1 },
+ { "sub/sub", 0 },
{ "sub/sub/file", 0 },
{ "sub/sub/ign", 1 },
- { "sub/sub/sub", 1 },
+ { "sub/sub/sub", 0 },
/* pattern "dir/" from .gitignore */
{ "dir", 1 },
{ "dir/", 1 },
{ "sub/dir", 1 },
{ "sub/dir/", 1 },
+ { "sub/dir/file", 1 }, /* contained in ignored parent */
{ "sub/sub/dir", 0 }, /* dir is not actually a dir, but a file */
{ NULL, 0 }
}, *one_test;
@@ -172,6 +176,61 @@ void test_status_ignore__ignore_pattern_ignorecase(void)
cl_assert(flags == ignore_case ? GIT_STATUS_IGNORED : GIT_STATUS_WT_NEW);
}
+void test_status_ignore__subdirectories(void)
+{
+ status_entry_single st;
+ int ignored;
+ git_status_options opts;
+
+ g_repo = cl_git_sandbox_init("empty_standard_repo");
+
+ cl_git_mkfile(
+ "empty_standard_repo/ignore_me", "I'm going to be ignored!");
+
+ cl_git_rewritefile("empty_standard_repo/.gitignore", "ignore_me\n");
+
+ memset(&st, 0, sizeof(st));
+ cl_git_pass(git_status_foreach(g_repo, cb_status__single, &st));
+ cl_assert_equal_i(2, st.count);
+ cl_assert(st.status == GIT_STATUS_IGNORED);
+
+ cl_git_pass(git_status_file(&st.status, g_repo, "ignore_me"));
+ cl_assert(st.status == GIT_STATUS_IGNORED);
+
+ cl_git_pass(git_status_should_ignore(&ignored, g_repo, "ignore_me"));
+ cl_assert(ignored);
+
+
+ /* So, interestingly, as per the comment in diff_from_iterators() the
+ * following file is ignored, but in a way so that it does not show up
+ * in status even if INCLUDE_IGNORED is used. This actually matches
+ * core git's behavior - if you follow these steps and try running "git
+ * status -uall --ignored" then the following file and directory will
+ * not show up in the output at all.
+ */
+
+ 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!");
+
+ opts.show = GIT_STATUS_SHOW_INDEX_AND_WORKDIR;
+ opts.flags = GIT_STATUS_OPT_INCLUDE_IGNORED |
+ GIT_STATUS_OPT_INCLUDE_UNTRACKED |
+ GIT_STATUS_OPT_RECURSE_UNTRACKED_DIRS;
+
+ memset(&st, 0, sizeof(st));
+ cl_git_pass(git_status_foreach(g_repo, cb_status__single, &st));
+ cl_assert_equal_i(2, st.count);
+
+ cl_git_pass(git_status_file(&st.status, g_repo, "test/ignore_me/file"));
+ cl_assert(st.status == GIT_STATUS_IGNORED);
+
+ cl_git_pass(
+ git_status_should_ignore(&ignored, g_repo, "test/ignore_me/file"));
+ cl_assert(ignored);
+}
+
void test_status_ignore__adding_internal_ignores(void)
{
int ignored;
@@ -234,3 +293,49 @@ void test_status_ignore__add_internal_as_first_thing(void)
cl_git_pass(git_status_should_ignore(&ignored, g_repo, "two.bar"));
cl_assert(!ignored);
}
+
+void test_status_ignore__internal_ignores_inside_deep_paths(void)
+{
+ int ignored;
+ const char *add_me = "Debug\nthis/is/deep\npatterned*/dir\n";
+
+ g_repo = cl_git_sandbox_init("empty_standard_repo");
+
+ cl_git_pass(git_ignore_add_rule(g_repo, add_me));
+
+ cl_git_pass(git_status_should_ignore(&ignored, g_repo, "Debug"));
+ cl_assert(ignored);
+ cl_git_pass(git_status_should_ignore(&ignored, g_repo, "and/Debug"));
+ cl_assert(ignored);
+ cl_git_pass(git_status_should_ignore(&ignored, g_repo, "really/Debug/this/file"));
+ cl_assert(ignored);
+ cl_git_pass(git_status_should_ignore(&ignored, g_repo, "Debug/what/I/say"));
+ cl_assert(ignored);
+
+ cl_git_pass(git_status_should_ignore(&ignored, g_repo, "and/NoDebug"));
+ cl_assert(!ignored);
+ cl_git_pass(git_status_should_ignore(&ignored, g_repo, "NoDebug/this"));
+ cl_assert(!ignored);
+ cl_git_pass(git_status_should_ignore(&ignored, g_repo, "please/NoDebug/this"));
+ cl_assert(!ignored);
+
+ cl_git_pass(git_status_should_ignore(&ignored, g_repo, "this/is/deep"));
+ cl_assert(ignored);
+ /* pattern containing slash gets FNM_PATHNAME so all slashes must match */
+ cl_git_pass(git_status_should_ignore(&ignored, g_repo, "and/this/is/deep"));
+ cl_assert(!ignored);
+ cl_git_pass(git_status_should_ignore(&ignored, g_repo, "this/is/deep/too"));
+ cl_assert(ignored);
+ /* pattern containing slash gets FNM_PATHNAME so all slashes must match */
+ cl_git_pass(git_status_should_ignore(&ignored, g_repo, "but/this/is/deep/and/ignored"));
+ cl_assert(!ignored);
+
+ cl_git_pass(git_status_should_ignore(&ignored, g_repo, "this/is/not/deep"));
+ cl_assert(!ignored);
+ cl_git_pass(git_status_should_ignore(&ignored, g_repo, "is/this/not/as/deep"));
+ cl_assert(!ignored);
+ cl_git_pass(git_status_should_ignore(&ignored, g_repo, "this/is/deepish"));
+ cl_assert(!ignored);
+ cl_git_pass(git_status_should_ignore(&ignored, g_repo, "xthis/is/deep"));
+ cl_assert(!ignored);
+}