Auto merge of #3847 - SergioBenitez:master, r=alexcrichton

bors · bors · commit ec8e82489535 · 2017-04-12T19:03:16.000Z
Always emit build script warnings for crates that fail to build Resolves #3777. r? @alexcrichton
diff --git a/src/cargo/ops/cargo_rustc/job_queue.rs b/src/cargo/ops/cargo_rustc/job_queue.rs
@@ -182,13 +182,17 @@ impl<'a> JobQueue<'a> {
                     match result {
                         Ok(()) => self.finish(key, cx)?,
                         Err(e) => {
+                            let msg = "The following warnings were emitted during compilation:";
+                            self.emit_warnings(Some(msg), key, cx)?;
+
                             if self.active > 0 {
                                 error = Some(human("build failed"));
                                 handle_error(&*e, &mut *cx.config.shell());
                                 cx.config.shell().say(
                                             "Build failed, waiting for other \
                                              jobs to finish...", YELLOW)?;
                             }
+
                             if error.is_none() {
                                 error = Some(e);
                             }
@@ -252,15 +256,33 @@ 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() {
-                    cx.config.shell().warn(warning)?;
+    fn emit_warnings(&self, msg: Option<&str>, key: Key<'a>, cx: &mut Context) -> CargoResult<()> {
+        let output = cx.build_state.outputs.lock().unwrap();
+        if let Some(output) = output.get(&(key.pkg.clone(), key.kind)) {
+            if let Some(msg) = msg {
+                if !output.warnings.is_empty() {
+                    writeln!(cx.config.shell().err(), "{}\n", msg)?;
                 }
             }
+
+            for warning in output.warnings.iter() {
+                cx.config.shell().warn(warning)?;
+            }
+
+            if !output.warnings.is_empty() && msg.is_some() {
+                // Output an empty line.
+                writeln!(cx.config.shell().err(), "")?;
+            }
         }
+
+        Ok(())
+    }
+
+    fn finish(&mut self, key: Key<'a>, cx: &mut Context) -> CargoResult<()> {
+        if key.profile.run_custom_build && cx.show_warnings(key.pkg) {
+            self.emit_warnings(None, key, cx)?;
+        }
+
         let state = self.pending.get_mut(&key).unwrap();
         state.amt -= 1;
         if state.amt == 0 {
diff --git a/tests/warn-on-failure.rs b/tests/warn-on-failure.rs
@@ -0,0 +1,92 @@
+extern crate cargotest;
+extern crate hamcrest;
+
+use cargotest::support::{project, execs, ProjectBuilder};
+use cargotest::support::registry::Package;
+use hamcrest::assert_that;
+
+static WARNING1: &'static str = "Hello! I'm a warning. :)";
+static WARNING2: &'static str = "And one more!";
+
+fn make_lib(lib_src: &str) {
+    Package::new("foo", "0.0.1")
+        .file("Cargo.toml", r#"
+            [package]
+            name = "foo"
+            authors = []
+            version = "0.0.1"
+            build = "build.rs"
+        "#)
+        .file("build.rs", &format!(r#"
+            fn main() {{
+                use std::io::Write;
+                println!("cargo:warning={{}}", "{}");
+                println!("hidden stdout");
+                write!(&mut ::std::io::stderr(), "hidden stderr");
+                println!("cargo:warning={{}}", "{}");
+            }}
+        "#, WARNING1, WARNING2))
+        .file("src/lib.rs", &format!("fn f() {{ {} }}", lib_src))
+        .publish();
+}
+
+fn make_upstream(main_src: &str) -> ProjectBuilder {
+    project("bar")
+        .file("Cargo.toml", r#"
+            [package]
+            name = "bar"
+            version = "0.0.1"
+            authors = []
+
+            [dependencies]
+            foo = "*"
+        "#)
+        .file("src/main.rs", &format!("fn main() {{ {} }}", main_src))
+}
+
+#[test]
+fn no_warning_on_success() {
+    make_lib("");
+    let upstream = make_upstream("");
+    assert_that(upstream.cargo_process("build"),
+                execs().with_status(0)
+                       .with_stderr("\
+[UPDATING] registry `[..]`
+[DOWNLOADING] foo v0.0.1 ([..])
+[COMPILING] foo v0.0.1
+[COMPILING] bar v0.0.1 ([..])
+[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
+"));
+}
+
+#[test]
+fn no_warning_on_bin_failure() {
+    make_lib("");
+    let upstream = make_upstream("hi()");
+    assert_that(upstream.cargo_process("build"),
+                execs().with_status(101)
+                       .with_stdout_does_not_contain("hidden stdout")
+                       .with_stderr_does_not_contain("hidden stderr")
+                       .with_stderr_does_not_contain(&format!("[WARNING] {}", WARNING1))
+                       .with_stderr_does_not_contain(&format!("[WARNING] {}", WARNING2))
+                       .with_stderr_contains("[UPDATING] registry `[..]`")
+                       .with_stderr_contains("[DOWNLOADING] foo v0.0.1 ([..])")
+                       .with_stderr_contains("[COMPILING] foo v0.0.1")
+                       .with_stderr_contains("[COMPILING] bar v0.0.1 ([..])"));
+}
+
+#[test]
+fn warning_on_lib_failure() {
+    make_lib("err()");
+    let upstream = make_upstream("");
+    assert_that(upstream.cargo_process("build"),
+                execs().with_status(101)
+                       .with_stdout_does_not_contain("hidden stdout")
+                       .with_stderr_does_not_contain("hidden stderr")
+                       .with_stderr_does_not_contain("[COMPILING] bar v0.0.1 ([..])")
+                       .with_stderr_contains("[UPDATING] registry `[..]`")
+                       .with_stderr_contains("[DOWNLOADING] foo v0.0.1 ([..])")
+                       .with_stderr_contains("[COMPILING] foo v0.0.1")
+                       .with_stderr_contains(&format!("[WARNING] {}", WARNING1))
+                       .with_stderr_contains(&format!("[WARNING] {}", WARNING2)));
+}
