Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add build script warnings, streaming output from build scripts to the console #2630

Merged
merged 3 commits into from
Jun 14, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
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
  • Loading branch information
alexcrichton committed Jun 14, 2016
commit a30f612ed9d744d967ec9708241d93ccd0da3328
1 change: 1 addition & 0 deletions src/cargo/ops/cargo_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 4 additions & 0 deletions src/cargo/ops/cargo_rustc/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -646,6 +646,10 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
pub fn rustflags_args(&self, unit: &Unit) -> CargoResult<Vec<String>> {
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
Expand Down
5 changes: 5 additions & 0 deletions src/cargo/ops/cargo_rustc/custom_build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>,
/// Warnings generated by this build,
pub warnings: Vec<String>,
}

pub type BuildMap = HashMap<(PackageId, Kind), BuildOutput>;
Expand Down Expand Up @@ -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') {
Expand Down Expand Up @@ -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())),
}
Expand All @@ -331,6 +335,7 @@ impl BuildOutput {
cfgs: cfgs,
metadata: metadata,
rerun_if_changed: rerun_if_changed,
warnings: warnings,
})
}

Expand Down
33 changes: 23 additions & 10 deletions src/cargo/ops/cargo_rustc/job_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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)
Expand Down Expand Up @@ -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) {}
Expand Down Expand Up @@ -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
Expand Down
7 changes: 2 additions & 5 deletions src/cargo/ops/cargo_rustc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -211,10 +211,7 @@ fn rustc(cx: &mut Context, unit: &Unit) -> CargoResult<Work> {
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 {
Expand Down
6 changes: 5 additions & 1 deletion src/doc/build-script.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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]
Expand Down
68 changes: 68 additions & 0 deletions tests/build-script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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(""));
}