Skip to content

Commit

Permalink
Include archive bucket version in archive pointers (astral-sh#11306)
Browse files Browse the repository at this point in the history
We've never bumped the version of this bucket, and we may never do so...
But it's still incorrect for us to omit it from these serialized structs
in the cache. Specifically, these structs include a pointer into the
archive bucket (namely, the ID). But we don't include the bucket
version! So, in theory, we could end up pointing to archives that don't
match the current bucket version expected in the code.
  • Loading branch information
charliermarsh authored and Loïc LESCOAT committed Mar 2, 2025
1 parent 0f6123d commit 7d81211
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 9 deletions.
10 changes: 6 additions & 4 deletions crates/uv-cache/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -797,17 +797,19 @@ impl CacheBucket {
fn to_str(self) -> &'static str {
match self {
// Note that when bumping this, you'll also need to bump it
// in crates/uv/tests/cache_prune.rs.
// in `crates/uv/tests/it/cache_prune.rs`.
Self::SourceDistributions => "sdists-v7",
Self::FlatIndex => "flat-index-v2",
Self::Git => "git-v0",
Self::Interpreter => "interpreter-v4",
// Note that when bumping this, you'll also need to bump it
// in crates/uv/tests/cache_clean.rs.
// in `crates/uv/tests/it/cache_clean.rs`.
Self::Simple => "simple-v15",
// Note that when bumping this, you'll also need to bump it
// in crates/uv/tests/cache_prune.rs.
Self::Wheels => "wheels-v3",
// in `crates/uv/tests/it/cache_prune.rs`.
Self::Wheels => "wheels-v4",
// Note that when bumping this, you'll also need to bump it
// in `crates/uv-distribution/src/archive.rs`.
Self::Archive => "archive-v0",
Self::Builds => "builds-v0",
Self::Environments => "environments-v1",
Expand Down
15 changes: 13 additions & 2 deletions crates/uv-distribution/src/archive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,35 @@ use uv_cache::{ArchiveId, Cache};
use uv_distribution_types::Hashed;
use uv_pypi_types::HashDigest;

/// The version of the archive bucket.
///
/// Must be kept in-sync with the version in [`uv_cache::CacheBucket::to_str`].
const ARCHIVE_VERSION: u8 = 0;

/// An archive (unzipped wheel) that exists in the local cache.
#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)]
pub struct Archive {
/// The unique ID of the entry in the wheel's archive bucket.
pub id: ArchiveId,
/// The computed hashes of the archive.
pub hashes: Vec<HashDigest>,
/// The version of the archive bucket.
pub version: u8,
}

impl Archive {
/// Create a new [`Archive`] with the given ID and hashes.
pub(crate) fn new(id: ArchiveId, hashes: Vec<HashDigest>) -> Self {
Self { id, hashes }
Self {
id,
hashes,
version: ARCHIVE_VERSION,
}
}

/// Returns `true` if the archive exists in the cache.
pub(crate) fn exists(&self, cache: &Cache) -> bool {
cache.archive(&self.id).exists()
self.version == ARCHIVE_VERSION && cache.archive(&self.id).exists()
}
}

Expand Down
18 changes: 16 additions & 2 deletions crates/uv-distribution/src/index/cached_wheel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,14 @@ impl CachedWheel {
// Read the pointer.
let pointer = HttpArchivePointer::read_from(path).ok()??;
let cache_info = pointer.to_cache_info();
let Archive { id, hashes } = pointer.into_archive();
let archive = pointer.into_archive();

// Ignore stale pointers.
if !archive.exists(cache) {
return None;
}

let Archive { id, hashes, .. } = archive;

let entry = cache.entry(CacheBucket::Archive, "", id);

Expand All @@ -151,7 +158,14 @@ impl CachedWheel {
// Read the pointer.
let pointer = LocalArchivePointer::read_from(path).ok()??;
let cache_info = pointer.to_cache_info();
let Archive { id, hashes } = pointer.into_archive();
let archive = pointer.into_archive();

// Ignore stale pointers.
if !archive.exists(cache) {
return None;
}

let Archive { id, hashes, .. } = archive;

// Convert to a cached wheel.
let entry = cache.entry(CacheBucket::Archive, "", id);
Expand Down
2 changes: 1 addition & 1 deletion crates/uv/tests/it/cache_prune.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ fn prune_stale_symlink() -> Result<()> {
.success();

// Remove the wheels directory, causing the symlink to become stale.
let wheels = context.cache_dir.child("wheels-v3");
let wheels = context.cache_dir.child("wheels-v4");
fs_err::remove_dir_all(wheels)?;

let filters: Vec<_> = context
Expand Down

0 comments on commit 7d81211

Please sign in to comment.