Skip to content

Stop the excluded directory rule from firing on a re-included ancestor - #144

Open
KaizenShogun wants to merge 1 commit into
cpburnz:masterfrom
KaizenShogun:fix/137b-reincluded-ancestor
Open

KaizenShogun wants to merge 1 commit into
cpburnz:masterfrom
KaizenShogun:fix/137b-reincluded-ancestor

Conversation

@KaizenShogun

@KaizenShogun KaizenShogun commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Fixes part B of #137 — the excluded-directory rule fires on an ancestor that the same spec re-includes.

GitIgnoreSpec.from_lines(['.*', '!**/node_modules/**']).match_file(
    'vendor/deps/npm/node_modules/.bin/x.txt')   # True; git stages the file

dir_include is resolved among patterns that produce a _DIR_MARK match, 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: !*/ against sub/d/ returns sub/, so classifying on that one match makes !*/ an ancestor exclusion and never the directory it re-includes — which breaks test_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:

base this branch
tests/ 215 pass 216 pass
corpus, 9,852 cases / 43 repos 1 0
targeted sweep, 1,248 cases 49 / 25 / 25 43 / 18 / 18 — 6/7/7 fixed, 0 new
µs per check 23.9 / 2.1 / 1.7 31.0 / 2.8 / 2.5

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 --stdin for files, a canary probe for directories, since check-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 -v and what git add -A actually 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

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
KaizenShogun force-pushed the fix/137b-reincluded-ancestor branch from 98e0378 to c842c94 Compare September 14, 2026 18:18
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.

1 participant