Upgrade UV and delete orphaned lock files #185
Conversation
bradhe
requested review from
Copilot,
jo-sm,
konstantinoscs,
sammuti and
socksy
There was a problem hiding this comment.
Pull request overview
This PR upgrades UV from version 0.7.13 to 0.9.27 and implements lock file cleanup functionality to address orphaned lock files in the system temp directory.
Changes:
- Upgraded UV version from 0.7.13 to 0.9.27
- Added functions to compute, track, and clean up UV lock files based on working directory hashes
- Implemented opportunistic cleanup of stale lock files during UV initialization
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| crates/tower-uv/src/lib.rs | Core implementation of lock file cleanup logic, hash computation, and comprehensive test coverage |
| crates/tower-uv/Cargo.toml | Added dependencies for file locking (fs2), hashing (seahash), and hex encoding |
| Cargo.toml | Workspace-level dependency declarations for new libraries |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
sammuti
left a comment
sammuti
left a comment
There was a problem hiding this comment.
Change looks good but there is a question on how to call the cleanup logic IMO
|
|
||
| // UV_VERSION is the version of UV to download and install when setting up a local UV deployment. | ||
| pub const UV_VERSION: &str = "0.7.13"; | ||
| pub const UV_VERSION: &str = "0.9.27"; |
There was a problem hiding this comment.
Wonder if we should make this a runtime config
There was a problem hiding this comment.
I think we want to lock it down at least for now because we want to ensure it works with our software.
crates/tower-uv/src/lib.rs
Outdated
|
|
||
| // Only process files matching the uv-*.lock pattern | ||
| if let Some(file_name) = path.file_name().and_then(|n| n.to_str()) { | ||
| if !file_name.starts_with("uv-") || !file_name.ends_with(".lock") { |
There was a problem hiding this comment.
Probably best to make this a stricter regex
crates/tower-uv/src/lib.rs
Outdated
| impl Uv { | ||
| pub async fn new(cache_dir: Option<PathBuf>, protected_mode: bool) -> Result<Self, Error> { | ||
| // Opportunistically clean up any stale UV lock files left behind from previous runs. | ||
| cleanup_stale_uv_lock_files(); |
There was a problem hiding this comment.
I think this may be surprising behavior in a constructor, why not expose the function and have runner call this?
There was a problem hiding this comment.
Good call, I've updated this.
| debug!("Cleaned up stale UV lock file: {:?}", path); | ||
| } | ||
| } | ||
| // If we couldn't get the lock, another process is using it, so leave it alone |
There was a problem hiding this comment.
if we assume one runner is running on one device, maybe we should clean up all locks on runner startup, regardless of whether they're POSIX locked?
Title says it all really. We found that we have loads of orphaned lock files laying around. This PR cleans them up by recreating the associated hash from the current uv run. It also upgrades UV to the latest version, we were/are using a majorly out of date version.
We should also land the changes that @sammuti proposed around temp directory and cleaning up after a run.