From 7c97c5bf694df1a67603e4806accd2bef88635ee Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 10 Nov 2015 16:39:15 -0800 Subject: [PATCH] Allow build scripts to specify dependencies Currently Cargo is quite conservative in how it determines whether a build script should be run. The heuristic used is "did any file in the project directory change", but this is almost always guaranteed to be too coarse grained in situations like: * If the build script takes a long time to run it's advantageous to run it as few times as possible. Being able to inform Cargo about precisely when a build script should be run should provide more robust support here. * Build scripts may not always have all of their dependencies in-tree or in the crate root. Sometimes a dependency could be elsewhere in a repository and scripts need a method of informing Cargo about this (as currently these compiles don't happen then they should). This commit adds this support in build scripts via a new `rerun-if-changed` directive which can be printed to standard output (using the standard Cargo metadata format). The value for this key is a path relative to the crate root, and Cargo will only look at these paths when determining whether to rerun the build script. Any other file changes will not trigger the build script to be rerun. Future extensions could possibly include the usage of glob patterns in build script paths like the `include` and `exclude` features of `Cargo.toml`, but these should be backwards compatible to add in the future. Closes #1162 --- src/cargo/ops/cargo_compile.rs | 1 + src/cargo/ops/cargo_rustc/context.rs | 2 + src/cargo/ops/cargo_rustc/custom_build.rs | 42 +++++++++--- src/cargo/ops/cargo_rustc/fingerprint.rs | 60 ++++++++++++---- src/doc/build-script.md | 6 ++ tests/test_cargo_compile_custom_build.rs | 84 ++++++++++++++++++++++- 6 files changed, 171 insertions(+), 24 deletions(-) diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 073d5929335..38469a85f05 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -475,6 +475,7 @@ fn scrape_target_config(config: &Config, triple: &str) library_links: Vec::new(), cfgs: Vec::new(), metadata: Vec::new(), + rerun_if_changed: Vec::new(), }; let key = format!("{}.{}", key, lib_name); let table = try!(config.get_table(&key)).unwrap().0; diff --git a/src/cargo/ops/cargo_rustc/context.rs b/src/cargo/ops/cargo_rustc/context.rs index 89a2c1a08ec..993d3295fc6 100644 --- a/src/cargo/ops/cargo_rustc/context.rs +++ b/src/cargo/ops/cargo_rustc/context.rs @@ -32,6 +32,7 @@ pub struct Context<'a, 'cfg: 'a> { pub sources: &'a SourceMap<'cfg>, pub compilation: Compilation<'cfg>, pub build_state: Arc, + pub build_explicit_deps: HashMap, (PathBuf, Vec)>, pub exec_engine: Arc>, pub fingerprints: HashMap, Arc>, pub compiled: HashSet>, @@ -93,6 +94,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> { profiles: profiles, compiled: HashSet::new(), build_scripts: HashMap::new(), + build_explicit_deps: HashMap::new(), }) } diff --git a/src/cargo/ops/cargo_rustc/custom_build.rs b/src/cargo/ops/cargo_rustc/custom_build.rs index 8431e81c5f9..ee38d24fd56 100644 --- a/src/cargo/ops/cargo_rustc/custom_build.rs +++ b/src/cargo/ops/cargo_rustc/custom_build.rs @@ -1,7 +1,7 @@ use std::collections::{HashMap, BTreeSet}; use std::fs; use std::io::prelude::*; -use std::path::PathBuf; +use std::path::{PathBuf, Path}; use std::str; use std::sync::{Mutex, Arc}; @@ -25,6 +25,8 @@ pub struct BuildOutput { pub cfgs: Vec, /// Metadata to pass to the immediate dependencies pub metadata: Vec<(String, String)>, + /// Glob paths to trigger a rerun of this build script. + pub rerun_if_changed: Vec, } pub type BuildMap = HashMap<(PackageId, Kind), BuildOutput>; @@ -45,8 +47,8 @@ pub struct BuildScripts { /// prepare work for. If the requirement is specified as both the target and the /// host platforms it is assumed that the two are equal and the build script is /// only run once (not twice). -pub fn prepare(cx: &mut Context, unit: &Unit) - -> CargoResult<(Work, Work, Freshness)> { +pub fn prepare<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) + -> CargoResult<(Work, Work, Freshness)> { let _p = profile::start(format!("build script prepare: {}/{}", unit.pkg, unit.target.name())); let key = (unit.pkg.package_id().clone(), unit.kind); @@ -65,7 +67,8 @@ pub fn prepare(cx: &mut Context, unit: &Unit) Ok((work_dirty.then(dirty), work_fresh.then(fresh), freshness)) } -fn build_work(cx: &mut Context, unit: &Unit) -> CargoResult<(Work, Work)> { +fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) + -> CargoResult<(Work, Work)> { let (script_output, build_output) = { (cx.layout(unit.pkg, Kind::Host).build(unit.pkg), cx.layout(unit.pkg, unit.kind).build_out(unit.pkg)) @@ -119,11 +122,20 @@ fn build_work(cx: &mut Context, unit: &Unit) -> CargoResult<(Work, Work)> { let pkg_name = unit.pkg.to_string(); let build_state = cx.build_state.clone(); let id = unit.pkg.package_id().clone(); + let output_file = build_output.parent().unwrap().join("output"); let all = (id.clone(), pkg_name.clone(), build_state.clone(), - build_output.clone()); + output_file.clone()); let build_scripts = super::load_build_deps(cx, unit); let kind = unit.kind; + // Check to see if the build script as 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(); + if let Some(ref prev) = prev_output { + let val = (output_file.clone(), prev.rerun_if_changed.clone()); + cx.build_explicit_deps.insert(*unit, val); + } + try!(fs::create_dir_all(&cx.layout(unit.pkg, Kind::Host).build(unit.pkg))); try!(fs::create_dir_all(&cx.layout(unit.pkg, unit.kind).build(unit.pkg))); @@ -177,8 +189,7 @@ fn build_work(cx: &mut Context, unit: &Unit) -> CargoResult<(Work, Work)> { pkg_name, e.desc); Human(e) })); - try!(paths::write(&build_output.parent().unwrap().join("output"), - &output.stdout)); + try!(paths::write(&output_file, &output.stdout)); // 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 @@ -199,10 +210,11 @@ fn build_work(cx: &mut Context, unit: &Unit) -> CargoResult<(Work, Work)> { // itself to run when we actually end up just discarding what we calculated // above. let fresh = Work::new(move |_tx| { - let (id, pkg_name, build_state, build_output) = all; - let contents = try!(paths::read(&build_output.parent().unwrap() - .join("output"))); - let output = try!(BuildOutput::parse(&contents, &pkg_name)); + let (id, pkg_name, build_state, output_file) = all; + let output = match prev_output { + Some(output) => output, + None => try!(BuildOutput::parse_file(&output_file, &pkg_name)), + }; build_state.insert(id, kind, output); Ok(()) }); @@ -242,6 +254,11 @@ impl BuildState { } impl BuildOutput { + pub fn parse_file(path: &Path, pkg_name: &str) -> CargoResult { + let contents = try!(paths::read(path)); + BuildOutput::parse(&contents, pkg_name) + } + // Parses the output of a script. // The `pkg_name` is used for error messages. pub fn parse(input: &str, pkg_name: &str) -> CargoResult { @@ -249,6 +266,7 @@ impl BuildOutput { let mut library_links = Vec::new(); let mut cfgs = Vec::new(); let mut metadata = Vec::new(); + let mut rerun_if_changed = Vec::new(); let whence = format!("build script of `{}`", pkg_name); for line in input.lines() { @@ -284,6 +302,7 @@ impl BuildOutput { "rustc-link-lib" => library_links.push(value.to_string()), "rustc-link-search" => library_paths.push(PathBuf::from(value)), "rustc-cfg" => cfgs.push(value.to_string()), + "rerun-if-changed" => rerun_if_changed.push(value.to_string()), _ => metadata.push((key.to_string(), value.to_string())), } } @@ -293,6 +312,7 @@ impl BuildOutput { library_links: library_links, cfgs: cfgs, metadata: metadata, + rerun_if_changed: rerun_if_changed, }) } diff --git a/src/cargo/ops/cargo_rustc/fingerprint.rs b/src/cargo/ops/cargo_rustc/fingerprint.rs index 945b42f23e4..3dcc79b1cf4 100644 --- a/src/cargo/ops/cargo_rustc/fingerprint.rs +++ b/src/cargo/ops/cargo_rustc/fingerprint.rs @@ -124,7 +124,7 @@ impl Fingerprint { match self.local { LocalFingerprint::MtimeBased(ref slot, ref path) => { let mut slot = slot.0.lock().unwrap(); - if force || slot.is_none() { + if force { let meta = try!(fs::metadata(path).chain_error(|| { internal(format!("failed to stat {:?}", path)) })); @@ -316,12 +316,6 @@ fn calculate<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) let local = if use_dep_info(unit) { let dep_info = dep_info_loc(cx, unit); let mtime = try!(calculate_target_mtime(&dep_info)); - - // if the mtime listed is not fresh, then remove the `dep_info` file to - // ensure that future calls to `resolve()` won't work. - if mtime.is_none() { - let _ = fs::remove_file(&dep_info); - } LocalFingerprint::MtimeBased(MtimeSlot(Mutex::new(mtime)), dep_info) } else { let fingerprint = try!(calculate_pkg_fingerprint(cx, unit.pkg)); @@ -382,14 +376,29 @@ pub fn prepare_build_cmd(cx: &mut Context, unit: &Unit) // 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 new_fingerprint = { + let local = { let state = cx.build_state.outputs.lock().unwrap(); match state.get(&(unit.pkg.package_id().clone(), unit.kind)) { Some(output) => { - format!("overridden build state with hash: {}", - util::hash_u64(output)) + let s = format!("overridden build state with hash: {}", + util::hash_u64(output)); + LocalFingerprint::Precalculated(s) + } + None => { + match cx.build_explicit_deps.get(unit) { + Some(&(ref output, ref deps)) if deps.len() > 0 => { + let mtime = try!(calculate_explicit_fingerprint(unit, + output, + deps)); + LocalFingerprint::MtimeBased(MtimeSlot(Mutex::new(mtime)), + output.clone()) + } + _ => { + let s = try!(calculate_pkg_fingerprint(cx, unit.pkg)); + LocalFingerprint::Precalculated(s) + } + } } - None => try!(calculate_pkg_fingerprint(cx, unit.pkg)), } }; let new_fingerprint = Arc::new(Fingerprint { @@ -398,7 +407,7 @@ pub fn prepare_build_cmd(cx: &mut Context, unit: &Unit) profile: 0, features: String::new(), deps: Vec::new(), - local: LocalFingerprint::Precalculated(new_fingerprint), + local: local, resolved: Mutex::new(None), }); @@ -548,6 +557,33 @@ fn calculate_pkg_fingerprint(cx: &Context, source.fingerprint(pkg) } +fn calculate_explicit_fingerprint(unit: &Unit, + output: &Path, + deps: &[String]) + -> CargoResult> { + let meta = match fs::metadata(output) { + Ok(meta) => meta, + Err(..) => return Ok(None), + }; + let mtime = FileTime::from_last_modification_time(&meta); + + for path in deps.iter().map(|p| unit.pkg.root().join(p)) { + let meta = match fs::metadata(&path) { + Ok(meta) => meta, + Err(..) => { + info!("bs stale: {} -- missing", path.display()); + return Ok(None) + } + }; + let mtime2 = FileTime::from_last_modification_time(&meta); + if mtime2 > mtime { + info!("bs stale: {} -- {} vs {}", path.display(), mtime2, mtime); + return Ok(None) + } + } + Ok(Some(mtime)) +} + fn filename(unit: &Unit) -> String { let kind = match *unit.target.kind() { TargetKind::Lib(..) => "lib", diff --git a/src/doc/build-script.md b/src/doc/build-script.md index 8874bf91260..f26931b63b3 100644 --- a/src/doc/build-script.md +++ b/src/doc/build-script.md @@ -68,6 +68,12 @@ build script is for is built: * `rustc-cfg` indicates that the specified directive will be passed as a `--cfg` flag to the compiler. This is often useful for performing compile-time detection of various features. +* `rerun-if-changed` is a path to a file or directory which indicates that the + build script should be re-run if it changes (detected by a more-recent + last-modified timestamp on the file). Normally build scripts are re-run if + any file inside the crate root changes, but this can be used to scope changes + to just a small set of files. If this path points to a directory the entire + directory will be traversed for changes. Any other element is a user-defined metadata that will be passed to dependencies. More information about this can be found in the [`links`][links] diff --git a/tests/test_cargo_compile_custom_build.rs b/tests/test_cargo_compile_custom_build.rs index 2555ee9c247..587185b4cfe 100644 --- a/tests/test_cargo_compile_custom_build.rs +++ b/tests/test_cargo_compile_custom_build.rs @@ -1,5 +1,6 @@ -use std::fs::File; +use std::fs::{self, File}; use std::io::prelude::*; +use std::thread; use support::{project, execs}; use support::{COMPILING, RUNNING, DOCTEST, FRESH, DOCUMENTING}; @@ -1707,3 +1708,84 @@ test!(changing_an_override_invalidates { {running} `rustc [..] -L native=bar` ", compiling = COMPILING, running = RUNNING))); }); + +test!(rebuild_only_on_explicit_paths { + let p = project("a") + .file("Cargo.toml", r#" + [project] + name = "a" + version = "0.5.0" + authors = [] + build = "build.rs" + "#) + .file("src/lib.rs", "") + .file("build.rs", r#" + fn main() { + println!("cargo:rerun-if-changed=foo"); + println!("cargo:rerun-if-changed=bar"); + } + "#); + p.build(); + + assert_that(p.cargo("build").arg("-v"), + execs().with_status(0)); + + // files don't exist, so should always rerun if they don't exist + println!("run without"); + assert_that(p.cargo("build").arg("-v"), + execs().with_status(0).with_stdout(&format!("\ +{compiling} a v0.5.0 ([..]) +{running} `[..]build-script-build[..]` +{running} `rustc src[..]lib.rs [..]` +", running = RUNNING, compiling = COMPILING))); + + thread::sleep_ms(1000); + File::create(p.root().join("foo")).unwrap(); + File::create(p.root().join("bar")).unwrap(); + + // now the exist, so run once, catch the mtime, then shouldn't run again + println!("run with"); + assert_that(p.cargo("build").arg("-v"), + execs().with_status(0).with_stdout(&format!("\ +{compiling} a v0.5.0 ([..]) +{running} `[..]build-script-build[..]` +{running} `rustc src[..]lib.rs [..]` +", running = RUNNING, compiling = COMPILING))); + + println!("run with2"); + assert_that(p.cargo("build").arg("-v"), + execs().with_status(0).with_stdout(&format!("\ +{fresh} a v0.5.0 ([..]) +", fresh = FRESH))); + + thread::sleep_ms(1000); + + // random other files do not affect freshness + println!("run baz"); + File::create(p.root().join("baz")).unwrap(); + assert_that(p.cargo("build").arg("-v"), + execs().with_status(0).with_stdout(&format!("\ +{fresh} a v0.5.0 ([..]) +", fresh = FRESH))); + + // but changing dependent files does + println!("run foo change"); + File::create(p.root().join("foo")).unwrap(); + assert_that(p.cargo("build").arg("-v"), + execs().with_status(0).with_stdout(&format!("\ +{compiling} a v0.5.0 ([..]) +{running} `[..]build-script-build[..]` +{running} `rustc src[..]lib.rs [..]` +", running = RUNNING, compiling = COMPILING))); + + // .. as does deleting a file + println!("run foo delete"); + fs::remove_file(p.root().join("bar")).unwrap(); + assert_that(p.cargo("build").arg("-v"), + execs().with_status(0).with_stdout(&format!("\ +{compiling} a v0.5.0 ([..]) +{running} `[..]build-script-build[..]` +{running} `rustc src[..]lib.rs [..]` +", running = RUNNING, compiling = COMPILING))); +}); +