From a30f612ed9d744d967ec9708241d93ccd0da3328 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 12 Apr 2016 14:28:52 -0700 Subject: [PATCH 1/3] 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("")); +} From 805dfe4e7964da509ea6dbc733b40ffcca21d874 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 12 Apr 2016 14:55:19 -0700 Subject: [PATCH 2/3] Allow specifying -v multiple times This commit modifies the CLI interface to allow the verbose (-v) flag to be specified multiple times. This'll be used soon to have `-vv` indicate that more output should be generated than `-v` during a normal build. Currently this commit changes the behavior of whether warnings are printed to print warnings for the **entire DAG of dependencies** if `-vv` is specified. That is, `--cap-lints` is never passed nor is any warning from build scripts if `-vv` is specified. --- src/bin/bench.rs | 4 +-- src/bin/build.rs | 4 +-- src/bin/cargo.rs | 4 +-- src/bin/clean.rs | 4 +-- src/bin/doc.rs | 4 +-- src/bin/fetch.rs | 4 +-- src/bin/generate_lockfile.rs | 4 +-- src/bin/git_checkout.rs | 4 +-- src/bin/init.rs | 4 +-- src/bin/install.rs | 4 +-- src/bin/login.rs | 4 +-- src/bin/metadata.rs | 4 +-- src/bin/new.rs | 4 +-- src/bin/owner.rs | 4 +-- src/bin/package.rs | 4 +-- src/bin/pkgid.rs | 4 +-- src/bin/publish.rs | 4 +-- src/bin/run.rs | 4 +-- src/bin/rustc.rs | 4 +-- src/bin/rustdoc.rs | 4 +-- src/bin/search.rs | 4 +-- src/bin/test.rs | 4 +-- src/bin/uninstall.rs | 4 +-- src/bin/update.rs | 4 +-- src/bin/verify_project.rs | 4 +-- src/bin/yank.rs | 4 +-- src/cargo/ops/cargo_rustc/context.rs | 3 +- src/cargo/util/config.rs | 11 ++++++- tests/build-script.rs | 46 ++++++++++++++++++++++++---- tests/registry.rs | 25 +++++++++++++++ 30 files changed, 129 insertions(+), 60 deletions(-) diff --git a/src/bin/bench.rs b/src/bin/bench.rs index 3aff7fb1d81..fc412ef1aaa 100644 --- a/src/bin/bench.rs +++ b/src/bin/bench.rs @@ -11,7 +11,7 @@ pub struct Options { flag_no_default_features: bool, flag_target: Option, flag_manifest_path: Option, - flag_verbose: Option, + flag_verbose: u32, flag_quiet: Option, flag_color: Option, flag_lib: bool, @@ -42,7 +42,7 @@ Options: --no-default-features Do not build the `default` feature --target TRIPLE Build for the target triple --manifest-path PATH Path to the manifest to build benchmarks for - -v, --verbose Use verbose output + -v, --verbose ... Use verbose output -q, --quiet No output printed to stdout --color WHEN Coloring: auto, always, never diff --git a/src/bin/build.rs b/src/bin/build.rs index 5a66ca75496..25d69440edf 100644 --- a/src/bin/build.rs +++ b/src/bin/build.rs @@ -13,7 +13,7 @@ pub struct Options { flag_no_default_features: bool, flag_target: Option, flag_manifest_path: Option, - flag_verbose: Option, + flag_verbose: u32, flag_quiet: Option, flag_color: Option, flag_release: bool, @@ -44,7 +44,7 @@ Options: --no-default-features Do not build the `default` feature --target TRIPLE Build for the target triple --manifest-path PATH Path to the manifest to compile - -v, --verbose Use verbose output + -v, --verbose ... Use verbose output -q, --quiet No output printed to stdout --color WHEN Coloring: auto, always, never diff --git a/src/bin/cargo.rs b/src/bin/cargo.rs index d7bd169cd98..3142bdd0a9d 100644 --- a/src/bin/cargo.rs +++ b/src/bin/cargo.rs @@ -21,7 +21,7 @@ use cargo::util::process_builder::process; pub struct Flags { flag_list: bool, flag_version: bool, - flag_verbose: Option, + flag_verbose: u32, flag_quiet: Option, flag_color: Option, flag_explain: Option, @@ -41,7 +41,7 @@ Options: -V, --version Print version info and exit --list List installed commands --explain CODE Run `rustc --explain CODE` - -v, --verbose Use verbose output + -v, --verbose ... Use verbose output -q, --quiet No output printed to stdout --color WHEN Coloring: auto, always, never diff --git a/src/bin/clean.rs b/src/bin/clean.rs index 5bcb10aeb25..af3968cf816 100644 --- a/src/bin/clean.rs +++ b/src/bin/clean.rs @@ -9,7 +9,7 @@ pub struct Options { flag_package: Vec, flag_target: Option, flag_manifest_path: Option, - flag_verbose: Option, + flag_verbose: u32, flag_quiet: Option, flag_color: Option, flag_release: bool, @@ -27,7 +27,7 @@ Options: --manifest-path PATH Path to the manifest to the package to clean --target TRIPLE Target triple to clean output for (default all) --release Whether or not to clean release artifacts - -v, --verbose Use verbose output + -v, --verbose ... Use verbose output -q, --quiet No output printed to stdout --color WHEN Coloring: auto, always, never diff --git a/src/bin/doc.rs b/src/bin/doc.rs index b7dbb1ade0c..37b5f60406d 100644 --- a/src/bin/doc.rs +++ b/src/bin/doc.rs @@ -12,7 +12,7 @@ pub struct Options { flag_no_deps: bool, flag_open: bool, flag_release: bool, - flag_verbose: Option, + flag_verbose: u32, flag_quiet: Option, flag_color: Option, flag_package: Vec, @@ -39,7 +39,7 @@ Options: --no-default-features Do not build the `default` feature --target TRIPLE Build for the target triple --manifest-path PATH Path to the manifest to document - -v, --verbose Use verbose output + -v, --verbose ... Use verbose output -q, --quiet No output printed to stdout --color WHEN Coloring: auto, always, never diff --git a/src/bin/fetch.rs b/src/bin/fetch.rs index 1f32e874154..0d72fff44b5 100644 --- a/src/bin/fetch.rs +++ b/src/bin/fetch.rs @@ -5,7 +5,7 @@ use cargo::util::important_paths::find_root_manifest_for_wd; #[derive(RustcDecodable)] pub struct Options { flag_manifest_path: Option, - flag_verbose: Option, + flag_verbose: u32, flag_quiet: Option, flag_color: Option, } @@ -19,7 +19,7 @@ Usage: Options: -h, --help Print this message --manifest-path PATH Path to the manifest to fetch dependencies for - -v, --verbose Use verbose output + -v, --verbose ... Use verbose output -q, --quiet No output printed to stdout --color WHEN Coloring: auto, always, never diff --git a/src/bin/generate_lockfile.rs b/src/bin/generate_lockfile.rs index 7e50eff265f..83b44d4eeef 100644 --- a/src/bin/generate_lockfile.rs +++ b/src/bin/generate_lockfile.rs @@ -7,7 +7,7 @@ use cargo::util::important_paths::find_root_manifest_for_wd; #[derive(RustcDecodable)] pub struct Options { flag_manifest_path: Option, - flag_verbose: Option, + flag_verbose: u32, flag_quiet: Option, flag_color: Option, } @@ -21,7 +21,7 @@ Usage: Options: -h, --help Print this message --manifest-path PATH Path to the manifest to generate a lockfile for - -v, --verbose Use verbose output + -v, --verbose ... Use verbose output -q, --quiet No output printed to stdout --color WHEN Coloring: auto, always, never "; diff --git a/src/bin/git_checkout.rs b/src/bin/git_checkout.rs index 8d3e29ac2eb..d3f49db6cde 100644 --- a/src/bin/git_checkout.rs +++ b/src/bin/git_checkout.rs @@ -6,7 +6,7 @@ use cargo::util::{Config, CliResult, CliError, human, ToUrl}; pub struct Options { flag_url: String, flag_reference: String, - flag_verbose: Option, + flag_verbose: u32, flag_quiet: Option, flag_color: Option, } @@ -20,7 +20,7 @@ Usage: Options: -h, --help Print this message - -v, --verbose Use verbose output + -v, --verbose ... Use verbose output -q, --quiet No output printed to stdout --color WHEN Coloring: auto, always, never "; diff --git a/src/bin/init.rs b/src/bin/init.rs index 8856f8b3599..3f02f057f3a 100644 --- a/src/bin/init.rs +++ b/src/bin/init.rs @@ -5,7 +5,7 @@ use cargo::util::{CliResult, Config}; #[derive(RustcDecodable)] pub struct Options { - flag_verbose: Option, + flag_verbose: u32, flag_quiet: Option, flag_color: Option, flag_bin: bool, @@ -28,7 +28,7 @@ Options: control at all (none) overriding a global configuration. --bin Use a binary instead of a library template --name NAME Set the resulting package name - -v, --verbose Use verbose output + -v, --verbose ... Use verbose output -q, --quiet No output printed to stdout --color WHEN Coloring: auto, always, never "; diff --git a/src/bin/install.rs b/src/bin/install.rs index 3f8d50baebe..5422375518f 100644 --- a/src/bin/install.rs +++ b/src/bin/install.rs @@ -10,7 +10,7 @@ pub struct Options { flag_debug: bool, flag_bin: Vec, flag_example: Vec, - flag_verbose: Option, + flag_verbose: u32, flag_quiet: Option, flag_color: Option, flag_root: Option, @@ -53,7 +53,7 @@ Build and install options: --bin NAME Only install the binary NAME --example EXAMPLE Install the example EXAMPLE instead of binaries --root DIR Directory to install packages into - -v, --verbose Use verbose output + -v, --verbose ... Use verbose output -q, --quiet Less output printed to stdout --color WHEN Coloring: auto, always, never diff --git a/src/bin/login.rs b/src/bin/login.rs index bfb8418e40e..50d8249be7f 100644 --- a/src/bin/login.rs +++ b/src/bin/login.rs @@ -10,7 +10,7 @@ use cargo::util::{CliResult, Config, human, ChainError}; pub struct Options { flag_host: Option, arg_token: Option, - flag_verbose: Option, + flag_verbose: u32, flag_quiet: Option, flag_color: Option, } @@ -24,7 +24,7 @@ Usage: Options: -h, --help Print this message --host HOST Host to set the token for - -v, --verbose Use verbose output + -v, --verbose ... Use verbose output -q, --quiet No output printed to stdout --color WHEN Coloring: auto, always, never diff --git a/src/bin/metadata.rs b/src/bin/metadata.rs index c8375d95c95..b959be2c0e3 100644 --- a/src/bin/metadata.rs +++ b/src/bin/metadata.rs @@ -16,7 +16,7 @@ pub struct Options { flag_no_default_features: bool, flag_no_deps: bool, flag_quiet: Option, - flag_verbose: Option, + flag_verbose: u32, } pub const USAGE: &'static str = " @@ -35,7 +35,7 @@ Options: --manifest-path PATH Path to the manifest --format-version VERSION Format version [default: 1] Valid values: 1 - -v, --verbose Use verbose output + -v, --verbose ... Use verbose output -q, --quiet No output printed to stdout --color WHEN Coloring: auto, always, never "; diff --git a/src/bin/new.rs b/src/bin/new.rs index 340fab90b4e..70f9018e497 100644 --- a/src/bin/new.rs +++ b/src/bin/new.rs @@ -5,7 +5,7 @@ use cargo::util::{CliResult, Config}; #[derive(RustcDecodable)] pub struct Options { - flag_verbose: Option, + flag_verbose: u32, flag_quiet: Option, flag_color: Option, flag_bin: bool, @@ -28,7 +28,7 @@ Options: control at all (none) overriding a global configuration. --bin Use a binary instead of a library template --name NAME Set the resulting package name - -v, --verbose Use verbose output + -v, --verbose ... Use verbose output -q, --quiet No output printed to stdout --color WHEN Coloring: auto, always, never "; diff --git a/src/bin/owner.rs b/src/bin/owner.rs index 33f49e51140..1eb02429576 100644 --- a/src/bin/owner.rs +++ b/src/bin/owner.rs @@ -8,7 +8,7 @@ pub struct Options { flag_add: Option>, flag_remove: Option>, flag_index: Option, - flag_verbose: Option, + flag_verbose: u32, flag_quiet: Option, flag_color: Option, flag_list: bool, @@ -27,7 +27,7 @@ Options: -l, --list List owners of a crate --index INDEX Registry index to modify owners for --token TOKEN API token to use when authenticating - -v, --verbose Use verbose output + -v, --verbose ... Use verbose output -q, --quiet No output printed to stdout --color WHEN Coloring: auto, always, never diff --git a/src/bin/package.rs b/src/bin/package.rs index be4a94cad6c..0336331a21b 100644 --- a/src/bin/package.rs +++ b/src/bin/package.rs @@ -4,7 +4,7 @@ use cargo::util::important_paths::find_root_manifest_for_wd; #[derive(RustcDecodable)] pub struct Options { - flag_verbose: Option, + flag_verbose: u32, flag_quiet: Option, flag_color: Option, flag_manifest_path: Option, @@ -27,7 +27,7 @@ Options: --no-metadata Ignore warnings about a lack of human-usable metadata --allow-dirty Allow dirty working directories to be packaged --manifest-path PATH Path to the manifest to compile - -v, --verbose Use verbose output + -v, --verbose ... Use verbose output -q, --quiet No output printed to stdout --color WHEN Coloring: auto, always, never diff --git a/src/bin/pkgid.rs b/src/bin/pkgid.rs index 68e53236f20..beff81e3db9 100644 --- a/src/bin/pkgid.rs +++ b/src/bin/pkgid.rs @@ -4,7 +4,7 @@ use cargo::util::important_paths::{find_root_manifest_for_wd}; #[derive(RustcDecodable)] pub struct Options { - flag_verbose: Option, + flag_verbose: u32, flag_quiet: Option, flag_color: Option, flag_manifest_path: Option, @@ -20,7 +20,7 @@ Usage: Options: -h, --help Print this message --manifest-path PATH Path to the manifest to the package to clean - -v, --verbose Use verbose output + -v, --verbose ... Use verbose output -q, --quiet No output printed to stdout --color WHEN Coloring: auto, always, never diff --git a/src/bin/publish.rs b/src/bin/publish.rs index 73466c27fbb..32b7f5e400c 100644 --- a/src/bin/publish.rs +++ b/src/bin/publish.rs @@ -7,7 +7,7 @@ pub struct Options { flag_host: Option, flag_token: Option, flag_manifest_path: Option, - flag_verbose: Option, + flag_verbose: u32, flag_quiet: Option, flag_color: Option, flag_no_verify: bool, @@ -27,7 +27,7 @@ Options: --no-verify Don't verify package tarball before publish --allow-dirty Allow publishing with a dirty source directory --manifest-path PATH Path to the manifest of the package to publish - -v, --verbose Use verbose output + -v, --verbose ... Use verbose output -q, --quiet No output printed to stdout --color WHEN Coloring: auto, always, never diff --git a/src/bin/run.rs b/src/bin/run.rs index 789c90e4c89..7515eba5b8e 100644 --- a/src/bin/run.rs +++ b/src/bin/run.rs @@ -11,7 +11,7 @@ pub struct Options { flag_no_default_features: bool, flag_target: Option, flag_manifest_path: Option, - flag_verbose: Option, + flag_verbose: u32, flag_quiet: Option, flag_color: Option, flag_release: bool, @@ -34,7 +34,7 @@ Options: --no-default-features Do not build the `default` feature --target TRIPLE Build for the target triple --manifest-path PATH Path to the manifest to execute - -v, --verbose Use verbose output + -v, --verbose ... Use verbose output -q, --quiet No output printed to stdout --color WHEN Coloring: auto, always, never diff --git a/src/bin/rustc.rs b/src/bin/rustc.rs index 51fec6d8680..2c2e850539b 100644 --- a/src/bin/rustc.rs +++ b/src/bin/rustc.rs @@ -14,7 +14,7 @@ pub struct Options { flag_no_default_features: bool, flag_target: Option, flag_manifest_path: Option, - flag_verbose: Option, + flag_verbose: u32, flag_quiet: Option, flag_color: Option, flag_release: bool, @@ -47,7 +47,7 @@ Options: --no-default-features Do not compile default features for the package --target TRIPLE Target triple which compiles will be for --manifest-path PATH Path to the manifest to fetch dependencies for - -v, --verbose Use verbose output + -v, --verbose ... Use verbose output -q, --quiet No output printed to stdout --color WHEN Coloring: auto, always, never diff --git a/src/bin/rustdoc.rs b/src/bin/rustdoc.rs index ed53d769ba8..8ed63d991f9 100644 --- a/src/bin/rustdoc.rs +++ b/src/bin/rustdoc.rs @@ -11,7 +11,7 @@ pub struct Options { flag_manifest_path: Option, flag_no_default_features: bool, flag_open: bool, - flag_verbose: Option, + flag_verbose: u32, flag_release: bool, flag_quiet: Option, flag_color: Option, @@ -44,7 +44,7 @@ Options: --no-default-features Do not build the `default` feature --target TRIPLE Build for the target triple --manifest-path PATH Path to the manifest to document - -v, --verbose Use verbose output + -v, --verbose ... Use verbose output -q, --quiet No output printed to stdout --color WHEN Coloring: auto, always, never diff --git a/src/bin/search.rs b/src/bin/search.rs index 2eae9173c9c..4d1f1ea3bf1 100644 --- a/src/bin/search.rs +++ b/src/bin/search.rs @@ -6,7 +6,7 @@ use std::cmp; #[derive(RustcDecodable)] pub struct Options { flag_host: Option, - flag_verbose: Option, + flag_verbose: u32, flag_quiet: Option, flag_color: Option, flag_limit: Option, @@ -23,7 +23,7 @@ Usage: Options: -h, --help Print this message --host HOST Host of a registry to search in - -v, --verbose Use verbose output + -v, --verbose ... Use verbose output -q, --quiet No output printed to stdout --color WHEN Coloring: auto, always, never --limit LIMIT Limit the number of results (default: 10, max: 100) diff --git a/src/bin/test.rs b/src/bin/test.rs index 8f2d0feef25..2d4dca94313 100644 --- a/src/bin/test.rs +++ b/src/bin/test.rs @@ -18,7 +18,7 @@ pub struct Options { flag_example: Vec, flag_test: Vec, flag_bench: Vec, - flag_verbose: Option, + flag_verbose: u32, flag_quiet: Option, flag_color: Option, flag_release: bool, @@ -47,7 +47,7 @@ Options: --no-default-features Do not build the `default` feature --target TRIPLE Build for the target triple --manifest-path PATH Path to the manifest to build tests for - -v, --verbose Use verbose output + -v, --verbose ... Use verbose output -q, --quiet No output printed to stdout --color WHEN Coloring: auto, always, never --no-fail-fast Run all tests regardless of failure diff --git a/src/bin/uninstall.rs b/src/bin/uninstall.rs index ace6a25007e..046997a44fb 100644 --- a/src/bin/uninstall.rs +++ b/src/bin/uninstall.rs @@ -5,7 +5,7 @@ use cargo::util::{CliResult, Config}; pub struct Options { flag_bin: Vec, flag_root: Option, - flag_verbose: Option, + flag_verbose: u32, flag_quiet: Option, flag_color: Option, @@ -23,7 +23,7 @@ Options: -h, --help Print this message --root DIR Directory to uninstall packages from --bin NAME Only uninstall the binary NAME - -v, --verbose Use verbose output + -v, --verbose ... Use verbose output -q, --quiet Less output printed to stdout --color WHEN Coloring: auto, always, never diff --git a/src/bin/update.rs b/src/bin/update.rs index 77e4c470cc4..12d26883888 100644 --- a/src/bin/update.rs +++ b/src/bin/update.rs @@ -10,7 +10,7 @@ pub struct Options { flag_aggressive: bool, flag_precise: Option, flag_manifest_path: Option, - flag_verbose: Option, + flag_verbose: u32, flag_quiet: Option, flag_color: Option, } @@ -27,7 +27,7 @@ Options: --aggressive Force updating all dependencies of as well --precise PRECISE Update a single dependency to exactly PRECISE --manifest-path PATH Path to the crate's manifest - -v, --verbose Use verbose output + -v, --verbose ... Use verbose output -q, --quiet No output printed to stdout --color WHEN Coloring: auto, always, never diff --git a/src/bin/verify_project.rs b/src/bin/verify_project.rs index 7079a4a6e97..0ce407e08c4 100644 --- a/src/bin/verify_project.rs +++ b/src/bin/verify_project.rs @@ -13,7 +13,7 @@ pub type Error = HashMap; #[derive(RustcDecodable)] pub struct Flags { flag_manifest_path: Option, - flag_verbose: Option, + flag_verbose: u32, flag_quiet: Option, flag_color: Option, } @@ -28,7 +28,7 @@ Usage: Options: -h, --help Print this message --manifest-path PATH Path to the manifest to verify - -v, --verbose Use verbose output + -v, --verbose ... Use verbose output -q, --quiet No output printed to stdout --color WHEN Coloring: auto, always, never "; diff --git a/src/bin/yank.rs b/src/bin/yank.rs index 60718c7645d..a869f36fffc 100644 --- a/src/bin/yank.rs +++ b/src/bin/yank.rs @@ -7,7 +7,7 @@ pub struct Options { flag_token: Option, flag_vers: Option, flag_index: Option, - flag_verbose: Option, + flag_verbose: u32, flag_quiet: Option, flag_color: Option, flag_undo: bool, @@ -25,7 +25,7 @@ Options: --undo Undo a yank, putting a version back into the index --index INDEX Registry index to yank from --token TOKEN API token to use when authenticating - -v, --verbose Use verbose output + -v, --verbose ... Use verbose output -q, --quiet No output printed to stdout --color WHEN Coloring: auto, always, never diff --git a/src/cargo/ops/cargo_rustc/context.rs b/src/cargo/ops/cargo_rustc/context.rs index 74e14cf3783..bf458bcbf5c 100644 --- a/src/cargo/ops/cargo_rustc/context.rs +++ b/src/cargo/ops/cargo_rustc/context.rs @@ -648,7 +648,8 @@ impl<'a, 'cfg> Context<'a, 'cfg> { } pub fn show_warnings(&self, pkg: &PackageId) -> bool { - pkg == self.resolve.root() || pkg.source_id().is_path() + pkg == self.resolve.root() || pkg.source_id().is_path() || + self.config.extra_verbose() } } diff --git a/src/cargo/util/config.rs b/src/cargo/util/config.rs index 53ad520e609..2ca6a2001c0 100644 --- a/src/cargo/util/config.rs +++ b/src/cargo/util/config.rs @@ -31,6 +31,7 @@ pub struct Config { rustc: PathBuf, rustdoc: PathBuf, target_dir: RefCell>, + extra_verbose: Cell, } impl Config { @@ -47,6 +48,7 @@ impl Config { rustc: PathBuf::from("rustc"), rustdoc: PathBuf::from("rustdoc"), target_dir: RefCell::new(None), + extra_verbose: Cell::new(false), }; try!(cfg.scrape_tool_config()); @@ -287,9 +289,11 @@ impl Config { } pub fn configure_shell(&self, - verbose: Option, + verbose: u32, quiet: Option, color: &Option) -> CargoResult<()> { + let extra_verbose = verbose >= 2; + let verbose = if verbose == 0 {None} else {Some(true)}; let cfg_verbose = try!(self.get_bool("term.verbose")).map(|v| v.val); let cfg_color = try!(self.get_string("term.color")).map(|v| v.val); let color = color.as_ref().or(cfg_color.as_ref()); @@ -320,10 +324,15 @@ impl Config { self.shell().set_verbosity(verbosity); try!(self.shell().set_color_config(color.map(|s| &s[..]))); + self.extra_verbose.set(extra_verbose); Ok(()) } + pub fn extra_verbose(&self) -> bool { + self.extra_verbose.get() + } + fn load_values(&self) -> CargoResult<()> { let mut cfg = CV::Table(HashMap::new(), PathBuf::from(".")); diff --git a/tests/build-script.rs b/tests/build-script.rs index 1a5ef7c032e..f555c22a326 100644 --- a/tests/build-script.rs +++ b/tests/build-script.rs @@ -4,15 +4,10 @@ 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 cargotest::support::registry::Package; use hamcrest::{assert_that, existing_file, existing_dir}; #[test] @@ -2087,3 +2082,42 @@ fn warnings_hidden_for_upstream() { execs().with_status(0) .with_stderr("")); } + +#[test] +fn warnings_printed_on_vv() { + 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("-vv"), + execs().with_status(0) + .with_stderr("\ +warning: foo +warning: bar +")); +} diff --git a/tests/registry.rs b/tests/registry.rs index cb517c91c1b..83ddb6b7427 100644 --- a/tests/registry.rs +++ b/tests/registry.rs @@ -1045,3 +1045,28 @@ fn resolve_and_backtracking() { assert_that(p.cargo("build"), execs().with_status(0)); } + +#[test] +fn upstream_warnings_on_extra_verbose() { + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "bar" + version = "0.5.0" + authors = [] + + [dependencies] + foo = "*" + "#) + .file("src/main.rs", "fn main() {}"); + p.build(); + + Package::new("foo", "0.1.0") + .file("src/lib.rs", "fn unused() {}") + .publish(); + + assert_that(p.cargo("build").arg("-vv"), + execs().with_status(0).with_stderr_contains("\ +[..] warning: function is never used[..] +")); +} From 26690d33e483ea83236413ea424d748251aa2696 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 13 Apr 2016 23:37:57 -0700 Subject: [PATCH 3/3] Stream build script output to the console This commit alters Cargo's behavior when the `-vv` option is passed (two verbose flags) to stream output of all build scripts to the console. Cargo makes not attempt to prevent interleaving or indicate *which* build script is producing output, rather it simply forwards all output to one to the console. Cargo still acts as a middle-man, capturing the output, to parse build script output and interpret the results. The parsing is still deferred to completion but the stream output happens while the build script is running. On Unix this is implemented via `select` and on Windows this is implemented via IOCP. Closes #1106 --- Cargo.lock | 38 +++++ Cargo.toml | 1 + src/cargo/ops/cargo_rustc/custom_build.rs | 66 +++++++- src/cargo/ops/cargo_rustc/job.rs | 20 +-- src/cargo/ops/cargo_rustc/job_queue.rs | 98 +++++++---- src/cargo/ops/cargo_rustc/mod.rs | 8 +- src/cargo/util/errors.rs | 8 + src/cargo/util/mod.rs | 2 + src/cargo/util/read2.rs | 194 ++++++++++++++++++++++ tests/build-script.rs | 60 ++++++- 10 files changed, 442 insertions(+), 53 deletions(-) create mode 100644 src/cargo/util/read2.rs diff --git a/Cargo.lock b/Cargo.lock index b82576568df..7db2c69a6f8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -21,6 +21,7 @@ dependencies = [ "libc 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)", "libgit2-sys 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)", + "miow 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", "num_cpus 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)", "regex 0.1.58 (registry+https://github.com/rust-lang/crates.io-index)", "rustc-serialize 0.3.18 (registry+https://github.com/rust-lang/crates.io-index)", @@ -81,6 +82,11 @@ dependencies = [ "winapi 0.2.6 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "cfg-if" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" + [[package]] name = "cmake" version = "0.1.16" @@ -317,6 +323,29 @@ dependencies = [ "libc 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "miow" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "kernel32-sys 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)", + "net2 0.2.24 (registry+https://github.com/rust-lang/crates.io-index)", + "winapi 0.2.6 (registry+https://github.com/rust-lang/crates.io-index)", + "ws2_32-sys 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)", +] + +[[package]] +name = "net2" +version = "0.2.24" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "cfg-if 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", + "kernel32-sys 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)", + "libc 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)", + "winapi 0.2.6 (registry+https://github.com/rust-lang/crates.io-index)", + "ws2_32-sys 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "nom" version = "1.2.2" @@ -486,3 +515,12 @@ name = "winapi-build" version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" +[[package]] +name = "ws2_32-sys" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "winapi 0.2.6 (registry+https://github.com/rust-lang/crates.io-index)", + "winapi-build 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)", +] + diff --git a/Cargo.toml b/Cargo.toml index 996bc04e143..3b0402079b6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,6 +33,7 @@ glob = "0.2" kernel32-sys = "0.2" libc = "0.2" log = "0.3" +miow = "0.1" num_cpus = "0.2" regex = "0.1" rustc-serialize = "0.3" diff --git a/src/cargo/ops/cargo_rustc/custom_build.rs b/src/cargo/ops/cargo_rustc/custom_build.rs index 95df5a42b8a..8660a65ee7b 100644 --- a/src/cargo/ops/cargo_rustc/custom_build.rs +++ b/src/cargo/ops/cargo_rustc/custom_build.rs @@ -3,13 +3,16 @@ use std::fs; use std::path::{PathBuf, Path}; use std::str; use std::sync::{Mutex, Arc}; +use std::process::{Stdio, Output}; use core::PackageId; use util::{CargoResult, Human}; use util::{internal, ChainError, profile, paths}; -use util::Freshness; +use util::{Freshness, ProcessBuilder, read2}; +use util::errors::{process_error, ProcessError}; use super::job::Work; +use super::job_queue::JobState; use super::{fingerprint, Kind, Context, Unit}; use super::CommandType; @@ -159,14 +162,12 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) 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))); - let exec_engine = cx.exec_engine.clone(); - // Prepare the unit of "dirty work" which will actually run the custom build // command. // // Note that this has to do some extra work just before running the command // to determine extra environment variables and such. - let dirty = Work::new(move |desc_tx| { + let dirty = Work::new(move |state| { // Make sure that OUT_DIR exists. // // If we have an old build directory, then just move it into place, @@ -203,8 +204,9 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) } // And now finally, run the build command itself! - desc_tx.send(p.to_string()).ok(); - let output = try!(exec_engine.exec_with_output(p).map_err(|mut e| { + state.running(&p); + let cmd = p.into_process_builder(); + let output = try!(stream_output(state, &cmd).map_err(|mut e| { e.desc = format!("failed to run custom build command for `{}`\n{}", pkg_name, e.desc); Human(e) @@ -438,3 +440,55 @@ pub fn build_map<'b, 'cfg>(cx: &mut Context<'b, 'cfg>, } } } + +fn stream_output(state: &JobState, cmd: &ProcessBuilder) + -> Result { + let mut stdout = Vec::new(); + let mut stderr = Vec::new(); + + let status = try!((|| { + let mut cmd = cmd.build_command(); + cmd.stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .stdin(Stdio::null()); + let mut child = try!(cmd.spawn()); + let out = child.stdout.take().unwrap(); + let err = child.stderr.take().unwrap(); + + try!(read2(out, err, &mut |is_out, data, eof| { + let idx = if eof { + data.len() + } else { + match data.iter().rposition(|b| *b == b'\n') { + Some(i) => i + 1, + None => return, + } + }; + let data = data.drain(..idx); + let dst = if is_out {&mut stdout} else {&mut stderr}; + let start = dst.len(); + dst.extend(data); + let s = String::from_utf8_lossy(&dst[start..]); + if is_out { + state.stdout(&s); + } else { + state.stderr(&s); + } + })); + child.wait() + })().map_err(|e| { + let msg = format!("could not exeute process {}", cmd); + process_error(&msg, Some(e), None, None) + })); + let output = Output { + stdout: stdout, + stderr: stderr, + status: status, + }; + if !output.status.success() { + let msg = format!("process didn't exit successfully: {}", cmd); + Err(process_error(&msg, None, Some(&status), Some(&output))) + } else { + Ok(output) + } +} diff --git a/src/cargo/ops/cargo_rustc/job.rs b/src/cargo/ops/cargo_rustc/job.rs index 0c270cfdf23..ae7ba303738 100644 --- a/src/cargo/ops/cargo_rustc/job.rs +++ b/src/cargo/ops/cargo_rustc/job.rs @@ -1,14 +1,14 @@ -use std::sync::mpsc::Sender; use std::fmt; use util::{CargoResult, Fresh, Dirty, Freshness}; +use super::job_queue::JobState; pub struct Job { dirty: Work, fresh: Work } /// Each proc should send its description before starting. /// It should send either once or close immediately. pub struct Work { - inner: Box, CargoResult<()>> + Send>, + inner: Box FnBox<&'a JobState<'b>, CargoResult<()>> + Send>, } trait FnBox { @@ -23,7 +23,7 @@ impl R> FnBox for F { impl Work { pub fn new(f: F) -> Work - where F: FnOnce(Sender) -> CargoResult<()> + Send + 'static + where F: FnOnce(&JobState) -> CargoResult<()> + Send + 'static { Work { inner: Box::new(f) } } @@ -32,14 +32,14 @@ impl Work { Work::new(|_| Ok(())) } - pub fn call(self, tx: Sender) -> CargoResult<()> { + pub fn call(self, tx: &JobState) -> CargoResult<()> { self.inner.call_box(tx) } pub fn then(self, next: Work) -> Work { - Work::new(move |tx| { - try!(self.call(tx.clone())); - next.call(tx) + Work::new(move |state| { + try!(self.call(state)); + next.call(state) }) } } @@ -52,10 +52,10 @@ impl Job { /// Consumes this job by running it, returning the result of the /// computation. - pub fn run(self, fresh: Freshness, tx: Sender) -> CargoResult<()> { + pub fn run(self, fresh: Freshness, state: &JobState) -> CargoResult<()> { match fresh { - Fresh => self.fresh.call(tx), - Dirty => self.dirty.call(tx), + Fresh => self.fresh.call(state), + Dirty => self.dirty.call(state), } } } diff --git a/src/cargo/ops/cargo_rustc/job_queue.rs b/src/cargo/ops/cargo_rustc/job_queue.rs index 4c855582d46..9179421903c 100644 --- a/src/cargo/ops/cargo_rustc/job_queue.rs +++ b/src/cargo/ops/cargo_rustc/job_queue.rs @@ -1,6 +1,7 @@ use std::collections::HashSet; use std::collections::hash_map::HashMap; use std::fmt; +use std::io::Write; use std::sync::mpsc::{channel, Sender, Receiver}; use crossbeam::{self, Scope}; @@ -12,6 +13,7 @@ use util::{CargoResult, profile, internal}; use super::{Context, Kind, Unit}; use super::job::Job; +use super::engine::CommandPrototype; /// A management structure of the entire dependency graph to compile. /// @@ -21,8 +23,8 @@ use super::job::Job; pub struct JobQueue<'a> { jobs: usize, queue: DependencyQueue, Vec<(Job, Freshness)>>, - tx: Sender>, - rx: Receiver>, + tx: Sender<(Key<'a>, Message)>, + rx: Receiver<(Key<'a>, Message)>, active: usize, pending: HashMap, PendingBuild>, compiled: HashSet<&'a PackageId>, @@ -47,9 +49,30 @@ struct Key<'a> { kind: Kind, } -struct Message<'a> { +pub struct JobState<'a> { + tx: Sender<(Key<'a>, Message)>, key: Key<'a>, - result: CargoResult<()>, +} + +enum Message { + Run(String), + Stdout(String), + Stderr(String), + Finish(CargoResult<()>), +} + +impl<'a> JobState<'a> { + pub fn running(&self, cmd: &CommandPrototype) { + let _ = self.tx.send((self.key, Message::Run(cmd.to_string()))); + } + + pub fn stdout(&self, out: &str) { + let _ = self.tx.send((self.key, Message::Stdout(out.to_string()))); + } + + pub fn stderr(&self, err: &str) { + let _ = self.tx.send((self.key, Message::Stderr(err.to_string()))); + } } impl<'a> JobQueue<'a> { @@ -107,8 +130,9 @@ impl<'a> JobQueue<'a> { // After a job has finished we update our internal state if it was // successful and otherwise wait for pending work to finish if it failed // and then immediately return. + let mut error = None; loop { - while self.active < self.jobs { + while error.is_none() && self.active < self.jobs { if !queue.is_empty() { let (key, job, fresh) = queue.remove(0); try!(self.run(key, fresh, job, cx.config, scope)); @@ -131,30 +155,46 @@ impl<'a> JobQueue<'a> { break } - // Now that all possible work has been scheduled, wait for a piece - // of work to finish. If any package fails to build then we stop - // scheduling work as quickly as possibly. - let msg = self.rx.recv().unwrap(); - info!("end: {:?}", msg.key); - self.active -= 1; - match msg.result { - Ok(()) => { - try!(self.finish(msg.key, cx)); + let (key, msg) = self.rx.recv().unwrap(); + + match msg { + Message::Run(cmd) => { + try!(cx.config.shell().verbose(|c| c.status("Running", &cmd))); + } + Message::Stdout(out) => { + if cx.config.extra_verbose() { + try!(write!(cx.config.shell().out(), "{}", out)); + } } - Err(e) => { - if self.active > 0 { - try!(cx.config.shell().say( - "Build failed, waiting for other \ - jobs to finish...", YELLOW)); - for _ in self.rx.iter().take(self.active as usize) {} + Message::Stderr(err) => { + if cx.config.extra_verbose() { + try!(write!(cx.config.shell().err(), "{}", err)); + } + } + Message::Finish(result) => { + info!("end: {:?}", key); + self.active -= 1; + match result { + Ok(()) => try!(self.finish(key, cx)), + Err(e) => { + if self.active > 0 { + try!(cx.config.shell().say( + "Build failed, waiting for other \ + jobs to finish...", YELLOW)); + } + if error.is_none() { + error = Some(e); + } + } } - return Err(e) } } } if self.queue.is_empty() { Ok(()) + } else if let Some(e) = error { + Err(e) } else { debug!("queue: {:#?}", self.queue); Err(internal("finished with jobs still left in the queue")) @@ -175,21 +215,17 @@ impl<'a> JobQueue<'a> { *self.counts.get_mut(key.pkg).unwrap() -= 1; let my_tx = self.tx.clone(); - let (desc_tx, desc_rx) = channel(); scope.spawn(move || { - my_tx.send(Message { + let res = job.run(fresh, &JobState { + tx: my_tx.clone(), key: key, - result: job.run(fresh, desc_tx), - }).unwrap(); + }); + my_tx.send((key, Message::Finish(res))).unwrap(); }); // Print out some nice progress information try!(self.note_working_on(config, &key, fresh)); - // only the first message of each job is processed - if let Ok(msg) = desc_rx.recv() { - try!(config.shell().verbose(|c| c.status("Running", &msg))); - } Ok(()) } @@ -219,7 +255,9 @@ impl<'a> JobQueue<'a> { // In general, we try to print "Compiling" for the first nontrivial task // run for a package, regardless of when that is. We then don't print // out any more information for a package after we've printed it once. - fn note_working_on(&mut self, config: &Config, key: &Key<'a>, + fn note_working_on(&mut self, + config: &Config, + key: &Key<'a>, fresh: Freshness) -> CargoResult<()> { if (self.compiled.contains(key.pkg) && !key.profile.doc) || (self.documented.contains(key.pkg) && key.profile.doc) { diff --git a/src/cargo/ops/cargo_rustc/mod.rs b/src/cargo/ops/cargo_rustc/mod.rs index 2919f07d0ac..6b07d8f413f 100644 --- a/src/cargo/ops/cargo_rustc/mod.rs +++ b/src/cargo/ops/cargo_rustc/mod.rs @@ -247,7 +247,7 @@ fn rustc(cx: &mut Context, unit: &Unit) -> CargoResult { let rustflags = try!(cx.rustflags_args(unit)); - return Ok(Work::new(move |desc_tx| { + return Ok(Work::new(move |state| { // Only at runtime have we discovered what the extra -L and -l // arguments are for native libraries, so we process those here. We // also need to be sure to add any -L paths for our plugins to the @@ -272,7 +272,7 @@ fn rustc(cx: &mut Context, unit: &Unit) -> CargoResult { // Add the arguments from RUSTFLAGS rustc.args(&rustflags); - desc_tx.send(rustc.to_string()).ok(); + state.running(&rustc); try!(exec_engine.exec(rustc).chain_error(|| { human(format!("Could not compile `{}`.", name)) })); @@ -410,13 +410,13 @@ fn rustdoc(cx: &mut Context, unit: &Unit) -> CargoResult { let key = (unit.pkg.package_id().clone(), unit.kind); let exec_engine = cx.exec_engine.clone(); - Ok(Work::new(move |desc_tx| { + Ok(Work::new(move |state| { if let Some(output) = build_state.outputs.lock().unwrap().get(&key) { for cfg in output.cfgs.iter() { rustdoc.arg("--cfg").arg(cfg); } } - desc_tx.send(rustdoc.to_string()).unwrap(); + state.running(&rustdoc); exec_engine.exec(rustdoc).chain_error(|| { human(format!("Could not document `{}`.", name)) }) diff --git a/src/cargo/util/errors.rs b/src/cargo/util/errors.rs index 3a84be560f6..8b41dd3e6ce 100644 --- a/src/cargo/util/errors.rs +++ b/src/cargo/util/errors.rs @@ -453,6 +453,10 @@ pub fn internal_error(error: &str, detail: &str) -> Box { } pub fn internal(error: S) -> Box { + _internal(&error) +} + +fn _internal(error: &fmt::Display) -> Box { Box::new(ConcreteCargoError { description: error.to_string(), detail: None, @@ -462,6 +466,10 @@ pub fn internal(error: S) -> Box { } pub fn human(error: S) -> Box { + _human(&error) +} + +fn _human(error: &fmt::Display) -> Box { Box::new(ConcreteCargoError { description: error.to_string(), detail: None, diff --git a/src/cargo/util/mod.rs b/src/cargo/util/mod.rs index d638691d774..3b18fb1e573 100644 --- a/src/cargo/util/mod.rs +++ b/src/cargo/util/mod.rs @@ -18,6 +18,7 @@ pub use self::sha256::Sha256; pub use self::to_semver::ToSemver; pub use self::to_url::ToUrl; pub use self::vcs::{GitRepo, HgRepo}; +pub use self::read2::read2; pub mod config; pub mod errors; @@ -41,3 +42,4 @@ mod shell_escape; mod vcs; mod lazy_cell; mod flock; +mod read2; diff --git a/src/cargo/util/read2.rs b/src/cargo/util/read2.rs new file mode 100644 index 00000000000..4596b260a13 --- /dev/null +++ b/src/cargo/util/read2.rs @@ -0,0 +1,194 @@ +pub use self::imp::read2; + +#[cfg(unix)] +mod imp { + use std::cmp; + use std::io::prelude::*; + use std::io; + use std::mem; + use std::os::unix::prelude::*; + use std::process::{ChildStdout, ChildStderr}; + use libc; + + pub fn read2(mut out_pipe: ChildStdout, + mut err_pipe: ChildStderr, + mut data: &mut FnMut(bool, &mut Vec, bool)) -> io::Result<()> { + unsafe { + libc::fcntl(out_pipe.as_raw_fd(), libc::F_SETFL, libc::O_NONBLOCK); + libc::fcntl(err_pipe.as_raw_fd(), libc::F_SETFL, libc::O_NONBLOCK); + } + + let mut out_done = false; + let mut err_done = false; + let mut out = Vec::new(); + let mut err = Vec::new(); + + let max = cmp::max(out_pipe.as_raw_fd(), err_pipe.as_raw_fd()); + loop { + // wait for either pipe to become readable using `select` + let r = unsafe { + let mut read: libc::fd_set = mem::zeroed(); + if !out_done { + libc::FD_SET(out_pipe.as_raw_fd(), &mut read); + } + if !err_done { + libc::FD_SET(err_pipe.as_raw_fd(), &mut read); + } + libc::select(max + 1, &mut read, 0 as *mut _, 0 as *mut _, + 0 as *mut _) + }; + if r == -1 { + let err = io::Error::last_os_error(); + if err.kind() == io::ErrorKind::Interrupted { + continue + } + return Err(err) + } + + // Read as much as we can from each pipe, ignoring EWOULDBLOCK or + // EAGAIN. If we hit EOF, then this will happen because the underlying + // reader will return Ok(0), in which case we'll see `Ok` ourselves. In + // this case we flip the other fd back into blocking mode and read + // whatever's leftover on that file descriptor. + let handle = |res: io::Result<_>| { + match res { + Ok(_) => Ok(true), + Err(e) => { + if e.kind() == io::ErrorKind::WouldBlock { + Ok(false) + } else { + Err(e) + } + } + } + }; + if !out_done && try!(handle(out_pipe.read_to_end(&mut out))) { + out_done = true; + } + data(true, &mut out, out_done); + if !err_done && try!(handle(err_pipe.read_to_end(&mut err))) { + err_done = true; + } + data(false, &mut err, err_done); + + if out_done && err_done { + return Ok(()) + } + } + } +} + +#[cfg(windows)] +mod imp { + extern crate miow; + extern crate winapi; + + use std::io; + use std::os::windows::prelude::*; + use std::process::{ChildStdout, ChildStderr}; + use std::slice; + + use self::miow::iocp::{CompletionPort, CompletionStatus}; + use self::miow::pipe::NamedPipe; + use self::miow::Overlapped; + use self::winapi::ERROR_BROKEN_PIPE; + + struct Pipe<'a> { + dst: &'a mut Vec, + overlapped: Overlapped, + pipe: NamedPipe, + done: bool, + } + + macro_rules! try { + ($e:expr) => (match $e { + Ok(e) => e, + Err(e) => { + println!("{} failed with {}", stringify!($e), e); + return Err(e) + } + }) + } + + pub fn read2(out_pipe: ChildStdout, + err_pipe: ChildStderr, + mut data: &mut FnMut(bool, &mut Vec, bool)) -> io::Result<()> { + let mut out = Vec::new(); + let mut err = Vec::new(); + + let port = try!(CompletionPort::new(1)); + try!(port.add_handle(0, &out_pipe)); + try!(port.add_handle(1, &err_pipe)); + + unsafe { + let mut out_pipe = Pipe::new(out_pipe, &mut out); + let mut err_pipe = Pipe::new(err_pipe, &mut err); + + try!(out_pipe.read()); + try!(err_pipe.read()); + + let mut status = [CompletionStatus::zero(), CompletionStatus::zero()]; + + while !out_pipe.done || !err_pipe.done { + for status in try!(port.get_many(&mut status, None)) { + if status.token() == 0 { + out_pipe.complete(status); + data(true, out_pipe.dst, out_pipe.done); + try!(out_pipe.read()); + } else { + err_pipe.complete(status); + data(false, err_pipe.dst, err_pipe.done); + try!(err_pipe.read()); + } + } + } + + Ok(()) + } + } + + impl<'a> Pipe<'a> { + unsafe fn new(p: P, dst: &'a mut Vec) -> Pipe<'a> { + Pipe { + dst: dst, + pipe: NamedPipe::from_raw_handle(p.into_raw_handle()), + overlapped: Overlapped::zero(), + done: false, + } + } + + unsafe fn read(&mut self) -> io::Result<()> { + let dst = slice_to_end(self.dst); + match self.pipe.read_overlapped(dst, &mut self.overlapped) { + Ok(_) => Ok(()), + Err(e) => { + if e.raw_os_error() == Some(ERROR_BROKEN_PIPE as i32) { + self.done = true; + Ok(()) + } else { + Err(e) + } + } + } + } + + unsafe fn complete(&mut self, status: &CompletionStatus) { + let prev = self.dst.len(); + self.dst.set_len(prev + status.bytes_transferred() as usize); + if status.bytes_transferred() == 0 { + self.done = true; + } + } + } + + unsafe fn slice_to_end(v: &mut Vec) -> &mut [u8] { + if v.capacity() == 0 { + v.reserve(16); + } + if v.capacity() == v.len() { + v.reserve(1); + } + slice::from_raw_parts_mut(v.as_mut_ptr().offset(v.len() as isize), + v.capacity() - v.len()) + } +} diff --git a/tests/build-script.rs b/tests/build-script.rs index f555c22a326..d0b947b823a 100644 --- a/tests/build-script.rs +++ b/tests/build-script.rs @@ -36,8 +36,7 @@ fn custom_build_script_failed() { [RUNNING] `rustc build.rs --crate-name build_script_build --crate-type bin [..]` [RUNNING] `[..]build-script-build[..]` [ERROR] failed to run custom build command for `foo v0.5.0 ({url})` -Process didn't exit successfully: `[..]build[..]build-script-build[..]` \ - (exit code: 101)", +process didn't exit successfully: `[..]build-script-build[..]` (exit code: 101)", url = p.url()))); } @@ -2042,8 +2041,12 @@ fn warnings_emitted() { assert_that(p.cargo_process("build").arg("-v"), execs().with_status(0) .with_stderr("\ +[COMPILING] foo v0.5.0 ([..]) +[RUNNING] `rustc [..]` +[RUNNING] `[..]` warning: foo warning: bar +[RUNNING] `rustc [..]` ")); } @@ -2080,7 +2083,16 @@ fn warnings_hidden_for_upstream() { assert_that(p.cargo_process("build").arg("-v"), execs().with_status(0) - .with_stderr("")); + .with_stderr("\ +[UPDATING] registry `[..]` +[DOWNLOADING] bar v0.1.0 ([..]) +[COMPILING] bar v0.1.0 ([..]) +[RUNNING] `rustc [..]` +[RUNNING] `[..]` +[RUNNING] `rustc [..]` +[COMPILING] foo v0.5.0 ([..]) +[RUNNING] `rustc [..]` +")); } #[test] @@ -2117,7 +2129,49 @@ fn warnings_printed_on_vv() { assert_that(p.cargo_process("build").arg("-vv"), execs().with_status(0) .with_stderr("\ +[UPDATING] registry `[..]` +[DOWNLOADING] bar v0.1.0 ([..]) +[COMPILING] bar v0.1.0 ([..]) +[RUNNING] `rustc [..]` +[RUNNING] `[..]` warning: foo warning: bar +[RUNNING] `rustc [..]` +[COMPILING] foo v0.5.0 ([..]) +[RUNNING] `rustc [..]` +")); +} + +#[test] +fn output_shows_on_vv() { + 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#" + use std::io::prelude::*; + + fn main() { + std::io::stderr().write_all(b"stderr\n").unwrap(); + std::io::stdout().write_all(b"stdout\n").unwrap(); + } + "#); + + assert_that(p.cargo_process("build").arg("-vv"), + execs().with_status(0) + .with_stdout("\ +stdout +") + .with_stderr("\ +[COMPILING] foo v0.5.0 ([..]) +[RUNNING] `rustc [..]` +[RUNNING] `[..]` +stderr +[RUNNING] `rustc [..]` ")); }