From 8e2bb932407772572c08559b11d21cef643b730f Mon Sep 17 00:00:00 2001 From: Amolith Date: Mon, 8 Dec 2025 21:16:57 -0700 Subject: [PATCH] refactor(update): simplify and fix Windows support - Remove platform-specific binary validation (ELF/Mach-O/PE); checksum verification is sufficient - Fix Windows: stage update to .exe.new, apply on next startup since running executables cannot be replaced - Add binaryName() function to share logic across platforms - Fix extraction cleanup on partial failure - Improve Homebrew detection on Linux Assisted-by: Claude Opus 4.5 via Crush --- internal/cmd/update.go | 7 ++- internal/update/update.go | 71 +++++------------------ internal/update/update_apply_unix.go | 69 ++++++++++++++++++++++ internal/update/update_darwin.go | 17 ------ internal/update/update_unix.go | 20 +------ internal/update/update_windows.go | 85 +++++++++++++++++++++++----- main.go | 6 ++ 7 files changed, 165 insertions(+), 110 deletions(-) create mode 100644 internal/update/update_apply_unix.go diff --git a/internal/cmd/update.go b/internal/cmd/update.go index 863df7a501a5b2dad9b5365461b210005326092d..bf0f75253ea32ded8bebd667019a82b244c8b8b3 100644 --- a/internal/cmd/update.go +++ b/internal/cmd/update.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "os" + "runtime" "time" "charm.land/lipgloss/v2" @@ -164,7 +165,11 @@ crush update apply --force } else { fmt.Fprintf(os.Stderr, "Successfully updated to v%s!\n", info.Latest) } - fmt.Fprintf(os.Stderr, "Run 'crush -v' to verify the new version.\n") + if runtime.GOOS == "windows" { + fmt.Fprintf(os.Stderr, "Restart Crush to use the new version.\n") + } else { + fmt.Fprintf(os.Stderr, "Run 'crush -v' to verify the new version.\n") + } return nil }, diff --git a/internal/update/update.go b/internal/update/update.go index 19f5306e1ed59446e7e1cbf6a08cf6d3bb2d4887..6f64ec5cfaddf0889246ee18ee5cdfc559dab6b9 100644 --- a/internal/update/update.go +++ b/internal/update/update.go @@ -30,6 +30,14 @@ const ( maxChecksumsSize = 1 * 1024 * 1024 // 1MB max for checksums.txt ) +// binaryName returns the expected binary name for the current platform. +func binaryName() string { + if runtime.GOOS == "windows" { + return "crush.exe" + } + return "crush" +} + // userAgent returns the user agent string for HTTP requests. func userAgent() string { return "crush/" + version.Version @@ -378,15 +386,12 @@ func Download(ctx context.Context, asset *Asset, release *Release) (string, erro binaryPath, err = extractTarGz(tmpFile.Name()) } if err != nil { + if binaryPath != "" { + os.Remove(binaryPath) + } return "", fmt.Errorf("failed to extract: %w", err) } - // Validate the extracted binary before returning. - if err := validateBinary(binaryPath); err != nil { - os.Remove(binaryPath) - return "", fmt.Errorf("invalid binary: %w", err) - } - return binaryPath, nil } @@ -462,7 +467,7 @@ func extractZip(archivePath string) (string, error) { } // Use exact name matching to avoid matching unintended files. - if filepath.Base(f.Name) == binaryName { + if filepath.Base(f.Name) == binaryName() { binaryPath, err := extractZipFile(f) if err != nil { return "", err @@ -546,7 +551,7 @@ func extractTarGz(archivePath string) (string, error) { } // Use exact name matching to avoid matching unintended files. - if filepath.Base(header.Name) == binaryName { + if filepath.Base(header.Name) == binaryName() { binaryPath, err := extractTarFile(tr) if err != nil { return "", err @@ -592,56 +597,6 @@ func extractTarFile(tr *tar.Reader) (string, error) { return tmpBinary.Name(), nil } -// Apply replaces the current executable with the downloaded binary. -func Apply(binaryPath string) error { - // Get path to current executable. - exe, err := os.Executable() - if err != nil { - return fmt.Errorf("failed to get executable path: %w", err) - } - - // Resolve symlinks. - exe, err = filepath.EvalSymlinks(exe) - if err != nil { - return fmt.Errorf("failed to resolve symlinks: %w", err) - } - - // Get the directory of the executable. - exeDir := filepath.Dir(exe) - - // Check if we have write permissions to the directory. - if err := checkWritePermission(exeDir); err != nil { - return fmt.Errorf("cannot write to %s: %w (you may need to run with elevated privileges)", exeDir, err) - } - - // Copy binary to exe directory first to ensure same filesystem. - // os.Rename fails across filesystems, and binaryPath may be in /tmp. - localBinary := filepath.Join(exeDir, ".crush-update-new") - if err := copyFile(binaryPath, localBinary); err != nil { - return fmt.Errorf("failed to copy new binary: %w", err) - } - - // Create a backup of the current executable. - backupPath := filepath.Join(exeDir, filepath.Base(exe)+".old") - if err := os.Rename(exe, backupPath); err != nil { - os.Remove(localBinary) - return fmt.Errorf("failed to backup current executable: %w", err) - } - - // Move new binary to executable location. - if err := os.Rename(localBinary, exe); err != nil { - // Try to restore backup on failure. - _ = os.Rename(backupPath, exe) - os.Remove(localBinary) - return fmt.Errorf("failed to install new version: %w", err) - } - - // Remove backup on success. - _ = os.Remove(backupPath) - - return nil -} - // checkWritePermission checks if we can write to the given directory. func checkWritePermission(dir string) error { testFile := filepath.Join(dir, ".crush-update-test") diff --git a/internal/update/update_apply_unix.go b/internal/update/update_apply_unix.go new file mode 100644 index 0000000000000000000000000000000000000000..226377c6b13e874123d1e6fe081cd1b0ad66b050 --- /dev/null +++ b/internal/update/update_apply_unix.go @@ -0,0 +1,69 @@ +//go:build !windows + +package update + +import ( + "fmt" + "os" + "path/filepath" +) + +// Apply replaces the current executable with the downloaded binary. +func Apply(binaryPath string) error { + // Get path to current executable. + exe, err := os.Executable() + if err != nil { + return fmt.Errorf("failed to get executable path: %w", err) + } + + // Resolve symlinks. + exe, err = filepath.EvalSymlinks(exe) + if err != nil { + return fmt.Errorf("failed to resolve symlinks: %w", err) + } + + // Get the directory of the executable. + exeDir := filepath.Dir(exe) + + // Check if we have write permissions to the directory. + if err := checkWritePermission(exeDir); err != nil { + return fmt.Errorf("cannot write to %s: %w (you may need to run with elevated privileges)", exeDir, err) + } + + // Copy binary to exe directory first to ensure same filesystem. + // os.Rename fails across filesystems, and binaryPath may be in /tmp. + localBinary := filepath.Join(exeDir, ".crush-update-new") + if err := copyFile(binaryPath, localBinary); err != nil { + return fmt.Errorf("failed to copy new binary: %w", err) + } + + // Create a backup of the current executable. + backupPath := filepath.Join(exeDir, filepath.Base(exe)+".old") + if err := os.Rename(exe, backupPath); err != nil { + os.Remove(localBinary) + return fmt.Errorf("failed to backup current executable: %w", err) + } + + // Move new binary to executable location. + if err := os.Rename(localBinary, exe); err != nil { + // Try to restore backup on failure. + _ = os.Rename(backupPath, exe) + os.Remove(localBinary) + return fmt.Errorf("failed to install new version: %w", err) + } + + // Remove backup on success. + _ = os.Remove(backupPath) + + return nil +} + +// HasPendingUpdate returns false on Unix since updates are applied immediately. +func HasPendingUpdate() bool { + return false +} + +// ApplyPendingUpdate is a no-op on Unix since updates are applied immediately. +func ApplyPendingUpdate() error { + return nil +} diff --git a/internal/update/update_darwin.go b/internal/update/update_darwin.go index c9966e3b0dbc2b929828449c91b246550651cafe..4b5242652cdea5953701d1fa2fc68b6627d5adcb 100644 --- a/internal/update/update_darwin.go +++ b/internal/update/update_darwin.go @@ -3,28 +3,11 @@ package update import ( - "debug/macho" - "fmt" "os" "path/filepath" "strings" ) -const binaryName = "crush" - -// validateBinary checks that the file at path is a valid Mach-O executable. -func validateBinary(path string) error { - f, err := macho.Open(path) - if err != nil { - return fmt.Errorf("not a valid Mach-O binary: %w", err) - } - defer f.Close() - if f.Type != macho.TypeExec { - return fmt.Errorf("Mach-O file is not an executable (type: %v)", f.Type) - } - return nil -} - // detectInstallMethod determines how Crush was installed on macOS. func detectInstallMethod(exePath string) InstallMethod { // Check for Homebrew installation. diff --git a/internal/update/update_unix.go b/internal/update/update_unix.go index fbd3f72fe8e4e82b410870fe4f495d340920abcd..aff4a5a45ba7044ba89c4cdce81929b9ae79b700 100644 --- a/internal/update/update_unix.go +++ b/internal/update/update_unix.go @@ -3,28 +3,11 @@ package update import ( - "debug/elf" - "fmt" "os" "path/filepath" "strings" ) -const binaryName = "crush" - -// validateBinary checks that the file at path is a valid ELF executable. -func validateBinary(path string) error { - f, err := elf.Open(path) - if err != nil { - return fmt.Errorf("not a valid ELF binary: %w", err) - } - defer f.Close() - if f.Type != elf.ET_EXEC && f.Type != elf.ET_DYN { - return fmt.Errorf("ELF file is not an executable (type: %v)", f.Type) - } - return nil -} - // detectInstallMethod determines how Crush was installed on Linux/BSD. func detectInstallMethod(exePath string) InstallMethod { // Check for Nix installation first (works on any platform with Nix). @@ -72,8 +55,7 @@ func detectInstallMethod(exePath string) InstallMethod { // Check for Homebrew on Linux. if strings.Contains(exePath, "/Cellar/") || - strings.HasPrefix(exePath, "/home/linuxbrew/") || - strings.Contains(exePath, "/.linuxbrew/") { + strings.Contains(exePath, "linuxbrew") { return InstallMethodHomebrew } diff --git a/internal/update/update_windows.go b/internal/update/update_windows.go index b66c2f10f0c859093f8ba63833da36e92bc4bf9d..af410c9f28a95fe4b194f6a538fc1fae8efbe725 100644 --- a/internal/update/update_windows.go +++ b/internal/update/update_windows.go @@ -3,26 +3,11 @@ package update import ( - "debug/pe" - "fmt" "os" "path/filepath" "strings" ) -const binaryName = "crush.exe" - -// validateBinary checks that the file at path is a valid PE executable. -func validateBinary(path string) error { - f, err := pe.Open(path) - if err != nil { - return fmt.Errorf("not a valid PE binary: %w", err) - } - defer f.Close() - // PE files opened successfully are valid executables. - return nil -} - // detectInstallMethod determines how Crush was installed on Windows. func detectInstallMethod(exePath string) InstallMethod { // Normalize path separators for comparison. @@ -81,3 +66,73 @@ func detectInstallMethod(exePath string) InstallMethod { return InstallMethodUnknown } + +// Apply stages the new binary for installation on next startup. +// On Windows, we cannot replace a running executable, so we copy the new +// binary to crush.exe.new and apply it on the next startup. +func Apply(binaryPath string) error { + pendingPath, err := pendingUpdatePath() + if err != nil { + return err + } + + // Copy the new binary to the pending location. + if err := copyFile(binaryPath, pendingPath); err != nil { + return err + } + + return nil +} + +// pendingUpdatePath returns the path where a pending update binary is stored. +func pendingUpdatePath() (string, error) { + exe, err := os.Executable() + if err != nil { + return "", err + } + return exe + ".new", nil +} + +// HasPendingUpdate checks if there's a pending update waiting to be applied. +func HasPendingUpdate() bool { + path, err := pendingUpdatePath() + if err != nil { + return false + } + _, err = os.Stat(path) + return err == nil +} + +// ApplyPendingUpdate applies a pending update that was staged previously. +// This should be called early in startup, before the main executable is locked. +func ApplyPendingUpdate() error { + pendingPath, err := pendingUpdatePath() + if err != nil { + return err + } + + if _, err := os.Stat(pendingPath); os.IsNotExist(err) { + return nil // No pending update. + } + + exe, err := os.Executable() + if err != nil { + return err + } + + // Rename current to .old, new to current. + oldPath := exe + ".old" + if err := os.Rename(exe, oldPath); err != nil { + return err + } + + if err := os.Rename(pendingPath, exe); err != nil { + // Try to restore. + _ = os.Rename(oldPath, exe) + return err + } + + // Clean up old binary. + _ = os.Remove(oldPath) + return nil +} diff --git a/main.go b/main.go index e75cb03e3575cf902c2ff4b44ddd15e0405f0b60..cfccd4b0e2aaa22121273fcb64301eeb6065aeb7 100644 --- a/main.go +++ b/main.go @@ -7,10 +7,16 @@ import ( "os" "github.com/charmbracelet/crush/internal/cmd" + "github.com/charmbracelet/crush/internal/update" _ "github.com/joho/godotenv/autoload" ) func main() { + // Apply any pending update from previous run (Windows-specific). + if err := update.ApplyPendingUpdate(); err != nil { + slog.Warn("Failed to apply pending update", "error", err) + } + if os.Getenv("CRUSH_PROFILE") != "" { go func() { slog.Info("Serving pprof at localhost:6060")