From ce9a6ab6483978b3a4f3b7cc3c86ff5df9609628 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sun, 10 Apr 2022 16:19:21 -0700 Subject: [PATCH 1/9] Mark .cargo/git and .cargo/registry as cache dirs Fixes #10457 (but still needs tests) --- crates/cargo-util/src/paths.rs | 10 +++++++++- src/cargo/sources/git/source.rs | 17 +++++++++++++++-- src/cargo/sources/registry/mod.rs | 14 ++++++++++++++ src/cargo/util/config/mod.rs | 11 ++++++++--- 4 files changed, 46 insertions(+), 6 deletions(-) diff --git a/crates/cargo-util/src/paths.rs b/crates/cargo-util/src/paths.rs index 0edb4d664e6..de7d2ae6311 100644 --- a/crates/cargo-util/src/paths.rs +++ b/crates/cargo-util/src/paths.rs @@ -632,7 +632,7 @@ pub fn create_dir_all_excluded_from_backups_atomic(p: impl AsRef) -> Resul let parent = path.parent().unwrap(); let base = path.file_name().unwrap(); create_dir_all(parent)?; - // We do this in two steps (first create a temporary directory and exlucde + // We do this in two steps (first create a temporary directory and exclude // it from backups, then rename it to the desired name. If we created the // directory directly where it should be and then excluded it from backups // we would risk a situation where cargo is interrupted right after the directory @@ -660,6 +660,14 @@ pub fn create_dir_all_excluded_from_backups_atomic(p: impl AsRef) -> Resul Ok(()) } +/// Mark an existing directory as excluded from backups and indexing. +pub fn exclude_from_backups_and_indexing(p: impl AsRef) -> Result<()> { + let path = p.as_ref(); + exclude_from_backups(path); + exclude_from_content_indexing(path); + Ok(()) +} + /// Marks the directory as excluded from archives/backups. /// /// This is recommended to prevent derived/temporary files from bloating backups. There are two diff --git a/src/cargo/sources/git/source.rs b/src/cargo/sources/git/source.rs index b166aff340d..af036376e0a 100644 --- a/src/cargo/sources/git/source.rs +++ b/src/cargo/sources/git/source.rs @@ -7,6 +7,7 @@ use crate::util::errors::CargoResult; use crate::util::hex::short_hash; use crate::util::Config; use anyhow::Context; +use cargo_util::paths::exclude_from_backups_and_indexing; use log::trace; use std::fmt::{self, Debug, Formatter}; use std::task::Poll; @@ -122,8 +123,20 @@ impl<'cfg> Source for GitSource<'cfg> { return Ok(()); } - let git_path = self.config.git_path(); - let git_path = self.config.assert_package_cache_locked(&git_path); + let git_fs = self.config.git_path(); + git_fs.create_dir()?; + let git_path = self.config.assert_package_cache_locked(&git_fs); + + // Before getting a checkout, make sure that `/git` is + // marked as excluded from indexing and backups. Older versions of Cargo + // didn't do this, so we do it here regardless of whether `` + // exists. + // + // This does not use `create_dir_all_excluded_from_backups_atomic` for + // the same reason: we want to exclude it even if the directory already + // exists. + exclude_from_backups_and_indexing(&git_path)?; + let db_path = git_path.join("db").join(&self.ident); let db = self.remote.db_at(&db_path).ok(); diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index f0a770c4c51..0d896db0cd5 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -167,6 +167,7 @@ use std::path::{Path, PathBuf}; use std::task::Poll; use anyhow::Context as _; +use cargo_util::paths::exclude_from_backups_and_indexing; use flate2::read::GzDecoder; use log::debug; use semver::Version; @@ -552,6 +553,19 @@ impl<'cfg> RegistrySource<'cfg> { } else { Box::new(remote::RemoteRegistry::new(source_id, config, &name)) as Box<_> }; + + // Before starting to work on the registry, make sure that + // `/registry` is marked as excluded from indexing and + // backups. Older versions of Cargo didn't do this, so we do it here + // regardless of whether `` exists. + // + // This does not use `create_dir_all_excluded_from_backups_atomic` for + // the same reason: we want to exclude it even if the directory already + // exists. + let registry_base = config.registry_base_path(); + registry_base.create_dir()?; + exclude_from_backups_and_indexing(®istry_base.into_path_unlocked())?; + Ok(RegistrySource::new( source_id, config, diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index 15414ece9af..c01318a110d 100644 --- a/src/cargo/util/config/mod.rs +++ b/src/cargo/util/config/mod.rs @@ -315,19 +315,24 @@ impl Config { self.home_path.join("git") } + /// Gets the Cargo base directory for all registry information (`/registry`). + pub fn registry_base_path(&self) -> Filesystem { + self.home_path.join("registry") + } + /// Gets the Cargo registry index directory (`/registry/index`). pub fn registry_index_path(&self) -> Filesystem { - self.home_path.join("registry").join("index") + self.registry_base_path().join("index") } /// Gets the Cargo registry cache directory (`/registry/path`). pub fn registry_cache_path(&self) -> Filesystem { - self.home_path.join("registry").join("cache") + self.registry_base_path().join("cache") } /// Gets the Cargo registry source directory (`/registry/src`). pub fn registry_source_path(&self) -> Filesystem { - self.home_path.join("registry").join("src") + self.registry_base_path().join("src") } /// Gets the default Cargo registry. From 7191fc05cbfe76e5a427cfb00b994b24e4690f95 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sun, 24 Apr 2022 09:27:09 -0700 Subject: [PATCH 2/9] Mark registry excluded from block_until_ready --- src/cargo/sources/registry/mod.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index 0d896db0cd5..c41cc938869 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -554,18 +554,6 @@ impl<'cfg> RegistrySource<'cfg> { Box::new(remote::RemoteRegistry::new(source_id, config, &name)) as Box<_> }; - // Before starting to work on the registry, make sure that - // `/registry` is marked as excluded from indexing and - // backups. Older versions of Cargo didn't do this, so we do it here - // regardless of whether `` exists. - // - // This does not use `create_dir_all_excluded_from_backups_atomic` for - // the same reason: we want to exclude it even if the directory already - // exists. - let registry_base = config.registry_base_path(); - registry_base.create_dir()?; - exclude_from_backups_and_indexing(®istry_base.into_path_unlocked())?; - Ok(RegistrySource::new( source_id, config, @@ -826,6 +814,18 @@ impl<'cfg> Source for RegistrySource<'cfg> { } fn block_until_ready(&mut self) -> CargoResult<()> { + // Before starting to work on the registry, make sure that + // `/registry` is marked as excluded from indexing and + // backups. Older versions of Cargo didn't do this, so we do it here + // regardless of whether `` exists. + // + // This does not use `create_dir_all_excluded_from_backups_atomic` for + // the same reason: we want to exclude it even if the directory already + // exists. + let registry_base = self.config.registry_base_path(); + registry_base.create_dir()?; + exclude_from_backups_and_indexing(®istry_base.into_path_unlocked())?; + self.ops.block_until_ready() } } From 5afc6c7021a998b4714f5ec39e63f9a3a56bd12c Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sat, 16 Apr 2022 07:55:30 -0700 Subject: [PATCH 3/9] Test .cargo/git/CACHEDIR.TAG is created --- tests/testsuite/git.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/testsuite/git.rs b/tests/testsuite/git.rs index 4db0f6ed6d5..043dac181fb 100644 --- a/tests/testsuite/git.rs +++ b/tests/testsuite/git.rs @@ -2247,6 +2247,11 @@ fn add_a_git_dep() { p.cargo("build").run(); + assert!(paths::home().is_dir()); + assert!(paths::home().join(".cargo").is_dir()); + assert!(paths::home().join(".cargo/git").is_dir()); + assert!(paths::home().join(".cargo/git/CACHEDIR.TAG").is_file()); + p.change_file( "a/Cargo.toml", &format!( @@ -2580,6 +2585,7 @@ fn use_the_cli() { "; project.cargo("build -v").with_stderr(stderr).run(); + assert!(paths::home().join(".cargo/git/CACHEDIR.TAG").is_file()); } #[cargo_test] From 7e366c2ef2e7d3c8488f7b8afc9cf26c24180f84 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sat, 16 Apr 2022 07:59:52 -0700 Subject: [PATCH 4/9] Test creation of .cargo/registry/CACHEDIR.TAG --- tests/testsuite/registry.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/testsuite/registry.rs b/tests/testsuite/registry.rs index e4284ec8972..35247a4d879 100644 --- a/tests/testsuite/registry.rs +++ b/tests/testsuite/registry.rs @@ -93,6 +93,8 @@ fn simple(cargo: fn(&Project, &str) -> Execs) { cargo(&p, "clean").run(); + assert!(paths::home().join(".cargo/registry/CACHEDIR.TAG").is_file()); + // Don't download a second time cargo(&p, "build") .with_stderr( @@ -150,6 +152,8 @@ fn deps(cargo: fn(&Project, &str) -> Execs) { ", ) .run(); + + assert!(paths::home().join(".cargo/registry/CACHEDIR.TAG").is_file()); } #[cargo_test] @@ -1231,6 +1235,7 @@ fn updating_a_dep(cargo: fn(&Project, &str) -> Execs) { ", ) .run(); + assert!(paths::home().join(".cargo/registry/CACHEDIR.TAG").is_file()); p.change_file( "a/Cargo.toml", From 0c525bbb0e819720b48055a5a4cc63f769bd6143 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sat, 16 Apr 2022 08:07:01 -0700 Subject: [PATCH 5/9] Test CACHEDIR.TAG is created in an existing registry --- tests/testsuite/registry.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/testsuite/registry.rs b/tests/testsuite/registry.rs index 35247a4d879..f9afa2919c6 100644 --- a/tests/testsuite/registry.rs +++ b/tests/testsuite/registry.rs @@ -1237,6 +1237,12 @@ fn updating_a_dep(cargo: fn(&Project, &str) -> Execs) { .run(); assert!(paths::home().join(".cargo/registry/CACHEDIR.TAG").is_file()); + // Now delete the CACHEDIR.TAG file: this is the situation we'll be in after + // upgrading from a version of Cargo that doesn't mark this directory, to one that + // does. It should be recreated. + fs::remove_file(paths::home().join(".cargo/registry/CACHEDIR.TAG")) + .expect("remove CACHEDIR.TAG"); + p.change_file( "a/Cargo.toml", r#" @@ -1265,6 +1271,11 @@ fn updating_a_dep(cargo: fn(&Project, &str) -> Execs) { ", ) .run(); + + assert!( + paths::home().join(".cargo/registry/CACHEDIR.TAG").is_file(), + "CACHEDIR.TAG recreated in existing registry" + ); } #[cargo_test] From c45ab7dd302679bd9f5ba9abd6e2409a6b8f39f5 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Wed, 20 Apr 2022 07:13:44 -0700 Subject: [PATCH 6/9] Remove redundant assertions --- tests/testsuite/git.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/testsuite/git.rs b/tests/testsuite/git.rs index 043dac181fb..ae38aa6fdaf 100644 --- a/tests/testsuite/git.rs +++ b/tests/testsuite/git.rs @@ -2247,9 +2247,6 @@ fn add_a_git_dep() { p.cargo("build").run(); - assert!(paths::home().is_dir()); - assert!(paths::home().join(".cargo").is_dir()); - assert!(paths::home().join(".cargo/git").is_dir()); assert!(paths::home().join(".cargo/git/CACHEDIR.TAG").is_file()); p.change_file( From ae5fd5e2e1bcb5670da2c5ac5bb9a72648d68202 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Mon, 25 Apr 2022 18:29:13 -0700 Subject: [PATCH 7/9] exclude_from_backups_and_indexing can't fail Ignore errors creating the registry directory --- crates/cargo-util/src/paths.rs | 5 +++-- src/cargo/sources/git/source.rs | 2 +- src/cargo/sources/registry/mod.rs | 7 +++++-- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/crates/cargo-util/src/paths.rs b/crates/cargo-util/src/paths.rs index de7d2ae6311..54607f62d04 100644 --- a/crates/cargo-util/src/paths.rs +++ b/crates/cargo-util/src/paths.rs @@ -661,11 +661,12 @@ pub fn create_dir_all_excluded_from_backups_atomic(p: impl AsRef) -> Resul } /// Mark an existing directory as excluded from backups and indexing. -pub fn exclude_from_backups_and_indexing(p: impl AsRef) -> Result<()> { +/// +/// Errors in marking it are ignored. +pub fn exclude_from_backups_and_indexing(p: impl AsRef) { let path = p.as_ref(); exclude_from_backups(path); exclude_from_content_indexing(path); - Ok(()) } /// Marks the directory as excluded from archives/backups. diff --git a/src/cargo/sources/git/source.rs b/src/cargo/sources/git/source.rs index af036376e0a..a10f02a9826 100644 --- a/src/cargo/sources/git/source.rs +++ b/src/cargo/sources/git/source.rs @@ -135,7 +135,7 @@ impl<'cfg> Source for GitSource<'cfg> { // This does not use `create_dir_all_excluded_from_backups_atomic` for // the same reason: we want to exclude it even if the directory already // exists. - exclude_from_backups_and_indexing(&git_path)?; + exclude_from_backups_and_indexing(&git_path); let db_path = git_path.join("db").join(&self.ident); diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index c41cc938869..6c47eac2ffc 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -822,9 +822,12 @@ impl<'cfg> Source for RegistrySource<'cfg> { // This does not use `create_dir_all_excluded_from_backups_atomic` for // the same reason: we want to exclude it even if the directory already // exists. + // + // IO errors in creating and marking it are ignored, e.g. in case we're on a + // read-only filesystem. let registry_base = self.config.registry_base_path(); - registry_base.create_dir()?; - exclude_from_backups_and_indexing(®istry_base.into_path_unlocked())?; + let _ = registry_base.create_dir()?; + exclude_from_backups_and_indexing(®istry_base.into_path_unlocked()); self.ops.block_until_ready() } From 8f04a65da39a3ff2f5ec43033a3aab9292a59c1c Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Mon, 25 Apr 2022 19:16:29 -0700 Subject: [PATCH 8/9] Actually ignore errors --- src/cargo/sources/git/source.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/cargo/sources/git/source.rs b/src/cargo/sources/git/source.rs index a10f02a9826..19a1274825c 100644 --- a/src/cargo/sources/git/source.rs +++ b/src/cargo/sources/git/source.rs @@ -124,7 +124,9 @@ impl<'cfg> Source for GitSource<'cfg> { } let git_fs = self.config.git_path(); - git_fs.create_dir()?; + // Ignore errors creating it, in case this is a read-only filesystem: + // perhaps the later operations can succeed anyhow. + let _ = git_fs.create_dir(); let git_path = self.config.assert_package_cache_locked(&git_fs); // Before getting a checkout, make sure that `/git` is From 4bcfd9e7e61407450294c86162ceba7231f52cc2 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Mon, 25 Apr 2022 19:16:46 -0700 Subject: [PATCH 9/9] Ignore errors creating the git directory Probably can't fail, but just in case --- src/cargo/sources/registry/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index 6c47eac2ffc..fc9c29510c1 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -826,7 +826,7 @@ impl<'cfg> Source for RegistrySource<'cfg> { // IO errors in creating and marking it are ignored, e.g. in case we're on a // read-only filesystem. let registry_base = self.config.registry_base_path(); - let _ = registry_base.create_dir()?; + let _ = registry_base.create_dir(); exclude_from_backups_and_indexing(®istry_base.into_path_unlocked()); self.ops.block_until_ready()