Repository navigation
Conversation
f565224 to
b059d80
Compare
b059d80 to
31475c2
Compare
Merging this PR will not alter performance
|
31475c2 to
d5d2405
Compare
| if let Err(Error::HashMismatch { .. }) = &result { | ||
| match fs_err::tokio::remove_file(&target_cache_file).await { |
There was a problem hiding this comment.
[P2] Coordinate eviction with shared-cache writers
On Unix, installs with different installation directories can share UV_PYTHON_CACHE_DIR; the callers' installation-directory locks do not serialize them. Two readers can open the corrupt archive, one can remove it, and a subsequent attempt can publish a valid replacement before the slower reader reaches this removal. That reader then deletes the replacement, potentially causing the downloader's subsequent File::open to fail with NotFound. Use a cache-entry lock covering opening, publication, validation, and eviction so removal targets the archive that failed verification.
| match fs_err::tokio::remove_file(&target_cache_file).await { | ||
| Ok(()) => {} | ||
| Err(err) if err.kind() == io::ErrorKind::NotFound => {} | ||
| Err(err) => return Err(err.into()), |
There was a problem hiding this comment.
[P2] Preserve the hash mismatch when eviction fails
If the cached archive is readable but cannot be deleted, such as in a read-only shared cache, this return replaces the confirmed HashMismatch with the cleanup I/O error. Callers consequently lose the integrity-failure diagnostic and its expected/computed hashes. Keep eviction best-effort and report its failure separately while returning the original mismatch, or retain both errors in a contextual error.
| let temp_dir = tempfile::tempdir().expect("temporary directory should be created"); | ||
| let cache_dir = temp_dir.path().join("cache"); | ||
| fs_err::create_dir_all(&cache_dir).expect("cache directory should be created"); |
There was a problem hiding this comment.
[P3] Exercise a cache hit in the entry-lock test
This fixture creates the cache directory and lock, but never writes ababababa-python.zip. It therefore exercises a cache miss: a refactor that locks only downloads while leaving cached readers unlocked would still pass, despite breaking eviction coordination. Seed the matching cached archive before acquiring the lock so this test verifies that existing-archive reads wait too.
| fs_err::write( | ||
| &cached_archive, | ||
| [ | ||
| 0x50, 0x4b, 0x03, 0x04, 0x0a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x4d, 0x1e, 0x48, 0x5d, | ||
| 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0d, 0x00, | ||
| 0x00, 0x00, 0x70, 0x79, 0x74, 0x68, 0x6f, 0x6e, 0x2f, 0x70, 0x79, 0x74, 0x68, 0x6f, |
There was a problem hiding this comment.
[P3] Make the archive fixture's contents explicit
These bytes encode a valid ZIP containing an empty python/python file. Successful extraction is essential here: malformed archive data would return ExtractError before reaching hash validation. The opaque byte array hides that requirement and makes fixture changes require understanding ZIP encoding. Prefer constructing named archive entries with the existing uv_test::archive::write_tar_gz helper in the python_install.rs integration module, and document why the expected digest deliberately differs.
d0e207a to
68dfb81
Compare
68dfb81 to
4dd00dc
Compare
| #[cfg(unix)] | ||
| { | ||
| use std::os::unix::fs::PermissionsExt; | ||
| fs_err::set_permissions(cache.path(), std::fs::Permissions::from_mode(0o555))?; | ||
| let (snapshot, _) = uv_test::run_and_format( |
There was a problem hiding this comment.
[P3] Separate the read-only cache scenario into a named test
This Unix-only block checks retention when lock acquisition fails, while the invocation below checks eviction after successful locking. These are independent regression scenarios: a failure here prevents the eviction assertion from running on Unix, and the read-only behavior cannot be selected independently. Following the repository’s test guidance, move this scenario into a separate #[cfg(unix)] test with explicit fixture setup, leaving the writable-cache eviction test cross-platform.
4dd00dc to
aa4191d
Compare
A downloaded Python archive is cached before its hash is verified, so a corrupt archive is reused on later runs. Remove it after a hash mismatch so the next run can download it again. This does not retry immediately or remove unrelated failed downloads. uv#19795 made cached Python downloads survive pruning, so corrupt entries can persist.