Skip to content

Commit

Permalink
Auto merge of #11613 - arriven:fix/11083-rerun-if-changed-published-s…
Browse files Browse the repository at this point in the history
…ources-dir, r=epage

Improve CI caching by skipping mtime checks for paths in $CARGO_HOME

Skip mtime checks for paths pointing into `$CARGO_HOME` to avoid rebuilds when only caching $CARGO_HOME/registry/{index, cache} and $CARGO_HOME/git/db and some of the dependencies have `rerun-if-changed=directory` in their `build.rs`

I considered skipping mtime checking only on `$CARGO_HOME/registry/src` but it looks like other functionality (like downloading a newer version of dependency) is unaffected by this and this way we also cover the same issue with git based dependencies (except the part where `cargo` is forced to re-fetch submodules if `$CARGO_HOME/git/checkouts` is missing) and it is more in line with the discussion in #9455

Fix #11083

Credit `@weihanglo` for the test (I did add a case of checking that dependency update still triggers a rebuild but they are the original author of the rest of the test)
  • Loading branch information
bors committed Jan 24, 2023
2 parents b862c6d + 9e01c8b commit 99f841c
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 0 deletions.
7 changes: 7 additions & 0 deletions src/cargo/core/compiler/fingerprint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1785,6 +1785,13 @@ where

for path in paths {
let path = path.as_ref();

// Assuming anything in cargo_home is immutable (see also #9455 about marking it readonly)
// which avoids rebuilds when CI caches $CARGO_HOME/registry/{index, cache} and
// $CARGO_HOME/git/db across runs, keeping the content the same but changing the mtime.
if let Ok(true) = home::cargo_home().map(|home| path.starts_with(home)) {
continue;
}
let path_mtime = match mtime_cache.entry(path.to_path_buf()) {
Entry::Occupied(o) => *o.get(),
Entry::Vacant(v) => {
Expand Down
79 changes: 79 additions & 0 deletions tests/testsuite/build_script.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Tests for build.rs scripts.
use cargo_test_support::compare::assert_match_exact;
use cargo_test_support::install::cargo_home;
use cargo_test_support::paths::CargoPathExt;
use cargo_test_support::registry::Package;
use cargo_test_support::tools;
Expand Down Expand Up @@ -4803,6 +4804,84 @@ fn rerun_if_directory() {
fresh();
}

#[cargo_test]
fn rerun_if_published_directory() {
// build script of a dependency contains a `rerun-if-changed` pointing to a directory
Package::new("mylib-sys", "1.0.0")
.file("mylib/balrog.c", "")
.file("src/lib.rs", "")
.file(
"build.rs",
r#"
fn main() {
// Changing to mylib/balrog.c will not trigger a rebuild
println!("cargo:rerun-if-changed=mylib");
}
"#,
)
.publish();

let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.0.1"
[dependencies]
mylib-sys = "1.0.0"
"#,
)
.file("src/main.rs", "fn main() {}")
.build();

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

// Delete regitry src to make directories being recreated with the latest timestamp.
cargo_home().join("registry/src").rm_rf();

p.cargo("check --verbose")
.with_stderr(
"\
[FRESH] mylib-sys v1.0.0
[FRESH] foo v0.0.1 ([CWD])
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
",
)
.run();

// Upgrade of a package should still trigger a rebuild
Package::new("mylib-sys", "1.0.1")
.file("mylib/balrog.c", "")
.file("mylib/balrog.h", "")
.file("src/lib.rs", "")
.file(
"build.rs",
r#"
fn main() {
println!("cargo:rerun-if-changed=mylib");
}
"#,
)
.publish();
p.cargo("update").run();
p.cargo("fetch").run();

p.cargo("check -v")
.with_stderr(format!(
"\
[COMPILING] mylib-sys [..]
[RUNNING] `rustc --crate-name build_script_build [..]
[RUNNING] `[..]build-script-build[..]`
[RUNNING] `rustc --crate-name mylib_sys [..]
[CHECKING] foo [..]
[RUNNING] `rustc --crate-name foo [..]
[FINISHED] [..]",
))
.run();
}

#[cargo_test]
fn test_with_dep_metadata() {
let p = project()
Expand Down

0 comments on commit 99f841c

Please sign in to comment.