Skip to content

Commit 13e842a

Browse files
committed
cli/config: add synchronisation for configDir (Dir, SetDir)
commit 8a30653 introduced a sync.Once to allow for the config-directory (and home-dir) to be looked up lazily instead of in an `init()`. However, the package-level `configDir` variable can be set through two separate paths; implicitly (through `config.Dir()`), and explicitly, through `config.SetDir()`. The existing code had no synchronisation for this, which could lead to a potential race-condition (code requesting `config.Dir()` and code setting a custom path through `config.SetDir()`). This patch adds synchronisation by triggering the `sync.Once` as part of `config.SetDir()` to prevent it being triggered later (overwriting the value that was set). It also restores the `resetConfigDir()` utility that was removed in 379122b, to allow resetting the `sync.Once` for this test. In general, we should get rid of this package-level variable, and store it as a config on the client (passing the option to locations where its used instead). Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
1 parent a26e601 commit 13e842a

2 files changed

Lines changed: 18 additions & 0 deletions

File tree

cli/config/config.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,13 @@ var (
2727
configDir string
2828
)
2929

30+
// resetConfigDir is used in testing to reset the "configDir" package variable
31+
// and its sync.Once to force re-lookup between tests.
32+
func resetConfigDir() {
33+
configDir = ""
34+
initConfigDir = new(sync.Once)
35+
}
36+
3037
// Dir returns the directory the configuration file is stored in
3138
func Dir() string {
3239
initConfigDir.Do(func() {
@@ -45,6 +52,8 @@ func ContextStoreDir() string {
4552

4653
// SetDir sets the directory the configuration file is stored in
4754
func SetDir(dir string) {
55+
// trigger the sync.Once to synchronise with Dir()
56+
initConfigDir.Do(func() {})
4857
configDir = filepath.Clean(dir)
4958
}
5059

cli/config/config_test.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -382,3 +382,12 @@ func TestConfigPath(t *testing.T) {
382382

383383
SetDir(oldDir)
384384
}
385+
386+
// TestSetDir verifies that Dir() does not overwrite the value set through
387+
// SetDir() if it has not been run before.
388+
func TestSetDir(t *testing.T) {
389+
const expected = "my_config_dir"
390+
resetConfigDir()
391+
SetDir(expected)
392+
assert.Check(t, is.Equal(Dir(), expected))
393+
}

0 commit comments

Comments
 (0)