From 308ae4329291c2d109729ab0d16c55c38ad23999 Mon Sep 17 00:00:00 2001 From: "m.kindritskiy" Date: Fri, 3 Apr 2026 15:44:43 +0300 Subject: [PATCH] Move upgrade into lets self command --- docs/docs/architecture.md | 3 +- docs/docs/changelog.md | 1 + docs/docs/cli.md | 3 +- docs/docs/installation.mdx | 4 +- internal/cli/cli.go | 39 ++++++++------------ internal/cli/cli_test.go | 45 +++++++++++++++++++---- internal/cmd/root.go | 1 - internal/cmd/root_test.go | 69 +++++++++++++++++++++++++++++++++++ internal/cmd/self.go | 1 + internal/cmd/upgrade.go | 39 ++++++++++++++++++++ internal/config/validate.go | 2 +- tests/command_group.bats | 1 - tests/command_group_long.bats | 1 - tests/config_version.bats | 2 +- tests/help.bats | 2 - tests/help_long.bats | 1 - 16 files changed, 170 insertions(+), 44 deletions(-) create mode 100644 internal/cmd/upgrade.go diff --git a/docs/docs/architecture.md b/docs/docs/architecture.md index d5b936f5..62589bdf 100644 --- a/docs/docs/architecture.md +++ b/docs/docs/architecture.md @@ -71,7 +71,8 @@ To achieve this we are creating so-called `root` command and `subcommands` from #### Root command Root command is responsible for: - - `lets` own command line flags such as `--version`, `--upgrade`, `--help` and so on. + - `lets` own command line flags such as `--version`, `--help` and so on. + - `lets self` subcommands such as `lets self upgrade` - `lets` commands autocompletion in terminal #### Subcommands diff --git a/docs/docs/changelog.md b/docs/docs/changelog.md index 9866273c..cfe8f42b 100644 --- a/docs/docs/changelog.md +++ b/docs/docs/changelog.md @@ -22,6 +22,7 @@ title: Changelog * `[Fixed]` Resolve `go to definition` from YAML merge aliases such as `<<: *test` to the referenced command in `lets self lsp`. * `[Fixed]` Resolve `go to definition` from command references such as `ref: build` to the referenced command in `lets self lsp`. * `[Added]` Load local mixin files into LSP storage and command index so mixin commands are available for navigation. +* `[Changed]` Replace the top-level `--upgrade` flag with the `lets self upgrade` command. ## [0.0.59](https://github.com/lets-cli/lets/releases/tag/v0.0.59) diff --git a/docs/docs/cli.md b/docs/docs/cli.md index 531c9301..d807d3d6 100644 --- a/docs/docs/cli.md +++ b/docs/docs/cli.md @@ -12,9 +12,10 @@ title: CLI options |`--init`|`bool`|false|creates a new lets.yaml in the current folder| |`--only`|`stringArray`||run only specified command(s) described in cmd as map| |`--exclude`|`stringArray`||run all but excluded command(s) described in cmd as map| -|`--upgrade`|`bool`|false|upgrade lets to latest version| |`--no-depends`|`bool`|false|skip 'depends' for running command| |`-c, --config`|`string`|lets.yaml|specify config| |`-d, --debug`|`bool`|false|verbose logs| |`-h, --help`|||help for lets| |`-v, --version`|||version for lets| + +Upgrade the lets binary with `lets self upgrade`. diff --git a/docs/docs/installation.mdx b/docs/docs/installation.mdx index bf2d81da..ea77aa68 100644 --- a/docs/docs/installation.mdx +++ b/docs/docs/installation.mdx @@ -126,13 +126,13 @@ brew install lets-cli/tap/lets > -Starting from version `0.0.30` lets has `--upgrade` flag which allows to do self-upgrades. +Starting from version `0.0.30` lets has a built-in self-upgrade command. It updates binary located at `which lets` ```bash -lets --upgrade +lets self upgrade ``` If your `lets` version is below `0.0.30` - use shell script and specify the latest version. diff --git a/internal/cli/cli.go b/internal/cli/cli.go index d65d7c78..b06a47df 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -112,20 +112,6 @@ func Main(version string, buildDate string) int { return 0 } - if rootFlags.upgrade { - upgrader, err := upgrade.NewBinaryUpgrader(registry.NewGithubRegistry(), version) - if err == nil { - err = upgrader.Upgrade(ctx) - } - - if err != nil { - log.Errorf("can not self-upgrade binary: %s", err) - return 1 - } - - return 0 - } - showUsage := rootFlags.help || (command.Name() == "help" && len(args) == 0) || (len(os.Args) == 1) if showUsage { @@ -204,6 +190,10 @@ func allowsMissingConfig(current *cobra.Command) bool { return true } + return isSelfCommand(current) +} + +func isSelfCommand(current *cobra.Command) bool { for cmd := current; cmd != nil; cmd = cmd.Parent() { parent := cmd.Parent() if cmd.Name() == "self" && parent != nil && parent.Name() == "lets" { @@ -220,7 +210,7 @@ func maybeStartUpdateCheck( command *cobra.Command, appSettings settings.Settings, ) (<-chan updateCheckResult, context.CancelFunc) { - if !shouldCheckForUpdate(command.Name(), isInteractiveStderr(), appSettings) { + if !shouldCheckForUpdate(command, isInteractiveStderr(), appSettings) { return nil, func() {} } @@ -273,17 +263,21 @@ func printUpdateNotice(updateCh <-chan updateCheckResult) { } } -func shouldCheckForUpdate(commandName string, interactive bool, appSettings settings.Settings) bool { +func shouldCheckForUpdate(command *cobra.Command, interactive bool, appSettings settings.Settings) bool { if !interactive || !appSettings.UpgradeNotify || os.Getenv("CI") != "" { return false } - switch commandName { - case "completion", "help", "lsp", "self": - return false - default: + if command == nil { return true } + + switch command.Name() { + case "completion", "help": + return false + } + + return !isSelfCommand(command) } func isInteractiveStderr() bool { @@ -298,7 +292,6 @@ type flags struct { version bool all bool init bool - upgrade bool } // We can not parse --config and --debug flags using cobra.Command.ParseFlags @@ -379,9 +372,7 @@ func parseRootFlags(args []string) (*flags, error) { f.init = true } case "--upgrade": - if !isFlagVisited("upgrade") { - f.upgrade = true - } + return nil, errors.New("--upgrade has been replaced with 'lets self upgrade'") } idx += 1 //nolint:revive,golint diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index 5147d5f4..25baebf9 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -65,20 +65,20 @@ func TestShouldCheckForUpdate(t *testing.T) { t.Run("should allow normal interactive commands", func(t *testing.T) { t.Setenv("CI", "") - if !shouldCheckForUpdate("lets", true, defaultSettings) { + if !shouldCheckForUpdate(&cobra.Command{Use: "run"}, true, defaultSettings) { t.Fatal("expected update check to be enabled") } }) t.Run("should skip non interactive sessions", func(t *testing.T) { - if shouldCheckForUpdate("lets", false, defaultSettings) { + if shouldCheckForUpdate(&cobra.Command{Use: "run"}, false, defaultSettings) { t.Fatal("expected non-interactive session to skip update check") } }) t.Run("should skip when CI is set", func(t *testing.T) { t.Setenv("CI", "1") - if shouldCheckForUpdate("lets", true, defaultSettings) { + if shouldCheckForUpdate(&cobra.Command{Use: "run"}, true, defaultSettings) { t.Fatal("expected CI to skip update check") } }) @@ -87,16 +87,45 @@ func TestShouldCheckForUpdate(t *testing.T) { disabled := settings.Default() disabled.UpgradeNotify = false - if shouldCheckForUpdate("lets", true, disabled) { + if shouldCheckForUpdate(&cobra.Command{Use: "run"}, true, disabled) { t.Fatal("expected disabled settings to skip update check") } }) - t.Run("should skip internal commands", func(t *testing.T) { - for _, name := range []string{"completion", "help", "lsp", "self"} { - if shouldCheckForUpdate(name, true, defaultSettings) { - t.Fatalf("expected %q to skip update check", name) + t.Run("should skip completion and help commands", func(t *testing.T) { + for _, command := range []*cobra.Command{{Use: "completion"}, {Use: "help"}} { + if shouldCheckForUpdate(command, true, defaultSettings) { + t.Fatalf("expected %q to skip update check", command.Name()) } } }) + + t.Run("should skip self subcommands", func(t *testing.T) { + root := cmdpkg.CreateRootCommand("v0.0.0-test", "") + cmdpkg.InitSelfCmd(root, "v0.0.0-test") + + for _, args := range [][]string{{"self"}, {"self", "doc"}, {"self", "upgrade"}} { + command, _, err := root.Find(args) + if err != nil { + t.Fatalf("unexpected error for %v: %v", args, err) + } + + if shouldCheckForUpdate(command, true, defaultSettings) { + t.Fatalf("expected %v to skip update check", args) + } + } + }) +} + +func TestParseRootFlags(t *testing.T) { + t.Run("should reject legacy upgrade flag", func(t *testing.T) { + _, err := parseRootFlags([]string{"--upgrade"}) + if err == nil { + t.Fatal("expected legacy upgrade flag error") + } + + if err.Error() != "--upgrade has been replaced with 'lets self upgrade'" { + t.Fatalf("unexpected error: %v", err) + } + }) } diff --git a/internal/cmd/root.go b/internal/cmd/root.go index d2ca8823..338a42e4 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -98,7 +98,6 @@ func initRootFlags(rootCmd *cobra.Command) { rootCmd.Flags().StringToStringP("env", "E", nil, "set env variable for running command KEY=VALUE") rootCmd.Flags().StringArray("only", []string{}, "run only specified command(s) described in cmd as map") rootCmd.Flags().StringArray("exclude", []string{}, "run all but excluded command(s) described in cmd as map") - rootCmd.Flags().Bool("upgrade", false, "upgrade lets to latest version") 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 diff --git a/internal/cmd/root_test.go b/internal/cmd/root_test.go index 1cd5ddfe..596600c2 100644 --- a/internal/cmd/root_test.go +++ b/internal/cmd/root_test.go @@ -2,11 +2,13 @@ package cmd import ( "bytes" + "context" "errors" "strings" "testing" "github.com/lets-cli/lets/internal/config/config" + "github.com/lets-cli/lets/internal/upgrade" "github.com/spf13/cobra" ) @@ -298,4 +300,71 @@ func TestSelfCmd(t *testing.T) { t.Fatalf("expected no suggestions, got %q", err.Error()) } }) + + t.Run("should run self upgrade subcommand", func(t *testing.T) { + bufOut := new(bytes.Buffer) + called := false + + rootCmd := CreateRootCommand("v0.0.0-test", "") + rootCmd.SetArgs([]string{"self", "upgrade"}) + rootCmd.SetOut(bufOut) + rootCmd.SetErr(bufOut) + selfCmd := &cobra.Command{ + Use: "self", + Short: "Manage lets CLI itself", + } + rootCmd.AddCommand(selfCmd) + + selfCmd.AddCommand(initUpgradeCommandWith(func() (upgrade.Upgrader, error) { + return mockUpgraderFunc(func(ctx context.Context) error { + called = true + + return nil + }), nil + })) + + err := rootCmd.Execute() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if !called { + t.Fatal("expected upgrader to be called") + } + }) + + t.Run("should return upgrader error for self upgrade command", func(t *testing.T) { + bufOut := new(bytes.Buffer) + + rootCmd := CreateRootCommand("v0.0.0-test", "") + rootCmd.SetArgs([]string{"self", "upgrade"}) + rootCmd.SetOut(bufOut) + rootCmd.SetErr(bufOut) + selfCmd := &cobra.Command{ + Use: "self", + Short: "Manage lets CLI itself", + } + rootCmd.AddCommand(selfCmd) + + selfCmd.AddCommand(initUpgradeCommandWith(func() (upgrade.Upgrader, error) { + return mockUpgraderFunc(func(ctx context.Context) error { + return errors.New("upgrade failed") + }), nil + })) + + err := rootCmd.Execute() + if err == nil { + t.Fatal("expected upgrader error") + } + + if !strings.Contains(err.Error(), "can not self-upgrade binary") { + t.Fatalf("expected self-upgrade error, got %q", err.Error()) + } + }) +} + +type mockUpgraderFunc func(context.Context) error + +func (f mockUpgraderFunc) Upgrade(ctx context.Context) error { + return f(ctx) } diff --git a/internal/cmd/self.go b/internal/cmd/self.go index 1d80b956..a71cd5c1 100644 --- a/internal/cmd/self.go +++ b/internal/cmd/self.go @@ -26,4 +26,5 @@ func initSelfCmd(rootCmd *cobra.Command, version string, openURL func(string) er selfCmd.AddCommand(initDocCommand(openURL)) selfCmd.AddCommand(initLspCommand(version)) + selfCmd.AddCommand(initUpgradeCommand(version)) } diff --git a/internal/cmd/upgrade.go b/internal/cmd/upgrade.go new file mode 100644 index 00000000..741db149 --- /dev/null +++ b/internal/cmd/upgrade.go @@ -0,0 +1,39 @@ +package cmd + +import ( + "fmt" + + "github.com/lets-cli/lets/internal/upgrade" + "github.com/lets-cli/lets/internal/upgrade/registry" + "github.com/spf13/cobra" +) + +type upgraderFactory func() (upgrade.Upgrader, error) + +func initUpgradeCommand(version string) *cobra.Command { + return initUpgradeCommandWith(func() (upgrade.Upgrader, error) { + return upgrade.NewBinaryUpgrader(registry.NewGithubRegistry(), version) + }) +} + +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() + if err != nil { + return fmt.Errorf("can not self-upgrade binary: %w", err) + } + + if err := upgrader.Upgrade(cmd.Context()); err != nil { + return fmt.Errorf("can not self-upgrade binary: %w", err) + } + + return nil + }, + } + + return upgradeCmd +} diff --git a/internal/config/validate.go b/internal/config/validate.go index 60bd5cb5..efd3b328 100644 --- a/internal/config/validate.go +++ b/internal/config/validate.go @@ -46,7 +46,7 @@ func validateVersion(cfg *config.Config, letsVersion string) error { return fmt.Errorf( "config version '%s' is not compatible with 'lets' version '%s'. "+ "Please upgrade 'lets' to '%s' "+ - "using 'lets --upgrade' command or following documentation at https://lets-cli.org/docs/installation'", + "using 'lets self upgrade' command or following documentation at https://lets-cli.org/docs/installation'", cfgVersionParsed, letsVersionParsed, cfgVersionParsed, diff --git a/tests/command_group.bats b/tests/command_group.bats index 0de5fa08..c0e5d4a5 100644 --- a/tests/command_group.bats +++ b/tests/command_group.bats @@ -39,7 +39,6 @@ Flags: --init create a new lets.yaml in the current folder --no-depends skip 'depends' for running command --only stringArray run only specified command(s) described in cmd as map - --upgrade upgrade lets to latest version -v, --version version for lets Use "lets help [command]" for more information about a command. diff --git a/tests/command_group_long.bats b/tests/command_group_long.bats index 8c8531f4..6a460b55 100644 --- a/tests/command_group_long.bats +++ b/tests/command_group_long.bats @@ -40,7 +40,6 @@ Flags: --init create a new lets.yaml in the current folder --no-depends skip 'depends' for running command --only stringArray run only specified command(s) described in cmd as map - --upgrade upgrade lets to latest version -v, --version version for lets Use "lets help [command]" for more information about a command. diff --git a/tests/config_version.bats b/tests/config_version.bats index e64717d3..05e2d148 100644 --- a/tests/config_version.bats +++ b/tests/config_version.bats @@ -25,7 +25,7 @@ teardown() { LETS_CONFIG=lets-with-version-0.0.3.yaml run ./lets assert_failure - assert_line --index 0 "lets: config error: config version '0.0.3' is not compatible with 'lets' version '0.0.2'. Please upgrade 'lets' to '0.0.3' using 'lets --upgrade' command or following documentation at https://lets-cli.org/docs/installation'" + assert_line --index 0 "lets: config error: config version '0.0.3' is not compatible with 'lets' version '0.0.2'. Please upgrade 'lets' to '0.0.3' using 'lets self upgrade' command or following documentation at https://lets-cli.org/docs/installation'" } @test "config_version: no version specified" { diff --git a/tests/help.bats b/tests/help.bats index de450366..e9246e50 100644 --- a/tests/help.bats +++ b/tests/help.bats @@ -31,7 +31,6 @@ Flags: --init create a new lets.yaml in the current folder --no-depends skip 'depends' for running command --only stringArray run only specified command(s) described in cmd as map - --upgrade upgrade lets to latest version -v, --version version for lets Use "lets help [command]" for more information about a command. @@ -64,7 +63,6 @@ Flags: --init create a new lets.yaml in the current folder --no-depends skip 'depends' for running command --only stringArray run only specified command(s) described in cmd as map - --upgrade upgrade lets to latest version -v, --version version for lets Use "lets help [command]" for more information about a command. diff --git a/tests/help_long.bats b/tests/help_long.bats index 8fb63a72..8ddc0000 100644 --- a/tests/help_long.bats +++ b/tests/help_long.bats @@ -32,7 +32,6 @@ Flags: --init create a new lets.yaml in the current folder --no-depends skip 'depends' for running command --only stringArray run only specified command(s) described in cmd as map - --upgrade upgrade lets to latest version -v, --version version for lets Use "lets help [command]" for more information about a command.