Skip to content

Commit 4fe90ca

Browse files
committed
Fix file handle leak in gitignore loading for Windows compatibility
Replace the go-gitignore library's NewRepository (which opens .gitignore files via os.Open but never closes the handles) with a custom repoMatcher that reads files via os.ReadFile and passes strings.NewReader to the library's New() function. This ensures all file handles are closed immediately after reading. The library's ignore.Relative() pattern matching is preserved — only the hierarchy walker and file loading are reimplemented. The matching algorithm follows standard git precedence: child .gitignore overrides parent, negation patterns in child override matching patterns in parent, and .git/info/exclude is checked last. Fixes Windows CI where leaked handles prevented t.TempDir() cleanup.
1 parent bd6bbc2 commit 4fe90ca

File tree

1 file changed

+146
-18
lines changed

1 file changed

+146
-18
lines changed

internal/discover/gitignore.go

Lines changed: 146 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,47 +1,38 @@
11
package discover
22

33
import (
4-
"log/slog"
54
"os"
65
"path/filepath"
6+
"strings"
7+
"sync"
78

89
gitignore "github.com/boyter/gocodewalker/go-gitignore"
910
)
1011

1112
// ignoreMatchers holds the layered ignore matchers for a repository.
1213
// Evaluation order: hardcoded patterns (fastest) → gitignore → cbmignore.
1314
type ignoreMatchers struct {
14-
gitignore gitignore.GitIgnore // .gitignore hierarchy + .git/info/exclude (nil for non-git repos)
15+
gitignore *repoMatcher // .gitignore hierarchy + .git/info/exclude (nil for non-git repos)
1516
cbmignore gitignore.GitIgnore // .cbmignore at repo root (nil if absent)
1617
}
1718

1819
// loadIgnoreMatchers loads all gitignore-style matchers for the repository.
1920
// .gitignore is loaded only for git repos (presence of .git dir).
2021
// .cbmignore stacks on top — patterns there additionally exclude from indexing.
22+
//
23+
// All files are read and closed immediately — no file handle leaks.
24+
// This is critical for Windows where open handles prevent t.TempDir() cleanup.
2125
func loadIgnoreMatchers(repoPath string) ignoreMatchers {
2226
var m ignoreMatchers
2327

2428
// .gitignore — only for git repos
2529
gitDir := filepath.Join(repoPath, ".git")
2630
if info, err := os.Stat(gitDir); err == nil && info.IsDir() {
27-
gi, err := gitignore.NewRepository(repoPath)
28-
if err != nil {
29-
slog.Warn("discover.gitignore_load_err", "err", err)
30-
} else {
31-
m.gitignore = gi
32-
}
31+
m.gitignore = newRepoMatcher(repoPath)
3332
}
3433

3534
// .cbmignore — repo-root file with gitignore-style patterns
36-
cbmPath := filepath.Join(repoPath, ".cbmignore")
37-
if _, err := os.Stat(cbmPath); err == nil {
38-
ci, err := gitignore.NewFromFile(cbmPath)
39-
if err != nil {
40-
slog.Warn("discover.cbmignore_load_err", "err", err)
41-
} else {
42-
m.cbmignore = ci
43-
}
44-
}
35+
m.cbmignore = safeLoadGitignore(filepath.Join(repoPath, ".cbmignore"))
4536

4637
return m
4738
}
@@ -50,7 +41,7 @@ func loadIgnoreMatchers(repoPath string) ignoreMatchers {
5041
// absPath must be absolute; isDir indicates whether the path is a directory.
5142
func (m *ignoreMatchers) shouldIgnore(absPath string, isDir bool) bool {
5243
if m.gitignore != nil {
53-
if match := m.gitignore.Absolute(absPath, isDir); match != nil && match.Ignore() {
44+
if m.gitignore.shouldIgnore(absPath, isDir) {
5445
return true
5546
}
5647
}
@@ -61,3 +52,140 @@ func (m *ignoreMatchers) shouldIgnore(absPath string, isDir bool) bool {
6152
}
6253
return false
6354
}
55+
56+
// safeLoadGitignore reads a gitignore-style file and returns a GitIgnore matcher.
57+
// The file is read completely and closed immediately — no handle leak.
58+
// Returns nil if the file doesn't exist or can't be read.
59+
func safeLoadGitignore(path string) gitignore.GitIgnore {
60+
content, err := os.ReadFile(path)
61+
if err != nil {
62+
return nil
63+
}
64+
base := filepath.Dir(path)
65+
return gitignore.New(strings.NewReader(string(content)), base, nil)
66+
}
67+
68+
// repoMatcher implements gitignore-style matching across a repository's
69+
// .gitignore hierarchy. It replaces the library's repository type to avoid
70+
// file handle leaks (the library's NewWithErrors opens files but never closes them).
71+
//
72+
// Pattern matching uses the library's ignore.Relative() — only the hierarchy
73+
// walker and file loading are reimplemented.
74+
type repoMatcher struct {
75+
base string
76+
exclude gitignore.GitIgnore // .git/info/exclude patterns
77+
mu sync.Mutex // protects cache
78+
cache map[string]gitignore.GitIgnore // dir path → loaded .gitignore (nil = no file)
79+
}
80+
81+
func newRepoMatcher(base string) *repoMatcher {
82+
m := &repoMatcher{
83+
base: base,
84+
cache: make(map[string]gitignore.GitIgnore),
85+
}
86+
m.exclude = safeLoadGitignore(filepath.Join(base, ".git", "info", "exclude"))
87+
return m
88+
}
89+
90+
// loadDir loads the .gitignore for a directory, caching the result.
91+
func (m *repoMatcher) loadDir(dir string) gitignore.GitIgnore {
92+
m.mu.Lock()
93+
defer m.mu.Unlock()
94+
if gi, ok := m.cache[dir]; ok {
95+
return gi // may be nil (cached miss)
96+
}
97+
gi := safeLoadGitignore(filepath.Join(dir, ".gitignore"))
98+
m.cache[dir] = gi
99+
return gi
100+
}
101+
102+
// shouldIgnore checks if an absolute path should be ignored according to
103+
// the repository's .gitignore hierarchy, following standard git precedence:
104+
// - child .gitignore overrides parent .gitignore
105+
// - negation patterns (!) in child override matching patterns in parent
106+
// - .git/info/exclude is checked last (lowest priority)
107+
func (m *repoMatcher) shouldIgnore(absPath string, isDir bool) bool {
108+
if !strings.HasPrefix(absPath, m.base) {
109+
return false
110+
}
111+
if absPath == m.base {
112+
return false
113+
}
114+
115+
rel, err := filepath.Rel(m.base, absPath)
116+
if err != nil {
117+
return false
118+
}
119+
rel = filepath.ToSlash(rel)
120+
if rel == "." {
121+
return false
122+
}
123+
124+
return m.matchRel(rel, isDir)
125+
}
126+
127+
// matchRel implements the core gitignore hierarchy matching algorithm.
128+
// Mirrors the logic in the library's repository.Relative().
129+
func (m *repoMatcher) matchRel(rel string, isDir bool) bool {
130+
rel = filepath.ToSlash(filepath.Clean(rel))
131+
if rel == "." {
132+
return false
133+
}
134+
135+
// First: is the parent directory ignored?
136+
// An ignored parent means the child is also ignored.
137+
parent, local := splitPath(rel)
138+
if parent != "" {
139+
if m.matchRel(parent, true) {
140+
return true
141+
}
142+
}
143+
144+
// Walk from the file's directory up to the repo root,
145+
// checking .gitignore at each level. First match wins.
146+
dir := parent
147+
curLocal := local
148+
for {
149+
absDir := filepath.Join(m.base, filepath.FromSlash(dir))
150+
if dir == "" {
151+
absDir = m.base
152+
}
153+
gi := m.loadDir(absDir)
154+
if gi != nil {
155+
if match := gi.Relative(curLocal, isDir); match != nil {
156+
return match.Ignore()
157+
}
158+
}
159+
160+
if dir == "" {
161+
break
162+
}
163+
164+
// Walk up: prepend the current directory component to local
165+
var last string
166+
dir, last = splitPath(dir)
167+
curLocal = last + "/" + curLocal
168+
}
169+
170+
// Finally check .git/info/exclude (lowest priority)
171+
if m.exclude != nil {
172+
if match := m.exclude.Relative(rel, isDir); match != nil {
173+
return match.Ignore()
174+
}
175+
}
176+
177+
return false
178+
}
179+
180+
// splitPath splits a forward-slash path into parent and last component.
181+
// "a/b/c" → ("a/b", "c"), "a" → ("", "a"), "" → ("", "")
182+
func splitPath(p string) (parent, last string) {
183+
if p == "" {
184+
return "", ""
185+
}
186+
i := strings.LastIndex(p, "/")
187+
if i < 0 {
188+
return "", p
189+
}
190+
return p[:i], p[i+1:]
191+
}

0 commit comments

Comments
 (0)