From ea2c6e8afe3262d5ed8a65220e07e9d347bd465a Mon Sep 17 00:00:00 2001 From: Paul Woolcock Date: Thu, 1 Dec 2016 22:42:45 -0500 Subject: [PATCH 1/5] Assume build = 'build.rs' by default This change makes cargo assume `build = "build.rs"` if there is a `build.rs` file in the same directory as `Cargo.toml`. However, you can set `build = false` to prevent this. --- src/cargo/util/toml.rs | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/src/cargo/util/toml.rs b/src/cargo/util/toml.rs index 5e5dc93a669..1d4f8608ad0 100644 --- a/src/cargo/util/toml.rs +++ b/src/cargo/util/toml.rs @@ -286,12 +286,18 @@ pub struct TomlProfile { panic: Option, } +#[derive(RustcDecodable, Clone, Debug)] +pub enum StringOrBool { + String(String), + Bool(bool), +} + #[derive(RustcDecodable)] pub struct TomlProject { name: String, version: TomlVersion, authors: Vec, - build: Option, + build: Option, links: Option, exclude: Option>, include: Option>, @@ -540,7 +546,11 @@ impl TomlManifest { } // processing the custom build script - let new_build = project.build.as_ref().map(PathBuf::from); + let manifest_file = util::important_paths::find_root_manifest_for_wd(None, &layout.root) + .chain_error(|| human("Could not find root manifest location"))?; + let base_dir = manifest_file.parent() + .ok_or(human("Could not get parent directory of manifest"))?; + let new_build = self.maybe_custom_build(&project.build, &base_dir); // Get targets let targets = normalize(&lib, @@ -767,6 +777,23 @@ impl TomlManifest { } Ok(replace) } + + fn maybe_custom_build(&self, build: &Option, project_dir: &Path) + -> Option { + let build_rs = project_dir.join("build.rs"); + match *build { + Some(StringOrBool::Bool(false)) => None, // explicitly no build script + Some(StringOrBool::Bool(true)) => Some(build_rs.into()), + Some(StringOrBool::String(ref s)) => Some(PathBuf::from(s)), + None => { + match fs::metadata(&build_rs) { + Ok(ref e) if e.is_file() => Some(build_rs.into()), + Ok(_) => None, + Err(_) => None, + } + } + } + } } /// Will check a list of toml targets, and make sure the target names are unique within a vector. From 73e024422f0fd6e46c8e97dab80ac81280b6b663 Mon Sep 17 00:00:00 2001 From: Paul Woolcock Date: Thu, 1 Dec 2016 23:01:51 -0500 Subject: [PATCH 2/5] Tests for new `package.build` behavior --- tests/build-script.rs | 71 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/tests/build-script.rs b/tests/build-script.rs index 46d8e144063..8524eb95072 100644 --- a/tests/build-script.rs +++ b/tests/build-script.rs @@ -2334,3 +2334,74 @@ fn switch_features_rerun() { assert_that(build.cargo("run").arg("-v").arg("--features=foo"), execs().with_status(0).with_stdout("foo\n")); } + +#[test] +fn assume_build_script_when_build_rs_present() { + let p = project("builder") + .file("Cargo.toml", r#" + [package] + name = "builder" + version = "0.0.1" + authors = [] + "#) + .file("src/main.rs", r#" + fn main() { + println!(include_str!(concat!(env!("OUT_DIR"), "/output"))); + } + "#) + .file("build.rs", r#" + use std::env; + use std::fs::File; + use std::io::Write; + use std::path::Path; + + fn main() { + let out_dir = env::var_os("OUT_DIR").unwrap(); + let out_dir = Path::new(&out_dir).join("output"); + let mut f = File::create(&out_dir).unwrap(); + f.write_all(b"foo").unwrap(); + } + "#); + p.build(); + + assert_that(p.cargo("run").arg("-v"), + execs().with_status(0).with_stdout("foo\n")); +} + +#[test] +fn if_build_set_to_false_dont_tread_build_rs_as_build_script() { + let p = project("builder") + .file("Cargo.toml", r#" + [package] + name = "builder" + version = "0.0.1" + authors = [] + build = false + "#) + .file("src/main.rs", r#" + use std::path::Path; + fn main() { + let f = env!("OUT_DIR"); + assert!( + ! Path::new(f).join("output").exists() + ) + } + "#) + .file("build.rs", r#" + use std::env; + use std::fs::File; + use std::io::Write; + use std::path::Path; + + fn main() { + let out_dir = env::var_os("OUT_DIR").unwrap(); + let out_dir = Path::new(&out_dir).join("output"); + let mut f = File::create(&out_dir).unwrap(); + f.write_all(b"foo").unwrap(); + } + "#); + p.build(); + + assert_that(p.cargo("run").arg("-v"), + execs().with_status(0)); +} From e4090499dbcf5dbbf041e0f4b87ebf6d0c361006 Mon Sep 17 00:00:00 2001 From: Paul Woolcock Date: Mon, 5 Dec 2016 09:41:03 -0500 Subject: [PATCH 3/5] Using layout.root here should be sufficient --- src/cargo/util/toml.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/cargo/util/toml.rs b/src/cargo/util/toml.rs index 1d4f8608ad0..fd5a36ca1f9 100644 --- a/src/cargo/util/toml.rs +++ b/src/cargo/util/toml.rs @@ -546,11 +546,7 @@ impl TomlManifest { } // processing the custom build script - let manifest_file = util::important_paths::find_root_manifest_for_wd(None, &layout.root) - .chain_error(|| human("Could not find root manifest location"))?; - let base_dir = manifest_file.parent() - .ok_or(human("Could not get parent directory of manifest"))?; - let new_build = self.maybe_custom_build(&project.build, &base_dir); + let new_build = self.maybe_custom_build(&project.build, &layout.root); // Get targets let targets = normalize(&lib, From d998dba882c2bc04d95498fc00d6fddc68bc0eba Mon Sep 17 00:00:00 2001 From: Paul Woolcock Date: Mon, 5 Dec 2016 13:02:13 -0500 Subject: [PATCH 4/5] correct typo --- tests/build-script.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/build-script.rs b/tests/build-script.rs index 8524eb95072..021ea084245 100644 --- a/tests/build-script.rs +++ b/tests/build-script.rs @@ -2369,7 +2369,7 @@ fn assume_build_script_when_build_rs_present() { } #[test] -fn if_build_set_to_false_dont_tread_build_rs_as_build_script() { +fn if_build_set_to_false_dont_treat_build_rs_as_build_script() { let p = project("builder") .file("Cargo.toml", r#" [package] From 1b9949161b95a121258f9d500d826a2c7ca8b77a Mon Sep 17 00:00:00 2001 From: Paul Woolcock Date: Tue, 6 Dec 2016 10:07:52 -0500 Subject: [PATCH 5/5] change this to only emit a warning right now --- src/cargo/util/toml.rs | 17 ++++++++++++++--- tests/build-script.rs | 15 +++++++++++++-- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/src/cargo/util/toml.rs b/src/cargo/util/toml.rs index fd5a36ca1f9..18b4d4c6cfc 100644 --- a/src/cargo/util/toml.rs +++ b/src/cargo/util/toml.rs @@ -546,7 +546,7 @@ impl TomlManifest { } // processing the custom build script - let new_build = self.maybe_custom_build(&project.build, &layout.root); + let new_build = self.maybe_custom_build(&project.build, &layout.root, &mut warnings); // Get targets let targets = normalize(&lib, @@ -774,7 +774,10 @@ impl TomlManifest { Ok(replace) } - fn maybe_custom_build(&self, build: &Option, project_dir: &Path) + fn maybe_custom_build(&self, + build: &Option, + project_dir: &Path, + warnings: &mut Vec) -> Option { let build_rs = project_dir.join("build.rs"); match *build { @@ -783,7 +786,15 @@ impl TomlManifest { Some(StringOrBool::String(ref s)) => Some(PathBuf::from(s)), None => { match fs::metadata(&build_rs) { - Ok(ref e) if e.is_file() => Some(build_rs.into()), + // Enable this after the warning has been visible for some time + // Ok(ref e) if e.is_file() => Some(build_rs.into()), + Ok(ref e) if e.is_file() => { + warnings.push("`build.rs` files in the same directory \ + as your `Cargo.toml` will soon be treated \ + as build scripts. Add `build = false` to \ + your `Cargo.toml` to prevent this".into()); + None + }, Ok(_) => None, Err(_) => None, } diff --git a/tests/build-script.rs b/tests/build-script.rs index 021ea084245..01ea55f9a51 100644 --- a/tests/build-script.rs +++ b/tests/build-script.rs @@ -2345,8 +2345,12 @@ fn assume_build_script_when_build_rs_present() { authors = [] "#) .file("src/main.rs", r#" + use std::path::Path; fn main() { - println!(include_str!(concat!(env!("OUT_DIR"), "/output"))); + let f = env!("OUT_DIR"); + assert!( + ! Path::new(f).join("output").exists() + ); } "#) .file("build.rs", r#" @@ -2365,7 +2369,14 @@ fn assume_build_script_when_build_rs_present() { p.build(); assert_that(p.cargo("run").arg("-v"), - execs().with_status(0).with_stdout("foo\n")); + execs().with_status(0).with_stderr("\ +warning: `build.rs` files in the same directory as your `Cargo.toml` will soon be treated \ +as build scripts. Add `build = false` to your `Cargo.toml` to prevent this + Compiling builder v0.0.1 ([..]) + Running [..] + Finished [..] + Running [..] +")); } #[test]