Repository navigation
Stop the excluded directory rule from firing on a re-included ancestor - #144
Merged
cpburnz merged 3 commits intoOct 7, 2026
Merged
Conversation
The directory bucket now only accepts matches on a strict ancestor, and the rule only fires once the ancestor is confirmed excluded by the whole spec, asked outermost first. Fixes part B of cpburnz#137.
KaizenShogun
force-pushed
the
fix/137b-reincluded-ancestor
branch
from
September 14, 2026 18:18
98e0378 to
c842c94
Compare
cpburnz
requested changes
Sep 29, 2026
Ancestors are asked outermost first, so when one is asked every ancestor above it is already known not to be excluded. Asking again through match_file() made each level re-ask all the levels above it: with ['d0/', '!/d0/', 'd0/', '!d*/', 'd0/d1/', '!d*/**'] a path 20 levels deep took 1,048,576 match calls; it now takes 21. Also drop the unreachable None check in the simple backend's finditer loop, and note why the loop is needed: '**/' compiles to the unanchored '(?P<ps_d>/)', which matches both an ancestor and the directory itself. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Owner
|
Thanks for fixing this. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes part B of #137 — the excluded-directory rule fires on an ancestor that the same spec re-includes.
dir_includeis resolved among patterns that produce a_DIR_MARKmatch, while whether an ancestor ends up excluded is decided by every pattern that matches it.!**/node_modules/**compiles without a dir mark, so it never displaces.*from the directory bucket even though it re-includes the very ancestor.*excluded.Two changes per backend: the directory bucket only accepts matches on a strict ancestor, and the rule only fires once
_ancestor_excluded()confirms that ancestor is excluded by the whole spec — asked outermost first, the order git stops descending in.One thing worth knowing before you read the diff. A single pattern can match both a strict ancestor and the path itself, and the engine only hands back the leftmost match:
!*/againstsub/d/returnssub/, so classifying on that one match makes!*/an ancestor exclusion and never the directory it re-includes — which breakstest_02_dir_reinclusion_whitelist. So the simple backend asks for every separator. In re2/hyperscan the same split is{base}/?$rather than a third expression per pattern, which costs ~9x.Measured on
f0fb3f4,GitIgnoreSpec, all three backends, git 2.55.0 as the oracle:tests/Sweep and per-check columns are simple / re2 / hyperscan. The sweep is 96 two-pattern specs over 13 queries, each verdict taken from a real repository —
check-ignore --stdinfor files, a canary probe for directories, sincecheck-ignore d/answers itself. Bench and corpus: https://github.com/KaizenShogun/gitignore-conformance — happy to run any variant you'd rather have through it.The new test's verdicts were taken two ways that agree on every row:
check-ignore -vand whatgit add -Aactually stages.@youdie006 reproduced the bug independently on git 2.43.0 (#137 comment) with the same verdicts I get on 2.55.0, so none of this is version skew.
— Midas