emrg: installer: write bin\pyvenv.cfg on Windows so bundled python venv works (#966 follow-up) - #969
Conversation
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle
Reviewed the full diff against rant 2026-08-24T21:46:53 (#966 Windows follow-up):
- make-installer.sh:
WritePyvenvCfgwrites{app}\\bin\\pyvenv.cfgwith absolutehome = {app}\\bin\\python-dist(ExpandConstant), called in ssPostInstall BEFORE AddBinDirToPath.CfgContent: AnsiStringmatches Inno Setup 6.7.1+SaveStringToFilesignature (UnicodeString would fail to compile) — the dedicated test pins this exactly. CRLF (#13#10) correct for Windows. - smoke-test.sh: Windows branch (${WINDIR:-}) replicates the installer-written cfg with
cygpath -wabsolute path and exercises bin\python.exe directly — good that the 'missing DLL' R100 conclusion is corrected (DLLs are in bin; the real missing piece was the cfg). - upgrade_prompt.j2: local-equivalent install also writes bin/pyvenv.cfg with native absolute path, POSIX skip documented — symmetric with the installer path.
- Test: text-assertion regression test (no iscc/cmd execution, CI-safe) covering installer [Code], smoke-test branch, and upgrade prompt; Agent.md count 1051→1052 consistent (doc-count guard green).
- CI: test + test-windows both green (run 32799604676).
Solid follow-up completing the #966 story on Windows.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle (2/3)
Re-checked this cycle: head 4c709a6 unchanged, CI test + test-windows still green (run 32799604676). Confirmations from re-review:
WritePyvenvCfgusesAnsiStringforSaveStringToFile(Inno Setup 6.7.1+ signature — UnicodeString would not compile) and writes CRLF-terminatedhome = {app}\bin\python-distabsolute path.- ssPostInstall ordering:
WritePyvenvCfgbeforeAddBinDirToPath(pinned by the regression test). - smoke-test.sh Windows branch replicates the cfg via
cygpath -wabsolute path before exercisingbin\python.exe— consistent with the installer path. - upgrade_prompt.j2 covers local-equivalent installs (POSIX skip documented) — symmetric.
- Doc count 1051→1052 matches the new test (#511 guard green).
No issues found; ready to merge once the 3rd LGTM lands.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle (3/3)
Final re-check: head 4c709a6 unchanged, CI test + test-windows green (run 32799604676), MERGEABLE. Three consecutive LGTMs from distinct cycles (100311, 101045, 101729) with no ❌. Windows pyvenv.cfg story (#966 follow-up) complete — merging.
Uh oh!
There was an error while loading. Please reload this page.
Follow-up to #966 (POSIX venv fix). On Windows, the bundled
bin\python.exe/bin\python3.exeare copies of the python-dist interpreter and require a same-directorypyvenv.cfgto locate the stdlib — without it,python -m venv <dir>produces a venv whose python fails withCould not find platform independent libraries+ModuleNotFoundError: No module named 'encodings', blocking tasks that install research dependencies.Driving rant (verbatim):
Changes:
[Code]procedureWritePyvenvCfgruns onssPostInstall, writing{app}\bin\pyvenv.cfgwithhome = {app}\bin\python-dist(absolute path via ExpandConstant — a relative home fails withFailed to import encodings, verified locally).CfgContentis declaredAnsiStringto matchSaveStringToFile's signature (same rule as the existingLogTextprecedent).bin\python.exeon Windows — regenerates the same cfg in-test (mirroring the installer's ssPostInstall) with an absolute home, then runspython -m venvand verifies stdlib import + pip. POSIX path unchanged (bin/python wrapper from emrg: packaging: fix bundled python venv (pyvenv.cfg home) via bin/python wrapper #966).bin/pyvenv.cfgwith the native absolute path during local equivalent installs.Verified locally: pytest 987 passed / 65 skipped, import + CLI checks green, Windows venv regression (bin\python.exe -m venv → venv OK, pip 25.2). Full installer/Inno compilation is validated by the windows Build Release CI.