Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: package cache lock file_name was incorrect #977

Merged
merged 1 commit into from
Dec 12, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions crates/rattler_cache/src/package_cache/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use rattler_package_streaming::{DownloadReporter, ExtractError};
pub use reporter::CacheReporter;
use reqwest::StatusCode;
use simple_spawn_blocking::Cancelled;
use tracing::instrument;
use url::Url;

use crate::validation::validate_package_directory;
Expand Down Expand Up @@ -209,6 +210,7 @@ impl PackageCache {
/// This is a convenience wrapper around `get_or_fetch` which fetches the
/// package from the given URL if the package could not be found in the
/// cache.
#[instrument(skip_all, fields(url=%url))]
pub async fn get_or_fetch_from_url_with_retry(
&self,
pkg: impl Into<CacheKey>,
Expand Down Expand Up @@ -306,7 +308,14 @@ where
{
// Acquire a read lock on the cache entry. This ensures that no other process is
// currently writing to the cache.
let lock_file_path = path.with_extension("lock");
let lock_file_path = {
// Append the `.lock` extension to the cache path to create the lock file path.
// `Path::with_extension` strips too much from the filename if it contains one
// or more dots.
let mut path_str = path.as_os_str().to_owned();
path_str.push(".lock");
PathBuf::from(path_str)
};

// Ensure the directory containing the lock-file exists.
if let Some(root_dir) = lock_file_path.parent() {
Expand All @@ -333,7 +342,8 @@ where
_ => false,
};

if path.is_dir() && !hash_mismatch {
let cache_dir_exists = path.is_dir();
if cache_dir_exists && !hash_mismatch {
let path_inner = path.clone();

let reporter = reporter.as_deref().map(|r| (r, r.on_validate_start()));
Expand Down Expand Up @@ -387,6 +397,12 @@ where
}
}
}
} else if !cache_dir_exists {
tracing::debug!("cache directory does not exist, fetching package");
} else if hash_mismatch {
tracing::warn!("hash mismatch, wanted a package with hash {} but the cached package has hash {}, fetching package",
given_sha.map_or(String::from("<unknown>"), |s| format!("{s:x}")),
locked_sha256.map_or(String::from("<unknown>"), |s| format!("{s:x}")));
}

// If the cache is stale, we need to fetch the package again. We have to acquire
Expand Down
Loading