From a30f612ed9d744d967ec9708241d93ccd0da3328 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 12 Apr 2016 14:28:52 -0700 Subject: [PATCH] Forward warnings from build scripts This adds support for forwarding warnings from build scripts to the main console for diagnosis. A new `warning` metadata key is recognized by Cargo and is printed after a build script has finished executing. The purpose of this key is for build dependencies to try to produce useful warnings for local crates being developed. For example a parser generator could emit warnings about ambiguous grammars or the gcc crate could print warnings about the C/C++ code that's compiled. Warnings are only emitted for path dependencies by default (like lints) so warnings printed in crates.io crates are suppressed. cc #1106 --- src/cargo/ops/cargo_compile.rs | 1 + src/cargo/ops/cargo_rustc/context.rs | 4 ++ src/cargo/ops/cargo_rustc/custom_build.rs | 5 ++ src/cargo/ops/cargo_rustc/job_queue.rs | 33 +++++++---- src/cargo/ops/cargo_rustc/mod.rs | 7 +-- src/doc/build-script.md | 6 +- tests/build-script.rs | 68 +++++++++++++++++++++++ 7 files changed, 108 insertions(+), 16 deletions(-) diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 12356ac8292..530062dd6b8 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -486,6 +486,7 @@ fn scrape_target_config(config: &Config, triple: &str) cfgs: Vec::new(), metadata: Vec::new(), rerun_if_changed: Vec::new(), + warnings: Vec::new(), }; let key = format!("{}.{}", key, lib_name); let table = try!(config.get_table(&key)).unwrap().val; diff --git a/src/cargo/ops/cargo_rustc/context.rs b/src/cargo/ops/cargo_rustc/context.rs index 6eb5ecca92c..74e14cf3783 100644 --- a/src/cargo/ops/cargo_rustc/context.rs +++ b/src/cargo/ops/cargo_rustc/context.rs @@ -646,6 +646,10 @@ impl<'a, 'cfg> Context<'a, 'cfg> { pub fn rustflags_args(&self, unit: &Unit) -> CargoResult> { rustflags_args(self.config, &self.build_config, unit.kind) } + + pub fn show_warnings(&self, pkg: &PackageId) -> bool { + pkg == self.resolve.root() || pkg.source_id().is_path() + } } // Acquire extra flags to pass to the compiler from the diff --git a/src/cargo/ops/cargo_rustc/custom_build.rs b/src/cargo/ops/cargo_rustc/custom_build.rs index 13d3ed0fc1b..95df5a42b8a 100644 --- a/src/cargo/ops/cargo_rustc/custom_build.rs +++ b/src/cargo/ops/cargo_rustc/custom_build.rs @@ -26,6 +26,8 @@ pub struct BuildOutput { pub metadata: Vec<(String, String)>, /// Glob paths to trigger a rerun of this build script. pub rerun_if_changed: Vec, + /// Warnings generated by this build, + pub warnings: Vec, } pub type BuildMap = HashMap<(PackageId, Kind), BuildOutput>; @@ -282,6 +284,7 @@ impl BuildOutput { let mut cfgs = Vec::new(); let mut metadata = Vec::new(); let mut rerun_if_changed = Vec::new(); + let mut warnings = Vec::new(); let whence = format!("build script of `{}`", pkg_name); for line in input.split(|b| *b == b'\n') { @@ -320,6 +323,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()), + "warning" => warnings.push(value.to_string()), "rerun-if-changed" => rerun_if_changed.push(value.to_string()), _ => metadata.push((key.to_string(), value.to_string())), } @@ -331,6 +335,7 @@ impl BuildOutput { cfgs: cfgs, metadata: metadata, rerun_if_changed: rerun_if_changed, + warnings: warnings, }) } diff --git a/src/cargo/ops/cargo_rustc/job_queue.rs b/src/cargo/ops/cargo_rustc/job_queue.rs index ff35daac2eb..4c855582d46 100644 --- a/src/cargo/ops/cargo_rustc/job_queue.rs +++ b/src/cargo/ops/cargo_rustc/job_queue.rs @@ -85,15 +85,15 @@ impl<'a> JobQueue<'a> { /// This function will spawn off `config.jobs()` workers to build all of the /// necessary dependencies, in order. Freshness is propagated as far as /// possible along each dependency chain. - pub fn execute(&mut self, config: &Config) -> CargoResult<()> { + pub fn execute(&mut self, cx: &mut Context) -> CargoResult<()> { let _p = profile::start("executing the job graph"); crossbeam::scope(|scope| { - self.drain_the_queue(config, scope) + self.drain_the_queue(cx, scope) }) } - fn drain_the_queue(&mut self, config: &Config, scope: &Scope<'a>) + fn drain_the_queue(&mut self, cx: &mut Context, scope: &Scope<'a>) -> CargoResult<()> { let mut queue = Vec::new(); trace!("queue: {:#?}", self.queue); @@ -111,7 +111,7 @@ impl<'a> JobQueue<'a> { while self.active < self.jobs { if !queue.is_empty() { let (key, job, fresh) = queue.remove(0); - try!(self.run(key, fresh, job, config, scope)); + try!(self.run(key, fresh, job, cx.config, scope)); } else if let Some((fresh, key, jobs)) = self.queue.dequeue() { let total_fresh = jobs.iter().fold(fresh, |fresh, &(_, f)| { f.combine(fresh) @@ -139,15 +139,11 @@ impl<'a> JobQueue<'a> { self.active -= 1; match msg.result { Ok(()) => { - let state = self.pending.get_mut(&msg.key).unwrap(); - state.amt -= 1; - if state.amt == 0 { - self.queue.finish(&msg.key, state.fresh); - } + try!(self.finish(msg.key, cx)); } Err(e) => { if self.active > 0 { - try!(config.shell().say( + try!(cx.config.shell().say( "Build failed, waiting for other \ jobs to finish...", YELLOW)); for _ in self.rx.iter().take(self.active as usize) {} @@ -197,6 +193,23 @@ impl<'a> JobQueue<'a> { Ok(()) } + fn finish(&mut self, key: Key<'a>, cx: &mut Context) -> CargoResult<()> { + if key.profile.run_custom_build && cx.show_warnings(key.pkg) { + let output = cx.build_state.outputs.lock().unwrap(); + if let Some(output) = output.get(&(key.pkg.clone(), key.kind)) { + for warning in output.warnings.iter() { + try!(cx.config.shell().warn(warning)); + } + } + } + let state = self.pending.get_mut(&key).unwrap(); + state.amt -= 1; + if state.amt == 0 { + self.queue.finish(&key, state.fresh); + } + Ok(()) + } + // This isn't super trivial because we don't want to print loads and // loads of information to the console, but we also want to produce a // faithful representation of what's happening. This is somewhat nuanced diff --git a/src/cargo/ops/cargo_rustc/mod.rs b/src/cargo/ops/cargo_rustc/mod.rs index 91626dccc99..2919f07d0ac 100644 --- a/src/cargo/ops/cargo_rustc/mod.rs +++ b/src/cargo/ops/cargo_rustc/mod.rs @@ -109,7 +109,7 @@ pub fn compile_targets<'a, 'cfg: 'a>(pkg_targets: &'a PackagesToBuild<'a>, } // Now that we've figured out everything that we're going to do, do it! - try!(queue.execute(cx.config)); + try!(queue.execute(&mut cx)); for unit in units.iter() { let out_dir = cx.layout(unit.pkg, unit.kind).build_out(unit.pkg) @@ -211,10 +211,7 @@ fn rustc(cx: &mut Context, unit: &Unit) -> CargoResult { let mut rustc = try!(prepare_rustc(cx, crate_types, unit)); let name = unit.pkg.name().to_string(); - let is_path_source = unit.pkg.package_id().source_id().is_path(); - let allow_warnings = unit.pkg.package_id() == cx.resolve.root() || - is_path_source; - if !allow_warnings { + if !cx.show_warnings(unit.pkg.package_id()) { if cx.config.rustc_info().cap_lints { rustc.arg("--cap-lints").arg("allow"); } else { diff --git a/src/doc/build-script.md b/src/doc/build-script.md index 20272113f44..cc50b4c7480 100644 --- a/src/doc/build-script.md +++ b/src/doc/build-script.md @@ -55,7 +55,7 @@ cargo:libdir=/path/to/foo/lib cargo:include=/path/to/foo/include ``` -There are a few special keys that Cargo recognizes, affecting how the +There are a few special keys that Cargo recognizes, some affecting how the crate is built: * `rustc-link-lib` indicates that the specified value should be passed to the @@ -75,6 +75,10 @@ crate is built: directory, depending on platform) will trigger a rebuild. To request a re-run on any changes within an entire directory, print a line for the directory and another line for everything inside it, recursively.) +* `warning` is a message that will be printed to the main console after a build + script has finished running. Warnings are only shown for path dependencies + (that is, those you're working on locally), so for example warnings printed + out in crates.io crates are not emitted by default. 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/build-script.rs b/tests/build-script.rs index 964a315f4aa..1a5ef7c032e 100644 --- a/tests/build-script.rs +++ b/tests/build-script.rs @@ -4,9 +4,15 @@ extern crate hamcrest; use std::fs::{self, File}; use std::io::prelude::*; +<<<<<<< 07c1d9900de40c59b898d08d64273447560ffbe3:tests/build-script.rs use cargotest::{rustc_host, is_nightly, sleep_ms}; use cargotest::support::{project, execs}; use cargotest::support::paths::CargoPathExt; +======= +use support::{project, execs}; +use support::paths::CargoPathExt; +use support::registry::Package; +>>>>>>> Forward warnings from build scripts:tests/test_cargo_compile_custom_build.rs use hamcrest::{assert_that, existing_file, existing_dir}; #[test] @@ -2019,3 +2025,65 @@ fn panic_abort_with_build_scripts() { assert_that(p.cargo_process("build").arg("-v").arg("--release"), execs().with_status(0)); } + +#[test] +fn warnings_emitted() { + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.5.0" + authors = [] + build = "build.rs" + "#) + .file("src/lib.rs", "") + .file("build.rs", r#" + fn main() { + println!("cargo:warning=foo"); + println!("cargo:warning=bar"); + } + "#); + + assert_that(p.cargo_process("build").arg("-v"), + execs().with_status(0) + .with_stderr("\ +warning: foo +warning: bar +")); +} + +#[test] +fn warnings_hidden_for_upstream() { + Package::new("bar", "0.1.0") + .file("build.rs", r#" + fn main() { + println!("cargo:warning=foo"); + println!("cargo:warning=bar"); + } + "#) + .file("Cargo.toml", r#" + [project] + name = "bar" + version = "0.1.0" + authors = [] + build = "build.rs" + "#) + .file("src/lib.rs", "") + .publish(); + + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.5.0" + authors = [] + + [dependencies] + bar = "*" + "#) + .file("src/lib.rs", ""); + + assert_that(p.cargo_process("build").arg("-v"), + execs().with_status(0) + .with_stderr("")); +}