Skip to content

Commit

Permalink
Auto merge of #3857 - antonlarin:rebuild-on-env-change, r=alexcrichton
Browse files Browse the repository at this point in the history
Include package props with corresponding env vars into target metadata

Previously, when changing package properties with corresponding environment variables (such as authors, which has CARGO_PKG_AUTHORS), it didn't invalidate the build, even though there could have been a dependency on such variables in the source code.

This commit includes 3 such properties: authors list, description and homepage in the target metadata.

I've added a test only for description change, can add more if necessary.
Fixes #3696.
  • Loading branch information
bors committed Mar 23, 2017
2 parents 4e95c6b + d70ca21 commit 20f6eff
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 0 deletions.
4 changes: 4 additions & 0 deletions src/cargo/ops/cargo_rustc/compilation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,10 @@ impl<'cfg> Compilation<'cfg> {
let cargo_exe = self.config.cargo_exe()?;
cmd.env(::CARGO_ENV, cargo_exe);

// When adding new environment variables depending on
// crate properties which might require rebuild upon change
// consider adding the corresponding properties to the hash
// in Context::target_metadata()
cmd.env("CARGO_MANIFEST_DIR", pkg.root())
.env("CARGO_PKG_VERSION_MAJOR", &pkg.version().major.to_string())
.env("CARGO_PKG_VERSION_MINOR", &pkg.version().minor.to_string())
Expand Down
7 changes: 7 additions & 0 deletions src/cargo/ops/cargo_rustc/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,13 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
// to pull crates from anywhere w/o worrying about conflicts
unit.pkg.package_id().hash(&mut hasher);

// Add package properties which map to environment variables
// exposed by Cargo
let manifest_metadata = unit.pkg.manifest().metadata();
manifest_metadata.authors.hash(&mut hasher);
manifest_metadata.description.hash(&mut hasher);
manifest_metadata.homepage.hash(&mut hasher);

// Also mix in enabled features to our metadata. This'll ensure that
// when changing feature sets each lib is separately cached.
self.resolve.features_sorted(unit.pkg.package_id()).hash(&mut hasher);
Expand Down
41 changes: 41 additions & 0 deletions tests/freshness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -680,3 +680,44 @@ fn rebuild_if_build_artifacts_move_forward_in_time() {
[FINISHED] [..]
"));
}

#[test]
fn rebuild_if_environment_changes() {
let p = project("env_change")
.file("Cargo.toml", r#"
[package]
name = "env_change"
description = "old desc"
version = "0.0.1"
authors = []
"#)
.file("src/main.rs", r#"
fn main() {
println!("{}", env!("CARGO_PKG_DESCRIPTION"));
}
"#);

assert_that(p.cargo_process("run"),
execs().with_status(0)
.with_stdout("old desc").with_stderr(&format!("\
[COMPILING] env_change v0.0.1 ({dir})
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
[RUNNING] `target[/]debug[/]env_change[EXE]`
", dir = p.url())));

File::create(&p.root().join("Cargo.toml")).unwrap().write_all(br#"
[package]
name = "env_change"
description = "new desc"
version = "0.0.1"
authors = []
"#).unwrap();

assert_that(p.cargo("run"),
execs().with_status(0)
.with_stdout("new desc").with_stderr(&format!("\
[COMPILING] env_change v0.0.1 ({dir})
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
[RUNNING] `target[/]debug[/]env_change[EXE]`
", dir = p.url())));
}

0 comments on commit 20f6eff

Please sign in to comment.