diff --git a/internal/gui/assets/app.js b/internal/gui/assets/app.js index 25e5b25e..7fcd050f 100644 --- a/internal/gui/assets/app.js +++ b/internal/gui/assets/app.js @@ -735,7 +735,8 @@ async function renderUpdateBadge() { return; } if (b.classList.contains("busy")) return; - label.textContent = t("Update"); + // a swap that failed says so where it was clicked, not only in the tooltip + label.textContent = u.state === "ready" && u.error ? t("Update failed") : t("Update"); b.title = u.state === "ready" ? t("Restart to update to {v}", { v: u.latest }) : u.state === "error" ? t("Couldn't download {v}", { v: u.latest }) + " · " + t("Click to try again") : u.stuck ? updateStuck(u) + " " + t("Click to open the download page.") diff --git a/internal/gui/assets/i18n.js b/internal/gui/assets/i18n.js index 5e60104b..dabe5405 100644 --- a/internal/gui/assets/i18n.js +++ b/internal/gui/assets/i18n.js @@ -539,6 +539,7 @@ const I18N = { "{v} is downloaded": "{v} 已下载", "Restart to update": "重启以更新", "Update": "更新", + "Update failed": "更新失败", "Restart to update to {v}": "重启以更新到 {v}", "Restarting…": "正在重启…", "{v} is out": "{v} 已发布", diff --git a/internal/gui/update.go b/internal/gui/update.go index 8dd2fa08..aa5df7ec 100644 --- a/internal/gui/update.go +++ b/internal/gui/update.go @@ -5,7 +5,6 @@ import ( "errors" "log" "net/http" - "os" "path/filepath" "runtime" "sync" @@ -61,7 +60,7 @@ func (u *updater) start() { } else if runtime.GOOS != "darwin" { if exe, err := update.Executable(); err == nil && (update.Writable(filepath.Dir(exe)) || update.CanElevate()) { u.exe = exe - os.Remove(exe + ".old") // what the last update on Windows moved aside + update.RemoveOld(exe) // what the last updates on Windows moved aside } } go func() { diff --git a/internal/update/install_windows_test.go b/internal/update/install_windows_test.go new file mode 100644 index 00000000..d3dcdb25 --- /dev/null +++ b/internal/update/install_windows_test.go @@ -0,0 +1,45 @@ +package update + +import ( + "os" + "os/exec" + "path/filepath" + "testing" +) + +// What the last update moved aside can still be running (a `magpie serve` +// started before it), and then can't be removed or replaced: the running +// exe goes beside it and the new version still goes in. +func TestInstallBinaryBesideARunningOld(t *testing.T) { + ping := filepath.Join(os.Getenv("SystemRoot"), "System32", "PING.EXE") + b, err := os.ReadFile(ping) + if err != nil { + t.Skip("no PING.EXE:", err) + } + dir := t.TempDir() + exe := filepath.Join(dir, "magpie.exe") + for _, f := range []string{exe, exe + ".old"} { + if err := os.WriteFile(f, b, 0o755); err != nil { + t.Fatal(err) + } + cmd := exec.Command(f, "-n", "30", "127.0.0.1") + if err := cmd.Start(); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { cmd.Process.Kill(); cmd.Wait() }) + } + if os.Remove(exe+".old") == nil { + t.Fatal("a running .old could be removed; the test proves nothing") + } + staged := exe + ".new" + os.WriteFile(staged, []byte("new version"), 0o755) + if err := InstallBinary(staged, exe); err != nil { + t.Fatal(err) + } + if got, _ := os.ReadFile(exe); string(got) != "new version" { + t.Fatalf("exe is not the new version: %d bytes", len(got)) + } + if _, err := os.Stat(exe + ".old-2"); err != nil { + t.Fatal("the running exe was not moved beside the old:", err) + } +} diff --git a/internal/update/update.go b/internal/update/update.go index 7f11d100..90a87525 100644 --- a/internal/update/update.go +++ b/internal/update/update.go @@ -351,10 +351,13 @@ func StageBinary(ctx context.Context, rel *Release) (string, error) { func InstallBinary(staged, exe string) error { if runtime.GOOS == "windows" { // A running .exe cannot be overwritten, but it can be moved aside. - os.Remove(exe + ".old") - if err := os.Rename(exe, exe+".old"); err != nil { - os.Remove(staged) - return err + // What the last update moved aside may still be running too (a + // `magpie serve` started before it), and can't be removed or + // replaced then: this one goes beside it, under a name of its own. + // The download is kept, for another try. + RemoveOld(exe) + if err := os.Rename(exe, oldName(exe)); err != nil { + return fmt.Errorf("couldn't move %s aside to put the new version in: %w", filepath.Base(exe), err) } } if err := os.Rename(staged, exe); err != nil { @@ -366,6 +369,28 @@ func InstallBinary(staged, exe string) error { return nil } +// oldName is where a running exe is moved aside to: exe.old, or when +// that is still there (in use), exe.old-2, exe.old-3… +func oldName(exe string) string { + name := exe + ".old" + for n := 2; ; n++ { + if _, err := os.Lstat(name); os.IsNotExist(err) { + return name + } + name = fmt.Sprintf("%s.old-%d", exe, n) + } +} + +// RemoveOld removes what earlier updates moved aside of exe, those no +// longer running. +func RemoveOld(exe string) { + os.Remove(exe + ".old") + olds, _ := filepath.Glob(exe + ".old-*") + for _, o := range olds { + os.Remove(o) + } +} + // InstallBinaryAsAdmin is InstallBinary with the administrator's password. func InstallBinaryAsAdmin(staged, exe string) error { err := asAdmin("mv -f " + shellQuote(staged) + " " + shellQuote(exe))