A negation is inherited by the contents of the directory it re-includes #26

Activity

1. andrew

   added

   bugSomething isn't workingSomething isn't working

   on Sep 10, 2026
2. KaizenShogun commented on Sep 10, 2026

   

   KaizenShogun

   on Sep 10, 2026

   ContributorAuthor

   More actions

   Correction to my own report: I said the 8 divergences were all from supabase's docker/.gitignore. Half of them aren't — 4 come from apps/docs/.gitignore, where the pattern has a different shape. So this doesn't need the X/** form:

   .gitignore: **/*/generated/**/*
               !**/*/generated/keepdir
   

   with sample/generated/keepdir/ an actual directory:

   path	git 2.55.0	this library (main, both merges in)
   sample/generated/keepdir	not ignored	not ignored
   sample/generated/keepdir/	ignored	not ignored
   sample/generated/keepdir/file	ignored	not ignored

   git check-ignore -v blames line 1 for the last two, and git add -A stages nothing under it. In supabase the re-included name is .gitkeep; the behaviour is the same with a plain directory name, so it isn't about the dot or the extension. Count of 8 stands, the attribution didn't.

   Separately, now that #22 and #24 are in: on the same 8,953 nested-.gitignore queries, main goes from 27 divergences to 12, and the 12 are a subset of the 27 — nothing regressed. Those 12 are exactly this issue (8) and #25 (4).
3. KaizenShogun commented on Sep 10, 2026

   

   KaizenShogun

   on Sep 10, 2026

   Last edited by KaizenShogun

   ContributorAuthor

   More actions

   Cause: appendTrailingDoubleStar (gitignore.go:594) appends an implicit ** to every pattern that
   isn't dirOnly, so !gen/keepdir compiles as gen/keepdir/** and matches gen/keepdir/f.txt.
   Not the descendant loop at :429, and not the literalSuffix fast-reject, which never fires here
   because extractLiteralSuffix returns "" when the last segment is **.

   Repro, no repo needed:

   gen/**/*
   !gen/keepdir
   

   with gen/keepdir/ a real directory:

   path	git 2.55.0	HEAD
   gen/keepdir/	not ignored	not ignored
   gen/keepdir/f.txt	ignored (gen/**/*)	not ignored

   MatchDetail blames !gen/keepdir for the second row. Same with !gen/keepdir/.

   Skipping the implicit ** for negations is three lines and fixes 10, breaks 66 (12 -> 68
   divergences over 8,953 queries). Removing the implicit ** outright breaks 1,828.

   What breaks, from angular's .gitignore:

   .vscode/*
   !.vscode/settings.json
   

   With settings.json as a directory, git says .vscode/settings.json/keep is not ignored:
   .vscode/* is one segment deep. The library gets that right today because of the implicit ** on
   the negation, which stands in for the ancestor walk the matcher doesn't do. Take it away and
   .vscode/*'s own implicit ** matches the file.

   So match() decides per pattern and git decides per directory, descending. The two cases that need
   telling apart are "ancestor re-included, nothing inside matches" (not ignored) and "ancestor
   re-included, something inside matches an exclusion" (ignored), and without walking the path's
   ancestors they look identical.

   That's a design call, so no PR. Say the word and I'll measure the ancestor walk.

   Bench and corpus: https://github.com/KaizenShogun/gitignore-conformance (33 repos' rule files,
   git check-ignore -v --no-index as the oracle).
4. KaizenShogun commented on Sep 11, 2026

   

   KaizenShogun

   on Sep 11, 2026

   Last edited by KaizenShogun

   ContributorAuthor

   More actions

   Correction to my last comment. Dropping the implicit ** from negations isn't the fix, but not
   because the ** is right: it stands in for an ancestor walk the matcher doesn't do. Add the walk
   and the ** can go entirely.

   match (gitignore.go:353) asks one question per path. git asks about every ancestor first, since it
   never descends into an excluded directory (#25). The implicit ** fakes the descent well
   enough for exclusions, which is why removing it alone breaks 66 cases.

   Measured on top of main, which is at 12 divergences / 8,953 queries after #22 and #24:

   build	divergences	vs main
   ancestor walk only	5	fixes 8, breaks 1
   ancestor walk + no ** on negations	67	fixes 12, breaks 67
   ancestor walk + no implicit ** at all	3	fixes 12, breaks 3

   Row 2 is the patch I proposed last time, and it's the worst of the three: exclusions keep the extra
   reach, negations lose it.

   The walk:

   func (m *Matcher) match(relPath string, isDir bool) bool {
   	pathSegs := strings.Split(relPath, "/")
   	for i := 1; i < len(pathSegs); i++ {
   		if m.matchSegs(pathSegs[:i], true) {
   			return true
   		}
   	}
   	return m.matchSegs(pathSegs, isDir)
   }

   with the old body moved into matchSegs(pathSegs []string, isDir bool) and
   appendTrailingDoubleStar returning segs unchanged.

   It fails three tests in your suite. I ran all three against git check-ignore 2.55.0 rather than
   assume they were wrong, and all three assert the opposite of git:

   test	rules	path	test expects	git 2.55.0
   TestMatchCannotReincludeUnderExcludedParent	dir/ + !dir/important.txt	dir/important.txt	not ignored	ignored, by dir/
   TestMatchNegationSubdirectoryFilter	abc + !abc/b	abc/b/b.js	not ignored	ignored, by abc
   TestMatchDoubleStarSlash	**/	a/b	not ignored	ignored, by **/

   The first quotes gitignore(5), "It is not possible to re-include a file if a parent directory of
   that file is excluded", and then asserts the library re-includes anyway, with a comment that
   callers SkipDir on excluded directories so they never ask. True for a walker, which is why the gap
   is invisible from Walk, but not for Match/MatchPath as public API: go-git is looking at
   delegating to this library (go-git/go-git#877), and 7 of its Status() divergences come from these
   two mechanisms, 6 from #26 and 1 from #25. The other two tests are inherited from
   sabhiram/go-gitignore.

   The 3 remaining divergences are unrelated to either issue: two are /node_modules in a nested
   .gitignore under a directory git prunes differently, one is a !volumes/functions/deno.json*
   negation whose sibling deno.jsonsample I still owe you a look at.

   No PR, since it changes three of your tests and that's your call. If you want it, tell me which
   shape and I'll justify each test change against check-ignore.

   Bench: https://github.com/KaizenShogun/gitignore-conformance (33 repos' rule files, 8,953 questions,
   git 2.55.0 as the oracle).
5. andrew commented on Sep 11, 2026

   

   andrew

   on Sep 11, 2026

   Contributor

   More actions

   Again it would be great to rewrite these to reduce the claudisms, would make it much easier to follow
6. KaizenShogun commented on Sep 12, 2026

   

   KaizenShogun

   on Sep 12, 2026

   ContributorAuthor

   More actions

   Rewritten in place, both of the long ones. Same numbers, same code, less prose around them. If it's
   still heavy, say so and I'll keep cutting.
7. andrew

   added a commit that references this issue on Sep 12, 2026

   Check parent directories in Match and add git check-ignore conformanc…

   ...

   This issue will close once commit 66f08d2 is merged into the 'main' branch.

   66f08d2
8. andrew commented on Sep 12, 2026

   

   andrew

   on Sep 12, 2026

   Contributor

   More actions

   Thanks for the analysis and the bench numbers. Went with the parent-directory check and dropped the implicit trailing ** entirely, plus the /** -> dir-only rewrite and the descendant loop, since foo/** matching foo/ itself was a separate bug that the rewrite caused. #27 has that along with a git check-ignore conformance suite and a fuzz test; the three tests you flagged are corrected there. Would be useful to run your 8,953-query bench against that branch if you have a moment.
9. andrew

   closed this as completedin 30f606bon Sep 12, 2026