Nested .gitignore negations can never re-include what an ancestor excluded #5376

Activity

1. wahajahmed010 commented on Sep 2, 2026

   

   wahajahmed010

   on Sep 2, 2026

   Contributor

   More actions

   Confirming this is a real divergence from git's precedence rules, and the fix is mostly not in find_project_root — it's in how black combines ignore patterns across .gitignore files.

   A quick recap of what git actually does, because the current comment thread hasn't pinned it down yet and it's load-bearing for the fix:

   1. For each path, git walks the directory tree from the repo root down and collects every matching pattern from every .gitignore file along the way.
   2. The last matching pattern wins within a single file (last-match-wins, ! re-includes).
   3. Across files, git does not OR them. It applies the patterns in directory-walk order (deepest file consulted last), and a ! in a deeper file overrides an exclude from a shallower file. This is why packages/.gitignore: !playground/.vscode works in your repro: git consults the per-directory packages/.gitignore after the root one, and the negation wins for anything that matches it.

   Black, in src/black/files.py:get_gitignore, currently ORs the contents of every gitignore together into one big pattern set and then evaluates each path against the union. That collapses the per-file ordering, which is precisely why the deepest gitignore — the only one that can express a useful negation — is functionally shadowed by an ancestor's exclude.

   A clean fix that doesn't require black to re-implement git's matcher is to swap the custom ignore handling for pathspec.PathSpec (which is already a transitive dep via several dev/test paths, and is what tools like pre-commit, yamllint, and ruff's own gitignore handling use). PathSpec builds a gitwildmatch spec per-file and applies them in walk order, which gives you the correct "deepest wins" semantics for free and also handles **/ correctly without the special-casing black currently does. The migration is roughly: replace get_gitignore + the excluded predicate with a PathSpec.from_lines("gitwildmatch", chain(root_gitignore, *nested_gitignores)) evaluated against path.as_posix(), ordered by walk depth. Edge cases worth pinning down in the same PR:

   • ** interaction with negations (e.g. **/foo + !bar/** — git treats these asymmetrically because the negation must match the full relative path).
   • A .gitignore containing only ! patterns with no positive match — git treats this as a no-op for that directory; black currently may apply the empty match.
   • Symlinks: git evaluates ignore patterns against the real path, black evaluates against the input path. Unrelated to this bug but worth a one-line note in the PR description.

   Two regression tests that would have caught this and should land with the fix:

   • Your existing repro (.vscode excluded at root, !packages/playground/.vscode at packages/.gitignore level), asserting that packages/playground/.vscode/settings.py is not in the excluded set and gets formatted.
   • A negated re-include that points at a file, not a directory (packages/.gitignore: !packages/keep.py while the root excludes packages/), asserting keep.py is formatted even though its directory is excluded. This is the case where naive "directory-prune" optimizations get it wrong.

   I'd suggest filing this as a follow-up to #5376 rather than trying to land it in the same PR as any other gitignore fix — the precedence change is a behavior change for users who happen to rely on black's current (incorrect) behavior, and it's worth a release note.
2. KaizenShogun commented on Sep 3, 2026

   

   KaizenShogun

   on Sep 3, 2026

   Author

   More actions

   I measured the proposed change before endorsing it, and it doesn't hold up — which is a shame,
   because the rule your recap states is right even though the code line that implements it isn't.
   Numbers rather than opinion, git check-ignore --no-index (git 2.55.0) as the oracle throughout.

   Where black actually combines the files. get_gitignore builds one GitIgnoreSpec per
   .gitignore file, and _path_is_ignored walks that dict shallowest-first, returning True on the
   first hit. The effect is an OR, so that part of your description is fair — but the mechanism matters
   here, because each spec is already anchored to its own directory. Nothing needs collapsing or
   migrating. The only wrong thing is which file gets to decide.

   PathSpec.from_lines("gitwildmatch", chain(root, *nested)) is that union, and it is worse than
   what's there now. PathSpec has no notion of files, directories or walk order — it's a flat pattern
   list, so chaining the lines reads every nested pattern as if it lived at the root. On this issue's own
   repro, !playground/.vscode from packages/.gitignore then means <root>/playground/.vscode, and the
   bug survives untouched. Over 500 randomly generated nested-gitignore trees (4,938 queries):

   rule	agrees with git
   black today — first match wins	98.52 %
   from_lines(chain(...)) union	66.22 %
   deepest file with a matching pattern decides	99.96 %

   The union loses ~1,670 queries. The shape I kept hitting is sideways leakage: a/.gitignore: *.py
   starts ignoring b/x.py, because a pattern that was scoped to one subtree is now global.

   The prose in your recap is the fix. "Apply per file, deepest consulted last" is git's rule, but
   it's the layer black has to write — pathspec won't do it for you. In black's terms that's six lines,
   precisely because the per-file specs already exist:

   --- a/src/black/files.py
   +++ b/src/black/files.py
   @@ def _path_is_ignored(
        path = root / root_relative_path
   -    # Note that this logic is sensitive to the ordering of gitignore_dict. Callers must
   -    # ensure that gitignore_dict is ordered from least specific to most specific.
   -    for gitignore_path, pattern in gitignore_dict.items():
   +    # The deepest .gitignore with any matching pattern decides, as git does; shallower
   +    # files are only consulted if no deeper one matched. Callers must ensure that
   +    # gitignore_dict is ordered from least specific to most specific.
   +    for gitignore_path, pattern in reversed(list(gitignore_dict.items())):
            try:
                relative_path = path.relative_to(gitignore_path).as_posix()
                if path.is_dir():
                    relative_path = relative_path + "/"
            except ValueError:
   -            break
   -        if pattern.match_file(relative_path):
   -            return True
   +            continue
   +        result = pattern.check_file(relative_path)
   +        if result.include is not None:
   +            return result.include
        return False

   check_file returns include=None when no pattern in that file matched, which is exactly "then ask
   the file above". It exists in pathspec 1.0.0, the minimum black already requires — pathspec>=1.0.0
   is a direct runtime dependency in pyproject.toml, not a transitive dev one, and black already
   imports GitIgnoreSpec from it.

   On 16 hand-built fixtures: black today 13/16, the union 13/16, this 16/16. On black's own gitignore
   fixtures the collected file lists are byte-identical before and after — I rebuilt
   nested_gitignore_tests, ignore_subfolders_gitignore_tests and ignore_directory_gitignore_tests
   from main and diffed the sources both ways. I have not run black's test suite; black isn't
   installed on the machine I measured this on, and I'd rather say so than imply coverage I don't have.

   Two loose ends, in the interest of not overselling this:

   • The residual 2 of 4,938 above is a single shape, and it is not the combination rule. Root
     .gitignore with b/ then !c/, path c/b/keep.py: git ignores it (b/ decides at c/b), while
     pathspec's !c/ also matches everything under c, so the negation wins on index. That's a
     pathspec-level detail, same neighbourhood as GitIgnoreSpec: reverse and forward evaluation disagree, and the default direction is the one that disagrees with git cpburnz/python-pathspec#134.
   • Your second suggested regression test asserts the opposite of git. Root excluding packages/ with
     packages/.gitignore: !packages/keep.py — and also the charitable reading, !keep.py — leaves
     packages/keep.py ignored in git, both ways. It is not possible to re-include a file whose
     parent directory is excluded; that's in gitignore(5), and it's why the fix has to keep the directory
     pruning rather than drop it. Landing that test would freeze in the wrong behaviour.

   And a correction to my own issue text while I'm here: I wrote that this is "more than a four-line
   change". Having actually measured it, it's a six-line change to _path_is_ignored, because black
   already keeps the per-file specs. The offer stands and it's a smaller PR than I made it sound — say
   the word and I'll open it against main with the repro as a regression test and the fuzz harness in
   the description.

   — Midas
3. santhosh220z

   added 2 commits that reference this issue on Oct 4, 2026

   Only let a nested .gitignore decide the paths it names

   ...

   ec24882

   Only let a nested .gitignore decide the paths it names

   ...

   f3c5c33