From 43a7d31ef8d2e928b22fcbe812f2bdddd9edc6db Mon Sep 17 00:00:00 2001 From: Andrew Nesbitt Date: Sat, 12 Sep 2026 19:26:28 -0400 Subject: [PATCH 1/3] Reduce matching allocations and skip unrelated ignore rules --- README.md | 14 ++- conformance_test.go | 44 +++++++ gitignore.go | 165 ++++++++++++++++++-------- gitignore_bench_test.go | 24 ++++ performance_test.go | 250 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 444 insertions(+), 53 deletions(-) create mode 100644 performance_test.go diff --git a/README.md b/README.md index 9502504..8af0b2f 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@ A Go library for matching paths against gitignore rules. Pattern matching uses a - Directory-only patterns (trailing `/`) with descendant matching - Match provenance via `MatchDetail` (which pattern, file, and line number matched) - Invalid pattern surfacing via `Errors()` -- Literal suffix fast-reject for common patterns like `*.log` +- Fast rejection for literal names and suffix patterns like `*.log` ```go import "github.com/git-pkgs/gitignore" @@ -111,7 +111,17 @@ A Matcher is safe for concurrent `Match`/`MatchPath`/`MatchDetail` calls once co ## Match semantics -Paths should use forward slashes and be relative to the repository root. Last-match-wins, same as git. +Paths should use forward slashes and be relative to the repository root. An ignored parent directory excludes all its descendants; otherwise, the last matching rule for the path wins. + +## Benchmarks + +Run benchmarks on an otherwise idle machine, using the same Go toolchain for both revisions. Record repeated samples and memory allocations: + +```sh +go test -run '^$' -bench . -benchmem -count=10 -cpu=1 +``` + +`BenchmarkCompile` includes filesystem reads and a Git subprocess through `New`; `BenchmarkAddPatterns` measures parsing without filesystem access. Compare samples with `benchstat` and check the timing spread before quoting speed changes. ## License diff --git a/conformance_test.go b/conformance_test.go index dc2e79b..7581680 100644 --- a/conformance_test.go +++ b/conformance_test.go @@ -148,6 +148,50 @@ func TestConformanceGitStartupFailure(t *testing.T) { } } +func TestConformanceNestedWalk(t *testing.T) { + requireGit(t) + isolateGitEnv(t) + paths := parsePathList("a/cache/x\na/keep.log\na/drop.log\na/sub/keep.log\na/sub/drop.log\nb/drop.log\nb/keep.log\nb/cache/x\nblocked/sub/file\n") + root := buildRepo(t, "*.log\nblocked/\n", paths) + for dir, patterns := range map[string]string{ + "a": "!keep.log\ncache/\n", "a/sub": "!drop.log\n", + "b": "!drop.log\n", "blocked/sub": "!file\n", + } { + if err := os.WriteFile(filepath.Join(root, dir, ".gitignore"), []byte(patterns), 0o644); err != nil { + t.Fatal(err) + } + } + want := gitCheckIgnore(t, root, paths) + m := gitignore.NewFromDirectory(root) + for _, p := range paths { + if got := m.Match(p.query()); got != want[p.rel] { + t.Errorf("NewFromDirectory Match(%q) = %v, git = %v", p.rel, got, want[p.rel]) + } + } + for _, start := range []string{"", "a", "a/sub", "b", "blocked/sub"} { + visited := make(map[string]bool) + visit := func(path string, _ os.DirEntry) error { + visited[filepath.ToSlash(path)] = true + return nil + } + var err error + if start == "" { + err = gitignore.Walk(root, visit) + } else { + err = gitignore.WalkFrom(root, start, visit) + } + if err != nil { + t.Fatal(err) + } + for _, p := range paths { + inScope := start == "" || strings.HasPrefix(p.rel, start+"/") + if expected := inScope && !want[p.rel]; visited[p.rel] != expected { + t.Errorf("walk from %q: visited %q = %v, want %v", start, p.rel, visited[p.rel], expected) + } + } + } +} + // TestConformanceFuzz generates random pattern sets and paths, then compares // the library against git check-ignore. It is skipped under -short. func TestConformanceFuzz(t *testing.T) { diff --git a/gitignore.go b/gitignore.go index c5e24b7..78cff7c 100644 --- a/gitignore.go +++ b/gitignore.go @@ -6,6 +6,7 @@ import ( "os" "os/exec" "path/filepath" + "slices" "strings" ) @@ -19,12 +20,12 @@ type pattern struct { negate bool dirOnly bool // trailing slash pattern tailDoubleStar bool // pattern ends in "/**" + baseOnly bool // ** followed by one concrete segment anchored bool - prefix []string // directory scope segments for nested .gitignore - text string // original pattern text before compilation - source string // file path this pattern came from, empty for programmatic - line int // 1-based line number in source file - literalSuffix string // fast-reject: last segment must end with this (e.g. ".log" from "*.log") + text string // original pattern text before compilation + source string // file path this pattern came from, empty for programmatic + line int // 1-based line number in source file + literalSuffix string // fast-reject: last segment must end with this (e.g. ".log" from "*.log") } // Matcher checks paths against gitignore rules collected from .gitignore files, @@ -42,9 +43,15 @@ type pattern struct { type Matcher struct { maxIgnoreFileSize int64 patterns []pattern + groups []patternGroup errors []PatternError } +type patternGroup struct { + prefix []string + start, end int +} + // PatternError records a pattern compilation error or a skipped oversized file. type PatternError struct { Pattern string // the original pattern text @@ -182,7 +189,7 @@ func expandTilde(path string) string { // in Errors. func NewFromDirectory(root string, opts ...Option) *Matcher { m := New(root, opts...) - _ = walkRecursive(root, "", m, nil, false) + _ = walkRecursive(root, "", m, nil, false, true, false) return m } @@ -201,7 +208,7 @@ func Walk(root string, fn func(path string, d fs.DirEntry) error, opts ...Option if err != nil { return err } - return walkRecursive(root, "", m, fn, true) + return walkRecursive(root, "", m, fn, true, false, false) } // WalkFrom walks the directory tree starting at a subdirectory of root, @@ -262,10 +269,23 @@ func WalkFrom(root, start string, fn func(path string, d fs.DirEntry) error, opt } } - return walkRecursive(root, start, m, fn, true) + return walkRecursive(root, start, m, fn, true, false, true) } -func walkRecursive(root, rel string, m *Matcher, fn func(string, fs.DirEntry) error, stopOnSizeError bool) error { +func walkRecursive(root, rel string, m *Matcher, fn func(string, fs.DirEntry) error, stopOnSizeError, retainPatterns, checkParents bool) error { + patternCount := len(m.patterns) + groupCount := len(m.groups) + if !retainPatterns { + defer func() { + clear(m.patterns[patternCount:]) + m.patterns = m.patterns[:patternCount] + clear(m.groups[groupCount:]) + m.groups = m.groups[:groupCount] + if groupCount > 0 { + m.groups[groupCount-1].end = patternCount + } + }() + } dir := root if rel != "" { dir = filepath.Join(root, rel) @@ -296,7 +316,7 @@ func walkRecursive(root, rel string, m *Matcher, fn func(string, fs.DirEntry) er entryRel = filepath.Join(rel, name) } - if m.MatchPath(filepath.ToSlash(entryRel), entry.IsDir()) { + if m.match(filepath.ToSlash(entryRel), entry.IsDir(), checkParents) { continue } @@ -307,7 +327,7 @@ func walkRecursive(root, rel string, m *Matcher, fn func(string, fs.DirEntry) er } if entry.IsDir() { - if err := walkRecursive(root, entryRel, m, fn, stopOnSizeError); err != nil { + if err := walkRecursive(root, entryRel, m, fn, stopOnSizeError, retainPatterns, false); err != nil { return err } } @@ -345,14 +365,14 @@ func (m *Matcher) addFromFile(absPath, relDir string) error { // Match returns true if the given path should be ignored. // The path should be slash-separated and relative to the repository root. // For directories, append a trailing slash (e.g. "vendor/"). -// Uses last-match-wins semantics: iterates patterns in reverse and returns -// on the first match. +// An ignored parent directory makes its descendants ignored. Otherwise, +// the last matching rule determines whether the path is ignored. func (m *Matcher) Match(relPath string) bool { isDir := strings.HasSuffix(relPath, "/") if isDir { relPath = relPath[:len(relPath)-1] } - return m.match(relPath, isDir) + return m.match(relPath, isDir, true) } // MatchPath returns true if the given path should be ignored. @@ -360,7 +380,7 @@ func (m *Matcher) Match(relPath string) bool { // a trailing slash convention. The path should be slash-separated, // relative to the repository root, and should not have a trailing slash. func (m *Matcher) MatchPath(relPath string, isDir bool) bool { - return m.match(relPath, isDir) + return m.match(relPath, isDir, true) } // MatchResult describes which pattern matched a path and whether @@ -388,13 +408,18 @@ func (m *Matcher) MatchDetail(relPath string) MatchResult { // match reports whether relPath is ignored. Git decides ignore status // while walking the tree and does not enter an excluded directory, so a // path is ignored if any of its parent directories is. That is checked -// here by testing each proper prefix of the path as a directory before -// testing the full path. -func (m *Matcher) match(relPath string, isDir bool) bool { - pathSegs := strings.Split(relPath, "/") - for end := 1; end < len(pathSegs); end++ { - if idx := m.findMatch(pathSegs[:end], true); idx >= 0 && !m.patterns[idx].negate { - return true +// here unless the directory walk has already checked the parents. +func (m *Matcher) match(relPath string, isDir, checkParents bool) bool { + if len(m.patterns) == 0 { + return false + } + var buf [pathBufferSize]string + pathSegs := splitPath(relPath, buf[:0]) + if checkParents { + for end := 1; end < len(pathSegs); end++ { + if idx := m.findMatch(pathSegs[:end], true); idx >= 0 && !m.patterns[idx].negate { + return true + } } } idx := m.findMatch(pathSegs, isDir) @@ -402,7 +427,11 @@ func (m *Matcher) match(relPath string, isDir bool) bool { } func (m *Matcher) matchDetail(relPath string, isDir bool) MatchResult { - pathSegs := strings.Split(relPath, "/") + if len(m.patterns) == 0 { + return MatchResult{} + } + var buf [pathBufferSize]string + pathSegs := splitPath(relPath, buf[:0]) for end := 1; end < len(pathSegs); end++ { if idx := m.findMatch(pathSegs[:end], true); idx >= 0 && !m.patterns[idx].negate { return m.resultFor(idx) @@ -414,17 +443,43 @@ func (m *Matcher) matchDetail(relPath string, isDir bool) MatchResult { return MatchResult{} } +const pathBufferSize = 16 + +func splitPath(path string, segments []string) []string { + for part := range strings.SplitSeq(path, "/") { + if len(segments) == cap(segments) { + return strings.Split(path, "/") + } + segments = append(segments, part) + } + return segments +} + // findMatch returns the index of the last pattern that matches pathSegs, // or -1 if none does. Patterns are scanned from last to first because // gitignore uses last-match-wins ordering. func (m *Matcher) findMatch(pathSegs []string, isDir bool) int { - lastSeg := pathSegs[len(pathSegs)-1] - for i := len(m.patterns) - 1; i >= 0; i-- { - p := &m.patterns[i] + for g := len(m.groups) - 1; g >= 0; g-- { + group := &m.groups[g] + if !matchScope(pathSegs, group.prefix) { + continue + } + segs := pathSegs[len(group.prefix):] + if idx := findPattern(m.patterns[group.start:group.end], segs, isDir); idx >= 0 { + return group.start + idx + } + } + return -1 +} + +func findPattern(patterns []pattern, segs []string, isDir bool) int { + lastSeg := segs[len(segs)-1] + for i := len(patterns) - 1; i >= 0; i-- { + p := &patterns[i] if p.literalSuffix != "" && !strings.HasSuffix(lastSeg, p.literalSuffix) { continue } - if matchPattern(p, pathSegs, isDir) { + if matchPattern(p, segs, isDir) { return i } } @@ -443,30 +498,35 @@ func (m *Matcher) resultFor(idx int) MatchResult { } } -// matchPattern checks whether pathSegs matches the compiled pattern, -// including the directory prefix scope and dirOnly handling. -func matchPattern(p *pattern, pathSegs []string, isDir bool) bool { - segs := pathSegs - if n := len(p.prefix); n > 0 { - // Rules from a nested .gitignore apply only to entries strictly - // inside that directory, never to the directory itself. - if len(segs) <= n { +// Nested rules cannot match their containing directory. +func matchScope(segs, prefix []string) bool { + if len(segs) <= len(prefix) { + return false + } + for i, part := range prefix { + if segs[i] != part { return false } - for i, ps := range p.prefix { - if segs[i] != ps { - return false - } - } - segs = segs[n:] } + return true +} + +func matchPattern(p *pattern, segs []string, isDir bool) bool { if p.dirOnly && !isDir { return false } + if p.baseOnly { + return matchSegment(p.segments[len(p.segments)-1].raw, segs[len(segs)-1]) + } return matchSegments(p.segments, segs, p.tailDoubleStar) } func (m *Matcher) addPatterns(data []byte, dir, source string) { + start := len(m.patterns) + var prefix []string + if dir != "" { + prefix = strings.Split(dir, "/") + } lineNum := 0 for len(data) > 0 { var raw []byte @@ -477,7 +537,7 @@ func (m *Matcher) addPatterns(data []byte, dir, source string) { if line == "" || line[0] == '#' { continue } - p, errMsg := compilePattern(line, dir) + p, errMsg := compilePattern(line) if errMsg != "" { m.errors = append(m.errors, PatternError{ Pattern: line, @@ -492,6 +552,13 @@ func (m *Matcher) addPatterns(data []byte, dir, source string) { p.line = lineNum m.patterns = append(m.patterns, p) } + if len(m.patterns) > start { + if n := len(m.groups); n > 0 && slices.Equal(m.groups[n-1].prefix, prefix) { + m.groups[n-1].end = len(m.patterns) + } else { + m.groups = append(m.groups, patternGroup{prefix: prefix, start: start, end: len(m.patterns)}) + } + } } // trimTrailingSpaces removes unescaped trailing spaces per gitignore spec. @@ -512,11 +579,8 @@ func trimTrailingSpaces(s string) string { // compilePattern compiles a gitignore pattern line into a pattern struct. // Returns the compiled pattern and an empty string on success, or a zero // pattern and an error message on failure. -func compilePattern(line, dir string) (pattern, string) { +func compilePattern(line string) (pattern, string) { var p pattern - if dir != "" { - p.prefix = strings.Split(dir, "/") - } // Handle negation if strings.HasPrefix(line, "!") { @@ -565,6 +629,8 @@ func compilePattern(line, dir string) (pattern, string) { } p.segments = segs + const basePatternSegments = 2 + p.baseOnly = len(segs) == basePatternSegments && segs[0].doubleStar && !segs[1].doubleStar p.literalSuffix = extractLiteralSuffix(segs) return p, "" } @@ -629,8 +695,8 @@ func validateSegmentBrackets(segs []segment) string { // extractLiteralSuffix finds the literal trailing portion of the last concrete // segment, for fast rejection. For example, "*.log" yields ".log", "test_*.go" -// yields ".go". Only extracts a suffix when the segment is a simple star-prefix -// glob with no brackets, escapes, or question marks in the suffix portion. +// yields ".go". Literal segments use the entire name. Suffixes containing +// brackets, escapes, or question marks are excluded. // // The suffix is only extracted when the last segment is concrete (not **), // because the fast-reject check compares against the final path segment. @@ -649,9 +715,6 @@ func extractLiteralSuffix(segs []segment) string { // Find the last * in the segment. Everything after it must be literal. starIdx := strings.LastIndex(last, "*") - if starIdx < 0 { - return "" - } suffix := last[starIdx+1:] if suffix == "" { return "" diff --git a/gitignore_bench_test.go b/gitignore_bench_test.go index 10c1fa4..fd76d18 100644 --- a/gitignore_bench_test.go +++ b/gitignore_bench_test.go @@ -112,6 +112,30 @@ func BenchmarkMatchDeepPath(b *testing.B) { } } +func BenchmarkMatchLiteral(b *testing.B) { + for _, tc := range []struct { + name string + path string + want bool + }{ + {"RootHit", "vendor/", true}, + {"DeepHit", "a/b/c/d/e/f/g/vendor/", true}, + {"DeepMiss", "a/b/c/d/e/f/g/source/", false}, + {"ExcludedParent", "vendor/a/b/c/d/e/f/g/source.go", true}, + {"SuffixCollision", "a/b/c/d/e/f/g/myvendor/", false}, + } { + b.Run(tc.name, func(b *testing.B) { + m := benchMatcher(b, realisticPatterns()) + if got := m.Match(tc.path); got != tc.want { + b.Fatalf("Match(%q) = %v, want %v", tc.path, got, tc.want) + } + for b.Loop() { + m.Match(tc.path) + } + }) + } +} + func BenchmarkMatchNestedPatterns(b *testing.B) { m := benchMatcher(b, realisticPatterns()) for _, dir := range []string{"src", "src/pkg", "src/pkg/internal", "src/pkg/internal/util"} { diff --git a/performance_test.go b/performance_test.go new file mode 100644 index 0000000..4bdae91 --- /dev/null +++ b/performance_test.go @@ -0,0 +1,250 @@ +package gitignore_test + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/git-pkgs/gitignore" +) + +func TestMatchPathDepth(t *testing.T) { + for _, depth := range []int{1, 16, 17, 64} { + t.Run(fmt.Sprint(depth), func(t *testing.T) { + m := gitignore.New("") + m.AddPatterns([]byte("*.log\n!keep.log\nbuild/\n"), "") + for _, tc := range []struct { + name string + want bool + }{ + {"app.log", true}, {"keep.log", false}, {"main.go", false}, + {"build/file.go", true}, {"build/", true}, + } { + path := strings.Repeat("d/", depth-1) + tc.name + if got := m.Match(path); got != tc.want { + t.Errorf("Match(%q) = %v, want %v", path, got, tc.want) + } + if got := m.MatchPath(strings.TrimSuffix(path, "/"), strings.HasSuffix(path, "/")); got != tc.want { + t.Errorf("MatchPath(%q) = %v, want %v", path, got, tc.want) + } + if got := m.MatchDetail(path).Ignored; got != tc.want { + t.Errorf("MatchDetail(%q) = %v, want %v", path, got, tc.want) + } + } + }) + } +} + +func TestMatchConcurrent(t *testing.T) { + m := gitignore.New("") + m.AddPatterns([]byte("*.log\n!keep.log\nbuild/\n"), "") + m.AddPatterns([]byte("!trace.log\n"), "src") + for _, tc := range []struct { + path string + want bool + }{ + {"src/app.log", true}, {"src/keep.log", false}, {"src/trace.log", false}, + {"docs/trace.log", true}, {"build/file.go", true}, + {strings.Repeat("d/", 64) + "app.log", true}, + } { + t.Run(tc.path, func(t *testing.T) { + t.Parallel() + for range 100 { + if m.Match(tc.path) != tc.want || m.MatchPath(tc.path, false) != tc.want || m.MatchDetail(tc.path).Ignored != tc.want { + t.Fatalf("inconsistent match for %q, want %v", tc.path, tc.want) + } + } + }) + } +} + +func BenchmarkAddPatterns(b *testing.B) { + for _, count := range []int{10, 100, 1000} { + for _, scope := range []string{"", "src/pkg/internal/generated"} { + b.Run(fmt.Sprintf("Rules%d/Scope%t", count, scope != ""), func(b *testing.B) { + rules := []string{"*.log\n", "/cache/\n", "!keep.log\n"} + var text strings.Builder + for i := range count { + text.WriteString(rules[i%len(rules)]) + } + data := []byte(text.String()) + for b.Loop() { + m := gitignore.New("") + m.AddPatterns(data, scope) + } + }) + } + } +} + +func BenchmarkMatchDepth(b *testing.B) { + for _, depth := range []int{1, 4, 16, 64} { + for _, empty := range []bool{false, true} { + b.Run(fmt.Sprintf("Depth%d/Empty%t", depth, empty), func(b *testing.B) { + m := gitignore.New("") + if !empty { + m.AddPatterns([]byte(realisticPatterns()), "") + } + path := strings.Repeat("src/", depth-1) + "main.go" + for b.Loop() { + m.Match(path) + } + }) + } + } +} + +func BenchmarkMatchScopes(b *testing.B) { + for _, count := range []int{10, 100, 1000} { + b.Run(fmt.Sprintf("Siblings%d", count), func(b *testing.B) { + m := gitignore.New("") + for i := range count { + m.AddPatterns([]byte("*.log\ncache/\n!keep.log\n"), fmt.Sprintf("pkg%d", i)) + } + for b.Loop() { + m.Match("pkg0/src/keep.log") + } + }) + } +} + +func BenchmarkMatchPatternOrder(b *testing.B) { + for _, count := range []int{10, 100, 1000} { + for _, position := range []string{"First", "Last", "Miss"} { + b.Run(fmt.Sprintf("Rules%d/%s", count, position), func(b *testing.B) { + m := gitignore.New("") + for i := range count { + m.AddPatterns([]byte(fmt.Sprintf("pattern_%d_*.log", i)), "") + } + index := 0 + switch position { + case "Last": + index = count - 1 + case "Miss": + index = count + } + path := fmt.Sprintf("src/pattern_%d_file.log", index) + if got := m.Match(path); got != (position != "Miss") { + b.Fatalf("unexpected result for %q: %v", path, got) + } + for b.Loop() { + m.Match(path) + } + }) + } + } +} + +func BenchmarkMatchShapes(b *testing.B) { + for _, tc := range []struct{ name, pattern, path string }{ + {"Literal", "target", "src/target"}, + {"Suffix", "*.log", "src/application.log"}, + {"Prefix", "generated*", "src/generated_file.go"}, + {"Stars", "a*b*c", "src/aaaaabbbbbc"}, + {"Brackets", "[a-z][a-z][0-9].log", "src/ab5.log"}, + {"POSIX", "[[:alpha:]][[:digit:]].log", "src/a5.log"}, + {"DoubleStars", "**/a/**/b/**/target", "x/a/y/a/z/b/q/target"}, + {"NearMiss", "a*a*a*a*b", "src/aaaaaaaaaaaaaaaaaaaaac"}, + } { + b.Run(tc.name, func(b *testing.B) { + m := gitignore.New("") + m.AddPatterns([]byte(tc.pattern), "") + if got := m.Match(tc.path); got != (tc.name != "NearMiss") { + b.Fatalf("unexpected result for %q: %v", tc.path, got) + } + for b.Loop() { + m.Match(tc.path) + } + }) + } +} + +func BenchmarkMatchMixedParallel(b *testing.B) { + m := gitignore.New("") + m.AddPatterns([]byte(realisticPatterns()), "") + paths := []string{"src/main.go", "vendor/pkg/file.go", "app.log", "important.log", "src/a/b/c/file.go"} + b.ResetTimer() + b.RunParallel(func(pb *testing.PB) { + i := 0 + for pb.Next() { + m.Match(paths[i]) + i = (i + 1) % len(paths) + } + }) +} + +func BenchmarkWalkTree(b *testing.B) { + for _, tc := range []struct { + name string + width int + depth int + ignored bool + }{ + {"Wide", 100, 1, false}, + {"Deep", 1, 32, false}, + {"Pruned", 100, 1, true}, + } { + b.Run(tc.name, func(b *testing.B) { + b.Setenv("GIT_CONFIG_GLOBAL", os.DevNull) + root := benchTree(b, tc.width, tc.depth, tc.ignored) + for b.Loop() { + if err := gitignore.Walk(root, func(_ string, _ os.DirEntry) error { return nil }); err != nil { + b.Fatal(err) + } + } + }) + } +} + +func benchTree(b *testing.B, width, depth int, ignored bool) string { + b.Helper() + root := b.TempDir() + for i := range width { + dir := filepath.Join(root, fmt.Sprintf("pkg%d", i)) + for range depth { + dir = filepath.Join(dir, "src") + if err := os.MkdirAll(dir, 0o755); err != nil { + b.Fatal(err) + } + for name, data := range map[string]string{ + ".gitignore": "*.log\ncache/\n!keep.log\n", + "main.go": "", "debug.log": "", "keep.log": "", + } { + if err := os.WriteFile(filepath.Join(dir, name), []byte(data), 0o644); err != nil { + b.Fatal(err) + } + } + } + } + if ignored { + if err := os.WriteFile(filepath.Join(root, ".gitignore"), []byte("pkg*\n!pkg0\n"), 0o644); err != nil { + b.Fatal(err) + } + } + return root +} + +func TestMatchInterleavedScopes(t *testing.T) { + m := gitignore.New("") + for _, step := range []struct { + pattern, scope string + ignored bool + }{ + {"*.log", "", true}, + {"!keep.log", "src", false}, + {"keep.log", "other", false}, + {"src/keep.log", "", true}, + {"!keep.log", "src", false}, + {"[broken", "src", false}, + } { + m.AddPatterns([]byte(step.pattern), step.scope) + if got := m.Match("src/keep.log"); got != step.ignored { + t.Errorf("after %q in %q: ignored = %v, want %v", step.pattern, step.scope, got, step.ignored) + } + if got := m.MatchDetail("src/keep.log").Ignored; got != step.ignored { + t.Errorf("after %q in %q: detail ignored = %v, want %v", step.pattern, step.scope, got, step.ignored) + } + } +} From c7b6e43b89d24be976265bb2aef901f5948928e5 Mon Sep 17 00:00:00 2001 From: Andrew Nesbitt Date: Sat, 12 Sep 2026 19:45:32 -0400 Subject: [PATCH 2/3] Simplify traversal state and share bracket syntax helpers --- gitignore.go | 134 +++++++++++++++++++++-------------------------- refactor_test.go | 79 ++++++++++++++++++++++++++++ wildmatch.go | 46 ++++++++-------- 3 files changed, 161 insertions(+), 98 deletions(-) create mode 100644 refactor_test.go diff --git a/gitignore.go b/gitignore.go index 78cff7c..e52b8aa 100644 --- a/gitignore.go +++ b/gitignore.go @@ -18,10 +18,9 @@ type segment struct { type pattern struct { segments []segment negate bool - dirOnly bool // trailing slash pattern - tailDoubleStar bool // pattern ends in "/**" - baseOnly bool // ** followed by one concrete segment - anchored bool + dirOnly bool // trailing slash pattern + tailDoubleStar bool // pattern ends in "/**" + baseOnly bool // ** followed by one concrete segment text string // original pattern text before compilation source string // file path this pattern came from, empty for programmatic line int // 1-based line number in source file @@ -188,9 +187,9 @@ func expandTilde(path string) string { // skipped. Oversized files under MaxIgnoreFileSize are skipped and recorded // in Errors. func NewFromDirectory(root string, opts ...Option) *Matcher { - m := New(root, opts...) - _ = walkRecursive(root, "", m, nil, false, true, false) - return m + w := walker{root: root, matcher: New(root, opts...), retainPatterns: true} + _ = w.walk("") + return w.matcher } // Walk walks the directory tree rooted at root, calling fn for each file @@ -208,7 +207,8 @@ func Walk(root string, fn func(path string, d fs.DirEntry) error, opts ...Option if err != nil { return err } - return walkRecursive(root, "", m, fn, true, false, false) + w := walker{root: root, matcher: m, fn: fn, stopOnSizeError: true} + return w.walk("") } // WalkFrom walks the directory tree starting at a subdirectory of root, @@ -241,7 +241,7 @@ func WalkFrom(root, start string, fn func(path string, d fs.DirEntry) error, opt } // Load .gitignore from each ancestor directory between root and start - // (exclusive of start itself, which walkRecursive loads). + // (exclusive of start itself, which the walker loads). { slashed := filepath.ToSlash(start) for off := 0; ; { @@ -263,37 +263,40 @@ func WalkFrom(root, start string, fn func(path string, d fs.DirEntry) error, opt return err } - if fn != nil { - if err := fn(start, fs.FileInfoToDirEntry(info)); err != nil { - return err - } + w := walker{root: root, start: start, matcher: m, fn: fn, stopOnSizeError: true} + if err := w.visit(start, fs.FileInfoToDirEntry(info)); err != nil { + return err } - return walkRecursive(root, start, m, fn, true, false, true) + return w.walk(start) } -func walkRecursive(root, rel string, m *Matcher, fn func(string, fs.DirEntry) error, stopOnSizeError, retainPatterns, checkParents bool) error { - patternCount := len(m.patterns) - groupCount := len(m.groups) - if !retainPatterns { - defer func() { - clear(m.patterns[patternCount:]) - m.patterns = m.patterns[:patternCount] - clear(m.groups[groupCount:]) - m.groups = m.groups[:groupCount] - if groupCount > 0 { - m.groups[groupCount-1].end = patternCount - } - }() +type walker struct { + root, start string + matcher *Matcher + fn func(string, fs.DirEntry) error + retainPatterns bool + stopOnSizeError bool +} + +func (w *walker) visit(path string, entry fs.DirEntry) error { + if w.fn == nil { + return nil } - dir := root - if rel != "" { - dir = filepath.Join(root, rel) + return w.fn(path, entry) +} + +func (w *walker) walk(rel string) error { + m := w.matcher + if !w.retainPatterns { + defer m.restorePatterns(len(m.patterns), len(m.groups)) } + dir := w.root // Load .gitignore for this directory before processing entries. if rel != "" { - if err := m.addFromFile(filepath.Join(dir, ".gitignore"), filepath.ToSlash(rel)); err != nil && stopOnSizeError { + dir = filepath.Join(w.root, rel) + if err := m.addFromFile(filepath.Join(dir, ".gitignore"), filepath.ToSlash(rel)); err != nil && w.stopOnSizeError { return err } } @@ -311,23 +314,18 @@ func walkRecursive(root, rel string, m *Matcher, fn func(string, fs.DirEntry) er continue } - entryRel := name - if rel != "" { - entryRel = filepath.Join(rel, name) - } - - if m.match(filepath.ToSlash(entryRel), entry.IsDir(), checkParents) { + entryRel := filepath.Join(rel, name) + // Descendant directories have already passed their parent checks. + if m.match(filepath.ToSlash(entryRel), entry.IsDir(), rel == w.start) { continue } - if fn != nil { - if err := fn(entryRel, entry); err != nil { - return err - } + if err := w.visit(entryRel, entry); err != nil { + return err } if entry.IsDir() { - if err := walkRecursive(root, entryRel, m, fn, stopOnSizeError, retainPatterns, false); err != nil { + if err := w.walk(entryRel); err != nil { return err } } @@ -336,6 +334,16 @@ func walkRecursive(root, rel string, m *Matcher, fn func(string, fs.DirEntry) er return nil } +func (m *Matcher) restorePatterns(patternCount, groupCount int) { + clear(m.patterns[patternCount:]) + m.patterns = m.patterns[:patternCount] + clear(m.groups[groupCount:]) + m.groups = m.groups[:groupCount] + if groupCount > 0 { + m.groups[groupCount-1].end = patternCount + } +} + // AddPatterns parses gitignore pattern lines from data and scopes them to // the given relative directory. Pass an empty dir for root-level patterns. func (m *Matcher) AddPatterns(data []byte, dir string) { @@ -621,8 +629,7 @@ func compilePattern(line string) (pattern, string) { p.tailDoubleStar = allStars(line[i+1:]) } - segs, anchored := buildSegments(line, hasLeadingSlash) - p.anchored = anchored + segs := buildSegments(line, hasLeadingSlash) if msg := validateSegmentBrackets(segs); msg != "" { return pattern{}, msg @@ -637,7 +644,7 @@ func compilePattern(line string) (pattern, string) { // buildSegments splits a pattern line into segments, prepends ** for unanchored // patterns, and collapses consecutive ** segments. -func buildSegments(line string, hasLeadingSlash bool) ([]segment, bool) { +func buildSegments(line string, hasLeadingSlash bool) []segment { rawSegs := strings.Split(line, "/") anchored := hasLeadingSlash || len(rawSegs) > 1 @@ -663,7 +670,7 @@ func buildSegments(line string, hasLeadingSlash bool) ([]segment, bool) { } collapsed = append(collapsed, segs[i]) } - return collapsed, anchored + return collapsed } // allStars reports whether s consists of two or more '*' bytes and nothing @@ -759,42 +766,23 @@ func validateBrackets(glob string) string { // Returns an error message if invalid, and the index of the closing ']' (or -1 // if the bracket has no closing ']' and should be treated as literal). func validateBracketAt(glob string, pos int) (string, int) { - j := pos + 1 - if j < len(glob) && (glob[j] == '!' || glob[j] == '^') { - j++ - } + j, _ := bracketStart(glob, pos) if j < len(glob) && glob[j] == ']' { j++ // ] as first char is literal } for j < len(glob) && glob[j] != ']' { - if glob[j] == '\\' && j+1 < len(glob) { - j += posixClassOffset - continue - } - if glob[j] == '[' && j+1 < len(glob) && glob[j+1] == ':' { - end := findPosixClassEnd(glob, j+posixClassOffset) - if end >= 0 { - name := glob[j+posixClassOffset : end] - if !validPosixClassName(name) { - return "unknown POSIX class [:" + name + ":]", -1 - } - j = end + posixClassOffset - continue + if end := posixClassEnd(glob, j); end >= 0 { + name := glob[j+posixClassOffset : end] + if _, ok := posixClassMatchers[name]; !ok { + return "unknown POSIX class [:" + name + ":]", -1 } + j = end + posixClassOffset + continue } - j++ + _, j = readBracketChar(glob, j) } if j >= len(glob) { return "unclosed bracket expression", -1 } return "", j } - -func validPosixClassName(name string) bool { - switch name { - case "alnum", "alpha", "blank", "cntrl", "digit", "graph", - "lower", "print", "punct", "space", "upper", "xdigit": - return true - } - return false -} diff --git a/refactor_test.go b/refactor_test.go new file mode 100644 index 0000000..1a32fc6 --- /dev/null +++ b/refactor_test.go @@ -0,0 +1,79 @@ +package gitignore_test + +import ( + "errors" + "os" + "path/filepath" + "testing" + + "github.com/git-pkgs/gitignore" +) + +func TestBracketValidationAndMatching(t *testing.T) { + isolateGitEnv(t) + for _, tc := range []struct { + pattern, path, message string + ignored bool + }{ + {"[!][:digit:]]", "a", "", true}, + {"[!][:digit:]]", "]", "", false}, + {"[!][:digit:]]", "5", "", false}, + {`[\[:bogus:]]`, "b]", "", true}, + {"[a-[:unknown:]]", "a", "unknown POSIX class [:unknown:]", false}, + {"[[:unknown:]]", "a", "unknown POSIX class [:unknown:]", false}, + {"[!]]", "a", "", true}, + {"[!]", "a", "unclosed bracket expression", false}, + {`[a\]`, "a", "unclosed bracket expression", false}, + } { + t.Run(tc.pattern+"/"+tc.path, func(t *testing.T) { + m := gitignore.New("") + m.AddPatterns([]byte(tc.pattern+"\n"), "") + if got := m.Match(tc.path); got != tc.ignored { + t.Errorf("Match(%q) = %v, want %v", tc.path, got, tc.ignored) + } + errs := m.Errors() + if tc.message == "" { + if len(errs) != 0 { + t.Fatalf("Errors() = %v", errs) + } + } else if len(errs) != 1 || errs[0].Message != tc.message { + t.Fatalf("Errors() = %v, want %q", errs, tc.message) + } + }) + } +} + +func TestWalkCallbackError(t *testing.T) { + isolateGitEnv(t) + root := t.TempDir() + if err := os.MkdirAll(filepath.Join(root, "src", "nested"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "src", "nested", "file.go"), nil, 0o644); err != nil { + t.Fatal(err) + } + for _, start := range []string{"", "src"} { + for _, stop := range []string{"src", "src/nested/file.go"} { + t.Run(start+"/"+stop, func(t *testing.T) { + want := errors.New("callback failed") + stopped := false + err := gitignore.WalkFrom(root, start, func(path string, _ os.DirEntry) error { + if stopped { + t.Fatal("callback called after error") + } + if filepath.ToSlash(path) == stop { + stopped = true + return want + } + return nil + }) + if !errors.Is(err, want) { + t.Fatalf("WalkFrom() = %v, want %v", err, want) + } + }) + } + if err := gitignore.WalkFrom(root, start, nil); err != nil { + t.Fatalf("WalkFrom with nil callback: %v", err) + } + } +} diff --git a/wildmatch.go b/wildmatch.go index 1f56d63..fb03325 100644 --- a/wildmatch.go +++ b/wildmatch.go @@ -126,28 +126,14 @@ func matchSegment(glob, text string) bool { // glob[pos] (the '['). Returns (matched, posAfterBracket, valid). // If the bracket has no closing ']', valid is false. func matchBracket(glob string, pos int, ch byte) (bool, int, bool) { - i := pos + 1 // skip opening [ - if i >= len(glob) { - return false, 0, false - } - - negate := false - if glob[i] == '!' || glob[i] == '^' { - negate = true - i++ - } - + i, negate := bracketStart(glob, pos) matched := false - first := true // ] is literal when it's the first char after [, [!, or [^ + first := i // A leading ] is literal. for i < len(glob) { - if glob[i] == ']' && !first { - if negate { - matched = !matched - } - return matched, i + 1, true + if glob[i] == ']' && i != first { + return matched != negate, i + 1, true } - first = false var hit bool hit, i = matchBracketElement(glob, i, ch) @@ -159,17 +145,27 @@ func matchBracket(glob string, pos int, ch byte) (bool, int, bool) { return false, 0, false } +func bracketStart(glob string, pos int) (int, bool) { + i := pos + 1 + if i < len(glob) && (glob[i] == '!' || glob[i] == '^') { + return i + 1, true + } + return i, false +} + +func posixClassEnd(glob string, i int) int { + if glob[i] == '[' && i+1 < len(glob) && glob[i+1] == ':' { + return findPosixClassEnd(glob, i+posixClassOffset) + } + return -1 +} + // matchBracketElement matches a single element inside a bracket expression: // a POSIX class ([:name:]), a range (lo-hi), or a literal character. // Returns whether ch matched and the new index past the element. func matchBracketElement(glob string, i int, ch byte) (bool, int) { - // POSIX character class: [:name:] - if glob[i] == '[' && i+1 < len(glob) && glob[i+1] == ':' { - end := findPosixClassEnd(glob, i+posixClassOffset) - if end >= 0 { - name := glob[i+posixClassOffset : end] - return matchPosixClass(name, ch), end + posixClassOffset - } + if end := posixClassEnd(glob, i); end >= 0 { + return matchPosixClass(glob[i+posixClassOffset:end], ch), end + posixClassOffset } lo, next := readBracketChar(glob, i) From 4e5369ecd29ae9a615c40d27c79fec9a0100966a Mon Sep 17 00:00:00 2001 From: Andrew Nesbitt Date: Sat, 12 Sep 2026 20:04:56 -0400 Subject: [PATCH 3/3] Move refactor tests into gitignore_test.go and note WalkTree subprocess cost --- README.md | 2 +- gitignore_test.go | 70 +++++++++++++++++++++++++++++++++++++++++ refactor_test.go | 79 ----------------------------------------------- 3 files changed, 71 insertions(+), 80 deletions(-) delete mode 100644 refactor_test.go diff --git a/README.md b/README.md index 8af0b2f..1395cf2 100644 --- a/README.md +++ b/README.md @@ -121,7 +121,7 @@ Run benchmarks on an otherwise idle machine, using the same Go toolchain for bot go test -run '^$' -bench . -benchmem -count=10 -cpu=1 ``` -`BenchmarkCompile` includes filesystem reads and a Git subprocess through `New`; `BenchmarkAddPatterns` measures parsing without filesystem access. Compare samples with `benchstat` and check the timing spread before quoting speed changes. +`BenchmarkCompile` and `BenchmarkWalkTree` include filesystem reads and a Git subprocess through `New`; `BenchmarkAddPatterns` measures parsing without filesystem access. Compare samples with `benchstat` and check the timing spread before quoting speed changes. ## License diff --git a/gitignore_test.go b/gitignore_test.go index cd224f2..c54a7ea 100644 --- a/gitignore_test.go +++ b/gitignore_test.go @@ -1,6 +1,7 @@ package gitignore_test import ( + "errors" "os" "os/exec" "path/filepath" @@ -1729,6 +1730,40 @@ func TestWildmatchBracketEdgeCases(t *testing.T) { } } +func TestBracketValidationAndMatching(t *testing.T) { + isolateGitEnv(t) + for _, tc := range []struct { + pattern, path, message string + ignored bool + }{ + {"[!][:digit:]]", "a", "", true}, + {"[!][:digit:]]", "]", "", false}, + {"[!][:digit:]]", "5", "", false}, + {`[\[:bogus:]]`, "b]", "", true}, + {"[a-[:unknown:]]", "a", "unknown POSIX class [:unknown:]", false}, + {"[[:unknown:]]", "a", "unknown POSIX class [:unknown:]", false}, + {"[!]]", "a", "", true}, + {"[!]", "a", "unclosed bracket expression", false}, + {`[a\]`, "a", "unclosed bracket expression", false}, + } { + t.Run(tc.pattern+"/"+tc.path, func(t *testing.T) { + m := gitignore.New("") + m.AddPatterns([]byte(tc.pattern+"\n"), "") + if got := m.Match(tc.path); got != tc.ignored { + t.Errorf("Match(%q) = %v, want %v", tc.path, got, tc.ignored) + } + errs := m.Errors() + if tc.message == "" { + if len(errs) != 0 { + t.Fatalf("Errors() = %v", errs) + } + } else if len(errs) != 1 || errs[0].Message != tc.message { + t.Fatalf("Errors() = %v, want %q", errs, tc.message) + } + }) + } +} + func TestWildmatchCharacterClassesExpanded(t *testing.T) { tests := []struct { pattern string @@ -2785,3 +2820,38 @@ func TestWalkFromRootEquivalentStart(t *testing.T) { }) } } + +func TestWalkCallbackError(t *testing.T) { + isolateGitEnv(t) + root := t.TempDir() + if err := os.MkdirAll(filepath.Join(root, "src", "nested"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "src", "nested", "file.go"), nil, 0o644); err != nil { + t.Fatal(err) + } + for _, start := range []string{"", "src"} { + for _, stop := range []string{"src", "src/nested/file.go"} { + t.Run(start+"/"+stop, func(t *testing.T) { + want := errors.New("callback failed") + stopped := false + err := gitignore.WalkFrom(root, start, func(path string, _ os.DirEntry) error { + if stopped { + t.Fatal("callback called after error") + } + if filepath.ToSlash(path) == stop { + stopped = true + return want + } + return nil + }) + if !errors.Is(err, want) { + t.Fatalf("WalkFrom() = %v, want %v", err, want) + } + }) + } + if err := gitignore.WalkFrom(root, start, nil); err != nil { + t.Fatalf("WalkFrom with nil callback: %v", err) + } + } +} diff --git a/refactor_test.go b/refactor_test.go deleted file mode 100644 index 1a32fc6..0000000 --- a/refactor_test.go +++ /dev/null @@ -1,79 +0,0 @@ -package gitignore_test - -import ( - "errors" - "os" - "path/filepath" - "testing" - - "github.com/git-pkgs/gitignore" -) - -func TestBracketValidationAndMatching(t *testing.T) { - isolateGitEnv(t) - for _, tc := range []struct { - pattern, path, message string - ignored bool - }{ - {"[!][:digit:]]", "a", "", true}, - {"[!][:digit:]]", "]", "", false}, - {"[!][:digit:]]", "5", "", false}, - {`[\[:bogus:]]`, "b]", "", true}, - {"[a-[:unknown:]]", "a", "unknown POSIX class [:unknown:]", false}, - {"[[:unknown:]]", "a", "unknown POSIX class [:unknown:]", false}, - {"[!]]", "a", "", true}, - {"[!]", "a", "unclosed bracket expression", false}, - {`[a\]`, "a", "unclosed bracket expression", false}, - } { - t.Run(tc.pattern+"/"+tc.path, func(t *testing.T) { - m := gitignore.New("") - m.AddPatterns([]byte(tc.pattern+"\n"), "") - if got := m.Match(tc.path); got != tc.ignored { - t.Errorf("Match(%q) = %v, want %v", tc.path, got, tc.ignored) - } - errs := m.Errors() - if tc.message == "" { - if len(errs) != 0 { - t.Fatalf("Errors() = %v", errs) - } - } else if len(errs) != 1 || errs[0].Message != tc.message { - t.Fatalf("Errors() = %v, want %q", errs, tc.message) - } - }) - } -} - -func TestWalkCallbackError(t *testing.T) { - isolateGitEnv(t) - root := t.TempDir() - if err := os.MkdirAll(filepath.Join(root, "src", "nested"), 0o755); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(root, "src", "nested", "file.go"), nil, 0o644); err != nil { - t.Fatal(err) - } - for _, start := range []string{"", "src"} { - for _, stop := range []string{"src", "src/nested/file.go"} { - t.Run(start+"/"+stop, func(t *testing.T) { - want := errors.New("callback failed") - stopped := false - err := gitignore.WalkFrom(root, start, func(path string, _ os.DirEntry) error { - if stopped { - t.Fatal("callback called after error") - } - if filepath.ToSlash(path) == stop { - stopped = true - return want - } - return nil - }) - if !errors.Is(err, want) { - t.Fatalf("WalkFrom() = %v, want %v", err, want) - } - }) - } - if err := gitignore.WalkFrom(root, start, nil); err != nil { - t.Fatalf("WalkFrom with nil callback: %v", err) - } - } -}