Skip to content

Commit 116ec17

Browse files
pieternclaude
andauthored
Plumb context through libs/git (#4716)
## Summary - Thread `context.Context` through the `libs/git` package's public API so that `os.UserHomeDir` and `os.Getenv` calls use context-aware equivalents (`env.UserHomeDir`, `env.Get`) - Resolve `XDG_CONFIG_HOME` once in `newConfig` and store on the `config` struct, eliminating duplication between `globalGitConfig` and `defaultCoreExcludesFile` - Follow-up to #4654 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 137ffde commit 116ec17

File tree

12 files changed

+72
-64
lines changed

12 files changed

+72
-64
lines changed

libs/git/config.go

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package git
22

33
import (
4+
"context"
45
"errors"
56
"fmt"
67
"io"
@@ -10,6 +11,7 @@ import (
1011
"regexp"
1112
"strings"
1213

14+
"github.com/databricks/cli/libs/env"
1315
"github.com/databricks/cli/libs/vfs"
1416
"gopkg.in/ini.v1"
1517
)
@@ -25,19 +27,28 @@ import (
2527
//
2628
// Also see: https://git-scm.com/docs/git-config.
2729
type config struct {
28-
home string
29-
variables map[string]string
30+
home string
31+
xdgConfigHome string
32+
variables map[string]string
3033
}
3134

32-
func newConfig() (*config, error) {
33-
home, err := os.UserHomeDir() //nolint:forbidigo // no ctx available in git config loading
35+
func newConfig(ctx context.Context) (*config, error) {
36+
home, err := env.UserHomeDir(ctx)
3437
if err != nil {
3538
return nil, err
3639
}
3740

41+
xdgConfigHome := env.Get(ctx, "XDG_CONFIG_HOME")
42+
if xdgConfigHome == "" {
43+
// If $XDG_CONFIG_HOME is either not set or empty,
44+
// $HOME/.config is used instead.
45+
xdgConfigHome = filepath.Join(home, ".config")
46+
}
47+
3848
return &config{
39-
home: home,
40-
variables: make(map[string]string),
49+
home: home,
50+
xdgConfigHome: xdgConfigHome,
51+
variables: make(map[string]string),
4152
}, nil
4253
}
4354

@@ -112,14 +123,7 @@ func (c config) loadFile(root vfs.Path, path string) error {
112123

113124
func (c config) defaultCoreExcludesFile() string {
114125
// Defaults to $XDG_CONFIG_HOME/git/ignore.
115-
xdgConfigHome := os.Getenv("XDG_CONFIG_HOME") //nolint:forbidigo // no ctx; plumbing it requires changing NewRepository and all callers
116-
if xdgConfigHome == "" {
117-
// If $XDG_CONFIG_HOME is either not set or empty,
118-
// $HOME/.config/git/ignore is used instead.
119-
xdgConfigHome = filepath.Join(c.home, ".config")
120-
}
121-
122-
return filepath.Join(xdgConfigHome, "git/ignore")
126+
return filepath.Join(c.xdgConfigHome, "git/ignore")
123127
}
124128

125129
func (c config) coreExcludesFile() (string, error) {
@@ -139,23 +143,19 @@ func (c config) coreExcludesFile() (string, error) {
139143
return path, nil
140144
}
141145

142-
func globalGitConfig() (*config, error) {
143-
config, err := newConfig()
146+
func globalGitConfig(ctx context.Context) (*config, error) {
147+
config, err := newConfig(ctx)
144148
if err != nil {
145149
return nil, err
146150
}
147-
xdgConfigHome := os.Getenv("XDG_CONFIG_HOME") //nolint:forbidigo // no ctx; plumbing it requires changing NewRepository and all callers
148-
if xdgConfigHome == "" {
149-
xdgConfigHome = filepath.Join(config.home, ".config")
150-
}
151151

152152
// From https://git-scm.com/docs/git-config#FILES:
153153
//
154154
// > If the global or the system-wide configuration files
155155
// > are missing or unreadable they will be ignored.
156156
//
157157
// We therefore ignore the error return value for the calls below.
158-
_ = config.loadFile(vfs.MustNew(xdgConfigHome), "git/config")
158+
_ = config.loadFile(vfs.MustNew(config.xdgConfigHome), "git/config")
159159
_ = config.loadFile(vfs.MustNew(config.home), ".gitconfig")
160160

161161
return config, nil

libs/git/config_test.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"path/filepath"
77
"testing"
88

9+
"github.com/databricks/cli/libs/env"
910
"github.com/stretchr/testify/assert"
1011
"github.com/stretchr/testify/require"
1112
)
@@ -69,13 +70,14 @@ func TestConfig(t *testing.T) {
6970
url = https://example.com/git
7071
`
7172

72-
c, err := newConfig()
73+
ctx := t.Context()
74+
c, err := newConfig(ctx)
7375
require.NoError(t, err)
7476

7577
err = c.load(bytes.NewBufferString(raw))
7678
require.NoError(t, err)
7779

78-
home, err := os.UserHomeDir() //nolint:forbidigo // test for config.go which has no ctx
80+
home, err := env.UserHomeDir(ctx)
7981
require.NoError(t, err)
8082

8183
assert.Equal(t, "false", c.variables["core.filemode"])
@@ -86,7 +88,8 @@ func TestConfig(t *testing.T) {
8688
}
8789

8890
func TestCoreExcludesFile(t *testing.T) {
89-
config, err := globalGitConfig()
91+
ctx := t.Context()
92+
config, err := globalGitConfig(ctx)
9093
require.NoError(t, err)
9194
path, err := config.coreExcludesFile()
9295
require.NoError(t, err)
@@ -118,7 +121,8 @@ func (h *testCoreExcludesHelper) initialize(t *testing.T) {
118121
}
119122

120123
func (h *testCoreExcludesHelper) coreExcludesFile() (string, error) {
121-
config, err := globalGitConfig()
124+
ctx := h.Context()
125+
config, err := globalGitConfig(ctx)
122126
require.NoError(h.T, err)
123127
return config.coreExcludesFile()
124128
}

libs/git/fileset.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package git
22

33
import (
4+
"context"
5+
46
"github.com/databricks/cli/libs/fileset"
57
"github.com/databricks/cli/libs/vfs"
68
)
@@ -14,9 +16,9 @@ type FileSet struct {
1416
}
1517

1618
// NewFileSet returns [FileSet] for the directory `root` which is contained within Git worktree located at `worktreeRoot`.
17-
func NewFileSet(worktreeRoot, root vfs.Path, paths ...[]string) (*FileSet, error) {
19+
func NewFileSet(ctx context.Context, worktreeRoot, root vfs.Path, paths ...[]string) (*FileSet, error) {
1820
fs := fileset.New(root, paths...)
19-
v, err := NewView(worktreeRoot, root)
21+
v, err := NewView(ctx, worktreeRoot, root)
2022
if err != nil {
2123
return nil, err
2224
}
@@ -27,8 +29,8 @@ func NewFileSet(worktreeRoot, root vfs.Path, paths ...[]string) (*FileSet, error
2729
}, nil
2830
}
2931

30-
func NewFileSetAtRoot(root vfs.Path, paths ...[]string) (*FileSet, error) {
31-
return NewFileSet(root, root, paths...)
32+
func NewFileSetAtRoot(ctx context.Context, root vfs.Path, paths ...[]string) (*FileSet, error) {
33+
return NewFileSet(ctx, root, root, paths...)
3234
}
3335

3436
func (f *FileSet) IgnoreFile(file string) (bool, error) {

libs/git/fileset_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import (
1111
)
1212

1313
func testFileSetAll(t *testing.T, worktreeRoot, root string) {
14-
fileSet, err := NewFileSet(vfs.MustNew(worktreeRoot), vfs.MustNew(root))
14+
fileSet, err := NewFileSet(t.Context(), vfs.MustNew(worktreeRoot), vfs.MustNew(root))
1515
require.NoError(t, err)
1616
files, err := fileSet.Files()
1717
require.NoError(t, err)
@@ -43,7 +43,7 @@ func TestFileSetNonCleanRoot(t *testing.T) {
4343
// Test what happens if the root directory can be simplified.
4444
// Path simplification is done by most filepath functions.
4545
// This should yield the same result as above test.
46-
fileSet, err := NewFileSetAtRoot(vfs.MustNew("./testdata/../testdata"))
46+
fileSet, err := NewFileSetAtRoot(t.Context(), vfs.MustNew("./testdata/../testdata"))
4747
require.NoError(t, err)
4848
files, err := fileSet.Files()
4949
require.NoError(t, err)

libs/git/info.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ func fetchRepositoryInfoDotGit(ctx context.Context, path string) (RepositoryInfo
109109

110110
result.WorktreeRoot = rootDir
111111

112-
repo, err := NewRepository(vfs.MustNew(rootDir))
112+
repo, err := NewRepository(ctx, vfs.MustNew(rootDir))
113113
if err != nil {
114114
log.Warnf(ctx, "failed to read .git: %s", err)
115115

libs/git/repository.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package git
22

33
import (
4+
"context"
45
"fmt"
56
"net/url"
67
"path"
@@ -125,8 +126,8 @@ func (r *Repository) OriginUrl() string {
125126
}
126127

127128
// loadConfig loads and combines user specific and repository specific configuration files.
128-
func (r *Repository) loadConfig() error {
129-
config, err := globalGitConfig()
129+
func (r *Repository) loadConfig(ctx context.Context) error {
130+
config, err := globalGitConfig(ctx)
130131
if err != nil {
131132
return fmt.Errorf("unable to load user specific gitconfig: %w", err)
132133
}
@@ -202,7 +203,7 @@ func (r *Repository) Ignore(relPath string) (bool, error) {
202203
return false, nil
203204
}
204205

205-
func NewRepository(rootDir vfs.Path) (*Repository, error) {
206+
func NewRepository(ctx context.Context, rootDir vfs.Path) (*Repository, error) {
206207
// Derive $GIT_DIR and $GIT_COMMON_DIR paths if this is a real repository.
207208
// If it isn't a real repository, they'll point to the (non-existent) `.git` directory.
208209
gitDir, gitCommonDir, err := resolveGitDirs(rootDir)
@@ -217,7 +218,7 @@ func NewRepository(rootDir vfs.Path) (*Repository, error) {
217218
ignore: make(map[string][]ignoreRules),
218219
}
219220

220-
err = repo.loadConfig()
221+
err = repo.loadConfig(ctx)
221222
if err != nil {
222223
// Error doesn't need to be rewrapped.
223224
return nil, err

libs/git/repository_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ func newTestRepository(t *testing.T) *testRepository {
4444
_, err = f2.WriteString(`ref: refs/heads/main`)
4545
require.NoError(t, err)
4646

47-
repo, err := NewRepository(vfs.MustNew(tmp))
47+
repo, err := NewRepository(t.Context(), vfs.MustNew(tmp))
4848
require.NoError(t, err)
4949

5050
return &testRepository{
@@ -99,7 +99,7 @@ func (testRepo *testRepository) addOriginUrl(url string) {
9999
require.NoError(testRepo.t, err)
100100

101101
// reload config to reflect the remote url
102-
err = testRepo.r.loadConfig()
102+
err = testRepo.r.loadConfig(testRepo.t.Context())
103103
require.NoError(testRepo.t, err)
104104
}
105105

@@ -128,7 +128,7 @@ func (testRepo *testRepository) assertOriginUrl(expected string) {
128128

129129
func TestRepository(t *testing.T) {
130130
// Load this repository as test.
131-
repo, err := NewRepository(vfs.MustNew("../.."))
131+
repo, err := NewRepository(t.Context(), vfs.MustNew("../.."))
132132
tr := testRepository{t, repo}
133133
require.NoError(t, err)
134134

@@ -192,7 +192,7 @@ func TestRepositoryGitConfigForSshUrl(t *testing.T) {
192192

193193
func TestRepositoryGitConfigWhenNotARepo(t *testing.T) {
194194
tmp := t.TempDir()
195-
repo, err := NewRepository(vfs.MustNew(tmp))
195+
repo, err := NewRepository(t.Context(), vfs.MustNew(tmp))
196196
require.NoError(t, err)
197197

198198
branch, err := repo.CurrentBranch()

libs/git/view.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package git
22

33
import (
4+
"context"
45
"fmt"
56
"os"
67
"path"
@@ -72,8 +73,8 @@ func (v *View) IgnoreDirectory(dir string) (bool, error) {
7273
return v.Ignore(dir + "/")
7374
}
7475

75-
func NewView(worktreeRoot, root vfs.Path) (*View, error) {
76-
repo, err := NewRepository(worktreeRoot)
76+
func NewView(ctx context.Context, worktreeRoot, root vfs.Path) (*View, error) {
77+
repo, err := NewRepository(ctx, worktreeRoot)
7778
if err != nil {
7879
return nil, err
7980
}
@@ -99,8 +100,8 @@ func NewView(worktreeRoot, root vfs.Path) (*View, error) {
99100
return result, nil
100101
}
101102

102-
func NewViewAtRoot(root vfs.Path) (*View, error) {
103-
return NewView(root, root)
103+
func NewViewAtRoot(ctx context.Context, root vfs.Path) (*View, error) {
104+
return NewView(ctx, root, root)
104105
}
105106

106107
func (v *View) SetupDefaults() {

libs/git/view_test.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -90,19 +90,19 @@ func testViewAtRoot(t *testing.T, tv testView) {
9090
}
9191

9292
func TestViewRootInBricksRepo(t *testing.T) {
93-
v, err := NewViewAtRoot(vfs.MustNew("./testdata"))
93+
v, err := NewViewAtRoot(t.Context(), vfs.MustNew("./testdata"))
9494
require.NoError(t, err)
9595
testViewAtRoot(t, testView{t, v})
9696
}
9797

9898
func TestViewRootInTempRepo(t *testing.T) {
99-
v, err := NewViewAtRoot(vfs.MustNew(createFakeRepo(t, "testdata")))
99+
v, err := NewViewAtRoot(t.Context(), vfs.MustNew(createFakeRepo(t, "testdata")))
100100
require.NoError(t, err)
101101
testViewAtRoot(t, testView{t, v})
102102
}
103103

104104
func TestViewRootInTempDir(t *testing.T) {
105-
v, err := NewViewAtRoot(vfs.MustNew(copyTestdata(t, "testdata")))
105+
v, err := NewViewAtRoot(t.Context(), vfs.MustNew(copyTestdata(t, "testdata")))
106106
require.NoError(t, err)
107107
testViewAtRoot(t, testView{t, v})
108108
}
@@ -125,21 +125,21 @@ func testViewAtA(t *testing.T, tv testView) {
125125
}
126126

127127
func TestViewAInBricksRepo(t *testing.T) {
128-
v, err := NewView(vfs.MustNew("."), vfs.MustNew("./testdata/a"))
128+
v, err := NewView(t.Context(), vfs.MustNew("."), vfs.MustNew("./testdata/a"))
129129
require.NoError(t, err)
130130
testViewAtA(t, testView{t, v})
131131
}
132132

133133
func TestViewAInTempRepo(t *testing.T) {
134134
repo := createFakeRepo(t, "testdata")
135-
v, err := NewView(vfs.MustNew(repo), vfs.MustNew(filepath.Join(repo, "a")))
135+
v, err := NewView(t.Context(), vfs.MustNew(repo), vfs.MustNew(filepath.Join(repo, "a")))
136136
require.NoError(t, err)
137137
testViewAtA(t, testView{t, v})
138138
}
139139

140140
func TestViewAInTempDir(t *testing.T) {
141141
// Since this is not a fake repo it should not traverse up the tree.
142-
v, err := NewViewAtRoot(vfs.MustNew(filepath.Join(copyTestdata(t, "testdata"), "a")))
142+
v, err := NewViewAtRoot(t.Context(), vfs.MustNew(filepath.Join(copyTestdata(t, "testdata"), "a")))
143143
require.NoError(t, err)
144144
tv := testView{t, v}
145145

@@ -176,21 +176,21 @@ func testViewAtAB(t *testing.T, tv testView) {
176176
}
177177

178178
func TestViewABInBricksRepo(t *testing.T) {
179-
v, err := NewView(vfs.MustNew("."), vfs.MustNew("./testdata/a/b"))
179+
v, err := NewView(t.Context(), vfs.MustNew("."), vfs.MustNew("./testdata/a/b"))
180180
require.NoError(t, err)
181181
testViewAtAB(t, testView{t, v})
182182
}
183183

184184
func TestViewABInTempRepo(t *testing.T) {
185185
repo := createFakeRepo(t, "testdata")
186-
v, err := NewView(vfs.MustNew(repo), vfs.MustNew(filepath.Join(repo, "a", "b")))
186+
v, err := NewView(t.Context(), vfs.MustNew(repo), vfs.MustNew(filepath.Join(repo, "a", "b")))
187187
require.NoError(t, err)
188188
testViewAtAB(t, testView{t, v})
189189
}
190190

191191
func TestViewABInTempDir(t *testing.T) {
192192
// Since this is not a fake repo it should not traverse up the tree.
193-
v, err := NewViewAtRoot(vfs.MustNew(filepath.Join(copyTestdata(t, "testdata"), "a", "b")))
193+
v, err := NewViewAtRoot(t.Context(), vfs.MustNew(filepath.Join(copyTestdata(t, "testdata"), "a", "b")))
194194
tv := testView{t, v}
195195
require.NoError(t, err)
196196

@@ -212,7 +212,7 @@ func TestViewABInTempDir(t *testing.T) {
212212
func TestViewAlwaysIgnoresLocalStateDir(t *testing.T) {
213213
repoPath := createFakeRepo(t, "testdata")
214214

215-
v, err := NewViewAtRoot(vfs.MustNew(repoPath))
215+
v, err := NewViewAtRoot(t.Context(), vfs.MustNew(repoPath))
216216
require.NoError(t, err)
217217

218218
// assert .databricks is still being ignored

0 commit comments

Comments
 (0)