diff --git a/src/cargo/core/compiler/context/unit_dependencies.rs b/src/cargo/core/compiler/context/unit_dependencies.rs index f522f3839a9..62b9f80b039 100644 --- a/src/cargo/core/compiler/context/unit_dependencies.rs +++ b/src/cargo/core/compiler/context/unit_dependencies.rs @@ -19,14 +19,16 @@ use super::{BuildContext, CompileMode, Kind, Unit}; use core::dependency::Kind as DepKind; use core::profiles::ProfileFor; use core::{Package, Target}; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use CargoResult; pub fn build_unit_dependencies<'a, 'cfg>( roots: &[Unit<'a>], bcx: &BuildContext<'a, 'cfg>, - mut deps: &mut HashMap, Vec>>, + deps: &mut HashMap, Vec>>, ) -> CargoResult<()> { + assert!(deps.len() == 0, "can only build unit deps once"); + for unit in roots.iter() { // Dependencies of tests/benches should not have `panic` set. // We check the global test mode to see if we are running in `cargo @@ -39,18 +41,20 @@ pub fn build_unit_dependencies<'a, 'cfg>( } else { ProfileFor::Any }; - deps_of(unit, bcx, &mut deps, profile_for)?; + deps_of(unit, bcx, deps, profile_for)?; } + connect_run_custom_build_deps(bcx, deps); + Ok(()) } -fn deps_of<'a, 'b, 'cfg>( +fn deps_of<'a, 'cfg>( unit: &Unit<'a>, bcx: &BuildContext<'a, 'cfg>, - deps: &'b mut HashMap, Vec>>, + deps: &mut HashMap, Vec>>, profile_for: ProfileFor, -) -> CargoResult<&'b [Unit<'a>]> { +) -> CargoResult<()> { // Currently the `deps` map does not include `profile_for`. This should // be safe for now. `TestDependency` only exists to clear the `panic` // flag, and you'll never ask for a `unit` with `panic` set as a @@ -58,14 +62,14 @@ fn deps_of<'a, 'b, 'cfg>( // requested unit's settings are the same as `Any`, `CustomBuild` can't // affect anything else in the hierarchy. if !deps.contains_key(unit) { - let unit_deps = compute_deps(unit, bcx, deps, profile_for)?; + let unit_deps = compute_deps(unit, bcx, profile_for)?; let to_insert: Vec<_> = unit_deps.iter().map(|&(unit, _)| unit).collect(); deps.insert(*unit, to_insert); for (unit, profile_for) in unit_deps { deps_of(&unit, bcx, deps, profile_for)?; } } - Ok(deps[unit].as_ref()) + Ok(()) } /// For a package, return all targets which are registered as dependencies @@ -75,11 +79,10 @@ fn deps_of<'a, 'b, 'cfg>( fn compute_deps<'a, 'b, 'cfg>( unit: &Unit<'a>, bcx: &BuildContext<'a, 'cfg>, - deps: &'b mut HashMap, Vec>>, profile_for: ProfileFor, ) -> CargoResult, ProfileFor)>> { if unit.mode.is_run_custom_build() { - return compute_deps_custom_build(unit, bcx, deps); + return compute_deps_custom_build(unit, bcx); } else if unit.mode.is_doc() && !unit.mode.is_any_test() { // Note: This does not include Doctest. return compute_deps_doc(unit, bcx); @@ -192,39 +195,27 @@ fn compute_deps<'a, 'b, 'cfg>( fn compute_deps_custom_build<'a, 'cfg>( unit: &Unit<'a>, bcx: &BuildContext<'a, 'cfg>, - deps: &mut HashMap, Vec>>, ) -> CargoResult, ProfileFor)>> { // When not overridden, then the dependencies to run a build script are: // // 1. Compiling the build script itself // 2. For each immediate dependency of our package which has a `links` // key, the execution of that build script. - let deps = deps - .iter() - .find(|(key, _deps)| key.pkg == unit.pkg && !key.target.is_custom_build()) - .expect("can't find package deps") - .1; - Ok(deps.iter() - .filter_map(|unit| { - if !unit.target.linkable() || unit.pkg.manifest().links().is_none() { - return None; - } - dep_build_script(unit, bcx) - }) - .chain(Some(( - new_unit( - bcx, - unit.pkg, - unit.target, - ProfileFor::CustomBuild, - Kind::Host, // build scripts always compiled for the host - CompileMode::Build, - ), - // All dependencies of this unit should use profiles for custom - // builds. - ProfileFor::CustomBuild, - ))) - .collect()) + // + // We don't have a great way of handling (2) here right now so this is + // deferred until after the graph of all unit dependencies has been + // constructed. + let unit = new_unit( + bcx, + unit.pkg, + unit.target, + ProfileFor::CustomBuild, + Kind::Host, // build scripts always compiled for the host + CompileMode::Build, + ); + // All dependencies of this unit should use profiles for custom + // builds. + Ok(vec![(unit, ProfileFor::CustomBuild)]) } /// Returns the dependencies necessary to document a package @@ -369,3 +360,74 @@ fn new_unit<'a>( mode, } } + +/// Fill in missing dependencies for units of the `RunCustomBuild` +/// +/// As mentioned above in `compute_deps_custom_build` each build script +/// execution has two dependencies. The first is compiling the build script +/// itself (already added) and the second is that all crates the package of the +/// build script depends on with `links` keys, their build script execution. (a +/// bit confusing eh?) +/// +/// Here we take the entire `deps` map and add more dependencies from execution +/// of one build script to execution of another build script. +fn connect_run_custom_build_deps<'a>( + bcx: &BuildContext, + deps: &mut HashMap, Vec>>, +) { + let mut new_deps = Vec::new(); + + { + // First up build a reverse dependency map. This is a mapping of all + // `RunCustomBuild` known steps to the unit which depends on them. For + // example a library might depend on a build script, so this map will + // have the build script as the key and the library would be in the + // value's set. + let mut reverse_deps = HashMap::new(); + for (unit, deps) in deps.iter() { + for dep in deps { + if dep.mode == CompileMode::RunCustomBuild { + reverse_deps.entry(dep) + .or_insert(HashSet::new()) + .insert(unit); + } + } + } + + // And next we take a look at all build scripts executions listed in the + // dependency map. Our job here is to take everything that depends on + // this build script (from our reverse map above) and look at the other + // package dependencies of these parents. + // + // If we depend on a linkable target and the build script mentions + // `links`, then we depend on that package's build script! Here we use + // `dep_build_script` to manufacture an appropriate build script unit to + // depend on. + for unit in deps.keys().filter(|k| k.mode == CompileMode::RunCustomBuild) { + let reverse_deps = match reverse_deps.get(unit) { + Some(set) => set, + None => continue, + }; + + let to_add = reverse_deps + .iter() + .flat_map(|reverse_dep| deps[reverse_dep].iter()) + .filter(|other| { + other.pkg != unit.pkg && + other.target.linkable() && + other.pkg.manifest().links().is_some() + }) + .filter_map(|other| dep_build_script(other, bcx).map(|p| p.0)) + .collect::>(); + + if !to_add.is_empty() { + new_deps.push((*unit, to_add)); + } + } + } + + // And finally, add in all the missing dependencies! + for (unit, new_deps) in new_deps { + deps.get_mut(&unit).unwrap().extend(new_deps); + } +} diff --git a/tests/testsuite/build_script.rs b/tests/testsuite/build_script.rs index 1754123364e..928b2904ae2 100644 --- a/tests/testsuite/build_script.rs +++ b/tests/testsuite/build_script.rs @@ -3080,9 +3080,11 @@ fn panic_abort_with_build_scripts() { execs().with_status(0), ); + p.root().join("target").rm_rf(); + assert_that( - p.cargo("test --release"), - execs().with_status(0) + p.cargo("test --release -v"), + execs().with_status(0).with_stderr_does_not_contain("[..]panic[..]"), ); } diff --git a/tests/testsuite/test.rs b/tests/testsuite/test.rs index 47ff910919c..ee28fb58577 100644 --- a/tests/testsuite/test.rs +++ b/tests/testsuite/test.rs @@ -4138,3 +4138,28 @@ fn json_artifact_includes_test_flag() { ), ); } + +#[test] +fn test_build_script_links() { + let p = project("foo") + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + links = 'something' + + [lib] + test = false + "#, + ) + .file("build.rs", "fn main() {}") + .file("src/lib.rs", "") + .build(); + + assert_that( + p.cargo("test --no-run"), + execs().with_status(0), + ); +}