diff --git a/commands/doctor_checks.go b/commands/doctor_checks.go index 1879360..0a4d473 100644 --- a/commands/doctor_checks.go +++ b/commands/doctor_checks.go @@ -205,11 +205,14 @@ func doctorCheckSystem(env *doctorEnv) []doctorResult { ctx, cancel := context.WithTimeout(env.ctx, doctorUpdateTimeout) latest, err := doctorFetchLatestVersion(ctx) cancel() + cmp := compareToLatest(version, latest) switch { case err != nil: update.Status, update.Message = doctorSkip, fmt.Sprintf("Couldn't check for updates: %v", err) - case model.NormalizeVersion(latest) == model.NormalizeVersion(version): + case cmp == 0: update.Status, update.Message = doctorOK, fmt.Sprintf("You're on the latest version (%s).", latest) + case cmp > 0: + update.Status, update.Message = doctorOK, fmt.Sprintf("You're ahead of the latest release (%s, you have %s).", latest, version) default: update.Status = doctorWarn update.Message = fmt.Sprintf("A newer version is available: %s (you have %s).", latest, version) @@ -221,7 +224,7 @@ func doctorCheckSystem(env *doctorEnv) []doctorResult { func doctorUpdateHint() string { if cliPath, err := model.ResolveCLIBinaryPath(); err == nil && model.DetectInstallKind(cliPath) == model.InstallKindHomebrew { - return "Run `brew upgrade shelltime/tap/shelltime`." + return fmt.Sprintf("Run `%s`.", model.HomebrewUpgradeCommand(cliPath)) } return "Run `shelltime update`." } diff --git a/commands/doctor_test.go b/commands/doctor_test.go index 2642b44..d207b82 100644 --- a/commands/doctor_test.go +++ b/commands/doctor_test.go @@ -164,6 +164,11 @@ func TestDoctorCheckSystem(t *testing.T) { assert.Contains(t, update.Message, "v0.2.0") assert.Regexp(t, "shelltime update|brew upgrade", update.Fix) + doctorFetchLatestVersion = func(context.Context) (string, error) { return "v0.1.0", nil } + update = findDoctorResult(t, doctorCheckSystem(env), "system.update") + assert.Equal(t, doctorOK, update.Status, "a build newer than the latest release needs no update") + assert.Contains(t, update.Message, "ahead") + env.offline = true assert.Contains(t, findDoctorResult(t, doctorCheckSystem(env), "system.update").Message, "--offline") diff --git a/commands/update.go b/commands/update.go index eda1e60..60fa93b 100644 --- a/commands/update.go +++ b/commands/update.go @@ -5,6 +5,7 @@ import ( "fmt" "log/slog" "os" + "os/exec" "path/filepath" "runtime" @@ -53,7 +54,7 @@ func commandUpdate(c *cli.Context) error { switch model.DetectInstallKind(cliPath) { case model.InstallKindHomebrew: color.Yellow.Println("📦 Detected Homebrew installation.") - color.Yellow.Println(" Run: brew upgrade shelltime/tap/shelltime") + color.Yellow.Printf(" Run: %s\n", model.HomebrewUpgradeCommand(cliPath)) return nil case model.InstallKindUnknown: color.Yellow.Printf("⚠️ Binary at %s is not in a known auto-updatable location.\n", cliPath) @@ -70,16 +71,18 @@ func commandUpdate(c *cli.Context) error { if current == "" { current = "dev" } - normalizedLatest := model.NormalizeVersion(latest) - normalizedCurrent := model.NormalizeVersion(current) + status := compareToLatest(current, latest) color.Cyan.Printf(" Current: %s\n", current) color.Cyan.Printf(" Latest: %s\n", latest) if check { - if normalizedLatest == normalizedCurrent { + switch { + case status == 0: color.Green.Println("✅ Already on the latest version.") - } else { + case status > 0: + color.Green.Println("✅ Already newer than the latest release.") + default: color.Yellow.Println("⬆️ An update is available. Run `shelltime update` to install it.") } return nil @@ -90,11 +93,16 @@ func commandUpdate(c *cli.Context) error { return nil } - if normalizedLatest == normalizedCurrent && !force { + if status == 0 && !force { color.Green.Println("✅ Already on the latest version. Use --force to reinstall.") return nil } + if status > 0 && !force { + color.Yellow.Printf("⚠️ %s is newer than the latest release. Use --force to downgrade to %s.\n", current, latest) + return nil + } + archiveName, err := model.BuildArchiveName(runtime.GOOS, runtime.GOARCH) if err != nil { return err @@ -103,8 +111,11 @@ func commandUpdate(c *cli.Context) error { expectedSum, ok, err := model.FetchChecksum(ctx, latest, archiveName) if err != nil { - color.Yellow.Printf("⚠️ Could not fetch checksums.txt: %v (proceeding without verification)\n", err) - } else if !ok { + // A missing checksums.txt comes back as !ok. Any other failure (5xx, + // network) must not quietly turn this into an unverified install. + return fmt.Errorf("fetch checksums.txt: %w", err) + } + if !ok { color.Yellow.Println("⚠️ No checksum entry for this archive — proceeding without verification.") } @@ -149,7 +160,7 @@ func commandUpdate(c *cli.Context) error { if shouldReinstallDaemon(ctx, skipDaemonReinstall) { color.Yellow.Println("🔁 Refreshing daemon service...") - if err := commandDaemonReinstall(c); err != nil { + if err := runDaemonReinstall(ctx, cliPath); err != nil { color.Yellow.Printf("⚠️ Daemon reinstall reported an error: %v\n", err) color.Yellow.Println(" You can rerun `shelltime daemon reinstall` manually.") } @@ -161,6 +172,30 @@ func commandUpdate(c *cli.Context) error { return nil } +// compareToLatest reports how the running version compares with the latest +// release: -1 when an update is available, 0 when up to date, and 1 when the +// running build is newer (e.g. a local build of an unreleased version). +func compareToLatest(current, latest string) int { + if cmp, ok := model.CompareVersions(current, latest); ok { + return cmp + } + if model.NormalizeVersion(current) == model.NormalizeVersion(latest) { + return 0 + } + return -1 +} + +// runDaemonReinstall runs `daemon reinstall` with the CLI that was just +// installed at cliPath. This process is still the previous release, so +// reinstalling in-process would write that release's service definition. +func runDaemonReinstall(ctx context.Context, cliPath string) error { + cmd := exec.CommandContext(ctx, cliPath, "daemon", "reinstall") + cmd.Stdin = os.Stdin + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + return cmd.Run() +} + // resolveDaemonDest returns the path the daemon binary should be written to. // Update only runs for curl-installer CLIs, so the daemon always goes next to // the CLI — never into a Homebrew prefix, where an unmanaged copy blocks diff --git a/commands/update_test.go b/commands/update_test.go index 6961549..b97fd28 100644 --- a/commands/update_test.go +++ b/commands/update_test.go @@ -1,8 +1,10 @@ package commands import ( + "context" "os" "path/filepath" + "runtime" "testing" "github.com/malamtime/cli/model" @@ -27,3 +29,48 @@ func TestResolveDaemonDest(t *testing.T) { t.Errorf("resolveDaemonDest() = %s, want %s", got, want) } } + +func TestCompareToLatest(t *testing.T) { + tests := []struct { + current, latest string + want int + }{ + {"0.1.93", "v0.1.93", 0}, + {"0.1.90", "v0.1.93", -1}, + {"0.1.94", "v0.1.93", 1}, + {"0.1.94-next", "v0.1.93", 1}, + {"dev", "v0.1.93", -1}, + } + for _, tt := range tests { + if got := compareToLatest(tt.current, tt.latest); got != tt.want { + t.Errorf("compareToLatest(%q, %q) = %d, want %d", tt.current, tt.latest, got, tt.want) + } + } +} + +// TestRunDaemonReinstallUsesNewBinary pins that the daemon refresh runs the +// CLI that was just installed, not this (previous-release) process, so the +// service definition comes from the new release. +func TestRunDaemonReinstallUsesNewBinary(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("uses a shell script as the fake CLI") + } + dir := t.TempDir() + argsFile := filepath.Join(dir, "args") + cliPath := filepath.Join(dir, "shelltime") + script := "#!/bin/sh\necho \"$@\" > " + argsFile + "\n" + if err := os.WriteFile(cliPath, []byte(script), 0o755); err != nil { + t.Fatal(err) + } + + if err := runDaemonReinstall(context.Background(), cliPath); err != nil { + t.Fatalf("runDaemonReinstall() error = %v", err) + } + got, err := os.ReadFile(argsFile) + if err != nil { + t.Fatal(err) + } + if string(got) != "daemon reinstall\n" { + t.Errorf("new CLI ran with args %q, want %q", got, "daemon reinstall\n") + } +} diff --git a/model/updater.go b/model/updater.go index b3d2e51..cd9ddb9 100644 --- a/model/updater.go +++ b/model/updater.go @@ -15,6 +15,7 @@ import ( "os" "path/filepath" "runtime" + "strconv" "strings" "time" ) @@ -369,20 +370,33 @@ func stripExe(name string) string { // this is safe even while the binary is running because the kernel keeps the // old inode alive for the current process. func ReplaceBinary(srcPath, destPath string) error { + // Stage next to destPath first. moveFile copies when srcPath is on another + // filesystem (e.g. a tmpfs /tmp), and destPath must not be missing while + // it does: the shell hooks run `shelltime` after every command. + staged := destPath + ".new" + _ = os.Remove(staged) + if err := moveFile(srcPath, staged); err != nil { + _ = os.Remove(staged) + return err + } + if err := os.Chmod(staged, 0o755); err != nil { + _ = os.Remove(staged) + return err + } + bak := destPath + ".bak" _ = os.Remove(bak) if _, err := os.Stat(destPath); err == nil { if err := os.Rename(destPath, bak); err != nil { + _ = os.Remove(staged) return fmt.Errorf("rename %s -> %s: %w", destPath, bak, err) } } - if err := moveFile(srcPath, destPath); err != nil { - // Try to restore .bak on failure so we don't leave the user without a binary. + if err := os.Rename(staged, destPath); err != nil { + // Restore .bak so we don't leave the user without a binary. _ = os.Rename(bak, destPath) - return err - } - if err := os.Chmod(destPath, 0o755); err != nil { - return err + _ = os.Remove(staged) + return fmt.Errorf("rename %s -> %s: %w", staged, destPath, err) } return nil } @@ -417,6 +431,58 @@ func NormalizeVersion(v string) string { return strings.TrimPrefix(strings.TrimSpace(v), "v") } +// CompareVersions compares two MAJOR.MINOR.PATCH versions with an optional +// "v" prefix and "-prerelease" suffix (e.g. "v0.1.93", "0.1.94-next"), +// returning -1, 0 or 1. A pre-release sorts before its release. ok is false +// when either side is not such a version (e.g. "dev"). +func CompareVersions(a, b string) (cmp int, ok bool) { + ac, apre, aok := parseVersion(a) + bc, bpre, bok := parseVersion(b) + if !aok || !bok { + return 0, false + } + for i := range ac { + if ac[i] != bc[i] { + if ac[i] < bc[i] { + return -1, true + } + return 1, true + } + } + switch { + case apre == bpre: + return 0, true + case apre == "": + return 1, true + case bpre == "": + return -1, true + default: + return strings.Compare(apre, bpre), true + } +} + +func parseVersion(v string) (core [3]int, pre string, ok bool) { + v = NormalizeVersion(v) + if i := strings.IndexAny(v, "-+"); i >= 0 { + if v[i] == '-' { + pre = v[i+1:] + } + v = v[:i] + } + parts := strings.Split(v, ".") + if len(parts) != 3 { + return core, "", false + } + for i, p := range parts { + n, err := strconv.Atoi(p) + if err != nil || n < 0 { + return core, "", false + } + core[i] = n + } + return core, pre, true +} + // ResolveCLIBinaryPath returns the real (symlink-resolved) path of the running // CLI binary. func ResolveCLIBinaryPath() (string, error) { @@ -444,18 +510,44 @@ const ( // install ($HOME/.shelltime/bin), or unknown. func DetectInstallKind(binPath string) InstallKind { clean := filepath.Clean(binPath) - if strings.Contains(clean, string(filepath.Separator)+"Cellar"+string(filepath.Separator)) || + sep := string(filepath.Separator) + // Casks resolve to /Caskroom///, which on Intel + // Macs (/usr/local) matches none of the other checks. + if strings.Contains(clean, sep+"Cellar"+sep) || + strings.Contains(clean, sep+"Caskroom"+sep) || strings.HasPrefix(clean, "/opt/homebrew/") || strings.HasPrefix(clean, "/home/linuxbrew/.linuxbrew/") { return InstallKindHomebrew } + // binPath is usually symlink-resolved (ResolveCLIBinaryPath), so also + // match the resolved bin dir in case $HOME or ~/.shelltime is a symlink. expected := filepath.Clean(filepath.Join(GetBaseStoragePath(), "bin")) - if strings.HasPrefix(clean, expected+string(filepath.Separator)) { + if strings.HasPrefix(clean, expected+sep) { + return InstallKindCurl + } + if resolved, err := filepath.EvalSymlinks(expected); err == nil && strings.HasPrefix(clean, resolved+sep) { return InstallKindCurl } return InstallKindUnknown } +// HomebrewUpgradeCommand returns the command that upgrades the Homebrew +// install at binPath. shelltime ships as a macOS-only cask now; a formula keg +// (Cellar) no longer gets releases, so it has to be swapped for the cask, or +// for the curl installer on Linuxbrew. +func HomebrewUpgradeCommand(binPath string) string { + clean := filepath.Clean(binPath) + sep := string(filepath.Separator) + switch { + case !strings.Contains(clean, sep+"Cellar"+sep): + return "brew upgrade shelltime/tap/shelltime" + case strings.Contains(clean, sep+".linuxbrew"+sep): + return "brew uninstall --formula shelltime && curl -sSL https://shelltime.xyz/i | bash" + default: + return "brew uninstall --formula shelltime && brew install --cask shelltime/tap/shelltime" + } +} + // CurrentPlatform returns the goos/goarch pair, exposed for tests and logging. func CurrentPlatform() (string, string) { return runtime.GOOS, runtime.GOARCH diff --git a/model/updater_cov_test.go b/model/updater_cov_test.go index a5aa1b9..cbeaabf 100644 --- a/model/updater_cov_test.go +++ b/model/updater_cov_test.go @@ -120,9 +120,9 @@ func TestWriteBinary_RoundTrip(t *testing.T) { assert.Equal(t, os.FileMode(0o755), info.Mode().Perm()) } -// TestReplaceBinary_RestoreOnMoveFailure covers ReplaceBinary's failure-recovery -// branch: when moveFile fails (src missing), the prior binary is restored from -// the .bak and an error is returned. +// TestReplaceBinary_RestoreOnMoveFailure covers ReplaceBinary's failure path: +// when moveFile fails (src missing), the prior binary is still at dest and an +// error is returned. func TestReplaceBinary_RestoreOnMoveFailure(t *testing.T) { dir := t.TempDir() dest := filepath.Join(dir, "shelltime") @@ -136,5 +136,5 @@ func TestReplaceBinary_RestoreOnMoveFailure(t *testing.T) { // The original binary must have been restored to dest. got, err := os.ReadFile(dest) require.NoError(t, err) - assert.Equal(t, "ORIGINAL", string(got), "prior binary restored after failed move") + assert.Equal(t, "ORIGINAL", string(got), "prior binary kept after failed move") } diff --git a/model/updater_test.go b/model/updater_test.go index 956f3a3..7eaf0c2 100644 --- a/model/updater_test.go +++ b/model/updater_test.go @@ -154,6 +154,7 @@ func TestDetectInstallKind(t *testing.T) { {"curl install", filepath.Join(base, "bin", "shelltime"), InstallKindCurl}, {"homebrew apple silicon", "/opt/homebrew/bin/shelltime", InstallKindHomebrew}, {"homebrew cellar", "/usr/local/Cellar/shelltime/0.1.0/bin/shelltime", InstallKindHomebrew}, + {"homebrew cask intel", "/usr/local/Caskroom/shelltime/0.1.93/shelltime", InstallKindHomebrew}, {"linuxbrew", "/home/linuxbrew/.linuxbrew/bin/shelltime", InstallKindHomebrew}, {"random location", "/usr/bin/shelltime", InstallKindUnknown}, } @@ -164,6 +165,59 @@ func TestDetectInstallKind(t *testing.T) { } } +// TestDetectInstallKindSymlinkedHome covers a $HOME reached through a symlink +// (e.g. /home -> /var/home): ResolveCLIBinaryPath returns the resolved path, +// which must still count as a curl install. +func TestDetectInstallKindSymlinkedHome(t *testing.T) { + realHome := t.TempDir() + binDir := filepath.Join(realHome, COMMAND_BASE_STORAGE_FOLDER, "bin") + require.NoError(t, os.MkdirAll(binDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(binDir, "shelltime"), []byte("x"), 0o755)) + + linkedHome := filepath.Join(t.TempDir(), "home") + require.NoError(t, os.Symlink(realHome, linkedHome)) + t.Setenv("HOME", linkedHome) + + resolved, err := filepath.EvalSymlinks(filepath.Join(linkedHome, COMMAND_BASE_STORAGE_FOLDER, "bin", "shelltime")) + require.NoError(t, err) + assert.Equal(t, InstallKindCurl, DetectInstallKind(resolved)) +} + +func TestHomebrewUpgradeCommand(t *testing.T) { + assert.Equal(t, "brew upgrade shelltime/tap/shelltime", + HomebrewUpgradeCommand("/opt/homebrew/Caskroom/shelltime/0.1.93/shelltime")) + assert.Equal(t, "brew uninstall --formula shelltime && brew install --cask shelltime/tap/shelltime", + HomebrewUpgradeCommand("/opt/homebrew/Cellar/shelltime/0.1.40/bin/shelltime")) + assert.Equal(t, "brew uninstall --formula shelltime && curl -sSL https://shelltime.xyz/i | bash", + HomebrewUpgradeCommand("/home/linuxbrew/.linuxbrew/Cellar/shelltime/0.1.40/bin/shelltime")) +} + +func TestCompareVersions(t *testing.T) { + tests := []struct { + a, b string + want int + wantOK bool + }{ + {"0.1.93", "v0.1.93", 0, true}, + {"0.1.92", "v0.1.93", -1, true}, + {"0.1.94", "v0.1.93", 1, true}, + {"0.1.100", "v0.1.99", 1, true}, + {"v1.0.0", "0.99.99", 1, true}, + {"0.1.94-next", "v0.1.93", 1, true}, + {"0.1.94-next", "v0.1.94", -1, true}, + {"dev", "v0.1.93", 0, false}, + {"0.1", "v0.1.93", 0, false}, + {"", "v0.1.93", 0, false}, + } + for _, tt := range tests { + t.Run(tt.a+"_vs_"+tt.b, func(t *testing.T) { + got, ok := CompareVersions(tt.a, tt.b) + assert.Equal(t, tt.wantOK, ok) + assert.Equal(t, tt.want, got) + }) + } +} + func TestExtractBinariesZip(t *testing.T) { tmp := t.TempDir() archivePath := filepath.Join(tmp, "release.zip") @@ -280,3 +334,27 @@ func TestReplaceBinaryNoExisting(t *testing.T) { _, err = os.Stat(dest + ".bak") assert.True(t, os.IsNotExist(err)) } + +// TestReplaceBinaryKeepsDestOnStagingFailure pins that the old binary stays at +// destPath until the new one is fully written next to it: the shell hooks run +// `shelltime` after every command and must never find it missing. +func TestReplaceBinaryKeepsDestOnStagingFailure(t *testing.T) { + tmp := t.TempDir() + dest := filepath.Join(tmp, "shelltime") + require.NoError(t, os.WriteFile(dest, []byte("OLD"), 0o755)) + + // A directory at the staging path makes staging fail. + require.NoError(t, os.MkdirAll(filepath.Join(dest+".new", "blocker"), 0o755)) + + src := filepath.Join(tmp, "src", "shelltime") + require.NoError(t, os.MkdirAll(filepath.Dir(src), 0o755)) + require.NoError(t, os.WriteFile(src, []byte("NEW"), 0o755)) + + require.Error(t, ReplaceBinary(src, dest)) + + body, err := os.ReadFile(dest) + require.NoError(t, err) + assert.Equal(t, "OLD", string(body)) + _, err = os.Stat(dest + ".bak") + assert.True(t, os.IsNotExist(err), "dest must not be moved aside before the new binary is staged") +}