diff --git a/docs/docs/changelog.md b/docs/docs/changelog.md index 1d23ac0d..7fd58685 100644 --- a/docs/docs/changelog.md +++ b/docs/docs/changelog.md @@ -6,6 +6,7 @@ title: Changelog ## [Unreleased](https://github.com/lets-cli/lets/releases/tag/v0.0.X) * `[Added]` Show interactive download progress for remote configs and remote mixins. Issue [#360](https://github.com/lets-cli/lets/issues/360) +* `[Fixed]` Make `--no-cache` re-download remote mixins for local and remote configs. Issue [#365](https://github.com/lets-cli/lets/issues/365) * `[Added]` Remote config support: `lets -c https://url` downloads and caches config to `~/.config/lets/remote-configs/`. Use `--no-cache` to force re-download. * `[Added]` Add `lets self skills` commands to show, install, and update the bundled lets agent skill. * `[Docs]` Document the bundled lets Agent Skill and link it from the config reference. diff --git a/internal/cli/cli.go b/internal/cli/cli.go index c9be4f3e..b8f499f4 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -90,6 +90,10 @@ func Main(version string, buildDate string) int { ))) } + if rootFlags.noCache { + loadOptions = append(loadOptions, loader.WithNoCache()) + } + if isRemoteURL(rootFlags.config) { if configDir != "" { log.Warnf("LETS_CONFIG_DIR is ignored when using a remote config URL") diff --git a/internal/cmd/root.go b/internal/cmd/root.go index cfa89cca..e354ce19 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -105,8 +105,8 @@ func initRootFlags(rootCmd *cobra.Command) { rootCmd.Flags().StringArray("exclude", []string{}, "run all but excluded command(s) described in cmd as map") rootCmd.Flags().Bool("init", false, "create a new lets.yaml in the current folder") rootCmd.Flags().Bool("no-depends", false, "skip 'depends' for running command") - rootCmd.Flags().CountP("debug", "d", "show debug logs (or use LETS_DEBUG=1). If used multiple times, shows more verbose logs") //nolint:lll + rootCmd.Flags().CountP("debug", "d", "show debug logs; repeat for verbose output") rootCmd.Flags().StringP("config", "c", "", "config file (default is lets.yaml)") rootCmd.Flags().Bool("all", false, "show all commands (including the ones with _)") - rootCmd.Flags().Bool("no-cache", false, "re-download remote config instead of using cached version") + rootCmd.Flags().Bool("no-cache", false, "re-download remote config and mixins instead of using cached versions") } diff --git a/internal/cmd/testdata/TestHelpGolden/basic.golden b/internal/cmd/testdata/TestHelpGolden/basic.golden index 29ebbdba..22a98d69 100644 --- a/internal/cmd/testdata/TestHelpGolden/basic.golden +++ b/internal/cmd/testdata/TestHelpGolden/basic.golden @@ -16,12 +16,12 @@ INTERNAL COMMANDS: FLAGS --all Show all commands (including the ones with _) -c --config Config file (default is lets.yaml) - -d --debug Show debug logs (or use LETS_DEBUG=1). If used multiple times, shows more verbose logs + -d --debug Show debug logs; repeat for verbose output -E --env Set env variable for running command KEY=VALUE --exclude Run all but excluded command(s) described in cmd as map -h --help Help for lets --init Create a new lets.yaml in the current folder - --no-cache Re-Download remote config instead of using cached version + --no-cache Re-Download remote config and mixins instead of using cached versions --no-depends Skip 'depends' for running command --only Run only specified command(s) described in cmd as map -v --version Version for lets diff --git a/internal/cmd/testdata/TestHelpGolden/grouped_commands.golden b/internal/cmd/testdata/TestHelpGolden/grouped_commands.golden index f98be8de..2cc7a1b2 100644 --- a/internal/cmd/testdata/TestHelpGolden/grouped_commands.golden +++ b/internal/cmd/testdata/TestHelpGolden/grouped_commands.golden @@ -21,12 +21,12 @@ INTERNAL COMMANDS: FLAGS --all Show all commands (including the ones with _) -c --config Config file (default is lets.yaml) - -d --debug Show debug logs (or use LETS_DEBUG=1). If used multiple times, shows more verbose logs + -d --debug Show debug logs; repeat for verbose output -E --env Set env variable for running command KEY=VALUE --exclude Run all but excluded command(s) described in cmd as map -h --help Help for lets --init Create a new lets.yaml in the current folder - --no-cache Re-Download remote config instead of using cached version + --no-cache Re-Download remote config and mixins instead of using cached versions --no-depends Skip 'depends' for running command --only Run only specified command(s) described in cmd as map -v --version Version for lets diff --git a/internal/cmd/testdata/TestHelpGolden/grouped_commands_long.golden b/internal/cmd/testdata/TestHelpGolden/grouped_commands_long.golden index d546f8ea..8dca1623 100644 --- a/internal/cmd/testdata/TestHelpGolden/grouped_commands_long.golden +++ b/internal/cmd/testdata/TestHelpGolden/grouped_commands_long.golden @@ -22,12 +22,12 @@ INTERNAL COMMANDS: FLAGS --all Show all commands (including the ones with _) -c --config Config file (default is lets.yaml) - -d --debug Show debug logs (or use LETS_DEBUG=1). If used multiple times, shows more verbose logs + -d --debug Show debug logs; repeat for verbose output -E --env Set env variable for running command KEY=VALUE --exclude Run all but excluded command(s) described in cmd as map -h --help Help for lets --init Create a new lets.yaml in the current folder - --no-cache Re-Download remote config instead of using cached version + --no-cache Re-Download remote config and mixins instead of using cached versions --no-depends Skip 'depends' for running command --only Run only specified command(s) described in cmd as map -v --version Version for lets diff --git a/internal/cmd/testdata/TestHelpGolden/long_command.golden b/internal/cmd/testdata/TestHelpGolden/long_command.golden index 78e3b850..effd022f 100644 --- a/internal/cmd/testdata/TestHelpGolden/long_command.golden +++ b/internal/cmd/testdata/TestHelpGolden/long_command.golden @@ -17,12 +17,12 @@ INTERNAL COMMANDS: FLAGS --all Show all commands (including the ones with _) -c --config Config file (default is lets.yaml) - -d --debug Show debug logs (or use LETS_DEBUG=1). If used multiple times, shows more verbose logs + -d --debug Show debug logs; repeat for verbose output -E --env Set env variable for running command KEY=VALUE --exclude Run all but excluded command(s) described in cmd as map -h --help Help for lets --init Create a new lets.yaml in the current folder - --no-cache Re-Download remote config instead of using cached version + --no-cache Re-Download remote config and mixins instead of using cached versions --no-depends Skip 'depends' for running command --only Run only specified command(s) described in cmd as map -v --version Version for lets diff --git a/internal/config/config/config.go b/internal/config/config/config.go index 1cfd0cd0..ed9c82fa 100644 --- a/internal/config/config/config.go +++ b/internal/config/config/config.go @@ -14,6 +14,7 @@ import ( "github.com/lets-cli/lets/internal/fetch" "github.com/lets-cli/lets/internal/set" "github.com/lets-cli/lets/internal/util" + log "github.com/sirupsen/logrus" "gopkg.in/yaml.v3" ) @@ -57,12 +58,14 @@ type Config struct { cachedEnv map[string]string downloadContext context.Context downloadProgress fetch.ProgressObserver + noCache bool isMixin bool // if true, we consider config as mixin and apply different parsing and validation } -func (c *Config) SetDownloadOptions(ctx context.Context, progress fetch.ProgressObserver) { +func (c *Config) SetDownloadOptions(ctx context.Context, progress fetch.ProgressObserver, noCache bool) { c.downloadContext = ctx c.downloadProgress = progress + c.noCache = noCache } func (c *Config) context() context.Context { @@ -236,18 +239,44 @@ func (c *Config) readMixin(mixin *Mixin) error { rm := mixin.Remote - data, err := rm.tryRead() - if err != nil { - return err - } + var ( + data []byte + err error + downloaded bool + ) - if data == nil { - data, err = rm.download(c.context(), c.downloadProgress) + if !c.noCache { + data, err = rm.tryRead() if err != nil { return err } } + if data == nil { + downloadedData, downloadErr := rm.download(c.context(), c.downloadProgress) + if downloadErr != nil { + if c.noCache { + cachedData, readErr := rm.tryRead() + if readErr != nil { + return readErr + } + + if cachedData != nil { + log.Warnf("failed to download remote mixin (%v), falling back to cached version", downloadErr) + + data = cachedData + } else { + return downloadErr + } + } else { + return downloadErr + } + } else { + data = downloadedData + downloaded = true + } + } + // TODO: what if multiple mixins have same commands // 1 option - fail and suggest use to namespace all commands in remote mixin // 2 option - namespace it (this may require specifying namespace in mixin config or in main config mixin section) @@ -263,8 +292,10 @@ func (c *Config) readMixin(mixin *Mixin) error { return fmt.Errorf("failed to merge remote mixin config '%s' with main config: %w", rm.URL, err) } - if err := rm.persist(data); err != nil { - return fmt.Errorf("failed to persist remote mixin config %s: %w", rm.URL, err) + if downloaded { + if err := rm.persist(data); err != nil { + return fmt.Errorf("failed to persist remote mixin config %s: %w", rm.URL, err) + } } } else { mixinAbsPath, err := path.GetFullConfigPath(mixin.FileName, c.WorkDir) @@ -373,7 +404,7 @@ func NewConfig(workDir string, configAbsPath string, dotLetsDir string) *Config func NewMixinConfig(cfg *Config, configAbsPath string) *Config { mixin := NewConfig(cfg.WorkDir, configAbsPath, cfg.DotLetsDir) mixin.isMixin = true - mixin.SetDownloadOptions(cfg.context(), cfg.downloadProgress) + mixin.SetDownloadOptions(cfg.context(), cfg.downloadProgress, cfg.noCache) return mixin } diff --git a/internal/config/config/mixin.go b/internal/config/config/mixin.go index 77e5fc8e..26166857 100644 --- a/internal/config/config/mixin.go +++ b/internal/config/config/mixin.go @@ -47,10 +47,11 @@ func (rm *RemoteMixin) Path() string { } func (rm *RemoteMixin) persist(data []byte) error { - f, err := os.OpenFile(rm.Path(), os.O_CREATE|os.O_WRONLY, 0o755) //nolint:nosnakecase + f, err := os.OpenFile(rm.Path(), os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o644) //nolint:nosnakecase if err != nil { return fmt.Errorf("can not open file %s to persist mixin: %w", rm.Path(), err) } + defer f.Close() _, err = f.Write(data) if err != nil { diff --git a/internal/config/load.go b/internal/config/load.go index baf87851..74aafb13 100644 --- a/internal/config/load.go +++ b/internal/config/load.go @@ -18,6 +18,7 @@ import ( type loadOptions struct { progress fetch.ProgressObserver + noCache bool } type LoadOption func(*loadOptions) @@ -28,6 +29,12 @@ func WithProgress(progress fetch.ProgressObserver) LoadOption { } } +func WithNoCache() LoadOption { + return func(opts *loadOptions) { + opts.noCache = true + } +} + func newLoadOptions(options []LoadOption) loadOptions { opts := loadOptions{} for _, option := range options { @@ -56,6 +63,9 @@ func LoadWithContext(ctx context.Context, configName string, configDir string, v // returns a Config with the working directory set to the caller's CWD. func LoadRemote(ctx context.Context, url string, noCache bool, version string, options ...LoadOption) (*config.Config, error) { opts := newLoadOptions(options) + if noCache { + opts.noCache = true + } cachedPath, err := ensureRemoteConfig(ctx, url, noCache, opts.progress) if err != nil { @@ -100,7 +110,7 @@ func loadConfigFromFile( defer f.Close() c := config.NewConfig(workDir, absPath, dotLetsDir) - c.SetDownloadOptions(ctx, opts.progress) + c.SetDownloadOptions(ctx, opts.progress, opts.noCache) if err := yaml.NewDecoder(f).Decode(c); err != nil { return nil, fmt.Errorf("failed to parse %s: %w", displayName, err) diff --git a/internal/config/load_test.go b/internal/config/load_test.go index 978c8432..92c7cdf0 100644 --- a/internal/config/load_test.go +++ b/internal/config/load_test.go @@ -208,7 +208,9 @@ func TestLoadRemote(t *testing.T) { t.Run("reports progress for remote mixin cache miss only", func(t *testing.T) { mixinConfig := "commands:\n mixed:\n cmd: echo mixed\n" + requests := 0 srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests++ w.Header().Set("Content-Type", "application/yaml") _, _ = w.Write([]byte(mixinConfig)) })) @@ -238,5 +240,73 @@ func TestLoadRemote(t *testing.T) { if len(cacheHitProgress.starts) != 0 { t.Fatalf("expected no progress starts for remote mixin cache hit, got %d", len(cacheHitProgress.starts)) } + if requests != 1 { + t.Fatalf("expected 1 HTTP request, got %d", requests) + } + }) + + t.Run("re-downloads remote mixins with --no-cache", func(t *testing.T) { + requests := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests++ + w.Header().Set("Content-Type", "application/yaml") + _, _ = w.Write([]byte("commands:\n mixed:\n cmd: echo mixed\n")) + })) + defer srv.Close() + + tempDir := t.TempDir() + mainConfig := "shell: bash\nmixins:\n - url: " + srv.URL + "\ncommands:\n ok:\n cmd: echo ok\n" + if err := os.WriteFile(filepath.Join(tempDir, "lets.yaml"), []byte(mainConfig), 0o644); err != nil { + t.Fatalf("write config: %v", err) + } + + if _, err := LoadWithContext(ctx, "", tempDir, "0.0.0-test"); err != nil { + t.Fatalf("prime cache error: %v", err) + } + progress := &recordingProgress{} + if _, err := LoadWithContext(ctx, "", tempDir, "0.0.0-test", WithNoCache(), WithProgress(progress)); err != nil { + t.Fatalf("no-cache load error: %v", err) + } + if requests != 2 { + t.Fatalf("expected 2 HTTP requests, got %d", requests) + } + if len(progress.starts) != 1 { + t.Fatalf("expected 1 progress start, got %d", len(progress.starts)) + } + if progress.starts[0].Kind != fetch.SourceRemoteMixin { + t.Fatalf("expected remote mixin progress, got %q", progress.starts[0].Kind) + } + }) + + t.Run("does not replace remote mixin cache until downloaded mixin is valid", func(t *testing.T) { + mixinConfig := "commands:\n mixed:\n cmd: echo mixed\n" + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/yaml") + _, _ = w.Write([]byte(mixinConfig)) + })) + defer srv.Close() + + tempDir := t.TempDir() + mainConfig := "shell: bash\nmixins:\n - url: " + srv.URL + "\ncommands:\n ok:\n cmd: echo ok\n" + if err := os.WriteFile(filepath.Join(tempDir, "lets.yaml"), []byte(mainConfig), 0o644); err != nil { + t.Fatalf("write config: %v", err) + } + + if _, err := LoadWithContext(ctx, "", tempDir, "0.0.0-test"); err != nil { + t.Fatalf("prime cache error: %v", err) + } + + mixinConfig = "commands:\n broken:\n xxx\n cmd: echo broken\n" + if _, err := LoadWithContext(ctx, "", tempDir, "0.0.0-test", WithNoCache()); err == nil { + t.Fatal("expected no-cache load to fail for invalid downloaded mixin") + } + + cfg, err := LoadWithContext(ctx, "", tempDir, "0.0.0-test") + if err != nil { + t.Fatalf("cached load error: %v", err) + } + if _, ok := cfg.Commands["mixed"]; !ok { + t.Fatal("expected valid cached mixin command") + } }) }