Fix GitIgnoreSpec re-including files under an excluded directory (#129)

[youdie006]

Fixes #129. Thanks to @eeshsaxena for the clear report and the directory-vs-contents distinction -- it made this straightforward to reproduce.

Problem

GitIgnoreSpec resolves patterns with a flat last-match, so a file-level negation can re-include a file whose parent directory is excluded, which git forbids ("It is not possible to re-include a file if a parent directory of that file is excluded"):

GitIgnoreSpec.from_lines(["build", "!keep.log"]).match_file("build/keep.log")
# -> False, but real `git check-ignore` treats build/keep.log as ignored

Proposed approach (open to a different design)

I want to be upfront that this is a core match-resolution change, so please treat the approach as a proposal -- I am happy to restructure it however you prefer.

The fix wraps the gitignore backend (_AncestorDirBackend). When a file is not already ignored by its own resolution, it walks the file's ancestor directory prefixes and, for each, asks whether that directory is excluded -- resolved as a directory (ancestor + "/") using git's plain last-match order via the existing util.check_match_file. If any ancestor directory is excluded, the file is ignored regardless of a later file-level negation.

Resolving the ancestor as a directory (rather than reusing the leaf-file resolution) is what keeps directory-level re-inclusions working, so this correctly distinguishes:

.gitignore	path	result
build + !keep.log	build/keep.log	ignored
build/* + !build/keep.log	build/keep.log	re-included

build/* only excludes the contents (it does not match the build directory itself), so re-inclusion is preserved -- that case, plus the * + !libfoo + !libfoo/** whitelist idiom (test_08_issue_81), the !*/ "scan all directories" idiom (test_07_issue_74), and !*.yaml/ (test_02_issue_41), all still behave as before.

Notes / tradeoffs I would value your opinion on:

• The check is O(path-depth) per file, and only runs when the file was not already ignored. It uses the plain per-pattern last-match (check_match_file) for the ancestor directory resolution rather than the compiled re2/hyperscan path, so it is backend-agnostic but does not use the fast combined regex for those extra directory lookups.
• To apply the wrapper uniformly (including through the internal _test_backend_factory hook used by the tests), I added a small _wrap_backend extension point on PathSpec (a no-op by default, overridden by GitIgnoreSpec). If you would rather fold the check in elsewhere -- e.g. inside each backend, or only in _make_backend -- I am glad to change it.

Tests

Added test_10_issue_129_{a,b,c} to tests/test_06_gitignore.py, running across all backends via parameterize_from_lines. Verified red-green: with the fix reverted the build/keep.log and a/keep.log ignore cases fail across every backend; with the fix they pass, and the build/* re-inclusion case passes both ways. Expected results confirmed against git check-ignore (2.54.0). Full suite (200 passed) and the strict docs build stay green.

---

This change was prepared with AI assistance and reviewed by me before submission.

[KaizenShogun]

I came at this from the other end — I have #133 open on the same file and wanted to know whether the two collide. They don't, and the harness I built to find that out says something useful about this PR, so here it is. Everything below was re-measured today against a99b129 on base 6568072; the scripts are boring enough to re-run.

The oracle

git check-ignore isn't a strong enough oracle for this particular bug. GitIgnoreSpec's own docstring says git "allows including files from excluded directories which directly contradicts the documentation" — so if that were true for these shapes, check-ignore would be the thing that's wrong and the current behaviour would be right. To rule that out I used a harder one: write the files, git add -A, and ask git ls-files. What git actually puts in the index is what a user means by "not ignored". Across the 23 pairs below, check-ignore, add and status --porcelain --ignored agree with each other on all 23, so the deviation the docstring describes doesn't apply to these shapes.

Result

23 path/pattern pairs, GitIgnoreSpec.match_file against that oracle:

tree	agrees with git
master	13 / 23
master + #132	22 / 23

The nine it fixes:

!**/node_modules/**  +  /node_modules      ->  node_modules/x.txt
!**/foo/**           +  foo/               ->  foo/x.txt, a/foo/x.txt
*  +  !src/**        +  src/generated/     ->  src/generated/x.py
!src/**              +  src/dist/          ->  src/dist/x.py
!**/*.md             +  docs/              ->  docs/a.md
build                +  !keep.log          ->  build/keep.log
a                    +  !keep.log          ->  a/keep.log
dir/                 +  !dir/file.txt      ->  dir/file.txt

And the controls it leaves alone, which is the part I was actually worried about: #74 (* + !*/), #81 (* + !libfoo + !libfoo/**), #41 (*.yaml + !*.yaml/), and build/* + !build/keep.md, which must stay re-included. Upstream suite: 197 OK → 200 OK.

Does this happen outside a test fixture?

Fair question to ask of any matcher bug, so I checked before believing my own fixtures. I pulled the real .gitignore of 43 widely-used repos (CPython, Django, React, Vue, Kubernetes, Rust, TensorFlow, VS Code, …), generated candidate paths from each file's own lines — 2012 of them — and compared against git. On master, four paths in two repos disagree. Two of them are real:

nodejs/node
  deps/npm/node_modules/.bin/x.txt        git: ignored    GitIgnoreSpec: NOT ignored
  deps/npm/node_modules/.bin/pkg/x.txt    git: ignored    GitIgnoreSpec: NOT ignored

Reduced, that's the first row of the table above: a broad !**/node_modules/** earlier in the file, a directory exclusion later. #132 fixes both. The other two are an artifact of my own path generator, and I'd rather flag it than have it read as a second finding: pandas' .gitignore has *\#*\#, my generator concretises it into a path containing literal backslashes, and GitIgnoreSpec calls that ignored where git doesn't. It's unchanged with or without this PR, so it's noise here (possibly worth its own look at escape handling one day, on a shape nobody writes).

So: rare in the wild, but not zero, and the one real hit is node's. Worth noting that the plain PathSpec (last-match-wins) gets that case right and GitIgnoreSpec gets it wrong on master, which is a slightly awkward place for the gitignore-specific class to be — after #132 they agree.

The one case it doesn't fix

.gitignore:  *  /  !src/**  /  src/generated/
src/a.py        git: ignored    with #132: NOT ignored

The mechanism, since it lives exactly in the code this PR adds: !src/** compiles to the regex ^src/, which matches the directory path src/. #132 resolves each ancestor as ancestor + "/", so that negation matches the ancestor itself and src comes out not-excluded. git disagrees — git check-ignore -v src/ blames line 1, *, because src/** requires something after the slash and never matches src itself. The split is visible from outside:

spec.match_file("src")   # True
spec.match_file("src/")  # False

I don't think this blocks the PR — master gets that case wrong too, so it's not a regression — but if you want a test for it, the ancestor resolution is where it would have to be fixed, and it's arguably a separate bug in how src/** compiles.

Overlap with #133

Orthogonal halves of the same wound, as far as I can measure. #132 fixes file paths under an excluded directory. #133 fixes directory paths — match_file('sub/') under the * + !*/ whitelist idiom, which #132 leaves untouched (still reports sub/ as ignored where git doesn't). Applied together on master, either order, they apply cleanly on each other: 202 tests OK, 22/23 on the file oracle above, and the trailing-slash directory queries go from wrong to right:

'*' + '!*/' + '!*.py',  git ignores none of these:
                     master   +#132    +#133   +both
match_file('sub/')     True    True    False   False
match_file('sub/d/')   True    True    False   False

(The slashless forms — match_file('sub') — still say True in every tree. That one I'd call caller error rather than a bug: without the trailing slash there's nothing in the string that says "directory", which is why black and friends append it before asking.)

Nice work on the report, @eeshsaxena, and on the directory-vs-contents distinction — that framing is what made this measurable. @youdie006, the ancestor-as-directory resolution is the right call; the residual above is the price of resolving it with a trailing slash, not of the approach.

— Midas

[youdie006]

Thanks for building that harness, and for pointing it at the controls rather than only at the cases the patch was aimed at — the #74/#81/#41 and build/* + !build/keep.md set is exactly what I would have wanted checked.

Your point about the oracle is well taken and I think it is the more important half of this comment. I leaned on git check-ignore, and you are right that GitIgnoreSpec's own docstring makes that circular for this bug class: if git really did re-include out of excluded directories, check-ignore would be the thing to distrust. git add -A + git ls-files is not subject to that objection, since it reports what git actually decided to track. I will use that in future work on this file.

I re-measured the residual you flagged, against real git rather than taking it from the report, on the * / !src/** / src/generated/ fixture:

path	git check-ignore -v	master	this PR
src	ignored (line 1, *)	ignored	ignored
src/	ignored (line 1, *)	not ignored	not ignored
src/a.py	ignored (line 1, *)	not ignored	not ignored
src/generated/x.py	ignored (line 1, *)	not ignored	ignored

So it reproduces, and your reading of it holds: the two wrong rows are identical on master, and the one row this PR changes in that fixture moves toward git, not away from it. Not a regression.

On the mechanism, I agree it is a separate bug, and I would put it one step earlier than the ancestor resolution. !src/** compiles to the regex ^src/, which matches the directory path src/ itself. gitignore(5) says a trailing /** "matches everything inside", i.e. abc/** matches files inside abc, so that pattern should not match abc/ at all — the regex wants something after the slash. Real git agrees: it blames * for src/, meaning !src/** never re-included the directory. Fixing the compilation of dir/** would make the ancestor lookup correct for free and would not need the ancestor code to special-case anything, which is why I would rather not paper over it inside the ancestor resolution this PR adds.

Happy to leave that for a separate change, and glad #132 and #133 turn out to be orthogonal — the directory-path half was the piece I knowingly did not touch here.

Disclosure: I use AI assistance in my work, and I review and verify everything before it goes out. The measurements above are my own re-runs.

[cpburnz]

I'm not thrilled with wrapping backends.

It looks like a singular segment matches stronger than the equivalent seg/* or seg/**. See this table:

.gitignore	path	git	GitIgnoreSpec.match_file
build + !ignore.log	build/ignore.log	ignored	not ignored ❌
build/* + !keep.log	build/keep.log	re-included	re-included ✅
build/** + !keep.log	build/keep.log	re-included	re-included ✅
build + !build/ignore.log	build/ignore.log	ignored	not ignored ❌
build/* + !build/keep.log	build/keep.log	re-included	re-included ✅
build/** + !build/keep.log	build/keep.log	re-included	re-included ✅

Perhaps there's a way to add another MARK (regex capture group) and use priority=3 in pathspec/_backends/simple/gitignore.py.