fix(desktop): keep venv interpreter unresolved in .desktop Exec - #96396
Closed
cosminfuica wants to merge 1 commit into
Closed
cosminfuica wants to merge 1 commit into
cosminfuica wants to merge 1 commit into
Conversation
resolve_exec_command() wrote Path(sys.executable).resolve() into Exec=. On a uv-created venv, venv/bin/python is a symlink into the shared interpreter store (~/.local/share/uv/python/cpython-X.Y.Z-.../bin/), so .resolve() emitted the BASE interpreter. That interpreter's sys.prefix is the uv install, not the venv, so none of Hermes' dependencies import: the app died on 'import yaml' before drawing a window. With Terminal=false the traceback is discarded and the launcher icon appears inert. The resolved path also pins a patch version that vanishes when uv upgrades 3.11.16 -> 3.11.17. _needs_interpreter() made the same mistake in reverse: it compared the shebang against the RESOLVED parent dir, so a venv console script (#!/.../venv/bin/python3) never matched and was needlessly prefixed. Use os.path.abspath(sys.executable) (already absolute; keeps the symlink) and match the shebang against both the unresolved and resolved bin dirs. Keeping the symlink also preserves PEP 405 venv detection. Adds regression coverage for the uv symlink layout.
13 of 19 tasks
Author
|
Closing in favor of #92090, the earlier canonical fix for this bug class (same repair: unresolved interpreter in What #92090 was missing was a run on an affected Linux host (its author is on Windows) — I've now done that end-to-end verification on my uv-venv machine and posted the results there: tests green, mutation proof against the merge-base, before/after For the consolidating maintainer: #96777 has a comparison table of all six open PRs for this defect. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
On Linux installs where the Hermes venv was created by
uv(the shell installer's default),hermes desktopwrites a non-runnableExec=into~/.local/share/applications/hermes.desktop. Clicking the launcher icon silently does nothing.Reported in #94058, #94110, #92086, #92095 (and #90292 is the same class). This fixes both halves of the bug and adds regression coverage.
Root cause
resolve_exec_command()writesPath(sys.executable).resolve().uv venv(likepython -m venv --symlinks, the POSIX default) makesvenv/bin/pythona symlink into a shared interpreter store:.resolve()follows that symlink, soExec=names the base interpreter. Itssys.prefixis the uv store, not the venv, sosite-packagesis never onsys.pathand Hermes dies on its first third-party import:Because the entry sets
Terminal=false, that traceback goes nowhere — the icon just appears inert. Reproduced on this machine:Two consequences, one cause:
cpython-3.11.16. When uv upgrades to3.11.17the path disappears, so an install that was working breaks at the next update. This is why the reports describe the icon breaking repeatedly after upgrades._needs_interpreter()had the mirror-image mistake: it compared the shebang against the resolved parent dir, so a venv console script whose shebang legitimately reads#!/…/venv/bin/python3never matched and got needlessly prefixed.Fix
Use
os.path.abspath(sys.executable)— already absolute, so.resolve()bought nothing — and keep the symlink intact. Match the shebang against both the unresolved and resolved bin dirs.Keeping the symlink also preserves PEP 405 venv detection, which locates
pyvenv.cfgrelative to the unresolved executable path.Before / after on a uv install:
Testing
pytest tests/hermes_cli/test_linux_desktop_entry.py→ 17 passed, 2 skippedtmp_pathand assert the version-pinned base interpreter never reachesExec=. Both fail onmainand pass with this patch (verified by reverting the one-line change).Path(sys.executable).resolve(), i.e. the buggy value itself. Its stated intent (Linux desktop entry generated with non-runnable Exec; icon launch always fails #90292 — "prefix the running interpreter") is preserved; it now asserts the interpreter is absolute and venv-internal rather than snapshotting the resolved path. Per AGENTS.md, behavior contracts over snapshots.desktop-file-validatepasses, and launching the emittedExecunderenv -i(no PATH, no venv activation — what the DE actually does) starts the app cleanly.Notes
Affects every Linux user whose venv python is a symlink — the default for both
uv venvandpython -m venvon POSIX. No behavior change on Windows, on non-symlinked venvs, or for the bash-wrapper and native-shim launcher paths, which are covered by existing tests.