Skip to content

Commit

Permalink
Auto merge of #10553 - sourcefrog:cachedir, r=weihanglo
Browse files Browse the repository at this point in the history
Mark .cargo/git and .cargo/registry as cache dirs

Fixes #10457

### What does this PR try to resolve?

As we previously discussed in #10457 this marks `~/.cargo/git` and `~/.cargo/registry` as not to be included in backups, and not subject to content indexing. These directories can be fairly large and frequently changed, and should only contain content that can be re-downloaded if necessary.

### How should we test and review this PR?

I did two manual tests:

1. Using the binary built from this tree, run `cargo update` in a source tree that has a git dependency, and observe that afterwards, there is a `CACHEDIR.TAG` in `~/.cargo/git`.
2. Similarly, run `cargo update` and observe that there's a `CACHEDIR.TAG` in `~/.cargo/registry`.

(I ran this on Linux. This code should also trigger OS-specific behavior on macOS and Windows that's the same as is currently applied to `target/`.)

I added some test assertions.
  • Loading branch information
bors committed Apr 27, 2022
2 parents f3a4448 + 4bcfd9e commit 7a3b56b
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 6 deletions.
11 changes: 10 additions & 1 deletion crates/cargo-util/src/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,7 @@ pub fn create_dir_all_excluded_from_backups_atomic(p: impl AsRef<Path>) -> 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
Expand Down Expand Up @@ -660,6 +660,15 @@ pub fn create_dir_all_excluded_from_backups_atomic(p: impl AsRef<Path>) -> Resul
Ok(())
}

/// Mark an existing directory as excluded from backups and indexing.
///
/// Errors in marking it are ignored.
pub fn exclude_from_backups_and_indexing(p: impl AsRef<Path>) {
let path = p.as_ref();
exclude_from_backups(path);
exclude_from_content_indexing(path);
}

/// Marks the directory as excluded from archives/backups.
///
/// This is recommended to prevent derived/temporary files from bloating backups. There are two
Expand Down
19 changes: 17 additions & 2 deletions src/cargo/sources/git/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -122,8 +123,22 @@ 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();
// 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 `<cargo_home>/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 `<cargo_home>`
// 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();
Expand Down
17 changes: 17 additions & 0 deletions src/cargo/sources/registry/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -552,6 +553,7 @@ impl<'cfg> RegistrySource<'cfg> {
} else {
Box::new(remote::RemoteRegistry::new(source_id, config, &name)) as Box<_>
};

Ok(RegistrySource::new(
source_id,
config,
Expand Down Expand Up @@ -812,6 +814,21 @@ impl<'cfg> Source for RegistrySource<'cfg> {
}

fn block_until_ready(&mut self) -> CargoResult<()> {
// Before starting to work on the registry, make sure that
// `<cargo_home>/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 `<cargo_home>` 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.
//
// 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();
exclude_from_backups_and_indexing(&registry_base.into_path_unlocked());

self.ops.block_until_ready()
}
}
Expand Down
11 changes: 8 additions & 3 deletions src/cargo/util/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,19 +315,24 @@ impl Config {
self.home_path.join("git")
}

/// Gets the Cargo base directory for all registry information (`<cargo_home>/registry`).
pub fn registry_base_path(&self) -> Filesystem {
self.home_path.join("registry")
}

/// Gets the Cargo registry index directory (`<cargo_home>/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 (`<cargo_home>/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 (`<cargo_home>/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.
Expand Down
3 changes: 3 additions & 0 deletions tests/testsuite/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2247,6 +2247,8 @@ fn add_a_git_dep() {

p.cargo("build").run();

assert!(paths::home().join(".cargo/git/CACHEDIR.TAG").is_file());

p.change_file(
"a/Cargo.toml",
&format!(
Expand Down Expand Up @@ -2580,6 +2582,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]
Expand Down
16 changes: 16 additions & 0 deletions tests/testsuite/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -150,6 +152,8 @@ fn deps(cargo: fn(&Project, &str) -> Execs) {
",
)
.run();

assert!(paths::home().join(".cargo/registry/CACHEDIR.TAG").is_file());
}

#[cargo_test]
Expand Down Expand Up @@ -1231,6 +1235,13 @@ 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",
Expand Down Expand Up @@ -1260,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]
Expand Down

0 comments on commit 7a3b56b

Please sign in to comment.