[Binary Provisioning] Change the default cache dir #4741
Conversation
internal/cmd/launcher.go
Outdated
| // nolint:forbidigo | ||
| // TODO: init this as part of the GlobalState | ||
| // and remote it from here | ||
| userdir, err := os.UserCacheDir() | ||
| if err != nil { | ||
| return "", err | ||
| } |
There was a problem hiding this comment.
I intend to address this having a field in GlobalState but I want to minimize the changes requires on this pull request.
The next week, I will open a separated pull request that I intend to merge only after we have merged Binary Provisioning.
There was a problem hiding this comment.
FYI: Here is a draft with that approach #4743
24794c5 to
3ab7d1e
Compare
| // TODO: init this as part of the GlobalState | ||
| // and remote it from here | ||
| userdir, err := os.UserCacheDir() | ||
| if err != nil { |
There was a problem hiding this comment.
I'm not sure we should return an error here. In the GlobalState when we fail to get the config dir, we default to .config. I prefer to be consistent in how we handle those errors here, mostly because when we move this logic to GlobalState we should do it anyway.
| return "", err | ||
| } | ||
| cacheDir := filepath.Join(userdir, "k6", "builds") | ||
| if err := fs.MkdirAll(cacheDir, 0o700); err != nil { |
There was a problem hiding this comment.
This is not necessary, the provider creates the directory if it doesn't exists.
| binaryCacheDir, err := getBinaryCacheDir(gs.FS) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to acquire the user's cache directory: %w", err) | ||
| } | ||
|
|
There was a problem hiding this comment.
Based on my comments in getBinaryCacheDir I think this code could be simplified to this:
| binaryCacheDir, err := getBinaryCacheDir(gs.FS) | |
| if err != nil { | |
| return nil, fmt.Errorf("failed to acquire the user's cache directory: %w", err) | |
| } | |
| userdir, err := os.UserCacheDir() | |
| if err != nil { | |
| userdir = ".cache" | |
| gs.Logger.WithError(err).Debug("failed to acquire the user's cache directory) | |
| } | |
| binaryCacheDir := filepath.Join(userdir, "k6", "builds") |
There was a problem hiding this comment.
FYI: I had implemented this approach here also considering overriding with the K6_BINARY_CACHE environment variable. #4742.
| t.Run("directory already exists", func(t *testing.T) { | ||
| t.Parallel() | ||
| fs := testutils.MakeMemMapFs(t, map[string][]byte{ | ||
| filepath.Join(userDir, "/k6/builds/marker.bin"): []byte(`example binary file`), |
There was a problem hiding this comment.
Shouldn't this (and other similar usage in the test) be:
filepath.Join(userDir, "k6", "builds", "marker.bin")|
Superseded by #4743 |
What?
It changes the default folder to
$HOME/.cache/k6/builds/*.Why?
The previous
k6providermight generate confusion for users, as it doesn't match any installed application.Checklist
make check) and all pass.Checklist: Documentation (only for k6 maintainers and if relevant)
Please do not merge this PR until the following items are filled out.
Related PR(s)/Issue(s)