From 330c6dcd664562f23453eec6f1749097ce15424a Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Fri, 1 Mar 2024 14:22:03 -0500 Subject: [PATCH] refactor(index): pass pkg name to cache manager instead of relative paths --- src/cargo/sources/registry/index/cache.rs | 44 +++++++++++++++-------- src/cargo/sources/registry/index/mod.rs | 20 +++++------ 2 files changed, 38 insertions(+), 26 deletions(-) diff --git a/src/cargo/sources/registry/index/cache.rs b/src/cargo/sources/registry/index/cache.rs index ccb42e9313d..5d3bb28500a 100644 --- a/src/cargo/sources/registry/index/cache.rs +++ b/src/cargo/sources/registry/index/cache.rs @@ -67,10 +67,11 @@ use std::fs; use std::io; -use std::path::Path; +use std::path::PathBuf; use std::str; use anyhow::bail; +use cargo_util::registry::make_dep_path; use semver::Version; use crate::util::cache_lock::CacheLockMode; @@ -221,45 +222,60 @@ impl<'a> SummariesCache<'a> { /// Manages the on-disk index caches. pub struct CacheManager<'gctx> { + /// The root path where caches are located. + cache_root: Filesystem, /// [`GlobalContext`] reference for convenience. gctx: &'gctx GlobalContext, } impl<'gctx> CacheManager<'gctx> { /// Creates a new instance of the on-disk index cache manager. - pub fn new(gctx: &'gctx GlobalContext) -> CacheManager<'gctx> { - CacheManager { gctx } + /// + /// `root` --- The root path where caches are located. + pub fn new(cache_root: Filesystem, gctx: &'gctx GlobalContext) -> CacheManager<'gctx> { + CacheManager { cache_root, gctx } } /// Gets the cache associated with the key. - pub fn get(&self, key: &Path) -> Option> { - match fs::read(key) { + pub fn get(&self, key: &str) -> Option> { + let cache_path = &self.cache_path(key); + match fs::read(cache_path) { Ok(contents) => Some(contents), Err(e) => { - tracing::debug!(?key, "cache missing: {e}"); + tracing::debug!(?cache_path, "cache missing: {e}"); None } } } /// Associates the value with the key. - pub fn put(&self, key: &Path, value: &[u8]) { - if fs::create_dir_all(key.parent().unwrap()).is_ok() { - let path = Filesystem::new(key.into()); + pub fn put(&self, key: &str, value: &[u8]) { + let cache_path = &self.cache_path(key); + if fs::create_dir_all(cache_path.parent().unwrap()).is_ok() { + let path = Filesystem::new(cache_path.clone()); self.gctx .assert_package_cache_locked(CacheLockMode::DownloadExclusive, &path); - if let Err(e) = fs::write(key, value) { - tracing::info!(?key, "failed to write cache: {e}"); + if let Err(e) = fs::write(cache_path, value) { + tracing::info!(?cache_path, "failed to write cache: {e}"); } } } /// Invalidates the cache associated with the key. - pub fn invalidate(&self, key: &Path) { - if let Err(e) = fs::remove_file(key) { + pub fn invalidate(&self, key: &str) { + let cache_path = &self.cache_path(key); + if let Err(e) = fs::remove_file(cache_path) { if e.kind() != io::ErrorKind::NotFound { - tracing::debug!(?key, "failed to remove from cache: {e}"); + tracing::debug!(?cache_path, "failed to remove from cache: {e}"); } } } + + fn cache_path(&self, key: &str) -> PathBuf { + let relative = make_dep_path(key, false); + // This is the file we're loading from cache or the index data. + // See module comment in `registry/mod.rs` for why this is structured + // the way it is. + self.cache_root.join(relative).into_path_unlocked() + } } diff --git a/src/cargo/sources/registry/index/mod.rs b/src/cargo/sources/registry/index/mod.rs index 1af1bec783b..0bc6196fad5 100644 --- a/src/cargo/sources/registry/index/mod.rs +++ b/src/cargo/sources/registry/index/mod.rs @@ -304,7 +304,7 @@ impl<'gctx> RegistryIndex<'gctx> { path: path.clone(), summaries_cache: HashMap::new(), gctx, - cache_manager: CacheManager::new(gctx), + cache_manager: CacheManager::new(path.join(".cache"), gctx), } } @@ -545,24 +545,20 @@ impl Summaries { cache_manager: &CacheManager<'_>, ) -> Poll>> { // This is the file we're loading from cache or the index data. - // See module comment in `registry/mod.rs` for why this is structured - // the way it is. - let relative = make_dep_path(&name.to_lowercase(), false); - // First up, attempt to load the cache. This could fail for all manner - // of reasons, but consider all of them non-fatal and just log their - // occurrence in case anyone is debugging anything. - let cache_path = root.join(".cache").join(&relative); + // See module comment in `registry/mod.rs` for why this is structured the way it is. + let name = &name.to_lowercase(); + let relative = make_dep_path(&name, false); let mut cached_summaries = None; let mut index_version = None; - if let Some(contents) = cache_manager.get(&cache_path) { + if let Some(contents) = cache_manager.get(name) { match Summaries::parse_cache(contents) { Ok((s, v)) => { cached_summaries = Some(s); index_version = Some(v); } Err(e) => { - tracing::debug!("failed to parse {:?} cache: {}", relative, e); + tracing::debug!("failed to parse {name:?} cache: {e}"); } } } @@ -575,7 +571,7 @@ impl Summaries { return Poll::Ready(Ok(cached_summaries)); } LoadResponse::NotFound => { - cache_manager.invalidate(&cache_path); + cache_manager.invalidate(name); return Poll::Ready(Ok(None)); } LoadResponse::Data { @@ -624,7 +620,7 @@ impl Summaries { // Once we have our `cache_bytes` which represents the `Summaries` we're // about to return, write that back out to disk so future Cargo // invocations can use it. - cache_manager.put(&cache_path, &cache_bytes); + cache_manager.put(name, &cache_bytes); // If we've got debug assertions enabled read back in the cached values // and assert they match the expected result.