fix(bootstrap): scrub PYTHONHOME/PYTHONPATH before uv so AppImage venv build succeeds (#144, #127) - #158
Conversation
…v build succeeds (#144, #127) On the Linux AppImage, the bundled runtime exports PYTHONHOME / PYTHONPATH (and sometimes LD_LIBRARY_PATH) pointing at the AppImage's *own* bundled Python. When first-run bootstrap shells out to `uv` to create/sync the venv, that build subprocess inherits those vars, so the freshly-built managed interpreter resolves its stdlib against the wrong (AppImage) Python and dies with `ModuleNotFoundError: No module named 'encodings'` while compiling a transitive dep (dora-search/demucs). This surfaces downstream as "Backend process exited (never started) — no error output captured" (#144). The backend spawn in backend.rs already scrubs these vars before launching uvicorn; the uv/venv/pip subprocesses in bootstrap.rs were scrubbing them too but via five inline copies, which is drift-prone. Factor the scrub into a single `scrub_python_env(cmd)` helper documenting the #144 root cause, and apply it at every uv/venv/pip call site (uvicorn import check, repair sync, venv create, sync, ROCm reinstall). Add a unit test asserting the helper queues removals for all three vars. Safe cross-platform: those vars are normally unset on macOS/Windows and `env_remove` on an unset var is a no-op, so default behavior is unchanged everywhere. Compile-validated (cargo check + cargo test pass); no AppImage in CI to reproduce the original failure end-to-end. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR consolidates Python environment variable scrubbing in Tauri bootstrap code by introducing a reusable ChangesEnvironment variable scrubbing consolidation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
| Filename | Overview |
|---|---|
| frontend/src-tauri/src/bootstrap.rs | Introduces scrub_python_env helper replacing 5 inline env_remove triples; adds unit test. The scrub is applied consistently at all uv/venv/python call sites in ensure_venv_ready. Correct and well-documented. |
Reviews (1): Last reviewed commit: "fix(bootstrap): scrub PYTHONHOME/PYTHONP..." | Re-trigger Greptile
| fn scrub_python_env(cmd: &mut Command) { | ||
| cmd.env_remove("PYTHONHOME") | ||
| .env_remove("PYTHONPATH") | ||
| .env_remove("LD_LIBRARY_PATH"); | ||
| } |
There was a problem hiding this comment.
Helper not reused in
backend.rs
scrub_python_env is fn (crate-private), so backend.rs retains its own inline scrub at line 259: cmd.env_remove("PYTHONHOME").env_remove("PYTHONPATH").env_remove("LD_LIBRARY_PATH"). If a future ticket adds a fourth variable to the scrub list (e.g. PYTHONEXECUTABLE), the two copies will silently diverge. Making the helper pub(crate) and calling it from backend.rs would eliminate this drift.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Problem
On the Linux AppImage, the bundled runtime exports
PYTHONHOME/PYTHONPATH(and sometimesLD_LIBRARY_PATH) pointing at the AppImage's own bundled Python. When the first-run bootstrap shells out touvto create/sync the Python venv, thatuvbuild subprocess inherits those env vars, so the freshly-built managed interpreter resolves its stdlib against the wrong (AppImage) Python and dies withModuleNotFoundError: No module named 'encodings'while compiling a transitive build dep (e.g.dora-search/demucs).This surfaces to the user downstream as:
— which is exactly what #144 and #127 report on AppImage first-run.
Root cause
The backend spawn in
backend.rsalready scrubs these vars before launching uvicorn:The
uv/venv/pip subprocesses inbootstrap.rsneed the same scrub. They were doing it, but via five inline copies of the same threeenv_removecalls — drift-prone (easy for a new uv call site to forget one).Fix
scrub_python_env(cmd: &mut Command)helper, with a doc comment explaining the [Bug] [Bug] OmniVoice‑Studio AppImage fails to start – Backend process exited (never started) #144 AppImage bundled-Python leak and that it mirrors the existingbackend.rsscrub.ensure_venv_ready:uvicornimport check (venv python)uv sync(venv exists but uvicorn not importable)uv venvcreate (mirror-cascade loop)uv sync(dependency install)uv pip install --reinstall(opt-in AMD path, [Feature] AMD GPU support (RocM/HIP or Vulkan) #124)Cross-platform safety
Safe on every platform:
PYTHONHOME/PYTHONPATH/LD_LIBRARY_PATHare normally unset on macOS/Windows, andenv_removeon an unset var is a no-op. No change to default behavior off the AppImage; no platform divergence.Validation
cargo check— passes clean.cargo test— 4 bootstrap tests pass, including the newscrub_python_env_removes_bundled_runtime_vars.backend.rspattern.Closes the AppImage first-run bootstrap failure behind #144 and #127 (pending reporter confirmation on a build from latest
main).🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests