Skip to content

Commit

Permalink
Include package props with env vars into target metadata
Browse files Browse the repository at this point in the history
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 such properties (there are 3 of them in total:
authors, description and homepage) in the target metadata.

Fixes #3696.
  • Loading branch information
antonlarin committed Mar 23, 2017
1 parent 3cac894 commit d70ca21
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 d70ca21

Please sign in to comment.