Skip to content

Commit

Permalink
Make metadata deserialization failures non-fatal in the cache (#11105)
Browse files Browse the repository at this point in the history 
## Summary

If we fail to deserialize cached metadata in the cache, we should just
ignore it, rather than failing.

Ideally, this never happens. If it does, it means we missed a cache
version bump. But if it does happen, it should still be non-fatal.

Closes #11043.

Closes #11101.

## Test Plan

Prior to this PR, the following would fail:

- `uvx uv@0.5.25 venv --python 3.12 --cache-dir foo`
- `uvx uv@0.5.25 pip install ./scripts/packages/hatchling_dynamic
--no-deps --python 3.12 --cache-dir foo`
- `uvx uv@0.5.18 venv --python 3.12 --cache-dir foo`
- `uvx uv@0.5.18 pip install ./scripts/packages/hatchling_dynamic
--no-deps --python 3.12 --cache-dir foo`

We can't go back and fix 0.5.18, but this will prevent such regressions
in the future.
  • Loading branch information
@charliermarsh
charliermarsh authored Jan 30, 2025
1 parent 1dfa650 commit d106ab1
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 65 deletions.
2 changes: 1 addition & 1 deletion crates/uv-cache/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -793,7 +793,7 @@ impl CacheBucket {
match self {
// Note that when bumping this, you'll also need to bump it
// in crates/uv/tests/cache_prune.rs.
Self::SourceDistributions => "sdists-v6",
Self::SourceDistributions => "sdists-v7",
Self::FlatIndex => "flat-index-v2",
Self::Git => "git-v0",
Self::Interpreter => "interpreter-v4",
Expand Down
153 changes: 90 additions & 63 deletions crates/uv-distribution/src/source/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -551,15 +551,21 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {

// If the cache contains compatible metadata, return it.
let metadata_entry = cache_shard.entry(METADATA);
if let Some(metadata) = CachedMetadata::read(&metadata_entry)
.await?
.filter(|metadata| metadata.matches(source.name(), source.version()))
{
debug!("Using cached metadata for: {source}");
return Ok(ArchiveMetadata {
metadata: Metadata::from_metadata23(metadata.into()),
hashes: revision.into_hashes(),
});
match CachedMetadata::read(&metadata_entry).await {
Ok(Some(metadata)) => {
if metadata.matches(source.name(), source.version()) {
debug!("Using cached metadata for: {source}");
return Ok(ArchiveMetadata {
metadata: Metadata::from_metadata23(metadata.into()),
hashes: revision.into_hashes(),
});
}
debug!("Cached metadata does not match expected name and version for: {source}");
}
Ok(None) => {}
Err(err) => {
debug!("Failed to deserialize cached metadata for: {source} ({err})");
}
}

// Otherwise, we need a wheel.
Expand Down Expand Up @@ -882,15 +888,21 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {

// If the cache contains compatible metadata, return it.
let metadata_entry = cache_shard.entry(METADATA);
if let Some(metadata) = CachedMetadata::read(&metadata_entry)
.await?
.filter(|metadata| metadata.matches(source.name(), source.version()))
{
debug!("Using cached metadata for: {source}");
return Ok(ArchiveMetadata {
metadata: Metadata::from_metadata23(metadata.into()),
hashes: revision.into_hashes(),
});
match CachedMetadata::read(&metadata_entry).await {
Ok(Some(metadata)) => {
if metadata.matches(source.name(), source.version()) {
debug!("Using cached metadata for: {source}");
return Ok(ArchiveMetadata {
metadata: Metadata::from_metadata23(metadata.into()),
hashes: revision.into_hashes(),
});
}
debug!("Cached metadata does not match expected name and version for: {source}");
}
Ok(None) => {}
Err(err) => {
debug!("Failed to deserialize cached metadata for: {source} ({err})");
}
}

// Otherwise, we need a source distribution.
Expand Down Expand Up @@ -1183,31 +1195,38 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {

// If the cache contains compatible metadata, return it.
let metadata_entry = cache_shard.entry(METADATA);
if let Some(metadata) = CachedMetadata::read(&metadata_entry)
.await?
.filter(|metadata| metadata.matches(source.name(), source.version()))
{
// If necessary, mark the metadata as dynamic.
let metadata = if dynamic {
ResolutionMetadata {
dynamic: true,
..metadata.into()
match CachedMetadata::read(&metadata_entry).await {
Ok(Some(metadata)) => {
if metadata.matches(source.name(), source.version()) {
debug!("Using cached metadata for: {source}");

// If necessary, mark the metadata as dynamic.
let metadata = if dynamic {
ResolutionMetadata {
dynamic: true,
..metadata.into()
}
} else {
metadata.into()
};
return Ok(ArchiveMetadata::from(
Metadata::from_workspace(
metadata,
resource.install_path.as_ref(),
None,
self.build_context.locations(),
self.build_context.sources(),
self.build_context.bounds(),
)
.await?,
));
}
} else {
metadata.into()
};

return Ok(ArchiveMetadata::from(
Metadata::from_workspace(
metadata,
resource.install_path.as_ref(),
None,
self.build_context.locations(),
self.build_context.sources(),
self.build_context.bounds(),
)
.await?,
));
debug!("Cached metadata does not match expected name and version for: {source}");
}
Ok(None) => {}
Err(err) => {
debug!("Failed to deserialize cached metadata for: {source} ({err})");
}
}

// If the backend supports `prepare_metadata_for_build_wheel`, use it.
Expand Down Expand Up @@ -1635,27 +1654,35 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
.map_err(Error::CacheRead)?
.is_fresh()
{
if let Some(metadata) = CachedMetadata::read(&metadata_entry)
.await?
.filter(|metadata| metadata.matches(source.name(), source.version()))
{
let git_member = GitWorkspaceMember {
fetch_root: fetch.path(),
git_source: resource,
};

debug!("Using cached metadata for: {source}");
return Ok(ArchiveMetadata::from(
Metadata::from_workspace(
metadata.into(),
&path,
Some(&git_member),
self.build_context.locations(),
self.build_context.sources(),
self.build_context.bounds(),
)
.await?,
));
match CachedMetadata::read(&metadata_entry).await {
Ok(Some(metadata)) => {
if metadata.matches(source.name(), source.version()) {
debug!("Using cached metadata for: {source}");

let git_member = GitWorkspaceMember {
fetch_root: fetch.path(),
git_source: resource,
};
return Ok(ArchiveMetadata::from(
Metadata::from_workspace(
metadata.into(),
&path,
Some(&git_member),
self.build_context.locations(),
self.build_context.sources(),
self.build_context.bounds(),
)
.await?,
));
}
debug!(
"Cached metadata does not match expected name and version for: {source}"
);
}
Ok(None) => {}
Err(err) => {
debug!("Failed to deserialize cached metadata for: {source} ({err})");
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/uv/tests/it/cache_prune.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ fn prune_stale_revision() -> Result<()> {
----- stderr -----
DEBUG uv [VERSION] ([COMMIT] DATE)
Pruning cache at: [CACHE_DIR]/
DEBUG Removing dangling source revision: [CACHE_DIR]/sdists-v6/[ENTRY]
DEBUG Removing dangling source revision: [CACHE_DIR]/sdists-v7/[ENTRY]
DEBUG Removing dangling cache archive: [CACHE_DIR]/archive-v0/[ENTRY]
Removed [N] files ([SIZE])
"###);
Expand Down

0 comments on commit d106ab1

Please sign in to comment.