update: on Windows, an exe something holds open for a moment still goes aside for the new version, and one held for good says why in full, with the release page a click away (freecss on Discord: "0.1.628 已下载 · couldn't move magpie-windows-amd64.exe aside to put the new version in: rename C:\Us…") — InstallBinary renamed the running exe to .old once and gave up on the first error, which Go prints as "rename <exe> <exe>.old: <reason>", two full paths ahead of the reason, and the version row cut it off on one line, so what failed couldn't be read. A leftover .old was already handled (RemoveOld, then .old-2…, TestInstallBinaryBesideARunningOld), and Windows renames a running image (it is mapped with FILE_SHARE_DELETE), so the likely cause is another program holding the exe open without delete sharing — an antivirus scan, OneDrive or a similar sync client — or a folder that refuses the change (access denied); which one isn't known until the full message is read. moveAside now tries again for about 3 s (0.1, 0.2, 0.4, 0.8, 1.5 s apart), clearing a read-only attribute after the first failure, with a fresh .old name each try; a missing exe isn't retried. When it still can't, the message is the reason without the paths and what to do: ERROR_SHARING_VIOLATION/ERROR_LOCK_VIOLATION says another program has it open (often an antivirus or OneDrive), access denied names the folder Windows won't let magpie change, anything else says to download the new version; the download is kept for another try, as before. The version row wraps a failed install's or download's reason (.sub.wraps) with the whole text as its tooltip, and a ready-but-failed install offers Download, which opens the release page. The Windows test box wasn't reachable from this session (ssh key refused), so nothing ran on Windows; TestInstallBinaryHeldOpen (windows-only: an exe held open without FILE_SHARE_DELETE for 0.5 s still installs; held for good, the error says another program has it open and the download is kept) was cross-compiled with GOOS=windows go test -c but not run. TestMoveAsideTriesAgain and TestMoveAsideSaysWhy (portable, with a fake rename) and the new update-check.test.cjs case (webkit, en and zh: the reason is not cut off, its tooltip is the full text, Download opens the release page, a later state is one line again) fail without the change; Chromium couldn't launch here (its Playwright cache has no ICU data). go vet ./internal/..., go build -tags nogui (darwin, GOOS=linux), GOOS=windows go build and vet, go test -tags nogui ./... pass (TestFXInState failed once on a TempDir cleanup race, passes on the rerun; untouched).
This commit is contained in:
1 parent
c2947dd6e7
commit
88cccc184e
6 files changed
+219
-2
No files matched your search
@@ -2006,6 +2006,8 @@ input.words { width: 240px; max-width: 100%; }
|
||||
.row.pref.rule-row { flex-wrap: wrap; row-gap: 8px; }
|
||||
.row.pref.rule-row .who { flex: 1 1 100%; }
|
||||
.row.pref.rule-row .sub { white-space: normal; }
|
||||
/* an update that failed says why in full (the version row) */
|
||||
.row .sub.wraps { white-space: normal; overflow-wrap: anywhere; }
|
||||
.prefs .val.rule-add { flex: 1 1 100%; max-width: none; margin-left: 0; gap: 6px; }
|
||||
input.words.rule-kind { flex: none; width: 110px; }
|
||||
input.words.rule-match { flex: 1 1 auto; width: auto; min-width: 0; }
|
||||
|
||||
@@ -12185,6 +12185,7 @@ async function renderUpdate(r, u) {
|
||||
const who = r.querySelector(".who"), val = r.querySelector(".val");
|
||||
const sub = who.querySelector(".sub") || who.appendChild(el("div", "sub"));
|
||||
sub.title = "";
|
||||
sub.classList.remove("wraps");
|
||||
for (const b of val.querySelectorAll("button")) b.remove();
|
||||
const btn = (label, fn, dim) => {
|
||||
const b = el("button", "text" + (dim ? " busy" : ""), label);
|
||||
@@ -12220,6 +12221,10 @@ async function renderUpdate(r, u) {
|
||||
switch (u.state) {
|
||||
case "ready":
|
||||
sub.textContent = t("{v} is downloaded", { v: u.latest }) + (u.error ? " · " + u.error : "");
|
||||
// a failed install says why in full, and offers the release page to
|
||||
// put the new version in by hand
|
||||
sub.title = u.error || "";
|
||||
if (u.error) sub.classList.add("wraps");
|
||||
// back with an answer only when it didn't restart
|
||||
btn(t("Restart to update"), async () => {
|
||||
const a = await api("update/install", installFrom()).catch(() => ({ state: "error" }));
|
||||
@@ -12227,6 +12232,7 @@ async function renderUpdate(r, u) {
|
||||
sub.textContent = t("Restarting…");
|
||||
backAsNew(u.current);
|
||||
});
|
||||
if (u.error && u.url) btn(t("Download"), () => (web ? window.open(u.url, "_blank", "noopener") : api("open", { url: u.url })));
|
||||
break;
|
||||
case "available":
|
||||
sub.textContent = t("{v} is out", { v: u.latest });
|
||||
@@ -12252,6 +12258,7 @@ async function renderUpdate(r, u) {
|
||||
// the reason in sight: "timed out" says try a proxy, a 404 says wait
|
||||
sub.textContent = t(u.latest ? "Couldn't download {v}" : "Couldn't check for updates", { v: u.latest }) + (u.error ? " · " + u.error.replace(/^Get "[^"]*": /, "") : "");
|
||||
sub.title = u.error || "";
|
||||
if (u.error) sub.classList.add("wraps");
|
||||
btn(t("Check"), check);
|
||||
break;
|
||||
case "": // not asked yet: with automatic updates off, only this asks
|
||||
|
||||
@@ -223,6 +223,35 @@ for (const engine of (process.env.BROWSER ? [process.env.BROWSER] : ["chromium",
|
||||
assert.equal(ctl.posts, 0, "watching a check already under way does not start another");
|
||||
assert.deepEqual(errors, []);
|
||||
});
|
||||
|
||||
// freecss on Discord: "0.1.628 已下载 · couldn't move
|
||||
// magpie-windows-amd64.exe aside to put the new version in: rename C:\Us…"
|
||||
// — the reason was cut off. It is read in full, and the release page is
|
||||
// a click away.
|
||||
await t.test(lang + ": an install that failed says why in full", async () => {
|
||||
const error = "couldn't move magpie-windows-amd64.exe aside to put the new version in: The process cannot access the file because it is being used by another process; another program has it open (often an antivirus or OneDrive): let magpie through it and restart to update again, or download the new version and put it in place of this one";
|
||||
const url = "https://github.com/yetone/magpie-releases/releases/tag/v0.1.628";
|
||||
const ctl = fresh({ update: { state: "ready", current: "0.1.627", latest: "0.1.628", error, url } });
|
||||
const { page, errors } = await open(lang, ctl);
|
||||
const opened = [];
|
||||
await page.route("**/api/open", async (route) => { opened.push(route.request().postDataJSON()); await route.fulfill({ json: {} }); });
|
||||
const row = rowOf(page, w.version);
|
||||
const sub = row.locator(".sub");
|
||||
await page.waitForFunction((e) => document.querySelector("#about .row.pref .sub")?.textContent.includes(e), error);
|
||||
assert.equal(await sub.getAttribute("title"), error);
|
||||
const cut = await sub.evaluate((s) => s.scrollWidth > s.clientWidth + 1);
|
||||
assert.equal(cut, false, "the reason must not be cut off");
|
||||
const download = row.locator("button", { hasText: lang === "zh" ? "下载" : "Download" });
|
||||
await download.click();
|
||||
assert.deepEqual(opened, [{ url }]);
|
||||
|
||||
// a later state is one line again
|
||||
ctl.update = { state: "latest", current: "0.1.628" };
|
||||
await page.evaluate(() => { void renderUpdate(document.querySelector("#about .row.pref")); });
|
||||
await page.waitForFunction((latest) => document.querySelector("#about .row.pref .sub")?.textContent.trim() === latest, w.latest);
|
||||
assert.equal(await sub.evaluate((s) => s.classList.contains("wraps")), false);
|
||||
assert.deepEqual(errors, []);
|
||||
});
|
||||
}
|
||||
});
|
||||
}
|
||||
@@ -0,0 +1,79 @@
|
||||
package update
|
||||
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
// fakeRename has renameFile fail with err the first n times, then rename.
|
||||
func fakeRename(t *testing.T, n int, err error) *int {
|
||||
calls := 0
|
||||
waits, rename := asideWaits, renameFile
|
||||
asideWaits = []time.Duration{time.Millisecond, time.Millisecond, time.Millisecond}
|
||||
renameFile = func(from, to string) error {
|
||||
calls++
|
||||
if calls <= n {
|
||||
return &os.LinkError{Op: "rename", Old: from, New: to, Err: err}
|
||||
}
|
||||
return os.Rename(from, to)
|
||||
}
|
||||
t.Cleanup(func() { asideWaits, renameFile = waits, rename })
|
||||
return &calls
|
||||
}
|
||||
|
||||
// Something holding the running exe open for a moment (an antivirus scan,
|
||||
// OneDrive) doesn't fail the update: moving it aside is tried again.
|
||||
func TestMoveAsideTriesAgain(t *testing.T) {
|
||||
exe := filepath.Join(t.TempDir(), "magpie-windows-amd64.exe")
|
||||
os.WriteFile(exe, []byte("v1"), 0o755)
|
||||
calls := fakeRename(t, 2, errSharingViolation)
|
||||
if err := moveAside(exe); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if *calls != 3 {
|
||||
t.Fatalf("%d tries, want 3", *calls)
|
||||
}
|
||||
if b, _ := os.ReadFile(exe + ".old"); string(b) != "v1" {
|
||||
t.Fatal("not moved to .old")
|
||||
}
|
||||
}
|
||||
|
||||
// One that keeps it says why and what to do, without the paths the
|
||||
// version row has no room for.
|
||||
func TestMoveAsideSaysWhy(t *testing.T) {
|
||||
exe := filepath.Join(t.TempDir(), "magpie-windows-amd64.exe")
|
||||
os.WriteFile(exe, []byte("v1"), 0o755)
|
||||
calls := fakeRename(t, 100, errSharingViolation)
|
||||
err := moveAside(exe)
|
||||
if err == nil {
|
||||
t.Fatal("moved")
|
||||
}
|
||||
if *calls != 4 {
|
||||
t.Fatalf("%d tries, want 4", *calls)
|
||||
}
|
||||
msg := err.Error()
|
||||
for _, want := range []string{"couldn't move magpie-windows-amd64.exe aside", strings.TrimRight(errSharingViolation.Error(), ". 。"), "another program has it open", "download the new version"} {
|
||||
if !strings.Contains(msg, want) {
|
||||
t.Errorf("%q lacks %q", msg, want)
|
||||
}
|
||||
}
|
||||
if strings.Contains(msg, exe) {
|
||||
t.Errorf("%q has the path", msg)
|
||||
}
|
||||
|
||||
// a folder magpie may not change says so
|
||||
fakeRename(t, 100, os.ErrPermission)
|
||||
if msg := moveAside(exe).Error(); !strings.Contains(msg, "doesn't let magpie change files in "+filepath.Dir(exe)) {
|
||||
t.Errorf("%q", msg)
|
||||
}
|
||||
|
||||
// gone is not tried again
|
||||
calls = fakeRename(t, 100, os.ErrNotExist)
|
||||
moveAside(exe)
|
||||
if *calls != 1 {
|
||||
t.Fatalf("%d tries of a missing exe", *calls)
|
||||
}
|
||||
}
|
||||
@@ -4,7 +4,10 @@ import (
|
||||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"syscall"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
// What the last update moved aside can still be running (a `magpie serve`
|
||||
@@ -43,3 +46,48 @@ func TestInstallBinaryBesideARunningOld(t *testing.T) {
|
||||
t.Fatal("the running exe was not moved beside the old:", err)
|
||||
}
|
||||
}
|
||||
|
||||
// holdOpen opens path as an antivirus scan does: reading, without letting
|
||||
// it be renamed or removed (no FILE_SHARE_DELETE).
|
||||
func holdOpen(t *testing.T, path string) syscall.Handle {
|
||||
p, _ := syscall.UTF16PtrFromString(path)
|
||||
h, err := syscall.CreateFile(p, syscall.GENERIC_READ, syscall.FILE_SHARE_READ|syscall.FILE_SHARE_WRITE, nil, syscall.OPEN_EXISTING, 0, 0)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return h
|
||||
}
|
||||
|
||||
// Held open for a moment, the exe still goes aside and the new version in;
|
||||
// held for good, the update says what holds it up and keeps the download.
|
||||
func TestInstallBinaryHeldOpen(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
exe := filepath.Join(dir, "magpie-windows-amd64.exe")
|
||||
staged := exe + ".new"
|
||||
os.WriteFile(exe, []byte("old version"), 0o755)
|
||||
os.WriteFile(staged, []byte("new version"), 0o755)
|
||||
|
||||
h := holdOpen(t, exe)
|
||||
if os.Rename(exe, exe+".x") == nil {
|
||||
t.Fatal("a held exe could be renamed; the test proves nothing")
|
||||
}
|
||||
go func() { time.Sleep(500 * time.Millisecond); syscall.CloseHandle(h) }()
|
||||
if err := InstallBinary(staged, exe); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if got, _ := os.ReadFile(exe); string(got) != "new version" {
|
||||
t.Fatalf("exe is %q", got)
|
||||
}
|
||||
|
||||
os.WriteFile(staged, []byte("newer version"), 0o755)
|
||||
h = holdOpen(t, exe)
|
||||
defer syscall.CloseHandle(h)
|
||||
err := InstallBinary(staged, exe)
|
||||
if err == nil || !strings.Contains(err.Error(), "another program has it open") {
|
||||
t.Fatalf("held for good: %v", err)
|
||||
}
|
||||
t.Log(err)
|
||||
if _, err := os.Stat(staged); err != nil {
|
||||
t.Fatal("the download was not kept for another try:", err)
|
||||
}
|
||||
}
|
||||
@@ -17,6 +17,7 @@ import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"io/fs"
|
||||
"net/http"
|
||||
"os"
|
||||
"path/filepath"
|
||||
@@ -24,6 +25,7 @@ import (
|
||||
"runtime"
|
||||
"strconv"
|
||||
"strings"
|
||||
"syscall"
|
||||
"time"
|
||||
|
||||
"github.com/yetone/magpie/internal/appdir"
|
||||
@@ -380,8 +382,8 @@ func InstallBinary(staged, exe string) error {
|
||||
// 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 := moveAside(exe); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
if err := os.Rename(staged, exe); err != nil {
|
||||
@@ -393,6 +395,56 @@ func InstallBinary(staged, exe string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// renameFile is os.Rename; tests make it fail.
|
||||
var renameFile = os.Rename
|
||||
|
||||
// asideWaits are the pauses between tries at moving the running exe aside:
|
||||
// an antivirus scanning it or a sync client (OneDrive) reading it holds it
|
||||
// open, which Windows won't rename under, for a moment, and lets go.
|
||||
var asideWaits = []time.Duration{100 * time.Millisecond, 200 * time.Millisecond, 400 * time.Millisecond, 800 * time.Millisecond, 1500 * time.Millisecond}
|
||||
|
||||
// Windows' errors for a file another program holds open without letting
|
||||
// it be renamed.
|
||||
const (
|
||||
errSharingViolation syscall.Errno = 32
|
||||
errLockViolation syscall.Errno = 33
|
||||
)
|
||||
|
||||
// moveAside moves the running exe out of the way of the new version, under
|
||||
// a name of its own, trying again for a few seconds while something has it
|
||||
// open. What it says when it can't is the reason and what to do: the error
|
||||
// is shown as is in the version row.
|
||||
func moveAside(exe string) error {
|
||||
var err error
|
||||
for i := 0; ; i++ {
|
||||
if err = renameFile(exe, oldName(exe)); err == nil {
|
||||
return nil
|
||||
}
|
||||
if errors.Is(err, fs.ErrNotExist) || i == len(asideWaits) {
|
||||
break
|
||||
}
|
||||
if i == 0 {
|
||||
os.Chmod(exe, 0o755) // a read-only exe: the attribute off
|
||||
}
|
||||
time.Sleep(asideWaits[i])
|
||||
}
|
||||
why := err.Error()
|
||||
var le *os.LinkError
|
||||
if errors.As(err, &le) {
|
||||
why = le.Err.Error() // the paths are known; the reason is what's new
|
||||
}
|
||||
var hint string
|
||||
switch {
|
||||
case errors.Is(err, errSharingViolation), errors.Is(err, errLockViolation):
|
||||
hint = "another program has it open (often an antivirus or OneDrive): let magpie through it and restart to update again, or download the new version and put it in place of this one"
|
||||
case errors.Is(err, fs.ErrPermission):
|
||||
hint = "Windows doesn't let magpie change files in " + filepath.Dir(exe) + " (an antivirus' folder protection, or the folder's permissions): let magpie through, or download the new version and put it in place of this one"
|
||||
default:
|
||||
hint = "download the new version and put it in place of this one"
|
||||
}
|
||||
return fmt.Errorf("couldn't move %s aside to put the new version in: %s; %s", filepath.Base(exe), strings.TrimRight(why, ". 。"), hint)
|
||||
}
|
||||
|
||||
// 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 {
|
||||
|
||||
Reference in new issue
Block a user