From 8552822527819c907b2d5c7511030e66dee46af3 Mon Sep 17 00:00:00 2001 From: Aria Desires Date: Tue, 18 Mar 2025 10:01:51 -0400 Subject: [PATCH 1/5] Make errors non-fatal for cache archive pointer reads in Planner --- crates/uv-installer/src/plan.rs | 90 +++++++++++++++++++-------------- 1 file changed, 52 insertions(+), 38 deletions(-) diff --git a/crates/uv-installer/src/plan.rs b/crates/uv-installer/src/plan.rs index c6bbdb5303a1a..cb51d42608318 100644 --- a/crates/uv-installer/src/plan.rs +++ b/crates/uv-installer/src/plan.rs @@ -180,24 +180,31 @@ impl<'a> Planner<'a> { .entry(format!("{}.http", wheel.filename.cache_key())); // Read the HTTP pointer. - if let Some(pointer) = HttpArchivePointer::read_from(&cache_entry)? { - let cache_info = pointer.to_cache_info(); - let archive = pointer.into_archive(); - if archive.satisfies(hasher.get(dist.as_ref())) { - let cached_dist = CachedDirectUrlDist { - filename: wheel.filename.clone(), - url: VerbatimParsedUrl { - parsed_url: wheel.parsed_url(), - verbatim: wheel.url.clone(), - }, - hashes: archive.hashes, - cache_info, - path: cache.archive(&archive.id), - }; - - debug!("URL wheel requirement already cached: {cached_dist}"); - cached.push(CachedDist::Url(cached_dist)); - continue; + match HttpArchivePointer::read_from(&cache_entry) { + Ok(Some(pointer)) => { + let cache_info = pointer.to_cache_info(); + let archive = pointer.into_archive(); + if archive.satisfies(hasher.get(dist.as_ref())) { + let cached_dist = CachedDirectUrlDist { + filename: wheel.filename.clone(), + url: VerbatimParsedUrl { + parsed_url: wheel.parsed_url(), + verbatim: wheel.url.clone(), + }, + hashes: archive.hashes, + cache_info, + path: cache.archive(&archive.id), + }; + + debug!("URL wheel requirement already cached: {cached_dist}"); + cached.push(CachedDist::Url(cached_dist)); + continue; + } + debug!("Cached URL wheel requirement does not match expected hash policy for: {wheel}"); + } + Ok(None) => {} + Err(err) => { + debug!("Failed to deserialize cached URL wheel requirement for: {wheel} ({err})"); } } } @@ -230,28 +237,35 @@ impl<'a> Planner<'a> { ) .entry(format!("{}.rev", wheel.filename.cache_key())); - if let Some(pointer) = LocalArchivePointer::read_from(&cache_entry)? { - let timestamp = Timestamp::from_path(&wheel.install_path)?; - if pointer.is_up_to_date(timestamp) { - let cache_info = pointer.to_cache_info(); - let archive = pointer.into_archive(); - if archive.satisfies(hasher.get(dist.as_ref())) { - let cached_dist = CachedDirectUrlDist { - filename: wheel.filename.clone(), - url: VerbatimParsedUrl { - parsed_url: wheel.parsed_url(), - verbatim: wheel.url.clone(), - }, - hashes: archive.hashes, - cache_info, - path: cache.archive(&archive.id), - }; - - debug!("Path wheel requirement already cached: {cached_dist}"); - cached.push(CachedDist::Url(cached_dist)); - continue; + match LocalArchivePointer::read_from(&cache_entry) { + Ok(Some(pointer)) => { + let timestamp = Timestamp::from_path(&wheel.install_path)?; + if pointer.is_up_to_date(timestamp) { + let cache_info = pointer.to_cache_info(); + let archive = pointer.into_archive(); + if archive.satisfies(hasher.get(dist.as_ref())) { + let cached_dist = CachedDirectUrlDist { + filename: wheel.filename.clone(), + url: VerbatimParsedUrl { + parsed_url: wheel.parsed_url(), + verbatim: wheel.url.clone(), + }, + hashes: archive.hashes, + cache_info, + path: cache.archive(&archive.id), + }; + + debug!("Path wheel requirement already cached: {cached_dist}"); + cached.push(CachedDist::Url(cached_dist)); + continue; + } + debug!("Cached path wheel requirement does not match expected hash policy for: {wheel}"); } } + Ok(None) => {} + Err(err) => { + debug!("Failed to deserialize cached path wheel requirement for: {wheel} ({err})"); + } } } Dist::Source(SourceDist::Registry(sdist)) => { From 233a6222de03fff8a257464fda63d2307a92bc18 Mon Sep 17 00:00:00 2001 From: Aria Desires Date: Tue, 18 Mar 2025 10:13:40 -0400 Subject: [PATCH 2/5] eliminate all other cache errors --- crates/uv-installer/src/plan.rs | 157 +++++++++++++++++++------------- 1 file changed, 96 insertions(+), 61 deletions(-) diff --git a/crates/uv-installer/src/plan.rs b/crates/uv-installer/src/plan.rs index cb51d42608318..f86f792c30537 100644 --- a/crates/uv-installer/src/plan.rs +++ b/crates/uv-installer/src/plan.rs @@ -97,17 +97,20 @@ impl<'a> Planner<'a> { [] => {} [installed] => { let source = RequirementSource::from(dist); - match RequirementSatisfaction::check(installed, &source)? { - RequirementSatisfaction::Mismatch => { + match RequirementSatisfaction::check(installed, &source) { + Ok(RequirementSatisfaction::Mismatch) => { debug!("Requirement installed, but mismatched:\n Installed: {installed:?}\n Requested: {source:?}"); } - RequirementSatisfaction::Satisfied => { + Ok(RequirementSatisfaction::Satisfied) => { debug!("Requirement already installed: {installed}"); continue; } - RequirementSatisfaction::OutOfDate => { + Ok(RequirementSatisfaction::OutOfDate) => { debug!("Requirement installed, but not fresh: {installed}"); } + Err(err) => { + debug!("Failed to deserialize cached requirements for: {installed} ({err})"); + } } reinstalls.push(installed.clone()); } @@ -195,7 +198,7 @@ impl<'a> Planner<'a> { cache_info, path: cache.archive(&archive.id), }; - + debug!("URL wheel requirement already cached: {cached_dist}"); cached.push(CachedDist::Url(cached_dist)); continue; @@ -239,27 +242,33 @@ impl<'a> Planner<'a> { match LocalArchivePointer::read_from(&cache_entry) { Ok(Some(pointer)) => { - let timestamp = Timestamp::from_path(&wheel.install_path)?; - if pointer.is_up_to_date(timestamp) { - let cache_info = pointer.to_cache_info(); - let archive = pointer.into_archive(); - if archive.satisfies(hasher.get(dist.as_ref())) { - let cached_dist = CachedDirectUrlDist { - filename: wheel.filename.clone(), - url: VerbatimParsedUrl { - parsed_url: wheel.parsed_url(), - verbatim: wheel.url.clone(), - }, - hashes: archive.hashes, - cache_info, - path: cache.archive(&archive.id), - }; - - debug!("Path wheel requirement already cached: {cached_dist}"); - cached.push(CachedDist::Url(cached_dist)); - continue; + match Timestamp::from_path(&wheel.install_path) { + Ok(timestamp) => { + if pointer.is_up_to_date(timestamp) { + let cache_info = pointer.to_cache_info(); + let archive = pointer.into_archive(); + if archive.satisfies(hasher.get(dist.as_ref())) { + let cached_dist = CachedDirectUrlDist { + filename: wheel.filename.clone(), + url: VerbatimParsedUrl { + parsed_url: wheel.parsed_url(), + verbatim: wheel.url.clone(), + }, + hashes: archive.hashes, + cache_info, + path: cache.archive(&archive.id), + }; + + debug!("Path wheel requirement already cached: {cached_dist}"); + cached.push(CachedDist::Url(cached_dist)); + continue; + } + debug!("Cached path wheel requirement does not match expected hash policy for: {wheel}"); + } + } + Err(err) => { + debug!("failed to get timestamp for wheel {wheel} ({err})"); } - debug!("Cached path wheel requirement does not match expected hash policy for: {wheel}"); } } Ok(None) => {} @@ -295,19 +304,27 @@ impl<'a> Planner<'a> { Dist::Source(SourceDist::DirectUrl(sdist)) => { // Find the most-compatible wheel from the cache, since we don't know // the filename in advance. - if let Some(wheel) = built_index.url(sdist)? { - if wheel.filename.name == sdist.name { - let cached_dist = wheel.into_url_dist(sdist); - debug!("URL source requirement already cached: {cached_dist}"); - cached.push(CachedDist::Url(cached_dist)); - continue; - } + match built_index.url(sdist) { + Ok(Some(wheel)) => { + if wheel.filename.name == sdist.name { + let cached_dist = wheel.into_url_dist(sdist); + debug!("URL source requirement already cached: {cached_dist}"); + cached.push(CachedDist::Url(cached_dist)); + continue; + } - warn!( - "Cached wheel filename does not match requested distribution for: `{}` (found: `{}`)", - sdist, - wheel.filename - ); + warn!( + "Cached wheel filename does not match requested distribution for: `{}` (found: `{}`)", + sdist, + wheel.filename + ); + } + Ok(None) => {} + Err(err) => { + debug!( + "Failed to deserialize cached wheel filename for: {sdist} ({err})" + ); + } } } Dist::Source(SourceDist::Git(sdist)) => { @@ -336,19 +353,27 @@ impl<'a> Planner<'a> { // Find the most-compatible wheel from the cache, since we don't know // the filename in advance. - if let Some(wheel) = built_index.path(sdist)? { - if wheel.filename.name == sdist.name { - let cached_dist = wheel.into_path_dist(sdist); - debug!("Path source requirement already cached: {cached_dist}"); - cached.push(CachedDist::Url(cached_dist)); - continue; - } + match built_index.path(sdist) { + Ok(Some(wheel)) => { + if wheel.filename.name == sdist.name { + let cached_dist = wheel.into_path_dist(sdist); + debug!("Path source requirement already cached: {cached_dist}"); + cached.push(CachedDist::Url(cached_dist)); + continue; + } - warn!( - "Cached wheel filename does not match requested distribution for: `{}` (found: `{}`)", - sdist, - wheel.filename - ); + warn!( + "Cached wheel filename does not match requested distribution for: `{}` (found: `{}`)", + sdist, + wheel.filename + ); + } + Ok(None) => {} + Err(err) => { + debug!( + "Failed to deserialize cached wheel filename for: {sdist} ({err})" + ); + } } } Dist::Source(SourceDist::Directory(sdist)) => { @@ -359,19 +384,29 @@ impl<'a> Planner<'a> { // Find the most-compatible wheel from the cache, since we don't know // the filename in advance. - if let Some(wheel) = built_index.directory(sdist)? { - if wheel.filename.name == sdist.name { - let cached_dist = wheel.into_directory_dist(sdist); - debug!("Directory source requirement already cached: {cached_dist}"); - cached.push(CachedDist::Url(cached_dist)); - continue; - } + match built_index.directory(sdist) { + Ok(Some(wheel)) => { + if wheel.filename.name == sdist.name { + let cached_dist = wheel.into_directory_dist(sdist); + debug!( + "Directory source requirement already cached: {cached_dist}" + ); + cached.push(CachedDist::Url(cached_dist)); + continue; + } - warn!( - "Cached wheel filename does not match requested distribution for: `{}` (found: `{}`)", - sdist, - wheel.filename - ); + warn!( + "Cached wheel filename does not match requested distribution for: `{}` (found: `{}`)", + sdist, + wheel.filename + ); + } + Ok(None) => {} + Err(err) => { + debug!( + "Failed to deserialize cached wheel filename for: {sdist} ({err})" + ); + } } } } From fcf830e72de6317d78ab1994e1376efe334b847c Mon Sep 17 00:00:00 2001 From: Aria Desires Date: Tue, 18 Mar 2025 10:17:00 -0400 Subject: [PATCH 3/5] bump cache version --- crates/uv-cache/src/lib.rs | 2 +- crates/uv/tests/it/cache_prune.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/uv-cache/src/lib.rs b/crates/uv-cache/src/lib.rs index 4e3ff1f955fa4..18b62846175fe 100644 --- a/crates/uv-cache/src/lib.rs +++ b/crates/uv-cache/src/lib.rs @@ -1000,7 +1000,7 @@ impl CacheBucket { match self { // Note that when bumping this, you'll also need to bump it // in `crates/uv/tests/it/cache_prune.rs`. - Self::SourceDistributions => "sdists-v8", + Self::SourceDistributions => "sdists-v9", Self::FlatIndex => "flat-index-v2", Self::Git => "git-v0", Self::Interpreter => "interpreter-v4", diff --git a/crates/uv/tests/it/cache_prune.rs b/crates/uv/tests/it/cache_prune.rs index 68749aaff0b86..8d0ada791b417 100644 --- a/crates/uv/tests/it/cache_prune.rs +++ b/crates/uv/tests/it/cache_prune.rs @@ -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-v8/[ENTRY] + DEBUG Removing dangling source revision: [CACHE_DIR]/sdists-v9/[ENTRY] DEBUG Removing dangling cache archive: [CACHE_DIR]/archive-v0/[ENTRY] Removed [N] files ([SIZE]) "###); From 34b6b631302726871b8666caeacf6e1c1aac78a2 Mon Sep 17 00:00:00 2001 From: Aria Desires Date: Tue, 18 Mar 2025 10:28:04 -0400 Subject: [PATCH 4/5] fix caps --- crates/uv-installer/src/plan.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/uv-installer/src/plan.rs b/crates/uv-installer/src/plan.rs index f86f792c30537..1edf91db47870 100644 --- a/crates/uv-installer/src/plan.rs +++ b/crates/uv-installer/src/plan.rs @@ -267,7 +267,7 @@ impl<'a> Planner<'a> { } } Err(err) => { - debug!("failed to get timestamp for wheel {wheel} ({err})"); + debug!("Failed to get timestamp for wheel {wheel} ({err})"); } } } From a56b6324124d9779d3ef26669efef606f91ece7f Mon Sep 17 00:00:00 2001 From: Aria Desires Date: Tue, 18 Mar 2025 11:00:06 -0400 Subject: [PATCH 5/5] remove fallibility from RequirementSatisfaction::check --- crates/uv-installer/src/plan.rs | 10 +-- crates/uv-installer/src/satisfies.rs | 100 ++++++++++++++--------- crates/uv-installer/src/site_packages.rs | 11 +-- 3 files changed, 73 insertions(+), 48 deletions(-) diff --git a/crates/uv-installer/src/plan.rs b/crates/uv-installer/src/plan.rs index 1edf91db47870..26bcf933ced17 100644 --- a/crates/uv-installer/src/plan.rs +++ b/crates/uv-installer/src/plan.rs @@ -98,18 +98,18 @@ impl<'a> Planner<'a> { [installed] => { let source = RequirementSource::from(dist); match RequirementSatisfaction::check(installed, &source) { - Ok(RequirementSatisfaction::Mismatch) => { + RequirementSatisfaction::Mismatch => { debug!("Requirement installed, but mismatched:\n Installed: {installed:?}\n Requested: {source:?}"); } - Ok(RequirementSatisfaction::Satisfied) => { + RequirementSatisfaction::Satisfied => { debug!("Requirement already installed: {installed}"); continue; } - Ok(RequirementSatisfaction::OutOfDate) => { + RequirementSatisfaction::OutOfDate => { debug!("Requirement installed, but not fresh: {installed}"); } - Err(err) => { - debug!("Failed to deserialize cached requirements for: {installed} ({err})"); + RequirementSatisfaction::CacheInvalid => { + // Already logged } } reinstalls.push(installed.clone()); diff --git a/crates/uv-installer/src/satisfies.rs b/crates/uv-installer/src/satisfies.rs index 07ba32bcbfa1d..f91c7ff0feca1 100644 --- a/crates/uv-installer/src/satisfies.rs +++ b/crates/uv-installer/src/satisfies.rs @@ -15,16 +15,14 @@ pub(crate) enum RequirementSatisfaction { Mismatch, Satisfied, OutOfDate, + CacheInvalid, } impl RequirementSatisfaction { /// Returns true if a requirement is satisfied by an installed distribution. /// /// Returns an error if IO fails during a freshness check for a local path. - pub(crate) fn check( - distribution: &InstalledDist, - source: &RequirementSource, - ) -> anyhow::Result { + pub(crate) fn check(distribution: &InstalledDist, source: &RequirementSource) -> Self { trace!( "Comparing installed with source: {:?} {:?}", distribution, @@ -35,9 +33,9 @@ impl RequirementSatisfaction { // If the requirement comes from a registry, check by name. RequirementSource::Registry { specifier, .. } => { if specifier.contains(distribution.version()) { - return Ok(Self::Satisfied); + return Self::Satisfied; } - Ok(Self::Mismatch) + Self::Mismatch } RequirementSource::Url { // We use the location since `direct_url.json` also stores this URL, e.g. @@ -55,7 +53,7 @@ impl RequirementSatisfaction { .. }) = &distribution else { - return Ok(Self::Mismatch); + return Self::Mismatch; }; let DirectUrl::ArchiveUrl { url: installed_url, @@ -63,37 +61,47 @@ impl RequirementSatisfaction { subdirectory: installed_subdirectory, } = direct_url.as_ref() else { - return Ok(Self::Mismatch); + return Self::Mismatch; }; if *editable { - return Ok(Self::Mismatch); + return Self::Mismatch; } if requested_subdirectory != installed_subdirectory { - return Ok(Self::Mismatch); + return Self::Mismatch; } if !CanonicalUrl::parse(installed_url) .is_ok_and(|installed_url| installed_url == CanonicalUrl::new(requested_url)) { - return Ok(Self::Mismatch); + return Self::Mismatch; } // If the requirement came from a local path, check freshness. if requested_url.scheme() == "file" { if let Ok(archive) = requested_url.to_file_path() { let Some(cache_info) = cache_info.as_ref() else { - return Ok(Self::OutOfDate); + return Self::OutOfDate; }; - if *cache_info != CacheInfo::from_path(&archive)? { - return Ok(Self::OutOfDate); + match CacheInfo::from_path(&archive) { + Ok(read_cache_info) => { + if *cache_info != read_cache_info { + return Self::OutOfDate; + } + } + Err(err) => { + debug!( + "Failed to read cached requirement for: {distribution} ({err})" + ); + return Self::CacheInvalid; + } } } } // Otherwise, assume the requirement is up-to-date. - Ok(Self::Satisfied) + Self::Satisfied } RequirementSource::Git { url: _, @@ -102,7 +110,7 @@ impl RequirementSatisfaction { } => { let InstalledDist::Url(InstalledDirectUrlDist { direct_url, .. }) = &distribution else { - return Ok(Self::Mismatch); + return Self::Mismatch; }; let DirectUrl::VcsUrl { url: installed_url, @@ -115,7 +123,7 @@ impl RequirementSatisfaction { subdirectory: installed_subdirectory, } = direct_url.as_ref() else { - return Ok(Self::Mismatch); + return Self::Mismatch; }; if requested_subdirectory != installed_subdirectory { @@ -123,7 +131,7 @@ impl RequirementSatisfaction { "Subdirectory mismatch: {:?} vs. {:?}", installed_subdirectory, requested_subdirectory ); - return Ok(Self::Mismatch); + return Self::Mismatch; } if !RepositoryUrl::parse(installed_url).is_ok_and(|installed_url| { @@ -134,7 +142,7 @@ impl RequirementSatisfaction { installed_url, requested_git.repository() ); - return Ok(Self::Mismatch); + return Self::Mismatch; } // TODO(charlie): It would be more consistent for us to compare the requested @@ -147,10 +155,10 @@ impl RequirementSatisfaction { installed_precise, requested_git.precise() ); - return Ok(Self::OutOfDate); + return Self::OutOfDate; } - Ok(Self::Satisfied) + Self::Satisfied } RequirementSource::Path { install_path: requested_path, @@ -163,7 +171,7 @@ impl RequirementSatisfaction { .. }) = &distribution else { - return Ok(Self::Mismatch); + return Self::Mismatch; }; let DirectUrl::ArchiveUrl { url: installed_url, @@ -171,14 +179,14 @@ impl RequirementSatisfaction { subdirectory: None, } = direct_url.as_ref() else { - return Ok(Self::Mismatch); + return Self::Mismatch; }; let Some(installed_path) = Url::parse(installed_url) .ok() .and_then(|url| url.to_file_path().ok()) else { - return Ok(Self::Mismatch); + return Self::Mismatch; }; if !(*requested_path == installed_path @@ -189,17 +197,25 @@ impl RequirementSatisfaction { requested_path, installed_path, ); - return Ok(Self::Mismatch); + return Self::Mismatch; } let Some(cache_info) = cache_info.as_ref() else { - return Ok(Self::OutOfDate); + return Self::OutOfDate; }; - if *cache_info != CacheInfo::from_path(requested_path)? { - return Ok(Self::OutOfDate); + match CacheInfo::from_path(requested_path) { + Ok(read_cache_info) => { + if *cache_info != read_cache_info { + return Self::OutOfDate; + } + } + Err(err) => { + debug!("Failed to read cached requirement for: {distribution} ({err})"); + return Self::CacheInvalid; + } } - Ok(Self::Satisfied) + Self::Satisfied } RequirementSource::Directory { install_path: requested_path, @@ -213,7 +229,7 @@ impl RequirementSatisfaction { .. }) = &distribution else { - return Ok(Self::Mismatch); + return Self::Mismatch; }; let DirectUrl::LocalDirectory { url: installed_url, @@ -223,7 +239,7 @@ impl RequirementSatisfaction { }, } = direct_url.as_ref() else { - return Ok(Self::Mismatch); + return Self::Mismatch; }; if *requested_editable != installed_editable.unwrap_or_default() { @@ -232,14 +248,14 @@ impl RequirementSatisfaction { *requested_editable, installed_editable.unwrap_or_default() ); - return Ok(Self::Mismatch); + return Self::Mismatch; } let Some(installed_path) = Url::parse(installed_url) .ok() .and_then(|url| url.to_file_path().ok()) else { - return Ok(Self::Mismatch); + return Self::Mismatch; }; if !(*requested_path == installed_path @@ -250,17 +266,25 @@ impl RequirementSatisfaction { requested_path, installed_path, ); - return Ok(Self::Mismatch); + return Self::Mismatch; } let Some(cache_info) = cache_info.as_ref() else { - return Ok(Self::OutOfDate); + return Self::OutOfDate; }; - if *cache_info != CacheInfo::from_path(requested_path)? { - return Ok(Self::OutOfDate); + match CacheInfo::from_path(requested_path) { + Ok(read_cache_info) => { + if *cache_info != read_cache_info { + return Self::OutOfDate; + } + } + Err(err) => { + debug!("Failed to read cached requirement for: {distribution} ({err})"); + return Self::CacheInvalid; + } } - Ok(Self::Satisfied) + Self::Satisfied } } } diff --git a/crates/uv-installer/src/site_packages.rs b/crates/uv-installer/src/site_packages.rs index 7fbb12f3159d7..7fa26be1d4a40 100644 --- a/crates/uv-installer/src/site_packages.rs +++ b/crates/uv-installer/src/site_packages.rs @@ -437,9 +437,10 @@ impl SitePackages { [distribution] => { // Validate that the requirement is satisfied. if requirement.evaluate_markers(Some(markers), &[]) { - match RequirementSatisfaction::check(distribution, &requirement.source)? { + match RequirementSatisfaction::check(distribution, &requirement.source) { RequirementSatisfaction::Mismatch - | RequirementSatisfaction::OutOfDate => { + | RequirementSatisfaction::OutOfDate + | RequirementSatisfaction::CacheInvalid => { return Ok(SatisfiesResult::Unsatisfied(requirement.to_string())) } RequirementSatisfaction::Satisfied => {} @@ -449,10 +450,10 @@ impl SitePackages { // Validate that the installed version satisfies the constraints. for constraint in constraints.get(name).into_iter().flatten() { if constraint.evaluate_markers(Some(markers), &[]) { - match RequirementSatisfaction::check(distribution, &constraint.source)? - { + match RequirementSatisfaction::check(distribution, &constraint.source) { RequirementSatisfaction::Mismatch - | RequirementSatisfaction::OutOfDate => { + | RequirementSatisfaction::OutOfDate + | RequirementSatisfaction::CacheInvalid => { return Ok(SatisfiesResult::Unsatisfied( requirement.to_string(), ))