Skip to content

Commit

Permalink
Forward warnings from build scripts
Browse files Browse the repository at this point in the history
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 rust-lang#1106
  • Loading branch information
alexcrichton committed Jun 14, 2016
1 parent 941f488 commit a30f612
Show file tree
Hide file tree
Showing 7 changed files with 108 additions and 16 deletions.
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(""));
}

0 comments on commit a30f612

Please sign in to comment.