From 7fd90c21439ad598d9f0fcc68b689951e03bbeee Mon Sep 17 00:00:00 2001 From: "m.kindritskiy" Date: Sun, 14 Jun 2026 22:19:09 +0300 Subject: [PATCH] Add download progress for lets self upgrade Wire a fetch.ProgressObserver through the registry download so the tar.gz stream reports byte progress while extracting, and disable HTTP compression so Content-Length stays usable for the bar. The upgrade command builds its own observer from cobra's stderr stream (cmd.ErrOrStderr) and the already-loaded settings value, rather than threading the observer down from the composition root. A fetch.NopObserver replaces nil as the no-progress default, removing nil checks across the download path. Unify terminal detection behind util.IsTerminalWriter (charmbracelet/x/term), used by cli, the upgrade command, and downloadprogress. This drops the direct mattn/go-isatty dependency; Cygwin/MSYS terminals are no longer treated as interactive, matching what logging and error rendering already did. --- docs/docs/changelog.md | 3 + go.mod | 4 +- internal/cli/cli.go | 15 ++- internal/cli/cli_test.go | 4 +- internal/cmd/help_golden_test.go | 3 +- internal/cmd/root_test.go | 19 ++-- internal/cmd/self.go | 13 ++- internal/cmd/self_skills_test.go | 13 +-- internal/cmd/upgrade.go | 31 +++++- internal/config/config/config.go | 16 +-- internal/fetch/fetch.go | 15 +++ .../progress.go | 6 +- .../progress_test.go | 2 +- internal/upgrade/notifier_test.go | 9 +- internal/upgrade/registry/registry.go | 43 +++++++- internal/upgrade/registry/registry_test.go | 101 ++++++++++++++++++ internal/upgrade/upgrade.go | 24 ++++- internal/upgrade/upgrade_test.go | 9 +- internal/util/terminal.go | 15 +++ 19 files changed, 287 insertions(+), 58 deletions(-) rename internal/{downloadprogress => progressbar}/progress.go (99%) rename internal/{downloadprogress => progressbar}/progress_test.go (99%) create mode 100644 internal/util/terminal.go diff --git a/docs/docs/changelog.md b/docs/docs/changelog.md index 4727de30..f50b0b88 100644 --- a/docs/docs/changelog.md +++ b/docs/docs/changelog.md @@ -6,6 +6,9 @@ title: Changelog ## [Unreleased](https://github.com/lets-cli/lets/releases/tag/v0.0.X) * `[Docs]` Convert design specs into ADRs. +* `[Refactoring]` Rename the internal download progress package to progressbar. +* `[Added]` Show interactive download progress for `lets self upgrade`. Issue [#367](https://github.com/lets-cli/lets/issues/367) +* `[Changed]` Unify terminal detection across the CLI; Cygwin/MSYS terminals are no longer treated as interactive (drops the direct `mattn/go-isatty` dependency). * `[Added]` Add `lets self config path` and `lets self config edit` for user settings. Issue [#370](https://github.com/lets-cli/lets/issues/370) * `[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) diff --git a/go.mod b/go.mod index f47d252d..4aa5c593 100644 --- a/go.mod +++ b/go.mod @@ -6,6 +6,7 @@ toolchain go1.26.0 require ( charm.land/bubbles/v2 v2.1.0 + charm.land/bubbletea/v2 v2.0.2 charm.land/lipgloss/v2 v2.0.2 github.com/charmbracelet/colorprofile v0.4.2 github.com/charmbracelet/x/ansi v0.11.6 @@ -17,7 +18,6 @@ require ( github.com/fatih/color v1.16.0 github.com/kindermax/docopt.go v0.8.0 github.com/lets-cli/fang v0.1.0 - github.com/mattn/go-isatty v0.0.20 github.com/odvcencio/gotreesitter v0.12.1 github.com/pkg/errors v0.9.1 github.com/sirupsen/logrus v1.9.3 @@ -28,7 +28,6 @@ require ( ) require ( - charm.land/bubbletea/v2 v2.0.2 // indirect github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect github.com/aymanbagabas/go-udiff v0.4.1 // indirect github.com/charmbracelet/harmonica v0.2.0 // indirect @@ -41,6 +40,7 @@ require ( github.com/iancoleman/strcase v0.3.0 // indirect github.com/lucasb-eyer/go-colorful v1.3.0 // indirect github.com/mattn/go-colorable v0.1.13 // indirect + github.com/mattn/go-isatty v0.0.20 // indirect github.com/mattn/go-runewidth v0.0.21 // indirect github.com/muesli/cancelreader v0.2.2 // indirect github.com/muesli/mango v0.1.0 // indirect diff --git a/internal/cli/cli.go b/internal/cli/cli.go index b8f499f4..680a02e3 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -14,16 +14,16 @@ import ( "github.com/lets-cli/lets/internal/cmd" loader "github.com/lets-cli/lets/internal/config" "github.com/lets-cli/lets/internal/config/config" - "github.com/lets-cli/lets/internal/downloadprogress" "github.com/lets-cli/lets/internal/env" "github.com/lets-cli/lets/internal/logging" + "github.com/lets-cli/lets/internal/progressbar" "github.com/lets-cli/lets/internal/set" "github.com/lets-cli/lets/internal/settings" "github.com/lets-cli/lets/internal/theme" "github.com/lets-cli/lets/internal/upgrade" "github.com/lets-cli/lets/internal/upgrade/registry" + "github.com/lets-cli/lets/internal/util" "github.com/lets-cli/lets/internal/workdir" - "github.com/mattn/go-isatty" log "github.com/sirupsen/logrus" "github.com/spf13/cobra" ) @@ -54,7 +54,7 @@ func Main(version string, buildDate string) int { rootCmd.InitDefaultHelpFlag() rootCmd.InitDefaultVersionFlag() reinitCompletionCmd := cmd.InitCompletionCmd(rootCmd, nil) - cmd.InitSelfCmd(rootCmd, version) + cmd.InitSelfCmd(rootCmd, version, appSettings) rootCmd.InitDefaultHelpCmd() command, args, err := rootCmd.Traverse(os.Args[1:]) @@ -83,10 +83,10 @@ func Main(version string, buildDate string) int { loadOptions := []loader.LoadOption{} if isInteractiveStderr() { - loadOptions = append(loadOptions, loader.WithProgress(downloadprogress.New( + loadOptions = append(loadOptions, loader.WithProgress(progressbar.New( os.Stderr, - downloadprogress.WithNoColor(appSettings.NoColor), - downloadprogress.WithTheme(appSettings.Theme), + progressbar.WithNoColor(appSettings.NoColor), + progressbar.WithTheme(appSettings.Theme), ))) } @@ -292,8 +292,7 @@ func shouldCheckForUpdate(command *cobra.Command, interactive bool, appSettings } func isInteractiveStderr() bool { - fd := os.Stderr.Fd() - return isatty.IsTerminal(fd) || isatty.IsCygwinTerminal(fd) + return util.IsTerminalWriter(os.Stderr) } func isRemoteURL(s string) bool { diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index 55199ffc..6fda4656 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -32,7 +32,7 @@ func TestAllowsMissingConfig(t *testing.T) { t.Run("self subcommand", func(t *testing.T) { root := cmdpkg.CreateRootCommand("v0.0.0-test", "") - cmdpkg.InitSelfCmd(root, "v0.0.0-test") + cmdpkg.InitSelfCmd(root, "v0.0.0-test", settings.Default()) command, _, err := root.Find([]string{"self", "doc"}) if err != nil { @@ -111,7 +111,7 @@ func TestShouldCheckForUpdate(t *testing.T) { t.Run("should skip self subcommands", func(t *testing.T) { root := cmdpkg.CreateRootCommand("v0.0.0-test", "") - cmdpkg.InitSelfCmd(root, "v0.0.0-test") + cmdpkg.InitSelfCmd(root, "v0.0.0-test", settings.Default()) for _, args := range [][]string{{"self"}, {"self", "doc"}, {"self", "upgrade"}} { command, _, err := root.Find(args) diff --git a/internal/cmd/help_golden_test.go b/internal/cmd/help_golden_test.go index 55769336..b7883da9 100644 --- a/internal/cmd/help_golden_test.go +++ b/internal/cmd/help_golden_test.go @@ -11,6 +11,7 @@ import ( "github.com/lets-cli/fang" "github.com/lets-cli/lets/internal/config" "github.com/lets-cli/lets/internal/executor" + "github.com/lets-cli/lets/internal/settings" "github.com/lets-cli/lets/internal/theme" "github.com/spf13/cobra" ) @@ -21,7 +22,7 @@ func newGoldenRoot(t *testing.T) *cobra.Command { root := CreateRootCommand("0.0.0-dev", "") root.InitDefaultHelpFlag() root.InitDefaultVersionFlag() - InitSelfCmd(root, "0.0.0-dev") + InitSelfCmd(root, "0.0.0-dev", settings.Default()) InitCompletionCmd(root, nil) root.InitDefaultHelpCmd() return root diff --git a/internal/cmd/root_test.go b/internal/cmd/root_test.go index c0bee6e4..ef556497 100644 --- a/internal/cmd/root_test.go +++ b/internal/cmd/root_test.go @@ -10,6 +10,7 @@ import ( "testing" "github.com/lets-cli/lets/internal/config/config" + "github.com/lets-cli/lets/internal/settings" "github.com/lets-cli/lets/internal/upgrade" "github.com/spf13/cobra" ) @@ -220,7 +221,7 @@ func TestSelfCmd(t *testing.T) { t.Run("should use help func when run without args", func(t *testing.T) { rootCmd := CreateRootCommand("v0.0.0-test", "") rootCmd.SetArgs([]string{"self"}) - initSelfCmd(rootCmd, "v0.0.0-test", func(string) error { return nil }, func(string) error { return nil }) + initSelfCmd(rootCmd, "v0.0.0-test", settings.Default(), func(string) error { return nil }, func(string) error { return nil }) called := false rootCmd.SetHelpFunc(func(c *cobra.Command, args []string) { @@ -247,7 +248,7 @@ func TestSelfCmd(t *testing.T) { rootCmd.SetArgs([]string{"self", "config", "path"}) rootCmd.SetOut(bufOut) rootCmd.SetErr(bufOut) - initSelfCmd(rootCmd, "v0.0.0-test", func(string) error { return nil }, func(string) error { return nil }) + initSelfCmd(rootCmd, "v0.0.0-test", settings.Default(), func(string) error { return nil }, func(string) error { return nil }) if err := rootCmd.Execute(); err != nil { t.Fatalf("unexpected error: %v", err) @@ -270,7 +271,7 @@ func TestSelfCmd(t *testing.T) { rootCmd.SetArgs([]string{"self", "config", "edit"}) rootCmd.SetOut(bufOut) rootCmd.SetErr(bufOut) - initSelfCmd(rootCmd, "v0.0.0-test", func(string) error { return nil }, func(path string) error { + initSelfCmd(rootCmd, "v0.0.0-test", settings.Default(), func(string) error { return nil }, func(path string) error { called = true gotPath = path @@ -316,7 +317,7 @@ func TestSelfCmd(t *testing.T) { rootCmd.SetArgs([]string{"self", "doc"}) rootCmd.SetOut(bufOut) rootCmd.SetErr(bufOut) - initSelfCmd(rootCmd, "v0.0.0-test", openURL, func(string) error { return nil }) + initSelfCmd(rootCmd, "v0.0.0-test", settings.Default(), openURL, func(string) error { return nil }) err := rootCmd.Execute() if err != nil { @@ -343,7 +344,7 @@ func TestSelfCmd(t *testing.T) { rootCmd.SetArgs([]string{"self", "doc"}) rootCmd.SetOut(bufOut) rootCmd.SetErr(bufOut) - initSelfCmd(rootCmd, "v0.0.0-test", openURL, func(string) error { return nil }) + initSelfCmd(rootCmd, "v0.0.0-test", settings.Default(), openURL, func(string) error { return nil }) err := rootCmd.Execute() if err == nil { @@ -362,7 +363,7 @@ func TestSelfCmd(t *testing.T) { rootCmd.SetArgs([]string{"self", "ls"}) rootCmd.SetOut(bufOut) rootCmd.SetErr(bufOut) - InitSelfCmd(rootCmd, "v0.0.0-test") + InitSelfCmd(rootCmd, "v0.0.0-test", settings.Default()) err := rootCmd.Execute() if err == nil { @@ -395,7 +396,7 @@ func TestSelfCmd(t *testing.T) { rootCmd.SetArgs([]string{"self", "zzzznotacommand"}) rootCmd.SetOut(bufOut) rootCmd.SetErr(bufOut) - InitSelfCmd(rootCmd, "v0.0.0-test") + InitSelfCmd(rootCmd, "v0.0.0-test", settings.Default()) err := rootCmd.Execute() if err == nil { @@ -431,7 +432,7 @@ func TestSelfCmd(t *testing.T) { } rootCmd.AddCommand(selfCmd) - selfCmd.AddCommand(initUpgradeCommandWith(func() (upgrade.Upgrader, error) { + selfCmd.AddCommand(initUpgradeCommandWith(func(_ *cobra.Command) (upgrade.Upgrader, error) { return mockUpgraderFunc(func(ctx context.Context) error { called = true @@ -462,7 +463,7 @@ func TestSelfCmd(t *testing.T) { } rootCmd.AddCommand(selfCmd) - selfCmd.AddCommand(initUpgradeCommandWith(func() (upgrade.Upgrader, error) { + selfCmd.AddCommand(initUpgradeCommandWith(func(_ *cobra.Command) (upgrade.Upgrader, error) { return mockUpgraderFunc(func(ctx context.Context) error { return errors.New("upgrade failed") }), nil diff --git a/internal/cmd/self.go b/internal/cmd/self.go index 6bf4c1f9..88bd1841 100644 --- a/internal/cmd/self.go +++ b/internal/cmd/self.go @@ -1,17 +1,22 @@ package cmd import ( + "github.com/lets-cli/lets/internal/settings" "github.com/lets-cli/lets/internal/util" "github.com/spf13/cobra" ) // InitSelfCmd intializes root 'self' subcommand. -func InitSelfCmd(rootCmd *cobra.Command, version string) { - initSelfCmd(rootCmd, version, util.OpenURL, util.OpenEditor) +func InitSelfCmd(rootCmd *cobra.Command, version string, appSettings settings.Settings) { + initSelfCmd(rootCmd, version, appSettings, util.OpenURL, util.OpenEditor) } func initSelfCmd( - rootCmd *cobra.Command, version string, openURL func(string) error, openEditor func(string) error, + rootCmd *cobra.Command, + version string, + appSettings settings.Settings, + openURL func(string) error, + openEditor func(string) error, ) { selfCmd := &cobra.Command{ Use: "self", @@ -30,5 +35,5 @@ func initSelfCmd( selfCmd.AddCommand(initDocCommand(openURL)) selfCmd.AddCommand(initLspCommand(version)) selfCmd.AddCommand(initSkillsCommand()) - selfCmd.AddCommand(initUpgradeCommand(version)) + selfCmd.AddCommand(initUpgradeCommand(version, appSettings)) } diff --git a/internal/cmd/self_skills_test.go b/internal/cmd/self_skills_test.go index 09a29a30..fdc57ad4 100644 --- a/internal/cmd/self_skills_test.go +++ b/internal/cmd/self_skills_test.go @@ -7,6 +7,7 @@ import ( "strings" "testing" + "github.com/lets-cli/lets/internal/settings" skillpkg "github.com/lets-cli/lets/internal/skills" ) @@ -17,7 +18,7 @@ func TestSelfSkillsCmd(t *testing.T) { rootCmd.SetArgs([]string{"self", "skills", "show"}) rootCmd.SetOut(bufOut) rootCmd.SetErr(new(bytes.Buffer)) - InitSelfCmd(rootCmd, "v0.0.0-test") + InitSelfCmd(rootCmd, "v0.0.0-test", settings.Default()) if err := rootCmd.Execute(); err != nil { t.Fatalf("unexpected error: %v", err) @@ -37,7 +38,7 @@ func TestSelfSkillsCmd(t *testing.T) { rootCmd.SetIn(strings.NewReader("local\n")) rootCmd.SetOut(bufOut) rootCmd.SetErr(bufErr) - InitSelfCmd(rootCmd, "v0.0.0-test") + InitSelfCmd(rootCmd, "v0.0.0-test", settings.Default()) if err := rootCmd.Execute(); err != nil { t.Fatalf("unexpected error: %v", err) @@ -65,7 +66,7 @@ func TestSelfSkillsCmd(t *testing.T) { rootCmd.SetIn(strings.NewReader("global\n")) rootCmd.SetOut(bufOut) rootCmd.SetErr(new(bytes.Buffer)) - InitSelfCmd(rootCmd, "v0.0.0-test") + InitSelfCmd(rootCmd, "v0.0.0-test", settings.Default()) if err := rootCmd.Execute(); err != nil { t.Fatalf("unexpected error: %v", err) @@ -83,7 +84,7 @@ func TestSelfSkillsCmd(t *testing.T) { rootCmd.SetIn(strings.NewReader("2\n")) rootCmd.SetOut(new(bytes.Buffer)) rootCmd.SetErr(new(bytes.Buffer)) - InitSelfCmd(rootCmd, "v0.0.0-test") + InitSelfCmd(rootCmd, "v0.0.0-test", settings.Default()) if err := rootCmd.Execute(); err != nil { t.Fatalf("unexpected error: %v", err) @@ -108,7 +109,7 @@ func TestSelfSkillsCmd(t *testing.T) { rootCmd.SetArgs([]string{"self", "skills", "install", "--local"}) rootCmd.SetOut(bufOut) rootCmd.SetErr(new(bytes.Buffer)) - InitSelfCmd(rootCmd, "v0.0.0-test") + InitSelfCmd(rootCmd, "v0.0.0-test", settings.Default()) if err := rootCmd.Execute(); err != nil { t.Fatalf("unexpected error: %v", err) @@ -142,7 +143,7 @@ func TestSelfSkillsCmd(t *testing.T) { rootCmd.SetArgs([]string{"self", "skills", "update"}) rootCmd.SetOut(bufOut) rootCmd.SetErr(new(bytes.Buffer)) - InitSelfCmd(rootCmd, "v0.0.0-test") + InitSelfCmd(rootCmd, "v0.0.0-test", settings.Default()) if err := rootCmd.Execute(); err != nil { t.Fatalf("unexpected error: %v", err) diff --git a/internal/cmd/upgrade.go b/internal/cmd/upgrade.go index 741db149..1d4d1047 100644 --- a/internal/cmd/upgrade.go +++ b/internal/cmd/upgrade.go @@ -2,27 +2,48 @@ package cmd import ( "fmt" + "io" + "github.com/lets-cli/lets/internal/fetch" + "github.com/lets-cli/lets/internal/progressbar" + "github.com/lets-cli/lets/internal/settings" "github.com/lets-cli/lets/internal/upgrade" "github.com/lets-cli/lets/internal/upgrade/registry" + "github.com/lets-cli/lets/internal/util" "github.com/spf13/cobra" ) -type upgraderFactory func() (upgrade.Upgrader, error) +type upgraderFactory func(cmd *cobra.Command) (upgrade.Upgrader, error) -func initUpgradeCommand(version string) *cobra.Command { - return initUpgradeCommandWith(func() (upgrade.Upgrader, error) { - return upgrade.NewBinaryUpgrader(registry.NewGithubRegistry(), version) +func initUpgradeCommand(version string, appSettings settings.Settings) *cobra.Command { + return initUpgradeCommandWith(func(cmd *cobra.Command) (upgrade.Upgrader, error) { + progress := upgradeProgress(cmd.ErrOrStderr(), appSettings) + + return upgrade.NewBinaryUpgrader(registry.NewGithubRegistry(), version, upgrade.WithProgress(progress)) }) } +// upgradeProgress builds a progress observer for the upgrade download, or a +// no-op observer when the stream is not an interactive terminal. +func upgradeProgress(stderr io.Writer, appSettings settings.Settings) fetch.ProgressObserver { //nolint:ireturn // Returns NopObserver or *progressbar.Observer. + if !util.IsTerminalWriter(stderr) { + return fetch.NopObserver{} + } + + return progressbar.New( + stderr, + progressbar.WithNoColor(appSettings.NoColor), + progressbar.WithTheme(appSettings.Theme), + ) +} + func initUpgradeCommandWith(createUpgrader upgraderFactory) *cobra.Command { upgradeCmd := &cobra.Command{ Use: "upgrade", Short: "Upgrade lets to latest version", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { - upgrader, err := createUpgrader() + upgrader, err := createUpgrader(cmd) if err != nil { return fmt.Errorf("can not self-upgrade binary: %w", err) } diff --git a/internal/config/config/config.go b/internal/config/config/config.go index 9e377b07..6d893d1b 100644 --- a/internal/config/config/config.go +++ b/internal/config/config/config.go @@ -55,16 +55,16 @@ type Config struct { RemoteSource string // cached env after config.SetupEnv, used in config.GetEnv - 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 + cachedEnv map[string]string + downloadContext context.Context + progressBar 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, noCache bool) { c.downloadContext = ctx - c.downloadProgress = progress + c.progressBar = progress c.noCache = noCache } @@ -253,7 +253,7 @@ func (c *Config) readMixin(mixin *Mixin) error { } if data == nil { - downloadedData, downloadErr := rm.download(c.context(), c.downloadProgress) + downloadedData, downloadErr := rm.download(c.context(), c.progressBar) if downloadErr != nil { // Always fall back to cached version on download failure, consistent with // ensureRemoteConfig's behavior for the top-level remote config. @@ -402,7 +402,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, cfg.noCache) + mixin.SetDownloadOptions(cfg.context(), cfg.progressBar, cfg.noCache) return mixin } diff --git a/internal/fetch/fetch.go b/internal/fetch/fetch.go index 9110495d..7ce9656c 100644 --- a/internal/fetch/fetch.go +++ b/internal/fetch/fetch.go @@ -40,6 +40,7 @@ type SourceKind string const ( SourceRemoteConfig SourceKind = "remote config" SourceRemoteMixin SourceKind = "remote mixin" + SourceSelfUpdate SourceKind = "self update" ) type ProgressInfo struct { @@ -165,3 +166,17 @@ func (r progressReader) Read(p []byte) (int, error) { return n, err } + +// NopObserver is a no-op ProgressObserver. Use it instead of nil to avoid +// nil checks throughout the call chain. +type NopObserver struct{} + +func (NopObserver) Start(ProgressInfo) ProgressTracker { //nolint:ireturn // Implements ProgressObserver. + return NopTracker{} +} + +// NopTracker is a no-op ProgressTracker returned by NopObserver. +type NopTracker struct{} + +func (NopTracker) Add(int64) {} +func (NopTracker) Done(error) {} diff --git a/internal/downloadprogress/progress.go b/internal/progressbar/progress.go similarity index 99% rename from internal/downloadprogress/progress.go rename to internal/progressbar/progress.go index 9d1360b1..9dafc0d9 100644 --- a/internal/downloadprogress/progress.go +++ b/internal/progressbar/progress.go @@ -1,4 +1,4 @@ -package downloadprogress +package progressbar import ( "fmt" @@ -17,6 +17,7 @@ import ( "github.com/charmbracelet/x/term" "github.com/lets-cli/lets/internal/fetch" "github.com/lets-cli/lets/internal/theme" + "github.com/lets-cli/lets/internal/util" ) const ( @@ -474,8 +475,7 @@ func detectWidth(writer io.Writer) int { } func isTerminal(writer io.Writer) bool { - file, ok := writer.(term.File) - return ok && term.IsTerminal(file.Fd()) + return util.IsTerminalWriter(writer) } func applyProgressColors(model *bubblesprogress.Model, fill, empty color.Color) { diff --git a/internal/downloadprogress/progress_test.go b/internal/progressbar/progress_test.go similarity index 99% rename from internal/downloadprogress/progress_test.go rename to internal/progressbar/progress_test.go index e0b16ff6..6ba46c6b 100644 --- a/internal/downloadprogress/progress_test.go +++ b/internal/progressbar/progress_test.go @@ -1,4 +1,4 @@ -package downloadprogress +package progressbar import ( "bytes" diff --git a/internal/upgrade/notifier_test.go b/internal/upgrade/notifier_test.go index 1a21211c..613a1f5b 100644 --- a/internal/upgrade/notifier_test.go +++ b/internal/upgrade/notifier_test.go @@ -7,6 +7,7 @@ import ( "testing" "time" + "github.com/lets-cli/lets/internal/fetch" "github.com/lets-cli/lets/internal/upgrade/registry" ) @@ -29,7 +30,13 @@ func (m *mockNotifierRegistry) GetLatestRelease(ctx context.Context) (string, er return m.release.TagName, nil } -func (m *mockNotifierRegistry) DownloadReleaseBinary(ctx context.Context, packageName string, version string, dstPath string) error { +func (m *mockNotifierRegistry) DownloadReleaseBinary( + ctx context.Context, + packageName string, + version string, + dstPath string, + progress fetch.ProgressObserver, +) error { return nil } diff --git a/internal/upgrade/registry/registry.go b/internal/upgrade/registry/registry.go index da6ec89c..fb4c716f 100644 --- a/internal/upgrade/registry/registry.go +++ b/internal/upgrade/registry/registry.go @@ -11,6 +11,7 @@ import ( "time" "github.com/codeclysm/extract" + "github.com/lets-cli/lets/internal/fetch" ) var archAdaptMap = map[string]string{ @@ -27,7 +28,13 @@ var osMap = map[string]string{ type RepoRegistry interface { GetLatestReleaseInfo(ctx context.Context) (*ReleaseInfo, error) GetLatestRelease(ctx context.Context) (string, error) - DownloadReleaseBinary(ctx context.Context, packageName string, version string, dstPath string) error + DownloadReleaseBinary( + ctx context.Context, + packageName string, + version string, + dstPath string, + progress fetch.ProgressObserver, + ) error GetPackageName(os string, arch string) (string, error) GetDownloadURL(repoURI string, packageName string, version string) string } @@ -42,8 +49,12 @@ type GithubRegistry struct { } func NewGithubRegistry() *GithubRegistry { + transport := http.DefaultTransport.(*http.Transport).Clone() + transport.DisableCompression = true + client := &http.Client{ - Timeout: 15 * 60 * time.Second, // global timeout + Timeout: 15 * 60 * time.Second, // global timeout + Transport: transport, } reg := &GithubRegistry{ @@ -78,6 +89,7 @@ func (reg *GithubRegistry) DownloadReleaseBinary( packageName string, version string, dstPath string, + progress fetch.ProgressObserver, ) error { downloadURL := reg.GetDownloadURL(reg.repoURI, packageName+".tar.gz", version) @@ -125,9 +137,17 @@ func (reg *GithubRegistry) DownloadReleaseBinary( } } - // TODO add download progress bar // TODO drop extract dependency, replace with own code - err = extract.Gz(ctx, resp.Body, dstDir, nil) + tracker := progress.Start(fetch.ProgressInfo{ + Kind: fetch.SourceSelfUpdate, + URL: downloadURL, + TotalBytes: resp.ContentLength, + }) + reader := progressReadCloser{ReadCloser: resp.Body, tracker: tracker} + + err = extract.Gz(ctx, reader, dstDir, nil) + tracker.Done(err) + if err != nil { return fmt.Errorf("failed to extract package: %w", err) } @@ -141,6 +161,21 @@ func (reg *GithubRegistry) DownloadReleaseBinary( return nil } +type progressReadCloser struct { + io.ReadCloser + + tracker fetch.ProgressTracker +} + +func (r progressReadCloser) Read(p []byte) (int, error) { + n, err := r.ReadCloser.Read(p) + if n > 0 { + r.tracker.Add(int64(n)) + } + + return n, err +} + type ReleaseInfo struct { TagName string `json:"tag_name"` PublishedAt time.Time `json:"published_at"` diff --git a/internal/upgrade/registry/registry_test.go b/internal/upgrade/registry/registry_test.go index 2aed645c..4b2bcbf5 100644 --- a/internal/upgrade/registry/registry_test.go +++ b/internal/upgrade/registry/registry_test.go @@ -1,12 +1,39 @@ package registry import ( + "archive/tar" + "bytes" + "compress/gzip" "context" "net/http" "net/http/httptest" + "os" + "path/filepath" + "strconv" "testing" + + "github.com/lets-cli/lets/internal/fetch" ) +type recordingProgress struct { + starts []fetch.ProgressInfo + adds []int64 + dones []error +} + +func (p *recordingProgress) Start(info fetch.ProgressInfo) fetch.ProgressTracker { + p.starts = append(p.starts, info) + return p +} + +func (p *recordingProgress) Add(n int64) { + p.adds = append(p.adds, n) +} + +func (p *recordingProgress) Done(err error) { + p.dones = append(p.dones, err) +} + func TestGithubRegistryGetLatestReleaseInfo(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if got := r.URL.Path; got != "/releases/latest" { @@ -54,3 +81,77 @@ func TestGithubRegistryGetLatestRelease(t *testing.T) { t.Fatalf("expected version v0.0.59, got %q", version) } } + +func TestGithubRegistryDownloadReleaseBinaryReportsProgress(t *testing.T) { + archive := releaseArchive(t, "updated binary") + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if got := r.URL.Path; got != "/releases/download/v0.0.2/lets_Test_x86_64.tar.gz" { + t.Fatalf("unexpected path %q", got) + } + + w.Header().Set("Content-Type", "application/gzip") + w.Header().Set("Content-Length", strconv.Itoa(len(archive))) + _, _ = w.Write(archive) + })) + defer server.Close() + + reg := NewGithubRegistry() + reg.repoURI = server.URL + + dstPath := filepath.Join(t.TempDir(), "lets") + progress := &recordingProgress{} + if err := reg.DownloadReleaseBinary(context.Background(), "lets_Test_x86_64", "v0.0.2", dstPath, progress); err != nil { + t.Fatalf("DownloadReleaseBinary() error = %v", err) + } + + data, err := os.ReadFile(dstPath) + if err != nil { + t.Fatalf("failed to read extracted binary: %v", err) + } + if string(data) != "updated binary" { + t.Fatalf("expected extracted binary, got %q", data) + } + + if len(progress.starts) != 1 { + t.Fatalf("expected one progress start, got %d", len(progress.starts)) + } + if progress.starts[0].Kind != fetch.SourceSelfUpdate { + t.Fatalf("expected self update progress, got %q", progress.starts[0].Kind) + } + if progress.starts[0].URL != server.URL+"/releases/download/v0.0.2/lets_Test_x86_64.tar.gz" { + t.Fatalf("unexpected progress URL %q", progress.starts[0].URL) + } + if progress.starts[0].TotalBytes != int64(len(archive)) { + t.Fatalf("expected total bytes %d, got %d", len(archive), progress.starts[0].TotalBytes) + } + if len(progress.adds) == 0 { + t.Fatal("expected progress byte updates") + } + if len(progress.dones) != 1 || progress.dones[0] != nil { + t.Fatalf("expected successful progress done, got %#v", progress.dones) + } +} + +func releaseArchive(t *testing.T, content string) []byte { + t.Helper() + + var buf bytes.Buffer + gz := gzip.NewWriter(&buf) + tw := tar.NewWriter(gz) + + data := []byte(content) + if err := tw.WriteHeader(&tar.Header{Name: "lets", Mode: 0o755, Size: int64(len(data))}); err != nil { + t.Fatalf("failed to write tar header: %v", err) + } + if _, err := tw.Write(data); err != nil { + t.Fatalf("failed to write tar content: %v", err) + } + if err := tw.Close(); err != nil { + t.Fatalf("failed to close tar: %v", err) + } + if err := gz.Close(); err != nil { + t.Fatalf("failed to close gzip: %v", err) + } + + return buf.Bytes() +} diff --git a/internal/upgrade/upgrade.go b/internal/upgrade/upgrade.go index 0fe17830..bb1ec7d4 100644 --- a/internal/upgrade/upgrade.go +++ b/internal/upgrade/upgrade.go @@ -11,6 +11,7 @@ import ( "runtime" "strings" + "github.com/lets-cli/lets/internal/fetch" "github.com/lets-cli/lets/internal/upgrade/registry" log "github.com/sirupsen/logrus" ) @@ -25,22 +26,38 @@ type BinaryUpgrader struct { binaryPath string downloadPath string backupPath string + progress fetch.ProgressObserver } -func NewBinaryUpgrader(reg registry.RepoRegistry, currentVersion string) (*BinaryUpgrader, error) { +type BinaryUpgraderOption func(*BinaryUpgrader) + +func WithProgress(progress fetch.ProgressObserver) BinaryUpgraderOption { + return func(upgrader *BinaryUpgrader) { + upgrader.progress = progress + } +} + +func NewBinaryUpgrader(reg registry.RepoRegistry, currentVersion string, options ...BinaryUpgraderOption) (*BinaryUpgrader, error) { executablePath, err := binaryPath() if err != nil { return nil, err } - return &BinaryUpgrader{ + upgrader := &BinaryUpgrader{ registry: reg, currentVersion: currentVersion, // TODO rewrite all paths with home dir binaryPath: executablePath, downloadPath: path.Join(os.TempDir(), "lets.download"), backupPath: path.Join(os.TempDir(), "lets.backup"), - }, nil + progress: fetch.NopObserver{}, + } + + for _, option := range options { + option(upgrader) + } + + return upgrader, nil } func (up *BinaryUpgrader) Upgrade(ctx context.Context) error { @@ -72,6 +89,7 @@ func (up *BinaryUpgrader) Upgrade(ctx context.Context) error { packageName, latestVersion, up.downloadPath, + up.progress, ) if err != nil { return fmt.Errorf("failed to download release %s version %s: %w", packageName, latestVersion, err) diff --git a/internal/upgrade/upgrade_test.go b/internal/upgrade/upgrade_test.go index 0cf67f99..d15807c6 100644 --- a/internal/upgrade/upgrade_test.go +++ b/internal/upgrade/upgrade_test.go @@ -9,6 +9,7 @@ import ( "testing" "time" + "github.com/lets-cli/lets/internal/fetch" "github.com/lets-cli/lets/internal/upgrade/registry" ) @@ -24,7 +25,13 @@ func (m MockRegistry) GetLatestReleaseInfo(ctx context.Context) (*registry.Relea return ®istry.ReleaseInfo{TagName: m.latestVersion}, nil } -func (m MockRegistry) DownloadReleaseBinary(ctx context.Context, packageName string, version string, dstPath string) error { +func (m MockRegistry) DownloadReleaseBinary( + ctx context.Context, + packageName string, + version string, + dstPath string, + progress fetch.ProgressObserver, +) error { file, err := os.Create(dstPath) if err != nil { return err diff --git a/internal/util/terminal.go b/internal/util/terminal.go new file mode 100644 index 00000000..1da2284b --- /dev/null +++ b/internal/util/terminal.go @@ -0,0 +1,15 @@ +package util + +import ( + "io" + + "github.com/charmbracelet/x/term" +) + +// IsTerminalWriter reports whether w is connected to an interactive terminal. +// Writers that are not backed by a file descriptor (e.g. buffers used in tests, +// or pipes) are treated as non-terminals. +func IsTerminalWriter(w io.Writer) bool { + file, ok := w.(term.File) + return ok && term.IsTerminal(file.Fd()) +}