mirror of
https://github.com/rust-lang/cargo.git
synced 2025-09-28 11:20:36 +00:00
feat: Added warning when failing to update index cache (#15014)
<!-- Thanks for submitting a pull request 🎉! Here are some tips for you: * If this is your first contribution, read "Cargo Contribution Guide" first: https://doc.crates.io/contrib/ * Run `cargo fmt --all` to format your code changes. * Small commits and pull requests are always preferable and easy to review. * If your idea is large and needs feedback from the community, read how: https://doc.crates.io/contrib/process/#working-on-large-features * Cargo takes care of compatibility. Read our design principles: https://doc.crates.io/contrib/design.html * When changing help text of cargo commands, follow the steps to generate docs: https://github.com/rust-lang/cargo/tree/master/src/doc#building-the-man-pages * If your PR is not finished, set it as "draft" PR or add "WIP" in its title. * It's ok to use the CI resources to test your PR, but please don't abuse them. ### What does this PR try to resolve? Explain the motivation behind this change. A clear overview along with an in-depth explanation are helpful. You can use `Fixes #<issue number>` to associate this PR to an existing issue. ### How should we test and review this PR? Demonstrate how you test this change and guide reviewers through your PR. With a smooth review process, a pull request usually gets reviewed quicker. If you don't know how to write and run your tests, please read the guide: https://doc.crates.io/contrib/tests ### Additional information Other information you want to mention in this PR, such as prior arts, future extensions, an unresolved problem, or a TODO list. --> ### What does this PR try to resolve? Fixes #13712 Adds a warning if there is an error updating the index cache. It also attempts to avoid warning spam as requested in [this comment](https://github.com/rust-lang/cargo/issues/13712#issuecomment-2514991597) Below is an example output ``` Updating crates.io index warning: failed to write cache, path: /home/ross/.cargo/registry/index/index.crates.io-1949cf8c6b5b557f/.cache/ba/se/base64, error: Permission denied (os error 13) Compiling serde v1.0.217 Compiling r-efi v5.2.0 Compiling base64 v0.22.1 Compiling cargo-13712 v0.1.0 (/home/ross/projects/cargo-13712) Finished `dev` profile [unoptimized + debuginfo] target(s) in 1.20s ``` ### How should we test and review this PR? I tested this on my machine by manually changing the owner/permission of the index files ```sh sudo chown root ~/.cargo/registry/index/.../.cache/r- sudo chmod 700 ~/.cargo/registry/index/.../.cache/r- # in a project that uses the `r-efi` crate cargo build ``` I wasn't quiet sure about the best way to add a test to the testsuite for this. 😅 If there is good way to create a test for this lmk
This commit is contained in:
commit
c518f225a9
@ -65,6 +65,7 @@
|
||||
//! [`IndexSummary::parse`]: super::IndexSummary::parse
|
||||
//! [`RemoteRegistry`]: crate::sources::registry::remote::RemoteRegistry
|
||||
|
||||
use std::cell::RefCell;
|
||||
use std::fs;
|
||||
use std::io;
|
||||
use std::path::PathBuf;
|
||||
@ -226,6 +227,9 @@ pub struct CacheManager<'gctx> {
|
||||
cache_root: Filesystem,
|
||||
/// [`GlobalContext`] reference for convenience.
|
||||
gctx: &'gctx GlobalContext,
|
||||
/// Keeps track of if we have sent a warning message if there was an error updating the cache.
|
||||
/// The motivation is to avoid warning spam if the cache is not writable.
|
||||
has_warned: RefCell<bool>,
|
||||
}
|
||||
|
||||
impl<'gctx> CacheManager<'gctx> {
|
||||
@ -233,7 +237,11 @@ impl<'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 }
|
||||
CacheManager {
|
||||
cache_root,
|
||||
gctx,
|
||||
has_warned: Default::default(),
|
||||
}
|
||||
}
|
||||
|
||||
/// Gets the cache associated with the key.
|
||||
@ -251,16 +259,28 @@ impl<'gctx> CacheManager<'gctx> {
|
||||
/// Associates the value with the key.
|
||||
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(cache_path, value) {
|
||||
tracing::info!(?cache_path, "failed to write cache: {e}");
|
||||
if let Err(e) = self.put_inner(cache_path, value) {
|
||||
tracing::info!(?cache_path, "failed to write cache: {e}");
|
||||
|
||||
if !*self.has_warned.borrow() {
|
||||
let _ = self.gctx.shell().warn(format!(
|
||||
"failed to write cache, path: {}, error: {e}",
|
||||
cache_path.to_str().unwrap_or_default()
|
||||
));
|
||||
*self.has_warned.borrow_mut() = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn put_inner(&self, cache_path: &PathBuf, value: &[u8]) -> std::io::Result<()> {
|
||||
fs::create_dir_all(cache_path.parent().unwrap())?;
|
||||
let path = Filesystem::new(cache_path.clone());
|
||||
self.gctx
|
||||
.assert_package_cache_locked(CacheLockMode::DownloadExclusive, &path);
|
||||
fs::write(cache_path, value)?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Invalidates the cache associated with the key.
|
||||
pub fn invalidate(&self, key: &str) {
|
||||
let cache_path = &self.cache_path(key);
|
||||
|
@ -2,7 +2,7 @@
|
||||
|
||||
use std::fmt::Write;
|
||||
use std::fs::{self, File};
|
||||
use std::path::Path;
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::sync::Arc;
|
||||
use std::sync::Mutex;
|
||||
|
||||
@ -3097,6 +3097,78 @@ fn readonly_registry_still_works() {
|
||||
}
|
||||
}
|
||||
|
||||
#[cargo_test(ignore_windows = "On Windows setting file attributes is a bit complicated")]
|
||||
fn unaccessible_registry_cache_still_works() {
|
||||
Package::new("foo", "0.1.0").publish();
|
||||
Package::new("fo2", "0.1.0").publish();
|
||||
|
||||
let p = project()
|
||||
.file(
|
||||
"Cargo.toml",
|
||||
r#"
|
||||
[package]
|
||||
name = "a"
|
||||
version = "0.5.0"
|
||||
edition = "2015"
|
||||
authors = []
|
||||
|
||||
[dependencies]
|
||||
foo = '0.1.0'
|
||||
fo2 = '0.1.0'
|
||||
"#,
|
||||
)
|
||||
.file("src/lib.rs", "")
|
||||
.build();
|
||||
|
||||
p.cargo("generate-lockfile").run();
|
||||
p.cargo("fetch --locked").run();
|
||||
|
||||
let cache_path = inner_dir(&paths::cargo_home().join("registry/index")).join(".cache");
|
||||
let f_cache_path = cache_path.join("3/f");
|
||||
|
||||
// Remove the permissions from the cache path that contains the "foo" crate
|
||||
set_permissions(&f_cache_path, 0o000);
|
||||
|
||||
// Now run a build and make sure we properly build and warn the user
|
||||
p.cargo("build")
|
||||
.with_stderr_data(str![[r#"
|
||||
[WARNING] failed to write cache, path: [ROOT]/home/.cargo/registry/index/-[HASH]/.cache/3/f/fo[..], [ERROR] Permission denied (os error 13)
|
||||
[COMPILING] fo[..] v0.1.0
|
||||
[COMPILING] fo[..] v0.1.0
|
||||
[COMPILING] a v0.5.0 ([ROOT]/foo)
|
||||
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
|
||||
|
||||
"#]])
|
||||
.run();
|
||||
// make sure we add the permissions to the files afterwards so "cargo clean" can remove them (#6934)
|
||||
set_permissions(&f_cache_path, 0o777);
|
||||
|
||||
fn set_permissions(path: &Path, permissions: u32) {
|
||||
#[cfg(not(windows))]
|
||||
{
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
let mut perms = t!(path.metadata()).permissions();
|
||||
perms.set_mode(permissions);
|
||||
t!(fs::set_permissions(path, perms));
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
panic!("This test is not supported on windows. See the reason in the #[cargo_test] macro");
|
||||
}
|
||||
|
||||
fn inner_dir(path: &Path) -> PathBuf {
|
||||
for entry in t!(path.read_dir()) {
|
||||
let path = t!(entry).path();
|
||||
|
||||
if path.is_dir() {
|
||||
return path;
|
||||
}
|
||||
}
|
||||
|
||||
panic!("could not find inner directory of {path:?}");
|
||||
}
|
||||
}
|
||||
|
||||
#[cargo_test]
|
||||
fn registry_index_rejected_http() {
|
||||
let _server = setup_http();
|
||||
|
Loading…
x
Reference in New Issue
Block a user