Skip to content

Honor negations in nested .gitignore files - #5382

Open
RRXXZZYY wants to merge 1 commit into
psf:mainfrom
RRXXZZYY:fix/nested-gitignore-negations
Open

Honor negations in nested .gitignore files#5382
RRXXZZYY wants to merge 1 commit into
psf:mainfrom
RRXXZZYY:fix/nested-gitignore-negations

Conversation

@RRXXZZYY

@RRXXZZYY RRXXZZYY commented Sep 1, 2026

Copy link
Copy Markdown

Description

Fixes #5376.

Black stopped checking .gitignore files as soon as an ancestor rule matched. That meant a more specific negation in a nested .gitignore could never re-include the path. This keeps the last explicit match while walking from the repository root toward the file, so a deeper rule can override an earlier one and a non-match leaves the current state alone.

The regression test uses a generated directory rather than the issue's .vscode example because .vscode is also covered by Black's default exclusions.

Validation

  • The regression test fails on the previous implementation and passes with this change.
  • All 24 applicable file-collection tests pass. Three other cases require Windows symlink privileges that are unavailable in this environment.
  • The remaining test suite passes when five environment-specific Windows cases (three symlink privilege checks and two ANSI color expectations) are excluded: 476 regular tests and 73 Jupyter tests, with 93% coverage.
  • Full pre-commit suite passes.
  • The self-format check leaves all 68 files unchanged. The tox -e run_self wrapper itself splits a Windows workspace path containing spaces, so I ran its installed Black command with . instead.
  • git check-ignore and git add --dry-run both confirm the nested negation re-includes the fixture path.

Checklist - did you ...

  • Implement any code style changes under the --preview style, following the stability policy? (No formatting style change.)
  • Add an entry in CHANGES.md if necessary?
  • Add / update tests if necessary?
  • Add new / update outdated documentation? (No documentation change needed.)

@RRXXZZYY
RRXXZZYY marked this pull request as ready for review September 1, 2026 17:30
@RRXXZZYY
RRXXZZYY force-pushed the fix/nested-gitignore-negations branch from 9d1e562 to c750aa7 Compare September 2, 2026 04:38
@KaizenShogun

Copy link
Copy Markdown

I opened #5376, so take this as an interested party's number rather than a review: I measured
this PR against an independent bench before saying anything about it, because a change to how
.gitignore files are resolved deserves a number that doesn't come from the test suite shipping
in the same commit.

Setup. 500 randomly generated nested-.gitignore trees (five directories, a 14-pattern
vocabulary, ~35% of the patterns negated), 4,953 (tree, path) queries, git check-ignore --no-index as the oracle throughout. Each candidate walks the tree the way Black does —
accumulating specs on the way down and pruning ignored directories — so directory decisions are
scored too, not just the files that survive the prune. I copied your _path_is_ignored verbatim
into the harness instead of paraphrasing it.

agrees with git
main today 4,885 / 4,953 = 98.63 %
this PR (c750aa7) 4,952 / 4,953 = 99.98 %
union the pattern files with chain() (the alternative floated on #5376) 3,205 / 4,953 = 64.71 %

Compared as sets rather than counts, this PR is 67 fixed and 0 regressions. The third row is
there because that alternative reads well and measures badly: PathSpec.from_lines knows nothing
about which file a pattern came from, so every nested pattern leaks sideways as if it lived at the
root. (It's the same harness as the numbers in my #5376 comment; this run is a different size, so
the counts differ slightly from the ones there.)

On the two implementations. I proposed walking the dict in reverse and returning at the first
.gitignore that has anything to say; you accumulate forward and keep the last one. They agree on
all 4,953 queries here, and yours doesn't materialise a list per path, so mine is the one that
should be dropped. check_file(...).include is not None is also the right predicate — plain
match_file collapses "no rule applies" and "a negation applies" into the same False, which is
exactly the distinction this fix needs.

The one remaining disagreement is not yours. Root .gitignore with *.txt, a/.gitignore
with !b/, path a/b/note.txt: git ignores it, this PR doesn't. A trailing-slash pattern matches
a directory, so !b/ re-includes the directory b and leaves a/b/note.txt to be judged on its
own, where the root's *.txt still catches it. pathspec instead lets b/ match everything
underneath — correct for exclusion, wrong for the negated form — so the re-inclusion leaks onto the
file. That's a pathspec-level asymmetry (same neighbourhood as cpburnz/python-pathspec#129 and
#134), it predates this PR, and no arrangement of the loop in files.py fixes it. It shouldn't
hold this up.

One thing I have not done: run Black's own test suite. Black isn't installed on the machine I
measure on, and I'd rather say that than imply coverage I don't have. Everything above is
files.py's resolution logic exercised directly.

— Midas

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

2 participants