-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Prevent concurrent updates of the environment in uv run
#14153
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This is very similar to the locking added for `uv sync`, `uv add`, and `uv remove` in #13869. Improving our (f)locking in general is tracked in #13883. This is a minimal change that un-breaks parallel invocations of `uv run`. It's too easy to confuse locking as in "lockfile generation" and locking as in "file locking to prevent race conditions", so we probably need a broader overhaul of how these things are named. It's also too easy to miss a callsite here, or to accidentally retain one of these locks across `run_to_completion`, so ideally we could move this file locking down to a lower-level function that's shared by all these callers, possibly `pip::operations::install`. I want to get this minimal change through testing and review before I tackle either of those.
|
I wonder if this is a flake? Reruning it. (Edit: Yep: #14160.) |
| .await? | ||
| .into_environment()?; | ||
|
|
||
| let _lock = environment.lock().await?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: we can more one statement later, just before update_environment
|
Manual testing for the two script cases. First unlocked: Fixed with this PR: And now the locked case: Fixed with this PR: |
|
As a minor note, I'd title this to be user-facing so it makes sense in the changelog What's the actual user-facing change? "Lock the Python environment during updates in |
|
Got it. Yes, I'd go with either of your descriptions above, or maybe "lock the virtual environment when |
uv runuv run
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [astral-sh/uv](https://github.com/astral-sh/uv) | patch | `0.7.13` -> `0.7.14` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>astral-sh/uv (astral-sh/uv)</summary> ### [`v0.7.14`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#0714) [Compare Source](astral-sh/uv@0.7.13...0.7.14) ##### Enhancements - Add XPU to `--torch-backend` ([#​14172](astral-sh/uv#14172)) - Add ROCm backends to `--torch-backend` ([#​14120](astral-sh/uv#14120)) - Remove preview label from `--torch-backend` ([#​14119](astral-sh/uv#14119)) - Add `[tool.uv.dependency-groups].mygroup.requires-python` ([#​13735](astral-sh/uv#13735)) - Add auto-detection for AMD GPUs ([#​14176](astral-sh/uv#14176)) - Show retries for HTTP status code errors ([#​13897](astral-sh/uv#13897)) - Support transparent Python patch version upgrades ([#​13954](astral-sh/uv#13954)) - Warn on empty index directory ([#​13940](astral-sh/uv#13940)) - Publish to DockerHub ([#​14088](astral-sh/uv#14088)) ##### Performance - Make cold resolves about 10% faster ([#​14035](astral-sh/uv#14035)) ##### Bug fixes - Don't use walrus operator in interpreter query script ([#​14108](astral-sh/uv#14108)) - Fix handling of changes to `requires-python` ([#​14076](astral-sh/uv#14076)) - Fix implied `platform_machine` marker for `win_amd64` platform tag ([#​14041](astral-sh/uv#14041)) - Only update existing symlink directories on preview uninstall ([#​14179](astral-sh/uv#14179)) - Serialize Python requests for tools as canonicalized strings ([#​14109](astral-sh/uv#14109)) - Support netrc and same-origin credential propagation on index redirects ([#​14126](astral-sh/uv#14126)) - Support reading `dependency-groups` from pyproject.tomls with no `[project]` ([#​13742](astral-sh/uv#13742)) - Handle an existing shebang in `uv init --script` ([#​14141](astral-sh/uv#14141)) - Prevent concurrent updates of the environment in `uv run` ([#​14153](astral-sh/uv#14153)) - Filter managed Python distributions by platform before querying when included in request ([#​13936](astral-sh/uv#13936)) ##### Documentation - Replace cuda124 with cuda128 ([#​14168](astral-sh/uv#14168)) - Document the way member sources shadow workspace sources ([#​14136](astral-sh/uv#14136)) - Sync documented PyTorch integration index for CUDA and ROCm versions from PyTorch website ([#​14100](astral-sh/uv#14100)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MC42Mi4xIiwidXBkYXRlZEluVmVyIjoiNDAuNjIuMSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiUmVub3ZhdGUgQm90Il19-->
This is very similar to the locking added for
uv sync,uv add, anduv removein #13869. Improving our (f)locking in general is tracked in#13883.
This is a minimal change that un-breaks parallel invocations of
uv run. It's too easy to confuse locking as in "lockfile generation" and locking as in "file locking to prevent race conditions", so we probably need a broader overhaul of how these things are named. It's also too easy to miss a callsite here, or to accidentally retain one of these locks acrossrun_to_completion, so ideally we could move this file locking down to a lower-level function that's shared by all these callers, possiblypip::operations::install. I want to get this minimal change through testing and review before I tackle either of those.Here's a repro of the current race condition in
uv run, which this PR fixes. Add a bunch of dependencies to provoke it, as suggested in #12751:With this change: