diff --git a/upgrade/upgrade.go b/upgrade/upgrade.go index 2f0c1e8..7fef5ab 100644 --- a/upgrade/upgrade.go +++ b/upgrade/upgrade.go @@ -193,10 +193,43 @@ func downloadAndReplace(url, destPath, expectedHash string) error { return fmt.Errorf("setting permissions: %w", err) } - if err := os.Rename(tmpPath, destPath); err != nil { + if err := replaceExecutable(tmpPath, destPath); err != nil { os.Remove(tmpPath) - return fmt.Errorf("replacing binary: %w (try running with sudo)", err) + if runtime.GOOS != "windows" { + return fmt.Errorf("replacing binary: %w (try running with sudo)", err) + } + return fmt.Errorf("replacing binary: %w", err) } return nil } + +func replaceExecutable(sourcePath, destPath string) error { + if runtime.GOOS == "windows" { + return replaceExecutableOnWindows(sourcePath, destPath) + } + return os.Rename(sourcePath, destPath) +} + +func replaceExecutableOnWindows(sourcePath, destPath string) error { + backupPath := filepath.Join(filepath.Dir(destPath), "."+filepath.Base(destPath)+".old") + if err := os.Remove(backupPath); err != nil && !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("removing previous binary backup: %w", err) + } + + if err := os.Rename(destPath, backupPath); err != nil { + return fmt.Errorf("moving current binary aside: %w", err) + } + + if err := os.Rename(sourcePath, destPath); err != nil { + if rollbackErr := os.Rename(backupPath, destPath); rollbackErr != nil { + return fmt.Errorf("installing new binary: %w; restoring current binary: %w", err, rollbackErr) + } + return fmt.Errorf("installing new binary: %w", err) + } + + // Windows keeps the running executable locked. A leftover backup is + // harmless and will be removed before the next upgrade. + _ = os.Remove(backupPath) + return nil +} diff --git a/upgrade/upgrade_test.go b/upgrade/upgrade_test.go index 296b401..9f5fea9 100644 --- a/upgrade/upgrade_test.go +++ b/upgrade/upgrade_test.go @@ -3,6 +3,7 @@ package upgrade import ( "crypto/sha256" "encoding/hex" + "errors" "io" "net/http" "net/http/httptest" @@ -184,6 +185,63 @@ func TestDownloadAndReplace(t *testing.T) { } } +func TestReplaceExecutableOnWindows(t *testing.T) { + dir := t.TempDir() + target := filepath.Join(dir, "cliamp.exe") + download := filepath.Join(dir, "cliamp-upgrade.exe") + backup := filepath.Join(dir, ".cliamp.exe.old") + + if err := os.WriteFile(target, []byte("OLD"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(download, []byte("NEW"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(backup, []byte("STALE"), 0o755); err != nil { + t.Fatal(err) + } + + if err := replaceExecutableOnWindows(download, target); err != nil { + t.Fatalf("replaceExecutableOnWindows: %v", err) + } + + got, err := os.ReadFile(target) + if err != nil { + t.Fatal(err) + } + if string(got) != "NEW" { + t.Fatalf("target content = %q, want NEW", got) + } + if _, err := os.Stat(download); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("download still exists or stat failed: %v", err) + } + if _, err := os.Stat(backup); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("backup still exists or stat failed: %v", err) + } +} + +func TestReplaceExecutableOnWindowsRollsBack(t *testing.T) { + dir := t.TempDir() + target := filepath.Join(dir, "cliamp.exe") + missingDownload := filepath.Join(dir, "missing.exe") + + if err := os.WriteFile(target, []byte("OLD"), 0o755); err != nil { + t.Fatal(err) + } + + err := replaceExecutableOnWindows(missingDownload, target) + if err == nil { + t.Fatal("replaceExecutableOnWindows should fail for a missing download") + } + got, readErr := os.ReadFile(target) + if readErr != nil { + t.Fatalf("reading restored target: %v", readErr) + } + if string(got) != "OLD" { + t.Fatalf("restored target content = %q, want OLD", got) + } +} + func TestDownloadAndReplaceHTTPError(t *testing.T) { dir := t.TempDir() target := filepath.Join(dir, "cliamp")