Skip to content

Fix ancestor exclusion checks after gitignore negations - #165

Merged
cpburnz merged 1 commit into
cpburnz:masterfrom
asnlsxx:fix/excluded-ancestor-directory-negation
Oct 18, 2026
Merged

cpburnz merged 1 commit into
cpburnz:masterfrom
asnlsxx:fix/excluded-ancestor-directory-negation

Conversation

@asnlsxx

@asnlsxx asnlsxx commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

With rules ['a/', '!a/b/'], GitIgnoreSpec currently reports a/b/file.txt as
not ignored even though a/ remains excluded. The same problem affects the
deep-file case explicitly deferred by #164: ['d/**/', '!d/child/'] incorrectly
re-includes d/child/grand/deep.txt while grand/ remains ignored.

This follows the ancestor-exclusion requirement discussed in #129, rather
than repeating the already merged #132 or #164 changes:
#129
#164

The ancestor walk currently runs only when dir_include is true. A later
directory negation makes that false; some matching patterns, such as a/**,
do not set a directory marker at all. Check ancestors whenever there is a
match, and return the first excluded ancestor's pattern index. Apply the
same change to Simple, RE2, and Hyperscan. Keep the non-recursive ancestor
walk and skip it for unmatched paths.

The patch adds regression tests using the existing backend/configuration
matrix, updates the previously documented incorrect expectation, and adds
a changelog entry. It introduces no runtime dependency or public signature
change. Checking additional ancestors can increase matching work for paths
that previously bypassed the rule; no speedup is claimed. A 40-level test
guards against recursive rechecking (at most 41 matcher calls).

Validation on Windows, CPython 3.10.11, Git 2.52.0.windows.1,
google-re2 1.1.20251105 and hyperscan 0.9.1:

  • Original suite: 278 tests, successful, 16 skipped.
  • New regressions on original source: 3 tests, 50 failing subtests.
  • Patched suite: 281 tests, successful, 16 skipped.
  • New focused regressions: 3 tests, successful, no skips.
  • Git oracle: 1,890 rule sets, 39,600 backend/path comparisons; zero
    differences in ignored status or the decisive index of ignored paths.
    On the same corpus, the original code has 1,885 ignored-status differences
    and 4,681 ignored-index differences (overlapping counts).
  • Built sdist and wheel, installed the wheel into an isolated target, then
    ran all 281 tests against that installed package: successful, 16 skipped.
  • git diff --cached --check and patch application check: successful.

Commands from the repository root, using the isolated environment's Python:
python -m unittest discover -t . -s tests/
python -m unittest tests.test_09_gitignore_ancestor_negation -v
python -m build --no-isolation --outdir ../packages
The Git-oracle counts above are supplemental local validation; the standalone
oracle and corpus are retained in the local delivery bundle, not added to this PR.
The committed regression tests can be run using the commands above.

Minimal API reproduction (False before this patch, True afterward):

from pathspec import GitIgnoreSpec
spec = GitIgnoreSpec.from_lines(["a/", "!a/b/"])
print(spec.match_file("a/b/file.txt"))

Limitations: the 16 skips are symlink-permission-dependent tests on Windows.
Other OS/Python combinations and upstream CI have not run. mypy 2.4.0
reports the same two pre-existing diagnostics in pathspec/_typing.py on
both the base and patched trees (lines 40 and 51); type checking is not
reported as passing. The finite oracle corpus is not exhaustive.

Base: ac29e19

AI assistance disclosure: Codex performed the research, implementation,
test authoring, local execution and draft preparation. The user's request
was to find a real, non-duplicate open-source need, implement and verify a
useful contribution, and obtain approval before any external publication.
The user authorized publication of this contribution. It has not received
independent human code review.

Check strict ancestors even when the last directory match is a negation
or the matching pattern has no directory marker. Return the excluding
ancestor's pattern index consistently across Simple, RE2, and Hyperscan.

Cover nested directory exceptions, terminal globstars, batch filtering,
re-inclusion controls, and bounded ancestor matching in regression tests.
@asnlsxx

asnlsxx commented Oct 18, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up validation for f70a1d2, including a measured performance cost:

  • Expanded the local Git oracle to 408 cases: eight pinned github/gitignore
    templates plus 400 seeded rule combinations, each with 246 synthetic paths
    including directory queries. Git receives existing directories without a
    trailing slash; pathspec receives a slash to indicate directory type. Git
    line numbers are mapped to pathspec indices after empty-string filtering.
  • On CPython 3.10.11 and 3.14.8, all 802,944 comparisons per interpreter pass
    across eight Simple/RE2/Hyperscan configurations. The original base has 6,978
    ignored-status and 32,574 ignored-index differences on the same corpus
    (overlapping counts). The base and patch both agree on all eight template
    cases; the differences arise in the generated rule combinations.
  • CPython 3.9.25 passes 200,736 comparisons using the two Simple configurations.
    Optional backends were unavailable in that environment and are not claimed.
  • Full unittest on 3.14.8: 281 test methods, success, 16 symlink-related skips.
    On 3.9.25: successful exit, but 799 test/subtest skip events, principally from
    missing optional backends; this is only base-backend coverage.
  • mypy 2.4.0 on Python 3.14.8 passes all 32 source files on both base and patch.
    The earlier Python 3.10 fallback diagnostics remain unchanged.

Performance tradeoff: with the Go.AllowList template and 246 synthetic paths,
matching takes about 2.30x (Simple), 2.25x (RE2), and 2.18x (Hyperscan) the base
time. Absolute medians per batch are 0.963 -> 2.211 ms, 0.447 -> 1.003 ms, and
0.458 -> 0.996 ms. The final !*/ re-includes directories, so this workload
previously skipped the ancestor walk even though its decisions were correct.
The extra checks now do real work. This is not a claimed speedup.

Each timing used seven samples of twenty batches, repeated in reverse
base/patch order; these medians combine both rounds. Compilation and filesystem
traversal are excluded, CPU scheduling was not controlled, and the paths are
synthetic. Other templates' aggregate ratios were approximately 0.89-1.26;
small differences should not be interpreted as reliable gains.

The supplemental scripts, complete oracle corpus, raw timing samples and five
oracle-adapter tests are retained in the local delivery bundle. They have not
been added to this PR or published separately. No library source changes were
made for this follow-up. Linux/macOS and upstream CI are not claimed as passing.
This investigation and the tools were prepared with Codex assistance.

@cpburnz

cpburnz commented Oct 18, 2026

Copy link
Copy Markdown
Owner

Thanks for finding and fixing this!

@cpburnz
cpburnz merged commit de04581 into cpburnz:master Oct 18, 2026
7 checks passed
cpburnz added a commit that referenced this pull request Oct 18, 2026
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.

2 participants