Reuse the venv on upgrade instead of recreating it - #17
Merged
Conversation
`brew upgrade` bumps VALCORE_VERSION, the stamp goes stale, and the launcher ran `uv venv` against a path that already held a venv. uv refuses that: it prompts to replace when a TTY is attached, and errors out when one is not. Under `set -e` the launcher then died before reaching the install, so every post-upgrade run from a script, cron job, or CI step failed until someone ran valcore by hand and answered the prompt. Answering it destroyed the venv and reinstalled ~115 packages to change one of them. Create the venv only when there is no usable interpreter at $venv/bin/python, and otherwise install the pinned version straight into the existing venv. A real 0.0.8 -> 0.0.9 upgrade now prepares and installs 1 package rather than 115, with no prompt and nothing interactive. `uv venv` gains --clear on the paths that do create one, so a half-written venv from an interrupted first run is replaced rather than aborting the same way. If installing into a reused venv fails -- most likely its interpreter no longer satisfies the new release's requires-python -- rebuild once and retry rather than leaving the user stuck. The "(first run)" message now says "upgrading" when that is what is happening. The stub uv in the launcher tests grew a `mkdir -p` where the real one refuses an existing venv, which is why the upgrade path looked healthy in CI while failing for real users. It now models the refusal, and can simulate an install that fails until the venv is rebuilt. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
brew upgradeleftvalcoreunable to start in any non-interactive context.The bug
A formula bump changes
VALCORE_VERSION, the stamp goes stale, and the launcher ranuv venvagainst a path that already held a venv. uv refuses that, and what happens next depends entirely on whether a terminal is attached:set -euo pipefailthe launcher died before reaching the install, so every post-upgrade run from a script, cron job, or CI step failed until a human ranvalcoreby hand and answered the prompt.Reproduced against the real script:
The fix
Create the venv only when there is no usable interpreter at
$venv/bin/python; otherwise install the pinned version straight into the existing venv and let uv work out what actually changed.A real 0.0.8 → 0.0.9 upgrade, stdin closed:
One package instead of 115, no prompt, exit 0.
Three related changes come with it:
uv venvtakes--clearon the paths that do create one, so a half-written venv from an interrupted first run is replaced rather than aborting for the same reason. On a fresh path it is a no-op.requires-python— rebuild once and retry, rather than leaving the user with a launcher that cannot recover on its own.(first run)message now saysupgradingwhen that is what is happening.Why CI didn't catch this
The stub
uvintest_launcher.pyimplementedvenvas a baremkdir -p, which succeeds whether or not the target already exists.test_stale_stamp_triggers_reprovisionthen asserted"venv --python >=3.11" in log, so the test actively enshrined the broken behavior: the upgrade path looked healthy in CI while failing for every real user without a TTY.The stub now refuses an existing venv the way real uv does, and
UV_FAIL_INSTALL_MARKERlets a test simulate an install that keeps failing until the venv is rebuilt.New coverage: upgrades reuse the venv rather than recreating it; the message distinguishes upgrade from first run; a reused venv that cannot take the install is rebuilt; and a partial venv from an interrupted run is cleared.
Testing
954 Python tests pass,
ruffclean. Verified end-to-end against the realuvand real PyPI packages for both a fresh install and an upgrade with stdin closed.The
shellchecktest skips on machines without shellcheck installed, so that assertion ran in CI rather than locally.🤖 Generated with Claude Code