emrg: installer pre-stop — gracefully close GUI/TUI/daemon before overwriting (rant 2026-08-10T08:50:44) - #606
Conversation
…rwriting (rant 2026-08-10T08:50:44)
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle
Reviewed the full diff on branch feature/installer-pre-stop and verified locally:
- Full suite: 674 passed (4 new tests in tests/test_installer_stop.py green), doc counts synced (README.md / README.cn.md / Agent.md 670→674), import + CLI checks green.
- bin/stop-emrg.cmd: GUI graceful WM_CLOSE → /F fallback, TUI command-line filter (wmic %% escaped LIKE + PowerShell CIM fallback, daemon pythonw -m emrg.server excluded), daemon protocol stop via old-install CLI + emrgd.pid poll (≤10s) + taskkill /F fallback; clean-install safe (skips to exit 0).
- bin/emrgd.cmd stop branch: label flow avoids %errorlevel% expansion trap; 'emrg server stop' confirmed present in emrg/main.py (line 62/104/158 _stop_daemon).
- make-installer.sh: [Files] dontcopy + [Code] PrepareToInstall via {cmd} SW_HIDE, non-zero exit aborts with actionable message — consistent with the #592 no-console-window discipline and Inno CloseApplications blind spot for windowless pythonw.
- build-runtime.sh bundles stop-emrg.cmd into runtime bin/.
Windows real-machine validation flows back via new rants per PR #605's updated status rules.
…ed parse-time %DPID% (rant 2026-08-10T08:50:44)
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle 20260810-091234. Re-reviewed the batch script and found + fixed a latent cmd.exe delayed-expansion bug in the :verify block (parse-time %DPID% inside a parenthesized block would make the daemon-alive check always false → false 'clean' exit). Now uses setlocal enabledelayedexpansion + !DPID!, with a discriminating regression assertion in tests/test_installer_stop.py. Local suite: 674 pytest green, import + CLI checks pass.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle 20260810-091736 (3rd approval). Final diff re-verified: delayed-expansion fix present, PrepareToInstall/dontcopy wiring intact, doc counts synced (674). CI green on updated branch.
…ompt_all_variables_substituted)
|
I tested this PR on the Windows host (the exact target platform) after it was merged, and the Windows-specific logic checks out:
One observation worth carrying into the host-side validation: on a GUI session that had been running ~15h (started the previous evening), two runs of the full script did not terminate it, while a direct Full local verification was done on master |
Windows installs of v0.2.10/v0.2.11 got stuck at "停止已有进程" (stopping existing processes) and the host had to reboot. Root cause: the Inno Setup installer overwrites
%USERPROFILE%\.emrg\install(the daemon's runtime dir) without stopping running processes — the windowlesspythonw.exe -m emrg.serverdaemon holds file locks, and InnoCloseApplicationscannot see windowless processes.Fix (pre-install graceful shutdown, order GUI → TUI → daemon):
bin/stop-emrg.cmd: ① GUItaskkill /IM EMRG.exe(graceful WM_CLOSE, ~5s, then/Ffallback) ② TUIpython.exe -m emrgvia command-line filter (wmic%%-escaped LIKE, PowerShell CIM fallback for Win11 24H2+ where wmic is removed; daemonpythonw -m emrg.serverexcluded) ③ daemon:emrg server stopprotocol shutdown via the old install's CLI (present since emrg: Phase 4 packaging 资产 + daemon ping 响应补 type 字段(rant #12 §3/§6/§10/§12/§13) #364 — version-safe),emrgd.pidpoll (≤10s), thentaskkill /F /PIDfallback. Returns 0 when nothing survives, 1 otherwise (installer aborts with a clear message instead of hanging). Clean installs skip everything.bin/emrgd.cmd: newstopbranch →python -m emrg server stop(reuses the tested_stop_daemonpath; label flow so%errorlevel%expands after the child exits).packaging/make-installer.sh: .iss gains[Files]dontcopyentry +[Code] PrepareToInstallthat extracts and runsstop-emrg.cmd(via{cmd},SW_HIDE) before file overwrite; non-zero exit aborts with actionable message.packaging/build-runtime.sh: bundlesstop-emrg.cmdinto runtimebin/.Verification on this host: 674 pytest green, import + CLI checks green. Windows behavior must be validated by the host on the next release (expected per the new status rules, feedback flows back via new rants).