From fe8bbb7a544e5bafadfb58b1f1c9132b9952c43f Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 5 Jun 2017 07:52:31 -0700 Subject: [PATCH] Implement `cargo:rerun-if-env-changed=FOO` This commit implements a new method of rerunning a build script if an environment variable changes. Environment variables are one of the primary methods of giving inputs to a build script today, and this'll help situations where if you change an env var you don't have to remember to clean out an old build directory to ensure fresh results. Closes #2776 --- src/cargo/ops/cargo_compile.rs | 5 +- src/cargo/ops/cargo_rustc/context.rs | 4 +- src/cargo/ops/cargo_rustc/custom_build.rs | 36 +++- src/cargo/ops/cargo_rustc/fingerprint.rs | 198 ++++++++++++++-------- src/doc/build-script.md | 9 + tests/build-script-env.rs | 106 ++++++++++++ 6 files changed, 276 insertions(+), 82 deletions(-) create mode 100644 tests/build-script-env.rs diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 888939a2203..f6fc2af94e5 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -703,6 +703,7 @@ fn scrape_target_config(config: &Config, triple: &str) env: Vec::new(), metadata: Vec::new(), rerun_if_changed: Vec::new(), + rerun_if_env_changed: Vec::new(), warnings: Vec::new(), }; // We require deterministic order of evaluation, so we must sort the pairs by key first. @@ -745,7 +746,9 @@ fn scrape_target_config(config: &Config, triple: &str) output.env.push((name.clone(), val.to_string())); } } - "warning" | "rerun-if-changed" => { + "warning" | + "rerun-if-changed" | + "rerun-if-env-changed" => { bail!("`{}` is not supported in build script overrides", k); } _ => { diff --git a/src/cargo/ops/cargo_rustc/context.rs b/src/cargo/ops/cargo_rustc/context.rs index 1b1242f9038..91d24d054e4 100644 --- a/src/cargo/ops/cargo_rustc/context.rs +++ b/src/cargo/ops/cargo_rustc/context.rs @@ -17,7 +17,7 @@ use util::{self, internal, Config, profile, Cfg, CfgExpr}; use util::errors::{CargoResult, CargoResultExt}; use super::TargetConfig; -use super::custom_build::{BuildState, BuildScripts}; +use super::custom_build::{BuildState, BuildScripts, BuildDeps}; use super::fingerprint::Fingerprint; use super::layout::Layout; use super::links::Links; @@ -38,7 +38,7 @@ pub struct Context<'a, 'cfg: 'a> { pub compilation: Compilation<'cfg>, pub packages: &'a PackageSet<'cfg>, pub build_state: Arc, - pub build_explicit_deps: HashMap, (PathBuf, Vec)>, + pub build_explicit_deps: HashMap, BuildDeps>, pub fingerprints: HashMap, Arc>, pub compiled: HashSet>, pub build_config: BuildConfig, diff --git a/src/cargo/ops/cargo_rustc/custom_build.rs b/src/cargo/ops/cargo_rustc/custom_build.rs index 6bb2620fdae..15c149bcc14 100644 --- a/src/cargo/ops/cargo_rustc/custom_build.rs +++ b/src/cargo/ops/cargo_rustc/custom_build.rs @@ -28,6 +28,8 @@ pub struct BuildOutput { pub metadata: Vec<(String, String)>, /// Paths to trigger a rerun of this build script. pub rerun_if_changed: Vec, + /// Environment variables which, when changed, will cause a rebuild. + pub rerun_if_env_changed: Vec, /// Warnings generated by this build, pub warnings: Vec, } @@ -59,6 +61,12 @@ pub struct BuildScripts { pub plugins: BTreeSet, } +pub struct BuildDeps { + pub build_script_output: PathBuf, + pub rerun_if_changed: Vec, + pub rerun_if_env_changed: Vec, +} + /// Prepares a `Work` that executes the target as a custom build script. /// /// The `req` given is the requirement which this run of the build script will @@ -181,11 +189,8 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) // Check to see if the build script has already run, and if it has keep // track of whether it has told us about some explicit dependencies let prev_output = BuildOutput::parse_file(&output_file, &pkg_name).ok(); - let rerun_if_changed = match prev_output { - Some(ref prev) => prev.rerun_if_changed.clone(), - None => Vec::new(), - }; - cx.build_explicit_deps.insert(*unit, (output_file.clone(), rerun_if_changed)); + let deps = BuildDeps::new(&output_file, prev_output.as_ref()); + cx.build_explicit_deps.insert(*unit, deps); fs::create_dir_all(&script_output)?; fs::create_dir_all(&build_output)?; @@ -246,8 +251,6 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) })?; - paths::write(&output_file, &output.stdout)?; - paths::write(&err_file, &output.stderr)?; // After the build command has finished running, we need to be sure to // remember all of its output so we can later discover precisely what it @@ -256,6 +259,8 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) // This is also the location where we provide feedback into the build // state informing what variables were discovered via our script as // well. + paths::write(&output_file, &output.stdout)?; + paths::write(&err_file, &output.stderr)?; let parsed_output = BuildOutput::parse(&output.stdout, &pkg_name)?; if json_messages { @@ -337,6 +342,7 @@ impl BuildOutput { let mut env = Vec::new(); let mut metadata = Vec::new(); let mut rerun_if_changed = Vec::new(); + let mut rerun_if_env_changed = Vec::new(); let mut warnings = Vec::new(); let whence = format!("build script of `{}`", pkg_name); @@ -378,6 +384,7 @@ impl BuildOutput { "rustc-env" => env.push(BuildOutput::parse_rustc_env(value, &whence)?), "warning" => warnings.push(value.to_string()), "rerun-if-changed" => rerun_if_changed.push(value.to_string()), + "rerun-if-env-changed" => rerun_if_env_changed.push(value.to_string()), _ => metadata.push((key.to_string(), value.to_string())), } } @@ -389,6 +396,7 @@ impl BuildOutput { env: env, metadata: metadata, rerun_if_changed: rerun_if_changed, + rerun_if_env_changed: rerun_if_env_changed, warnings: warnings, }) } @@ -436,6 +444,20 @@ impl BuildOutput { } } +impl BuildDeps { + pub fn new(output_file: &Path, output: Option<&BuildOutput>) -> BuildDeps { + BuildDeps { + build_script_output: output_file.to_path_buf(), + rerun_if_changed: output.map(|p| &p.rerun_if_changed) + .cloned() + .unwrap_or_default(), + rerun_if_env_changed: output.map(|p| &p.rerun_if_env_changed) + .cloned() + .unwrap_or_default(), + } + } +} + /// Compute the `build_scripts` map in the `Context` which tracks what build /// scripts each package depends on. /// diff --git a/src/cargo/ops/cargo_rustc/fingerprint.rs b/src/cargo/ops/cargo_rustc/fingerprint.rs index a3fc7ffe574..44f227f2a2c 100644 --- a/src/cargo/ops/cargo_rustc/fingerprint.rs +++ b/src/cargo/ops/cargo_rustc/fingerprint.rs @@ -1,3 +1,4 @@ +use std::env; use std::fs::{self, File, OpenOptions}; use std::hash::{self, Hasher}; use std::io::prelude::*; @@ -18,6 +19,7 @@ use util::paths; use super::job::Work; use super::context::{Context, Unit}; +use super::custom_build::BuildDeps; /// A tuple result of the `prepare_foo` functions in this module. /// @@ -136,7 +138,7 @@ pub struct Fingerprint { profile: u64, #[serde(serialize_with = "serialize_deps", deserialize_with = "deserialize_deps")] deps: Vec<(String, Arc)>, - local: LocalFingerprint, + local: Vec, #[serde(skip_serializing, skip_deserializing)] memoized_hash: Mutex>, rustflags: Vec, @@ -160,7 +162,7 @@ fn deserialize_deps<'de, D>(d: D) -> Result)>, D:: rustc: 0, target: 0, profile: 0, - local: LocalFingerprint::Precalculated(String::new()), + local: vec![LocalFingerprint::Precalculated(String::new())], features: String::new(), deps: Vec::new(), memoized_hash: Mutex::new(Some(hash)), @@ -173,25 +175,33 @@ fn deserialize_deps<'de, D>(d: D) -> Result)>, D:: enum LocalFingerprint { Precalculated(String), MtimeBased(MtimeSlot, PathBuf), + EnvBased(String, Option), } struct MtimeSlot(Mutex>); impl Fingerprint { fn update_local(&self) -> CargoResult<()> { - match self.local { - LocalFingerprint::MtimeBased(ref slot, ref path) => { - let meta = fs::metadata(path) - .chain_err(|| { - internal(format!("failed to stat `{}`", path.display())) - })?; - let mtime = FileTime::from_last_modification_time(&meta); - *slot.0.lock().unwrap() = Some(mtime); + let mut hash_busted = false; + for local in self.local.iter() { + match *local { + LocalFingerprint::MtimeBased(ref slot, ref path) => { + let meta = fs::metadata(path) + .chain_err(|| { + internal(format!("failed to stat `{}`", path.display())) + })?; + let mtime = FileTime::from_last_modification_time(&meta); + *slot.0.lock().unwrap() = Some(mtime); + } + LocalFingerprint::EnvBased(..) | + LocalFingerprint::Precalculated(..) => continue, } - LocalFingerprint::Precalculated(..) => return Ok(()) + hash_busted = true; } - *self.memoized_hash.lock().unwrap() = None; + if hash_busted { + *self.memoized_hash.lock().unwrap() = None; + } Ok(()) } @@ -220,32 +230,47 @@ impl Fingerprint { if self.rustflags != old.rustflags { return Err(internal("RUSTFLAGS has changed")) } - match (&self.local, &old.local) { - (&LocalFingerprint::Precalculated(ref a), - &LocalFingerprint::Precalculated(ref b)) => { - if a != b { - bail!("precalculated components have changed: {} != {}", - a, b) + if self.local.len() != old.local.len() { + bail!("local lens changed"); + } + for (new, old) in self.local.iter().zip(&old.local) { + match (new, old) { + (&LocalFingerprint::Precalculated(ref a), + &LocalFingerprint::Precalculated(ref b)) => { + if a != b { + bail!("precalculated components have changed: {} != {}", + a, b) + } } - } - (&LocalFingerprint::MtimeBased(ref on_disk_mtime, ref ap), - &LocalFingerprint::MtimeBased(ref previously_built_mtime, ref bp)) => { - let on_disk_mtime = on_disk_mtime.0.lock().unwrap(); - let previously_built_mtime = previously_built_mtime.0.lock().unwrap(); - - let should_rebuild = match (*on_disk_mtime, *previously_built_mtime) { - (None, None) => false, - (Some(_), None) | (None, Some(_)) => true, - (Some(on_disk), Some(previously_built)) => on_disk > previously_built, - }; - - if should_rebuild { - bail!("mtime based components have changed: previously {:?} now {:?}, \ - paths are {:?} and {:?}", - *previously_built_mtime, *on_disk_mtime, ap, bp) + (&LocalFingerprint::MtimeBased(ref on_disk_mtime, ref ap), + &LocalFingerprint::MtimeBased(ref previously_built_mtime, ref bp)) => { + let on_disk_mtime = on_disk_mtime.0.lock().unwrap(); + let previously_built_mtime = previously_built_mtime.0.lock().unwrap(); + + let should_rebuild = match (*on_disk_mtime, *previously_built_mtime) { + (None, None) => false, + (Some(_), None) | (None, Some(_)) => true, + (Some(on_disk), Some(previously_built)) => on_disk > previously_built, + }; + + if should_rebuild { + bail!("mtime based components have changed: previously {:?} now {:?}, \ + paths are {:?} and {:?}", + *previously_built_mtime, *on_disk_mtime, ap, bp) + } } + (&LocalFingerprint::EnvBased(ref akey, ref avalue), + &LocalFingerprint::EnvBased(ref bkey, ref bvalue)) => { + if *akey != *bkey { + bail!("env vars changed: {} != {}", akey, bkey); + } + if *avalue != *bvalue { + bail!("env var `{}` changed: previously {:?} now {:?}", + akey, bvalue, avalue) + } + } + _ => bail!("local fingerprint type has changed"), } - _ => bail!("local fingerprint type has changed"), } if self.deps.len() != old.deps.len() { @@ -359,7 +384,7 @@ fn calculate<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) profile: util::hash_u64(&unit.profile), features: format!("{:?}", cx.resolve.features_sorted(unit.pkg.package_id())), deps: deps, - local: local, + local: vec![local], memoized_hash: Mutex::new(None), rustflags: extra_flags, }); @@ -403,36 +428,7 @@ pub fn prepare_build_cmd<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) debug!("fingerprint at: {}", loc.display()); - // If this build script execution has been overridden, then the fingerprint - // is just a hash of what it was overridden with. Otherwise the fingerprint - // is that of the entire package itself as we just consider everything as - // input to the build script. - let (local, output_path) = { - let state = cx.build_state.outputs.lock().unwrap(); - match state.get(&(unit.pkg.package_id().clone(), unit.kind)) { - Some(output) => { - let s = format!("overridden build state with hash: {}", - util::hash_u64(output)); - (LocalFingerprint::Precalculated(s), None) - } - None => { - let &(ref output, ref deps) = &cx.build_explicit_deps[unit]; - - let local = if deps.is_empty() { - let s = pkg_fingerprint(cx, unit.pkg)?; - LocalFingerprint::Precalculated(s) - } else { - let deps = deps.iter().map(|p| unit.pkg.root().join(p)); - let mtime = mtime_if_fresh(output, deps); - let mtime = MtimeSlot(Mutex::new(mtime)); - LocalFingerprint::MtimeBased(mtime, output.clone()) - }; - - (local, Some(output.clone())) - } - } - }; - + let (local, output_path) = build_script_local_fingerprints(cx, unit)?; let mut fingerprint = Fingerprint { rustc: 0, target: 0, @@ -454,17 +450,19 @@ pub fn prepare_build_cmd<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) // necessary for that fingerprint. // // Hence, if there were some `rerun-if-changed` directives forcibly change - // the kind of fingerprint over to the `MtimeBased` variant where the - // relevant mtime is the output path of the build script. + // the kind of fingerprint by reinterpreting the dependencies output by the + // build script. let state = cx.build_state.clone(); let key = (unit.pkg.package_id().clone(), unit.kind); + let root = unit.pkg.root().to_path_buf(); let write_fingerprint = Work::new(move |_| { if let Some(output_path) = output_path { let outputs = state.outputs.lock().unwrap(); - if !outputs[&key].rerun_if_changed.is_empty() { - let slot = MtimeSlot(Mutex::new(None)); - fingerprint.local = LocalFingerprint::MtimeBased(slot, - output_path); + let outputs = &outputs[&key]; + if !outputs.rerun_if_changed.is_empty() || + !outputs.rerun_if_env_changed.is_empty() { + let deps = BuildDeps::new(&output_path, Some(outputs)); + fingerprint.local = local_fingerprints_deps(&deps, &root); fingerprint.update_local()?; } } @@ -474,6 +472,62 @@ pub fn prepare_build_cmd<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) Ok((if compare.is_ok() {Fresh} else {Dirty}, write_fingerprint, Work::noop())) } +fn build_script_local_fingerprints<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, + unit: &Unit<'a>) + -> CargoResult<(Vec, Option)> +{ + let state = cx.build_state.outputs.lock().unwrap(); + // First up, if this build script is entirely overridden, then we just + // return the hash of what we overrode it with. + // + // Note that the `None` here means tha twe don't want to update the local + // fingerprint afterwards because this is all just overridden. + if let Some(output) = state.get(&(unit.pkg.package_id().clone(), unit.kind)) { + debug!("override local fingerprints deps"); + let s = format!("overridden build state with hash: {}", + util::hash_u64(output)); + return Ok((vec![LocalFingerprint::Precalculated(s)], None)) + } + + // Next up we look at the previously listed dependencies for the build + // script. If there are none then we're in the "old mode" where we just + // assume that we're changed if anything in the packaged changed. The + // `Some` here though means that we want to update our local fingerprints + // after we're done as running this build script may have created more + // dependencies. + let deps = &cx.build_explicit_deps[unit]; + let output = deps.build_script_output.clone(); + if deps.rerun_if_changed.is_empty() && deps.rerun_if_env_changed.is_empty() { + debug!("old local fingerprints deps"); + let s = pkg_fingerprint(cx, unit.pkg)?; + return Ok((vec![LocalFingerprint::Precalculated(s)], Some(output))) + } + + // Ok so now we're in "new mode" where we can have files listed as + // dependencies as well as env vars listed as dependencies. Process them all + // here. + Ok((local_fingerprints_deps(deps, unit.pkg.root()), Some(output))) +} + +fn local_fingerprints_deps(deps: &BuildDeps, root: &Path) -> Vec { + debug!("new local fingerprints deps"); + let mut local = Vec::new(); + if !deps.rerun_if_changed.is_empty() { + let output = &deps.build_script_output; + let deps = deps.rerun_if_changed.iter().map(|p| root.join(p)); + let mtime = mtime_if_fresh(output, deps); + let mtime = MtimeSlot(Mutex::new(mtime)); + local.push(LocalFingerprint::MtimeBased(mtime, output.clone())); + } + + for var in deps.rerun_if_env_changed.iter() { + let val = env::var(var).ok(); + local.push(LocalFingerprint::EnvBased(var.clone(), val)); + } + + return local +} + fn write_fingerprint(loc: &Path, fingerprint: &Fingerprint) -> CargoResult<()> { let hash = fingerprint.hash(); debug!("write fingerprint: {}", loc.display()); diff --git a/src/doc/build-script.md b/src/doc/build-script.md index 6de0ab9117f..a0bfe0fd669 100644 --- a/src/doc/build-script.md +++ b/src/doc/build-script.md @@ -97,6 +97,15 @@ crate is built: then it's rebuilt and rerun unconditionally, so `cargo:rerun-if-changed=build.rs` is almost always redundant (unless you want to ignore changes in all other files except for `build.rs`). +* `rerun-if-env-changed=VAR` is the name of an environment variable which + indicates that if the environment variable's value changes the build script + should be rerun. This basically behaves the same as `rerun-if-changed` except + that it works with environment variables instead. Note that the environment + variables here are intended for global environment variables like `CC` and + such, it's not necessary to use this for env vars like `TARGET` that Cargo + sets. Also note that if `rerun-if-env-changed` is printed out then Cargo will + *only* rerun the build script if those environment variables change or if + files printed out by `rerun-if-changed` change. * `warning=MESSAGE` is a message that will be printed to the main console after a build script has finished running. Warnings are only shown for path diff --git a/tests/build-script-env.rs b/tests/build-script-env.rs new file mode 100644 index 00000000000..1230de34bfe --- /dev/null +++ b/tests/build-script-env.rs @@ -0,0 +1,106 @@ +extern crate cargotest; +extern crate hamcrest; + +use std::fs::File; + +use cargotest::sleep_ms; +use cargotest::support::{project, execs}; +use hamcrest::assert_that; + +#[test] +fn rerun_if_env_changes() { + let p = project("foo") + .file("Cargo.toml", r#" + [package] + name = "foo" + version = "0.5.0" + authors = [] + "#) + .file("src/main.rs", r#" + fn main() {} + "#) + .file("build.rs", r#" + fn main() { + println!("cargo:rerun-if-env-changed=FOO"); + } + "#); + p.build(); + + assert_that(p.cargo("build"), + execs().with_status(0) + .with_stderr("\ +[COMPILING] foo v0.5.0 ([..]) +[FINISHED] [..] +")); + assert_that(p.cargo("build").env("FOO", "bar"), + execs().with_status(0) + .with_stderr("\ +[COMPILING] foo v0.5.0 ([..]) +[FINISHED] [..] +")); + assert_that(p.cargo("build").env("FOO", "baz"), + execs().with_status(0) + .with_stderr("\ +[COMPILING] foo v0.5.0 ([..]) +[FINISHED] [..] +")); + assert_that(p.cargo("build").env("FOO", "baz"), + execs().with_status(0) + .with_stderr("\ +[FINISHED] [..] +")); + assert_that(p.cargo("build"), + execs().with_status(0) + .with_stderr("\ +[COMPILING] foo v0.5.0 ([..]) +[FINISHED] [..] +")); +} + +#[test] +fn rerun_if_env_or_file_changes() { + let p = project("foo") + .file("Cargo.toml", r#" + [package] + name = "foo" + version = "0.5.0" + authors = [] + "#) + .file("src/main.rs", r#" + fn main() {} + "#) + .file("build.rs", r#" + fn main() { + println!("cargo:rerun-if-env-changed=FOO"); + println!("cargo:rerun-if-changed=foo"); + } + "#) + .file("foo", ""); + p.build(); + + assert_that(p.cargo("build"), + execs().with_status(0) + .with_stderr("\ +[COMPILING] foo v0.5.0 ([..]) +[FINISHED] [..] +")); + assert_that(p.cargo("build").env("FOO", "bar"), + execs().with_status(0) + .with_stderr("\ +[COMPILING] foo v0.5.0 ([..]) +[FINISHED] [..] +")); + assert_that(p.cargo("build").env("FOO", "bar"), + execs().with_status(0) + .with_stderr("\ +[FINISHED] [..] +")); + sleep_ms(1000); + File::create(p.root().join("foo")).unwrap(); + assert_that(p.cargo("build").env("FOO", "bar"), + execs().with_status(0) + .with_stderr("\ +[COMPILING] foo v0.5.0 ([..]) +[FINISHED] [..] +")); +}