From 2be857af59beee3cbba42d714bf13716d1f1b94d Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Mon, 11 Jun 2018 10:44:55 -0700 Subject: [PATCH 1/4] Metabuild (RFC 2196) --- .../compiler/context/compilation_files.rs | 9 + src/cargo/core/compiler/custom_build.rs | 45 +- src/cargo/core/compiler/fingerprint.rs | 2 +- src/cargo/core/compiler/layout.rs | 7 + src/cargo/core/compiler/mod.rs | 25 +- src/cargo/core/features.rs | 3 + src/cargo/core/manifest.rs | 12 +- src/cargo/util/toml/mod.rs | 45 ++ src/cargo/util/toml/targets.rs | 37 +- src/doc/src/reference/unstable.md | 33 ++ tests/testsuite/main.rs | 1 + tests/testsuite/metabuild.rs | 414 ++++++++++++++++++ 12 files changed, 617 insertions(+), 16 deletions(-) create mode 100644 tests/testsuite/metabuild.rs diff --git a/src/cargo/core/compiler/context/compilation_files.rs b/src/cargo/core/compiler/context/compilation_files.rs index 5359ac751e8..76138db5ca8 100644 --- a/src/cargo/core/compiler/context/compilation_files.rs +++ b/src/cargo/core/compiler/context/compilation_files.rs @@ -161,6 +161,15 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> { self.layout(unit.kind).build().join(dir).join("out") } + pub fn metabuild_path(&self, unit: &Unit<'a>) -> PathBuf { + let metadata = self.metadata(unit).expect("metabuild metadata"); + self.layout(unit.kind).metabuild().join(format!( + "metabuild-{}-{}.rs", + unit.pkg.name(), + metadata + )) + } + /// Returns the file stem for a given target/profile combo (with metadata) pub fn file_stem(&self, unit: &Unit<'a>) -> String { match self.metas[unit] { diff --git a/src/cargo/core/compiler/custom_build.rs b/src/cargo/core/compiler/custom_build.rs index 2a00cf23b92..43b11744f62 100644 --- a/src/cargo/core/compiler/custom_build.rs +++ b/src/cargo/core/compiler/custom_build.rs @@ -134,6 +134,10 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoRes let build_plan = bcx.build_config.build_plan; let invocation_name = unit.buildkey(); + if let Some(deps) = unit.pkg.manifest().metabuild() { + prepare_metabuild(cx, build_script_unit, deps)?; + } + // Building the command to execute let to_exec = script_output.join(unit.target.name()); @@ -532,6 +536,44 @@ impl BuildOutput { } } +fn prepare_metabuild<'a, 'cfg>( + cx: &Context<'a, 'cfg>, + unit: &Unit<'a>, + deps: &[String], +) -> CargoResult<()> { + let mut output = Vec::new(); + let available_deps = cx.dep_targets(unit); + // Filter out optional dependencies, and look up the actual lib name. + let meta_deps: Vec<_> = deps + .iter() + .filter_map(|name| { + available_deps + .iter() + .find(|u| u.pkg.name().as_str() == name.as_str()) + .map(|dep| dep.target.crate_name()) + }) + .collect(); + for dep in &meta_deps { + output.push(format!("extern crate {};\n", dep)); + } + output.push("fn main() {\n".to_string()); + for dep in &meta_deps { + output.push(format!(" {}::metabuild();\n", dep)); + } + output.push("}".to_string()); + let output = output.join(""); + let path = cx.files().metabuild_path(unit); + let changed = if let Ok(orig) = fs::read_to_string(&path) { + orig != output + } else { + true + }; + if changed { + fs::write(&path, output)?; + } + Ok(()) +} + impl BuildDeps { pub fn new(output_file: &Path, output: Option<&BuildOutput>) -> BuildDeps { BuildDeps { @@ -580,7 +622,8 @@ pub fn build_map<'b, 'cfg>(cx: &mut Context<'b, 'cfg>, units: &[Unit<'b>]) -> Ca } { - let key = unit.pkg + let key = unit + .pkg .manifest() .links() .map(|l| (l.to_string(), unit.kind)); diff --git a/src/cargo/core/compiler/fingerprint.rs b/src/cargo/core/compiler/fingerprint.rs index eace5217c91..cc371662c04 100644 --- a/src/cargo/core/compiler/fingerprint.rs +++ b/src/cargo/core/compiler/fingerprint.rs @@ -474,7 +474,7 @@ fn calculate<'a, 'cfg>( profile: profile_hash, // Note that .0 is hashed here, not .1 which is the cwd. That doesn't // actually affect the output artifact so there's no need to hash it. - path: util::hash_u64(&super::path_args(&cx.bcx, unit).0), + path: util::hash_u64(&super::path_args(cx, unit).0), features: format!("{:?}", bcx.resolve.features_sorted(unit.pkg.package_id())), deps, local: vec![local], diff --git a/src/cargo/core/compiler/layout.rs b/src/cargo/core/compiler/layout.rs index d7a3e441304..27532fccad2 100644 --- a/src/cargo/core/compiler/layout.rs +++ b/src/cargo/core/compiler/layout.rs @@ -64,6 +64,7 @@ pub struct Layout { deps: PathBuf, native: PathBuf, build: PathBuf, + metabuild: PathBuf, incremental: PathBuf, fingerprint: PathBuf, examples: PathBuf, @@ -112,6 +113,7 @@ impl Layout { deps: root.join("deps"), native: root.join("native"), build: root.join("build"), + metabuild: root.join(".metabuild"), incremental: root.join("incremental"), fingerprint: root.join(".fingerprint"), examples: root.join("examples"), @@ -163,6 +165,7 @@ impl Layout { mkdir(&self.fingerprint)?; mkdir(&self.examples)?; mkdir(&self.build)?; + mkdir(&self.metabuild)?; return Ok(()); @@ -202,4 +205,8 @@ impl Layout { pub fn build(&self) -> &Path { &self.build } + /// Fetch the metabuild path. + pub fn metabuild(&self) -> &Path { + &self.metabuild + } } diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index d417ebb417d..559be6c4399 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -582,7 +582,7 @@ fn rustdoc<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoResult let mut rustdoc = cx.compilation.rustdoc_process(unit.pkg, unit.target)?; rustdoc.inherit_jobserver(&cx.jobserver); rustdoc.arg("--crate-name").arg(&unit.target.crate_name()); - add_path_args(bcx, unit, &mut rustdoc); + add_path_args(cx, unit, &mut rustdoc); add_cap_lints(bcx, unit, &mut rustdoc); add_color(bcx, &mut rustdoc); @@ -666,18 +666,23 @@ fn rustdoc<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoResult // // The first returned value here is the argument to pass to rustc, and the // second is the cwd that rustc should operate in. -fn path_args(bcx: &BuildContext, unit: &Unit) -> (PathBuf, PathBuf) { - let ws_root = bcx.ws.root(); - let src = unit.target.src_path(); +fn path_args<'a, 'cfg>(cx: &Context<'a, 'cfg>, unit: &Unit) -> (PathBuf, PathBuf) { + let ws_root = cx.bcx.ws.root(); + // TODO: this is a hack + let src = if unit.target.is_custom_build() && unit.pkg.manifest().metabuild().is_some() { + cx.files().metabuild_path(unit) + } else { + unit.target.src_path().to_path_buf() + }; assert!(src.is_absolute()); - match src.strip_prefix(ws_root) { - Ok(path) => (path.to_path_buf(), ws_root.to_path_buf()), - Err(_) => (src.to_path_buf(), unit.pkg.root().to_path_buf()), + if let Ok(path) = src.strip_prefix(ws_root) { + return (path.to_path_buf(), ws_root.to_path_buf()); } + (src, unit.pkg.root().to_path_buf()) } -fn add_path_args(bcx: &BuildContext, unit: &Unit, cmd: &mut ProcessBuilder) { - let (arg, cwd) = path_args(bcx, unit); +fn add_path_args<'a, 'cfg>(cx: &Context<'a, 'cfg>, unit: &Unit, cmd: &mut ProcessBuilder) { + let (arg, cwd) = path_args(cx, unit); cmd.arg(arg); cmd.cwd(cwd); } @@ -736,7 +741,7 @@ fn build_base_args<'a, 'cfg>( cmd.arg("--crate-name").arg(&unit.target.crate_name()); - add_path_args(bcx, unit, cmd); + add_path_args(cx, unit, cmd); add_color(bcx, cmd); add_error_format(bcx, cmd); diff --git a/src/cargo/core/features.rs b/src/cargo/core/features.rs index d18aad6a196..ead3e842312 100644 --- a/src/cargo/core/features.rs +++ b/src/cargo/core/features.rs @@ -189,6 +189,9 @@ features! { // "default-run" manifest option, [unstable] default_run: bool, + + // Declarative build scripts. + [unstable] metabuild: bool, } } diff --git a/src/cargo/core/manifest.rs b/src/cargo/core/manifest.rs index 4ae39baee94..7a7f4e5fdd5 100644 --- a/src/cargo/core/manifest.rs +++ b/src/cargo/core/manifest.rs @@ -44,6 +44,7 @@ pub struct Manifest { edition: Edition, im_a_teapot: Option, default_run: Option, + metabuild: Option>, } /// When parsing `Cargo.toml`, some warnings should silenced @@ -321,6 +322,7 @@ impl Manifest { im_a_teapot: Option, default_run: Option, original: Rc, + metabuild: Option>, ) -> Manifest { Manifest { summary, @@ -342,6 +344,7 @@ impl Manifest { im_a_teapot, default_run, publish_lockfile, + metabuild, } } @@ -464,6 +467,10 @@ impl Manifest { pub fn default_run(&self) -> Option<&str> { self.default_run.as_ref().map(|s| &s[..]) } + + pub fn metabuild(&self) -> Option<&Vec> { + self.metabuild.as_ref() + } } impl VirtualManifest { @@ -734,7 +741,10 @@ impl Target { self.kind == TargetKind::Bench } pub fn is_custom_build(&self) -> bool { - self.kind == TargetKind::CustomBuild + match self.kind { + TargetKind::CustomBuild => true, + _ => false, + } } /// Returns the arguments suitable for `--crate-type` to pass to rustc. diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 426fa2b0985..865056d8cca 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -487,6 +487,44 @@ impl TomlProfile { } } +#[derive(Clone, Debug, Serialize, Eq, PartialEq)] +pub struct StringOrVec(Vec); + +impl<'de> de::Deserialize<'de> for StringOrVec { + fn deserialize(deserializer: D) -> Result + where + D: de::Deserializer<'de>, + { + struct Visitor; + + impl<'de> de::Visitor<'de> for Visitor { + type Value = StringOrVec; + + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + formatter.write_str("string or list of strings") + } + + fn visit_str(self, s: &str) -> Result + where + E: de::Error, + { + Ok(StringOrVec(vec![s.to_string()])) + } + + fn visit_seq(self, v: V) -> Result + where + V: de::SeqAccess<'de>, + { + let seq = de::value::SeqAccessDeserializer::new(v); + Vec::deserialize(seq).map(StringOrVec) + + } + } + + deserializer.deserialize_any(Visitor) + } +} + #[derive(Clone, Debug, Serialize, Eq, PartialEq)] #[serde(untagged)] pub enum StringOrBool { @@ -581,6 +619,7 @@ pub struct TomlProject { version: semver::Version, authors: Option>, build: Option, + metabuild: Option, links: Option, exclude: Option>, include: Option>, @@ -784,6 +823,10 @@ impl TomlManifest { Edition::Edition2015 }; + if project.metabuild.is_some() { + features.require(Feature::metabuild())?; + } + // If we have no lib at all, use the inferred lib if available // If we have a lib with a path, we're done // If we have a lib with no path, use the inferred lib or_else package name @@ -794,6 +837,7 @@ impl TomlManifest { package_root, edition, &project.build, + &project.metabuild, &mut warnings, &mut errors, )?; @@ -984,6 +1028,7 @@ impl TomlManifest { project.im_a_teapot, project.default_run.clone(), Rc::clone(me), + project.metabuild.clone().map(|sov| sov.0), ); if project.license_file.is_some() && project.license.is_some() { manifest.warnings_mut().add_warning( diff --git a/src/cargo/util/toml/targets.rs b/src/cargo/util/toml/targets.rs index 3fcc4f44a01..786fca78717 100644 --- a/src/cargo/util/toml/targets.rs +++ b/src/cargo/util/toml/targets.rs @@ -16,8 +16,10 @@ use std::collections::HashSet; use core::{compiler, Edition, Feature, Features, Target}; use util::errors::{CargoResult, CargoResultExt}; -use super::{LibKind, PathValue, StringOrBool, TomlBenchTarget, TomlBinTarget, TomlExampleTarget, - TomlLibTarget, TomlManifest, TomlTarget, TomlTestTarget}; +use super::{ + LibKind, PathValue, StringOrBool, StringOrVec, TomlBenchTarget, TomlBinTarget, + TomlExampleTarget, TomlLibTarget, TomlManifest, TomlTarget, TomlTestTarget, +}; pub fn targets( features: &Features, @@ -26,6 +28,7 @@ pub fn targets( package_root: &Path, edition: Edition, custom_build: &Option, + metabuild: &Option, warnings: &mut Vec, errors: &mut Vec, ) -> CargoResult> { @@ -97,6 +100,9 @@ pub fn targets( // processing the custom build script if let Some(custom_build) = manifest.maybe_custom_build(custom_build, package_root) { + if metabuild.is_some() { + bail!("cannot specify both `metabuild` and `build`"); + } let name = format!( "build-script-{}", custom_build @@ -110,6 +116,24 @@ pub fn targets( edition, )); } + if let Some(metabuild) = metabuild { + // Verify names match available build deps. + let bdeps = manifest.build_dependencies.as_ref(); + for name in &metabuild.0 { + if !bdeps.map_or(false, |bd| bd.contains_key(name)) { + bail!( + "metabuild package `{}` must be specified in `build-dependencies`", + name + ); + } + } + + targets.push(Target::custom_build_target( + &package.name, + // TODO: Fix this. + package_root.join("metabuild.rs"), + )); + } Ok(targets) } @@ -504,7 +528,14 @@ fn clean_targets_with_legacy_path( validate_unique_names(&toml_targets, target_kind)?; let mut result = Vec::new(); for target in toml_targets { - let path = target_path(&target, inferred, target_kind, package_root, edition, legacy_path); + let path = target_path( + &target, + inferred, + target_kind, + package_root, + edition, + legacy_path, + ); let path = match path { Ok(path) => path, Err(e) => { diff --git a/src/doc/src/reference/unstable.md b/src/doc/src/reference/unstable.md index 28721ce522e..31bde2e9237 100644 --- a/src/doc/src/reference/unstable.md +++ b/src/doc/src/reference/unstable.md @@ -345,3 +345,36 @@ both `src/bin/a.rs` and `src/bin/b.rs`: [project] default-run = "a" ``` + +### Metabuild +* Tracking Issue: [rust-lang/rust#49803](https://github.com/rust-lang/rust/issues/49803) +* RFC: [#2196](https://github.com/rust-lang/rfcs/blob/master/text/2196-metabuild.md) + +Metabuild is a feature to have declarative build scripts. Instead of writing +a `build.rs` script, you specify a list of build dependencies in the +`metabuild` key in `Cargo.toml`. A build script is automatically generated +that runs each build dependency in order. Metabuild packages can then read +metadata from `Cargo.toml` to specify their behavior. + +Include `cargo-features` at the top of `Cargo.toml`, a `metadata` key in the +`package`, list the dependencies in `build-dependencies`, and add any metadata +that the metabuild packages require. Example: + +```toml +cargo-features = ["metabuild"] + +[package] +name = "mypackage" +version = "0.0.1" +metabuild = ["foo", "bar"] + +[build-dependencies] +foo = "1.0" +bar = "1.0" + +[package.metadata.foo] +extra-info = "qwerty" +``` + +Metabuild packages should have a public function called `metabuild` that +performs the same actions as a regular `build.rs` script would perform. diff --git a/tests/testsuite/main.rs b/tests/testsuite/main.rs index 7ec164ef00c..99fa5e641f7 100644 --- a/tests/testsuite/main.rs +++ b/tests/testsuite/main.rs @@ -62,6 +62,7 @@ mod jobserver; mod local_registry; mod lockfile_compat; mod login; +mod metabuild; mod metadata; mod net_config; mod new; diff --git a/tests/testsuite/metabuild.rs b/tests/testsuite/metabuild.rs new file mode 100644 index 00000000000..407dbaa6d23 --- /dev/null +++ b/tests/testsuite/metabuild.rs @@ -0,0 +1,414 @@ +use support::{basic_lib_manifest, execs, project}; +use support::{rustc_host, ChannelChanger}; +use support::hamcrest::assert_that; + +#[test] +fn metabuild_gated() { + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + metabuild = ["mb"] + "#, + ) + .file("src/lib.rs", "") + .build(); + + assert_that( + p.cargo("build").masquerade_as_nightly_cargo(), + execs().with_status(101).with_stderr_contains( + "\ +error: failed to parse manifest at `[..]` + +Caused by: + feature `metabuild` is required + +consider adding `cargo-features = [\"metabuild\"]` to the manifest +", + ), + ); +} + +#[test] +fn metabuild_basic() { + let p = project() + .file( + "Cargo.toml", + r#" + cargo-features = ["metabuild"] + [package] + name = "foo" + version = "0.0.1" + metabuild = ["mb", "mb-other"] + + [build-dependencies] + mb = {path="mb"} + mb-other = {path="mb-other"} + "#, + ) + .file("src/lib.rs", "") + .file("mb/Cargo.toml", &basic_lib_manifest("mb")) + .file( + "mb/src/lib.rs", + r#"pub fn metabuild() { println!("Hello mb"); }"#, + ) + .file( + "mb-other/Cargo.toml", + r#" + [package] + name = "mb-other" + version = "0.0.1" + "#, + ) + .file( + "mb-other/src/lib.rs", + r#"pub fn metabuild() { println!("Hello mb-other"); }"#, + ) + .build(); + + assert_that( + p.cargo("build -vv").masquerade_as_nightly_cargo(), + execs() + .with_status(0) + .with_stdout_contains("Hello mb") + .with_stdout_contains("Hello mb-other"), + ); +} + +#[test] +fn metabuild_error_both() { + let p = project() + .file( + "Cargo.toml", + r#" + cargo-features = ["metabuild"] + [package] + name = "foo" + version = "0.0.1" + metabuild = "mb" + + [build-dependencies] + mb = {path="mb"} + "#, + ) + .file("src/lib.rs", "") + .file("build.rs", r#"fn main() {}"#) + .file("mb/Cargo.toml", &basic_lib_manifest("mb")) + .file( + "mb/src/lib.rs", + r#"pub fn metabuild() { println!("Hello mb"); }"#, + ) + .build(); + + assert_that( + p.cargo("build -vv").masquerade_as_nightly_cargo(), + execs().with_status(101).with_stderr_contains( + "\ +error: failed to parse manifest at [..] + +Caused by: + cannot specify both `metabuild` and `build` +", + ), + ); +} + +#[test] +fn metabuild_missing_dep() { + let p = project() + .file( + "Cargo.toml", + r#" + cargo-features = ["metabuild"] + [package] + name = "foo" + version = "0.0.1" + metabuild = "mb" + "#, + ) + .file("src/lib.rs", "") + .build(); + + assert_that( + p.cargo("build -vv").masquerade_as_nightly_cargo(), + execs().with_status(101).with_stderr_contains( + "\ +error: failed to parse manifest at [..] + +Caused by: + metabuild package `mb` must be specified in `build-dependencies`", + ), + ); +} + +#[test] +fn metabuild_optional_dep() { + let p = project() + .file( + "Cargo.toml", + r#" + cargo-features = ["metabuild"] + [package] + name = "foo" + version = "0.0.1" + metabuild = "mb" + + [build-dependencies] + mb = {path="mb", optional=true} + "#, + ) + .file("src/lib.rs", "") + .file("mb/Cargo.toml", &basic_lib_manifest("mb")) + .file( + "mb/src/lib.rs", + r#"pub fn metabuild() { println!("Hello mb"); }"#, + ) + .build(); + + assert_that( + p.cargo("build -vv").masquerade_as_nightly_cargo(), + execs() + .with_status(0) + .with_stdout_does_not_contain("Hello mb"), + ); + + assert_that( + p.cargo("build -vv --features mb") + .masquerade_as_nightly_cargo(), + execs().with_status(0).with_stdout_contains("Hello mb"), + ); +} + +#[test] +fn metabuild_lib_name() { + // Test when setting `name` on [lib]. + let p = project() + .file( + "Cargo.toml", + r#" + cargo-features = ["metabuild"] + [package] + name = "foo" + version = "0.0.1" + metabuild = "mb" + + [build-dependencies] + mb = {path="mb"} + "#, + ) + .file("src/lib.rs", "") + .file( + "mb/Cargo.toml", + r#" + [package] + name = "mb" + version = "0.0.1" + [lib] + name = "other" + "#, + ) + .file( + "mb/src/lib.rs", + r#"pub fn metabuild() { println!("Hello mb"); }"#, + ) + .build(); + + assert_that( + p.cargo("build -vv").masquerade_as_nightly_cargo(), + execs().with_status(0).with_stdout_contains("Hello mb"), + ); +} + +#[test] +fn metabuild_fresh() { + // Check that rebuild is fresh. + let p = project() + .file( + "Cargo.toml", + r#" + cargo-features = ["metabuild"] + [package] + name = "foo" + version = "0.0.1" + metabuild = "mb" + + [build-dependencies] + mb = {path="mb"} + "#, + ) + .file("src/lib.rs", "") + .file("mb/Cargo.toml", &basic_lib_manifest("mb")) + .file( + "mb/src/lib.rs", + r#"pub fn metabuild() { println!("Hello mb"); }"#, + ) + .build(); + + assert_that( + p.cargo("build -vv").masquerade_as_nightly_cargo(), + execs().with_status(0).with_stdout_contains("Hello mb"), + ); + + assert_that( + p.cargo("build -vv").masquerade_as_nightly_cargo(), + execs() + .with_status(0) + .with_stdout_does_not_contain("Hello mb") + .with_stderr( + "\ +[FRESH] mb [..] +[FRESH] foo [..] +[FINISHED] dev [..] +", + ), + ); +} + +#[test] +fn metabuild_links() { + let p = project() + .file( + "Cargo.toml", + r#" + cargo-features = ["metabuild"] + [package] + name = "foo" + version = "0.0.1" + links = "cat" + metabuild = "mb" + + [build-dependencies] + mb = {path="mb"} + "#, + ) + .file("src/lib.rs", "") + .file("mb/Cargo.toml", &basic_lib_manifest("mb")) + .file( + "mb/src/lib.rs", + r#"pub fn metabuild() { + assert_eq!(std::env::var("CARGO_MANIFEST_LINKS"), + Ok("cat".to_string())); + println!("Hello mb"); + }"#, + ) + .build(); + + assert_that( + p.cargo("build -vv").masquerade_as_nightly_cargo(), + execs().with_status(0).with_stdout_contains("Hello mb"), + ); +} + +#[test] +fn metabuild_override() { + let p = project() + .file( + "Cargo.toml", + r#" + cargo-features = ["metabuild"] + [package] + name = "foo" + version = "0.0.1" + links = "cat" + metabuild = "mb" + + [build-dependencies] + mb = {path="mb"} + "#, + ) + .file("src/lib.rs", "") + .file("mb/Cargo.toml", &basic_lib_manifest("mb")) + .file( + "mb/src/lib.rs", + r#"pub fn metabuild() { panic!("should not run"); }"#, + ) + .file( + ".cargo/config", + &format!( + r#" + [target.{}.cat] + rustc-link-lib = ["a"] + "#, + rustc_host() + ), + ) + .build(); + + assert_that( + p.cargo("build -vv").masquerade_as_nightly_cargo(), + execs().with_status(0), + ); +} + +#[test] +fn metabuild_workspace() { + let p = project() + .file( + "Cargo.toml", + r#" + [workspace] + members = ["member1", "member2"] + "#, + ) + .file( + "member1/Cargo.toml", + r#" + cargo-features = ["metabuild"] + [package] + name = "member1" + version = "0.0.1" + metabuild = ["mb1", "mb2"] + + [build-dependencies] + mb1 = {path="../../mb1"} + mb2 = {path="../../mb2"} + "#, + ) + .file("member1/src/lib.rs", "") + .file( + "member2/Cargo.toml", + r#" + cargo-features = ["metabuild"] + [package] + name = "member2" + version = "0.0.1" + metabuild = ["mb1"] + + [build-dependencies] + mb1 = {path="../../mb1"} + "#, + ) + .file("member2/src/lib.rs", "") + .build(); + + project() + .at("mb1") + .file("Cargo.toml", &basic_lib_manifest("mb1")) + .file( + "src/lib.rs", + r#"pub fn metabuild() { println!("Hello mb1 {}", std::env::var("CARGO_MANIFEST_DIR").unwrap()); }"#, + ) + .build(); + + project() + .at("mb2") + .file("Cargo.toml", &basic_lib_manifest("mb2")) + .file( + "src/lib.rs", + r#"pub fn metabuild() { println!("Hello mb2 {}", std::env::var("CARGO_MANIFEST_DIR").unwrap()); }"#, + ) + .build(); + + assert_that( + p.cargo("build -vv --all").masquerade_as_nightly_cargo(), + execs() + .with_status(0) + .with_stdout_contains("Hello mb1 [..]member1") + .with_stdout_contains("Hello mb2 [..]member1") + .with_stdout_contains("Hello mb1 [..]member2") + .with_stdout_does_not_contain("Hello mb2 [..]member2"), + ); +} From ae5e84518da5cb08f9eef9ab100abc7342dea097 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Tue, 12 Jun 2018 14:23:43 -0700 Subject: [PATCH 2/4] Address review comments. - Add newline at end of file. - Remove unnecessary rustfmt changes. - Move file writing code to `write_if_changed`. --- src/cargo/core/compiler/custom_build.rs | 14 +++----------- src/cargo/util/paths.rs | 20 ++++++++++++++++++++ src/cargo/util/toml/targets.rs | 9 +-------- 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/src/cargo/core/compiler/custom_build.rs b/src/cargo/core/compiler/custom_build.rs index 43b11744f62..78ec8e6e3b1 100644 --- a/src/cargo/core/compiler/custom_build.rs +++ b/src/cargo/core/compiler/custom_build.rs @@ -560,17 +560,10 @@ fn prepare_metabuild<'a, 'cfg>( for dep in &meta_deps { output.push(format!(" {}::metabuild();\n", dep)); } - output.push("}".to_string()); + output.push("}\n".to_string()); let output = output.join(""); let path = cx.files().metabuild_path(unit); - let changed = if let Ok(orig) = fs::read_to_string(&path) { - orig != output - } else { - true - }; - if changed { - fs::write(&path, output)?; - } + paths::write_if_changed(path, &output)?; Ok(()) } @@ -622,8 +615,7 @@ pub fn build_map<'b, 'cfg>(cx: &mut Context<'b, 'cfg>, units: &[Unit<'b>]) -> Ca } { - let key = unit - .pkg + let key = unit.pkg .manifest() .links() .map(|l| (l.to_string(), unit.kind)); diff --git a/src/cargo/util/paths.rs b/src/cargo/util/paths.rs index e97bcf9903d..988d02eef26 100644 --- a/src/cargo/util/paths.rs +++ b/src/cargo/util/paths.rs @@ -142,6 +142,26 @@ pub fn write(path: &Path, contents: &[u8]) -> CargoResult<()> { Ok(()) } +pub fn write_if_changed, C: AsRef<[u8]>>(path: P, contents: C) -> CargoResult<()> { + (|| -> CargoResult<()> { + let contents = contents.as_ref(); + let mut f = OpenOptions::new() + .read(true) + .write(true) + .create(true) + .open(&path)?; + let mut orig = Vec::new(); + f.read_to_end(&mut orig)?; + if orig != contents { + f.set_len(0)?; + f.write_all(contents)?; + } + Ok(()) + })() + .chain_err(|| format!("failed to write `{}`", path.as_ref().display()))?; + Ok(()) +} + pub fn append(path: &Path, contents: &[u8]) -> CargoResult<()> { (|| -> CargoResult<()> { let mut f = OpenOptions::new() diff --git a/src/cargo/util/toml/targets.rs b/src/cargo/util/toml/targets.rs index 786fca78717..18cac61c665 100644 --- a/src/cargo/util/toml/targets.rs +++ b/src/cargo/util/toml/targets.rs @@ -528,14 +528,7 @@ fn clean_targets_with_legacy_path( validate_unique_names(&toml_targets, target_kind)?; let mut result = Vec::new(); for target in toml_targets { - let path = target_path( - &target, - inferred, - target_kind, - package_root, - edition, - legacy_path, - ); + let path = target_path(&target, inferred, target_kind, package_root, edition, legacy_path); let path = match path { Ok(path) => path, Err(e) => { From e685bbfe52cbb2e6f75ca8302e73b85661617ac1 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Tue, 12 Jun 2018 15:24:11 -0700 Subject: [PATCH 3/4] Remove unnecessary change. --- src/cargo/core/manifest.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/cargo/core/manifest.rs b/src/cargo/core/manifest.rs index 7a7f4e5fdd5..cde6b653315 100644 --- a/src/cargo/core/manifest.rs +++ b/src/cargo/core/manifest.rs @@ -741,10 +741,7 @@ impl Target { self.kind == TargetKind::Bench } pub fn is_custom_build(&self) -> bool { - match self.kind { - TargetKind::CustomBuild => true, - _ => false, - } + self.kind == TargetKind::CustomBuild } /// Returns the arguments suitable for `--crate-type` to pass to rustc. From ecc87b1795ddb181fede9a2fb6813e47e227c0f8 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Thu, 23 Aug 2018 21:49:43 -0700 Subject: [PATCH 4/4] New metabuild strategy using custom src_path enum. - Use new enum `TargertSourcePath` for Target::src_path to make it explicit that metabuild has a special path. - `cargo metadata` now skips the metabuild Target. - JSON artifacts include the true path to the metabuild source file. This may not be the best solution, but it's unclear what it should be, and I would prefer to avoid breaking the output. Alternatively it could just not emit anything? I'm not completely familiar with the use case of these artifact messages. - Place the file in `target/.metabuild/metabuild-pkgname-HASH.rs` instead of in the debug/release directory. Its contents do not depend on the profile. - Fix bug in write_if_changed. - More tests. --- .../compiler/context/compilation_files.rs | 9 - src/cargo/core/compiler/custom_build.rs | 3 +- src/cargo/core/compiler/fingerprint.rs | 2 +- src/cargo/core/compiler/layout.rs | 7 - src/cargo/core/compiler/mod.rs | 25 +- src/cargo/core/manifest.rs | 133 ++++-- src/cargo/core/package.rs | 16 +- src/cargo/ops/cargo_test.rs | 2 +- src/cargo/util/paths.rs | 1 + src/cargo/util/toml/mod.rs | 11 +- src/cargo/util/toml/targets.rs | 9 +- tests/testsuite/metabuild.rs | 381 +++++++++++++++--- tests/testsuite/support/registry.rs | 4 + 13 files changed, 463 insertions(+), 140 deletions(-) diff --git a/src/cargo/core/compiler/context/compilation_files.rs b/src/cargo/core/compiler/context/compilation_files.rs index 76138db5ca8..5359ac751e8 100644 --- a/src/cargo/core/compiler/context/compilation_files.rs +++ b/src/cargo/core/compiler/context/compilation_files.rs @@ -161,15 +161,6 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> { self.layout(unit.kind).build().join(dir).join("out") } - pub fn metabuild_path(&self, unit: &Unit<'a>) -> PathBuf { - let metadata = self.metadata(unit).expect("metabuild metadata"); - self.layout(unit.kind).metabuild().join(format!( - "metabuild-{}-{}.rs", - unit.pkg.name(), - metadata - )) - } - /// Returns the file stem for a given target/profile combo (with metadata) pub fn file_stem(&self, unit: &Unit<'a>) -> String { match self.metas[unit] { diff --git a/src/cargo/core/compiler/custom_build.rs b/src/cargo/core/compiler/custom_build.rs index 78ec8e6e3b1..58bdd7c76a4 100644 --- a/src/cargo/core/compiler/custom_build.rs +++ b/src/cargo/core/compiler/custom_build.rs @@ -562,7 +562,8 @@ fn prepare_metabuild<'a, 'cfg>( } output.push("}\n".to_string()); let output = output.join(""); - let path = cx.files().metabuild_path(unit); + let path = unit.pkg.manifest().metabuild_path(cx.bcx.ws.target_dir()); + fs::create_dir_all(path.parent().unwrap())?; paths::write_if_changed(path, &output)?; Ok(()) } diff --git a/src/cargo/core/compiler/fingerprint.rs b/src/cargo/core/compiler/fingerprint.rs index cc371662c04..eace5217c91 100644 --- a/src/cargo/core/compiler/fingerprint.rs +++ b/src/cargo/core/compiler/fingerprint.rs @@ -474,7 +474,7 @@ fn calculate<'a, 'cfg>( profile: profile_hash, // Note that .0 is hashed here, not .1 which is the cwd. That doesn't // actually affect the output artifact so there's no need to hash it. - path: util::hash_u64(&super::path_args(cx, unit).0), + path: util::hash_u64(&super::path_args(&cx.bcx, unit).0), features: format!("{:?}", bcx.resolve.features_sorted(unit.pkg.package_id())), deps, local: vec![local], diff --git a/src/cargo/core/compiler/layout.rs b/src/cargo/core/compiler/layout.rs index 27532fccad2..d7a3e441304 100644 --- a/src/cargo/core/compiler/layout.rs +++ b/src/cargo/core/compiler/layout.rs @@ -64,7 +64,6 @@ pub struct Layout { deps: PathBuf, native: PathBuf, build: PathBuf, - metabuild: PathBuf, incremental: PathBuf, fingerprint: PathBuf, examples: PathBuf, @@ -113,7 +112,6 @@ impl Layout { deps: root.join("deps"), native: root.join("native"), build: root.join("build"), - metabuild: root.join(".metabuild"), incremental: root.join("incremental"), fingerprint: root.join(".fingerprint"), examples: root.join("examples"), @@ -165,7 +163,6 @@ impl Layout { mkdir(&self.fingerprint)?; mkdir(&self.examples)?; mkdir(&self.build)?; - mkdir(&self.metabuild)?; return Ok(()); @@ -205,8 +202,4 @@ impl Layout { pub fn build(&self) -> &Path { &self.build } - /// Fetch the metabuild path. - pub fn metabuild(&self) -> &Path { - &self.metabuild - } } diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 559be6c4399..a3106e93144 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -8,6 +8,7 @@ use std::sync::Arc; use same_file::is_same_file; use serde_json; +use core::manifest::TargetSourcePath; use core::profiles::{Lto, Profile}; use core::shell::ColorChoice; use core::{PackageId, Target}; @@ -390,7 +391,6 @@ fn link_targets<'a, 'cfg>( let outputs = cx.outputs(unit)?; let export_dir = cx.files().export_dir(); let package_id = unit.pkg.package_id().clone(); - let target = unit.target.clone(); let profile = unit.profile; let unit_mode = unit.mode; let features = bcx.resolve @@ -399,6 +399,12 @@ fn link_targets<'a, 'cfg>( .map(|s| s.to_owned()) .collect(); let json_messages = bcx.build_config.json_messages(); + let mut target = unit.target.clone(); + if let TargetSourcePath::Metabuild = target.src_path() { + // Give it something to serialize. + let path = unit.pkg.manifest().metabuild_path(cx.bcx.ws.target_dir()); + target.set_src_path(TargetSourcePath::Path(path)); + } Ok(Work::new(move |_| { // If we're a "root crate", e.g. the target of this compilation, then we @@ -582,7 +588,7 @@ fn rustdoc<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoResult let mut rustdoc = cx.compilation.rustdoc_process(unit.pkg, unit.target)?; rustdoc.inherit_jobserver(&cx.jobserver); rustdoc.arg("--crate-name").arg(&unit.target.crate_name()); - add_path_args(cx, unit, &mut rustdoc); + add_path_args(bcx, unit, &mut rustdoc); add_cap_lints(bcx, unit, &mut rustdoc); add_color(bcx, &mut rustdoc); @@ -666,13 +672,12 @@ fn rustdoc<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoResult // // The first returned value here is the argument to pass to rustc, and the // second is the cwd that rustc should operate in. -fn path_args<'a, 'cfg>(cx: &Context<'a, 'cfg>, unit: &Unit) -> (PathBuf, PathBuf) { - let ws_root = cx.bcx.ws.root(); - // TODO: this is a hack +fn path_args(bcx: &BuildContext, unit: &Unit) -> (PathBuf, PathBuf) { + let ws_root = bcx.ws.root(); let src = if unit.target.is_custom_build() && unit.pkg.manifest().metabuild().is_some() { - cx.files().metabuild_path(unit) + unit.pkg.manifest().metabuild_path(bcx.ws.target_dir()) } else { - unit.target.src_path().to_path_buf() + unit.target.src_path().path().to_path_buf() }; assert!(src.is_absolute()); if let Ok(path) = src.strip_prefix(ws_root) { @@ -681,8 +686,8 @@ fn path_args<'a, 'cfg>(cx: &Context<'a, 'cfg>, unit: &Unit) -> (PathBuf, PathBuf (src, unit.pkg.root().to_path_buf()) } -fn add_path_args<'a, 'cfg>(cx: &Context<'a, 'cfg>, unit: &Unit, cmd: &mut ProcessBuilder) { - let (arg, cwd) = path_args(cx, unit); +fn add_path_args(bcx: &BuildContext, unit: &Unit, cmd: &mut ProcessBuilder) { + let (arg, cwd) = path_args(bcx, unit); cmd.arg(arg); cmd.cwd(cwd); } @@ -741,7 +746,7 @@ fn build_base_args<'a, 'cfg>( cmd.arg("--crate-name").arg(&unit.target.crate_name()); - add_path_args(cx, unit, cmd); + add_path_args(bcx, unit, cmd); add_color(bcx, cmd); add_error_format(bcx, cmd); diff --git a/src/cargo/core/manifest.rs b/src/cargo/core/manifest.rs index cde6b653315..88ede658c3e 100644 --- a/src/cargo/core/manifest.rs +++ b/src/cargo/core/manifest.rs @@ -1,6 +1,8 @@ +#![allow(deprecated)] // for SipHasher + use std::collections::{BTreeMap, HashMap}; use std::fmt; -use std::hash::{Hash, Hasher}; +use std::hash::{Hash, Hasher, SipHasher}; use std::path::{Path, PathBuf}; use std::rc::Rc; @@ -15,7 +17,7 @@ use core::{Dependency, PackageId, PackageIdSpec, SourceId, Summary}; use core::{Edition, Feature, Features, WorkspaceConfig}; use util::errors::*; use util::toml::TomlManifest; -use util::Config; +use util::{Config, Filesystem}; pub enum EitherManifest { Real(Manifest), @@ -191,7 +193,7 @@ pub struct Target { // as it's absolute currently and is otherwise a little too brittle for // causing rebuilds. Instead the hash for the path that we send to the // compiler is handled elsewhere. - src_path: NonHashedPathBuf, + src_path: TargetSourcePath, required_features: Option>, tested: bool, benched: bool, @@ -203,19 +205,50 @@ pub struct Target { } #[derive(Clone, PartialEq, Eq)] -struct NonHashedPathBuf { - path: PathBuf, +pub enum TargetSourcePath { + Path(PathBuf), + Metabuild, +} + +impl TargetSourcePath { + pub fn path(&self) -> &Path { + match self { + TargetSourcePath::Path(path) => path.as_ref(), + TargetSourcePath::Metabuild => panic!("metabuild not expected"), + } + } + + pub fn is_path(&self) -> bool { + match self { + TargetSourcePath::Path(_) => true, + _ => false, + } + } } -impl Hash for NonHashedPathBuf { +impl Hash for TargetSourcePath { fn hash(&self, _: &mut H) { // ... } } -impl fmt::Debug for NonHashedPathBuf { +impl fmt::Debug for TargetSourcePath { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - self.path.fmt(f) + match self { + TargetSourcePath::Path(path) => path.fmt(f), + TargetSourcePath::Metabuild => "metabuild".fmt(f), + } + } +} + +impl From for TargetSourcePath { + fn from(path: PathBuf) -> Self { + assert!( + path.is_absolute(), + "`{}` is not absolute", + path.display() + ); + TargetSourcePath::Path(path) } } @@ -240,7 +273,7 @@ impl ser::Serialize for Target { kind: &self.kind, crate_types: self.rustc_crate_types(), name: &self.name, - src_path: &self.src_path.path, + src_path: &self.src_path.path().to_path_buf(), edition: &self.edition.to_string(), required_features: self .required_features @@ -254,34 +287,43 @@ compact_debug! { impl fmt::Debug for Target { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { let (default, default_name) = { - let src = self.src_path().to_path_buf(); match &self.kind { TargetKind::Lib(kinds) => { ( Target::lib_target( &self.name, kinds.clone(), - src.clone(), - Edition::Edition2015, + self.src_path().path().to_path_buf(), + self.edition, ), - format!("lib_target({:?}, {:?}, {:?})", - self.name, kinds, src), + format!("lib_target({:?}, {:?}, {:?}, {:?})", + self.name, kinds, self.src_path, self.edition), ) } TargetKind::CustomBuild => { - ( - Target::custom_build_target( - &self.name, - src.clone(), - Edition::Edition2015, - ), - format!("custom_build_target({:?}, {:?})", - self.name, src), - ) + match self.src_path { + TargetSourcePath::Path(ref path) => { + ( + Target::custom_build_target( + &self.name, + path.to_path_buf(), + self.edition, + ), + format!("custom_build_target({:?}, {:?}, {:?})", + self.name, path, self.edition), + ) + } + TargetSourcePath::Metabuild => { + ( + Target::metabuild_target(&self.name), + format!("metabuild_target({:?})", self.name), + ) + } + } } _ => ( - Target::with_path(src.clone(), Edition::Edition2015), - format!("with_path({:?})", src), + Target::new(self.src_path.clone(), self.edition), + format!("with_path({:?}, {:?})", self.src_path, self.edition), ), } }; @@ -471,6 +513,16 @@ impl Manifest { pub fn metabuild(&self) -> Option<&Vec> { self.metabuild.as_ref() } + + pub fn metabuild_path(&self, target_dir: Filesystem) -> PathBuf { + let mut hasher = SipHasher::new_with_keys(0, 0); + self.package_id().hash(&mut hasher); + let hash = hasher.finish(); + target_dir + .into_path_unlocked() + .join(".metabuild") + .join(format!("metabuild-{}-{:016x}.rs", self.name(), hash)) + } } impl VirtualManifest { @@ -515,16 +567,11 @@ impl VirtualManifest { } impl Target { - fn with_path(src_path: PathBuf, edition: Edition) -> Target { - assert!( - src_path.is_absolute(), - "`{}` is not absolute", - src_path.display() - ); + fn new(src_path: TargetSourcePath, edition: Edition) -> Target { Target { kind: TargetKind::Bin, name: String::new(), - src_path: NonHashedPathBuf { path: src_path }, + src_path, required_features: None, doc: false, doctest: false, @@ -536,6 +583,10 @@ impl Target { } } + fn with_path(src_path: PathBuf, edition: Edition) -> Target { + Target::new(TargetSourcePath::from(src_path), edition) + } + pub fn lib_target( name: &str, crate_targets: Vec, @@ -582,6 +633,17 @@ impl Target { } } + pub fn metabuild_target(name: &str) -> Target { + Target { + kind: TargetKind::CustomBuild, + name: name.to_string(), + for_host: true, + benched: false, + tested: false, + ..Target::new(TargetSourcePath::Metabuild, Edition::Edition2015) + } + } + pub fn example_target( name: &str, crate_targets: Vec, @@ -641,8 +703,11 @@ impl Target { pub fn crate_name(&self) -> String { self.name.replace("-", "_") } - pub fn src_path(&self) -> &Path { - &self.src_path.path + pub fn src_path(&self) -> &TargetSourcePath { + &self.src_path + } + pub fn set_src_path(&mut self, src_path: TargetSourcePath) { + self.src_path = src_path; } pub fn required_features(&self) -> Option<&Vec> { self.required_features.as_ref() diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs index 201c5fe095a..09473864586 100644 --- a/src/cargo/core/package.rs +++ b/src/cargo/core/package.rs @@ -38,7 +38,7 @@ struct SerializedPackage<'a> { description: Option<&'a str>, source: &'a SourceId, dependencies: &'a [Dependency], - targets: &'a [Target], + targets: Vec<&'a Target>, features: &'a FeatureMap, manifest_path: &'a str, metadata: Option<&'a toml::Value>, @@ -48,6 +48,8 @@ struct SerializedPackage<'a> { readme: Option<&'a str>, repository: Option<&'a str>, edition: &'a str, + #[serde(skip_serializing_if = "Option::is_none")] + metabuild: Option<&'a Vec>, } impl ser::Serialize for Package { @@ -66,6 +68,15 @@ impl ser::Serialize for Package { let keywords = manmeta.keywords.as_ref(); let readme = manmeta.readme.as_ref().map(String::as_ref); let repository = manmeta.repository.as_ref().map(String::as_ref); + // Filter out metabuild targets. They are an internal implementation + // detail that is probably not relevant externally. There's also not a + // real path to show in `src_path`, and this avoids changing the format. + let targets: Vec<&Target> = self + .manifest + .targets() + .iter() + .filter(|t| t.src_path().is_path()) + .collect(); SerializedPackage { name: &*package_id.name(), @@ -76,7 +87,7 @@ impl ser::Serialize for Package { description, source: summary.source_id(), dependencies: summary.dependencies(), - targets: self.manifest.targets(), + targets, features: summary.features(), manifest_path: &self.manifest_path.display().to_string(), metadata: self.manifest.custom_metadata(), @@ -86,6 +97,7 @@ impl ser::Serialize for Package { readme, repository, edition: &self.manifest.edition().to_string(), + metabuild: self.manifest.metabuild(), }.serialize(s) } } diff --git a/src/cargo/ops/cargo_test.rs b/src/cargo/ops/cargo_test.rs index 163117d9bb7..f1db552506e 100644 --- a/src/cargo/ops/cargo_test.rs +++ b/src/cargo/ops/cargo_test.rs @@ -163,7 +163,7 @@ fn run_doc_tests( config.shell().status("Doc-tests", target.name())?; let mut p = compilation.rustdoc_process(package, target)?; p.arg("--test") - .arg(target.src_path()) + .arg(target.src_path().path()) .arg("--crate-name") .arg(&target.crate_name()); diff --git a/src/cargo/util/paths.rs b/src/cargo/util/paths.rs index 988d02eef26..bdc7ade548f 100644 --- a/src/cargo/util/paths.rs +++ b/src/cargo/util/paths.rs @@ -154,6 +154,7 @@ pub fn write_if_changed, C: AsRef<[u8]>>(path: P, contents: C) -> f.read_to_end(&mut orig)?; if orig != contents { f.set_len(0)?; + f.seek(io::SeekFrom::Start(0))?; f.write_all(contents)?; } Ok(()) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 865056d8cca..9d83668569a 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -13,7 +13,7 @@ use toml; use url::Url; use core::dependency::{Kind, Platform}; -use core::manifest::{LibKind, ManifestMetadata, Warnings}; +use core::manifest::{LibKind, ManifestMetadata, TargetSourcePath, Warnings}; use core::profiles::Profiles; use core::{Dependency, Manifest, PackageId, Summary, Target}; use core::{Edition, EitherManifest, Feature, Features, VirtualManifest}; @@ -1208,9 +1208,12 @@ impl TomlManifest { /// If not, the name of the offending build target is returned. fn unique_build_targets(targets: &[Target], package_root: &Path) -> Result<(), String> { let mut seen = HashSet::new(); - for v in targets.iter().map(|e| package_root.join(e.src_path())) { - if !seen.insert(v.clone()) { - return Err(v.display().to_string()); + for target in targets { + if let TargetSourcePath::Path(path) = target.src_path() { + let full = package_root.join(path); + if !seen.insert(full.clone()) { + return Err(full.display().to_string()); + } } } Ok(()) diff --git a/src/cargo/util/toml/targets.rs b/src/cargo/util/toml/targets.rs index 18cac61c665..cc40b536ce3 100644 --- a/src/cargo/util/toml/targets.rs +++ b/src/cargo/util/toml/targets.rs @@ -128,11 +128,10 @@ pub fn targets( } } - targets.push(Target::custom_build_target( - &package.name, - // TODO: Fix this. - package_root.join("metabuild.rs"), - )); + targets.push(Target::metabuild_target(&format!( + "metabuild-{}", + package.name + ))); } Ok(targets) diff --git a/tests/testsuite/metabuild.rs b/tests/testsuite/metabuild.rs index 407dbaa6d23..4600687755d 100644 --- a/tests/testsuite/metabuild.rs +++ b/tests/testsuite/metabuild.rs @@ -1,6 +1,10 @@ -use support::{basic_lib_manifest, execs, project}; -use support::{rustc_host, ChannelChanger}; -use support::hamcrest::assert_that; +use glob::glob; +use serde_json; +use std::str; +use support::{ + basic_lib_manifest, basic_manifest, execs, hamcrest::assert_that, project, registry::Package, + rustc_host, ChannelChanger, Project, +}; #[test] fn metabuild_gated() { @@ -13,8 +17,7 @@ fn metabuild_gated() { version = "0.0.1" metabuild = ["mb"] "#, - ) - .file("src/lib.rs", "") + ).file("src/lib.rs", "") .build(); assert_that( @@ -32,9 +35,8 @@ consider adding `cargo-features = [\"metabuild\"]` to the manifest ); } -#[test] -fn metabuild_basic() { - let p = project() +fn basic_project() -> Project { + project() .file( "Cargo.toml", r#" @@ -48,31 +50,30 @@ fn metabuild_basic() { mb = {path="mb"} mb-other = {path="mb-other"} "#, - ) - .file("src/lib.rs", "") + ).file("src/lib.rs", "") .file("mb/Cargo.toml", &basic_lib_manifest("mb")) .file( "mb/src/lib.rs", r#"pub fn metabuild() { println!("Hello mb"); }"#, - ) - .file( + ).file( "mb-other/Cargo.toml", r#" [package] name = "mb-other" version = "0.0.1" "#, - ) - .file( + ).file( "mb-other/src/lib.rs", r#"pub fn metabuild() { println!("Hello mb-other"); }"#, - ) - .build(); + ).build() +} +#[test] +fn metabuild_basic() { + let p = basic_project(); assert_that( p.cargo("build -vv").masquerade_as_nightly_cargo(), execs() - .with_status(0) .with_stdout_contains("Hello mb") .with_stdout_contains("Hello mb-other"), ); @@ -93,15 +94,13 @@ fn metabuild_error_both() { [build-dependencies] mb = {path="mb"} "#, - ) - .file("src/lib.rs", "") + ).file("src/lib.rs", "") .file("build.rs", r#"fn main() {}"#) .file("mb/Cargo.toml", &basic_lib_manifest("mb")) .file( "mb/src/lib.rs", r#"pub fn metabuild() { println!("Hello mb"); }"#, - ) - .build(); + ).build(); assert_that( p.cargo("build -vv").masquerade_as_nightly_cargo(), @@ -128,8 +127,7 @@ fn metabuild_missing_dep() { version = "0.0.1" metabuild = "mb" "#, - ) - .file("src/lib.rs", "") + ).file("src/lib.rs", "") .build(); assert_that( @@ -159,26 +157,22 @@ fn metabuild_optional_dep() { [build-dependencies] mb = {path="mb", optional=true} "#, - ) - .file("src/lib.rs", "") + ).file("src/lib.rs", "") .file("mb/Cargo.toml", &basic_lib_manifest("mb")) .file( "mb/src/lib.rs", r#"pub fn metabuild() { println!("Hello mb"); }"#, - ) - .build(); + ).build(); assert_that( p.cargo("build -vv").masquerade_as_nightly_cargo(), - execs() - .with_status(0) - .with_stdout_does_not_contain("Hello mb"), + execs().with_stdout_does_not_contain("Hello mb"), ); assert_that( p.cargo("build -vv --features mb") .masquerade_as_nightly_cargo(), - execs().with_status(0).with_stdout_contains("Hello mb"), + execs().with_stdout_contains("Hello mb"), ); } @@ -198,8 +192,7 @@ fn metabuild_lib_name() { [build-dependencies] mb = {path="mb"} "#, - ) - .file("src/lib.rs", "") + ).file("src/lib.rs", "") .file( "mb/Cargo.toml", r#" @@ -209,16 +202,14 @@ fn metabuild_lib_name() { [lib] name = "other" "#, - ) - .file( + ).file( "mb/src/lib.rs", r#"pub fn metabuild() { println!("Hello mb"); }"#, - ) - .build(); + ).build(); assert_that( p.cargo("build -vv").masquerade_as_nightly_cargo(), - execs().with_status(0).with_stdout_contains("Hello mb"), + execs().with_stdout_contains("Hello mb"), ); } @@ -238,24 +229,21 @@ fn metabuild_fresh() { [build-dependencies] mb = {path="mb"} "#, - ) - .file("src/lib.rs", "") + ).file("src/lib.rs", "") .file("mb/Cargo.toml", &basic_lib_manifest("mb")) .file( "mb/src/lib.rs", r#"pub fn metabuild() { println!("Hello mb"); }"#, - ) - .build(); + ).build(); assert_that( p.cargo("build -vv").masquerade_as_nightly_cargo(), - execs().with_status(0).with_stdout_contains("Hello mb"), + execs().with_stdout_contains("Hello mb"), ); assert_that( p.cargo("build -vv").masquerade_as_nightly_cargo(), execs() - .with_status(0) .with_stdout_does_not_contain("Hello mb") .with_stderr( "\ @@ -283,8 +271,7 @@ fn metabuild_links() { [build-dependencies] mb = {path="mb"} "#, - ) - .file("src/lib.rs", "") + ).file("src/lib.rs", "") .file("mb/Cargo.toml", &basic_lib_manifest("mb")) .file( "mb/src/lib.rs", @@ -293,12 +280,11 @@ fn metabuild_links() { Ok("cat".to_string())); println!("Hello mb"); }"#, - ) - .build(); + ).build(); assert_that( p.cargo("build -vv").masquerade_as_nightly_cargo(), - execs().with_status(0).with_stdout_contains("Hello mb"), + execs().with_stdout_contains("Hello mb"), ); } @@ -318,14 +304,12 @@ fn metabuild_override() { [build-dependencies] mb = {path="mb"} "#, - ) - .file("src/lib.rs", "") + ).file("src/lib.rs", "") .file("mb/Cargo.toml", &basic_lib_manifest("mb")) .file( "mb/src/lib.rs", r#"pub fn metabuild() { panic!("should not run"); }"#, - ) - .file( + ).file( ".cargo/config", &format!( r#" @@ -334,13 +318,9 @@ fn metabuild_override() { "#, rustc_host() ), - ) - .build(); + ).build(); - assert_that( - p.cargo("build -vv").masquerade_as_nightly_cargo(), - execs().with_status(0), - ); + assert_that(p.cargo("build -vv").masquerade_as_nightly_cargo(), execs()); } #[test] @@ -352,8 +332,7 @@ fn metabuild_workspace() { [workspace] members = ["member1", "member2"] "#, - ) - .file( + ).file( "member1/Cargo.toml", r#" cargo-features = ["metabuild"] @@ -366,8 +345,7 @@ fn metabuild_workspace() { mb1 = {path="../../mb1"} mb2 = {path="../../mb2"} "#, - ) - .file("member1/src/lib.rs", "") + ).file("member1/src/lib.rs", "") .file( "member2/Cargo.toml", r#" @@ -380,8 +358,7 @@ fn metabuild_workspace() { [build-dependencies] mb1 = {path="../../mb1"} "#, - ) - .file("member2/src/lib.rs", "") + ).file("member2/src/lib.rs", "") .build(); project() @@ -405,10 +382,282 @@ fn metabuild_workspace() { assert_that( p.cargo("build -vv --all").masquerade_as_nightly_cargo(), execs() - .with_status(0) .with_stdout_contains("Hello mb1 [..]member1") .with_stdout_contains("Hello mb2 [..]member1") .with_stdout_contains("Hello mb1 [..]member2") .with_stdout_does_not_contain("Hello mb2 [..]member2"), ); } + +#[test] +fn metabuild_metadata() { + // The metabuild Target is filtered out of the `metadata` results. + let p = basic_project(); + + let output = p + .cargo("metadata --format-version=1") + .masquerade_as_nightly_cargo() + .exec_with_output() + .expect("cargo metadata failed"); + let stdout = str::from_utf8(&output.stdout).unwrap(); + let meta: serde_json::Value = serde_json::from_str(stdout).expect("failed to parse json"); + let mb_info: Vec<&str> = meta["packages"] + .as_array() + .unwrap() + .iter() + .filter(|p| p["name"].as_str().unwrap() == "foo") + .next() + .unwrap()["metabuild"] + .as_array() + .unwrap() + .iter() + .map(|s| s.as_str().unwrap()) + .collect(); + assert_eq!(mb_info, ["mb", "mb-other"]); +} + +#[test] +fn metabuild_build_plan() { + let p = basic_project(); + + assert_that( + p.cargo("build --build-plan -Zunstable-options") + .masquerade_as_nightly_cargo(), + execs().with_json( + r#" +{ + "invocations": [ + { + "package_name": "mb", + "package_version": "0.5.0", + "target_kind": ["lib"], + "kind": "Host", + "deps": [], + "outputs": ["[..]/target/debug/deps/libmb-[..].rlib"], + "links": {}, + "program": "rustc", + "args": "{...}", + "env": "{...}", + "cwd": "[..]" + }, + { + "package_name": "mb-other", + "package_version": "0.0.1", + "target_kind": ["lib"], + "kind": "Host", + "deps": [], + "outputs": ["[..]/target/debug/deps/libmb_other-[..].rlib"], + "links": {}, + "program": "rustc", + "args": "{...}", + "env": "{...}", + "cwd": "[..]" + }, + { + "package_name": "foo", + "package_version": "0.0.1", + "target_kind": ["custom-build"], + "kind": "Host", + "deps": [0, 1], + "outputs": ["[..]/target/debug/build/foo-[..]/metabuild_foo-[..][EXE]"], + "links": "{...}", + "program": "rustc", + "args": "{...}", + "env": "{...}", + "cwd": "[..]" + }, + { + "package_name": "foo", + "package_version": "0.0.1", + "target_kind": ["custom-build"], + "kind": "Host", + "deps": [2], + "outputs": [], + "links": {}, + "program": "[..]/foo/target/debug/build/foo-[..]/metabuild-foo", + "args": [], + "env": "{...}", + "cwd": "[..]" + }, + { + "package_name": "foo", + "package_version": "0.0.1", + "target_kind": ["lib"], + "kind": "Host", + "deps": [3], + "outputs": ["[..]/foo/target/debug/deps/libfoo-[..].rlib"], + "links": "{...}", + "program": "rustc", + "args": "{...}", + "env": "{...}", + "cwd": "[..]" + } + ], + "inputs": [ + "[..]/foo/Cargo.toml", + "[..]/foo/mb/Cargo.toml", + "[..]/foo/mb-other/Cargo.toml" + ] +} +"#, + ), + ); + + assert_eq!( + glob( + &p.root() + .join("target/.metabuild/metabuild-foo-*.rs") + .to_str() + .unwrap() + ).unwrap() + .count(), + 1 + ); +} + +#[test] +fn metabuild_two_versions() { + // Two versions of a metabuild dep with the same name. + let p = project() + .at("ws") + .file( + "Cargo.toml", + r#" + [workspace] + members = ["member1", "member2"] + "#, + ).file( + "member1/Cargo.toml", + r#" + cargo-features = ["metabuild"] + [package] + name = "member1" + version = "0.0.1" + metabuild = ["mb"] + + [build-dependencies] + mb = {path="../../mb1"} + "#, + ).file("member1/src/lib.rs", "") + .file( + "member2/Cargo.toml", + r#" + cargo-features = ["metabuild"] + [package] + name = "member2" + version = "0.0.1" + metabuild = ["mb"] + + [build-dependencies] + mb = {path="../../mb2"} + "#, + ).file("member2/src/lib.rs", "") + .build(); + + project().at("mb1") + .file("Cargo.toml", r#" + [package] + name = "mb" + version = "0.0.1" + "#) + .file( + "src/lib.rs", + r#"pub fn metabuild() { println!("Hello mb1 {}", std::env::var("CARGO_MANIFEST_DIR").unwrap()); }"#, + ) + .build(); + + project().at("mb2") + .file("Cargo.toml", r#" + [package] + name = "mb" + version = "0.0.2" + "#) + .file( + "src/lib.rs", + r#"pub fn metabuild() { println!("Hello mb2 {}", std::env::var("CARGO_MANIFEST_DIR").unwrap()); }"#, + ) + .build(); + + assert_that( + p.cargo("build -vv --all").masquerade_as_nightly_cargo(), + execs() + .with_stdout_contains("Hello mb1 [..]member1") + .with_stdout_contains("Hello mb2 [..]member2"), + ); + + assert_eq!( + glob( + &p.root() + .join("target/.metabuild/metabuild-member?-*.rs") + .to_str() + .unwrap() + ).unwrap() + .count(), + 2 + ); +} + +#[test] +fn metabuild_external_dependency() { + Package::new("mb", "1.0.0") + .file("Cargo.toml", &basic_manifest("mb", "1.0.0")) + .file( + "src/lib.rs", + r#"pub fn metabuild() { println!("Hello mb"); }"#, + ).publish(); + Package::new("dep", "1.0.0") + .file( + "Cargo.toml", + r#" + cargo-features = ["metabuild"] + [package] + name = "dep" + version = "1.0.0" + metabuild = ["mb"] + + [build-dependencies] + mb = "1.0" + "#, + ).file("src/lib.rs", "") + .build_dep("mb", "1.0.0") + .publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + [dependencies] + dep = "1.0" + "#, + ).file("src/lib.rs", "extern crate dep;") + .build(); + + assert_that( + p.cargo("build -vv").masquerade_as_nightly_cargo(), + execs().with_stdout_contains("Hello mb"), + ); + + assert_eq!( + glob( + &p.root() + .join("target/.metabuild/metabuild-dep-*.rs") + .to_str() + .unwrap() + ).unwrap() + .count(), + 1 + ); +} + +#[test] +fn metabuild_json_artifact() { + let p = basic_project(); + assert_that( + p.cargo("build --message-format=json") + .masquerade_as_nightly_cargo(), + execs(), + ); +} diff --git a/tests/testsuite/support/registry.rs b/tests/testsuite/support/registry.rs index 3e07113d872..cdea8836c69 100644 --- a/tests/testsuite/support/registry.rs +++ b/tests/testsuite/support/registry.rs @@ -184,6 +184,10 @@ impl Package { self.full_dep(name, vers, None, "dev", &[], None) } + pub fn build_dep(&mut self, name: &str, vers: &str) -> &mut Package { + self.full_dep(name, vers, None, "build", &[], None) + } + fn full_dep( &mut self, name: &str,