Always emit build script warnings for crates that fail to build

[SergioBenitez]

Resolves #3777.

r? @alexcrichton

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[SergioBenitez]

Apparently RFC#1623 wasn't stabilized in 1.16. Added the static lifetime.

[SergioBenitez]

The failure on Travis looks unrelated.

[alexcrichton]

Thanks! Could you gist a few snippets of what this looks like? It'd also be great to add a test for the full exhaustive match of the output.

Also, would you mind following the style that the rest of the tests/*.rs tests take for assertions?

[SergioBenitez]

Sorry, which style? Is this change sufficient?

@@ -49,12 +49,12 @@ fn no_warning_on_success() {
     make_lib("");
     let upstream = make_upstream("");
     let cargo_proc = upstream.cargo_process("build");
-    let expected_output = execs().with_status(0).with_stderr(
-        format!("[UPDATING] registry `[..]`\n\
-                 [DOWNLOADING] foo v0.0.1 ([..])\n\
-                 [COMPILING] foo v0.0.1\n\
-                 [COMPILING] bar v0.0.1 ([..])\n\
-                 [FINISHED] dev [unoptimized + debuginfo] target(s) in [..]\n"));
+    let expected_output = 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 [..] ");

Or do you mean I should make assertions look more like this (taken from a test)?

    assert_that(p.cargo("build").arg("-v"),
                execs().with_status(0)
                       .with_stderr("\
[COMPILING] foo v0.5.0 (file://[..])
[RUNNING] `[..][/]build-script-build`
[RUNNING] `rustc --crate-name foo [..]`
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
"));

If the latter, I consider that borderline unreadable, and it goes against what unofficial guidelines/rustfmt recommend.

Here's a snippet of how this looks:

    Updating registry `file:///Users/sbenitez/Local/cargo/target/cit/t0/registry`
 Downloading foo v0.0.1 (registry file:///Users/sbenitez/Local/cargo/target/cit/t0/registry)
   Compiling foo v0.0.1
error[E0425]: cannot find function `err` in this scope
 --> /Users/sbenitez/Local/cargo/target/cit/t0/home/.cargo/registry/src/-f674845f9099ef49/foo-0.0.1/src/lib.rs:1:10
  |
1 | fn f() { err() }
  |          ^^^ did you mean `Err`?

error: aborting due to previous error

The following warnings were emitted during compilation:

warning: Hello! I'm a warning. :)
warning: And one more!

error: Could not compile `foo`.

Finally, while I'd like to match against exact output, the output depends on the error message given by rustc meaning that if the error message changes, then the test will break. Is there a good way to deal with this?

[SergioBenitez]

@alexcrichton Ping!

[alexcrichton]

If the latter, I consider that borderline unreadable, and it goes against what unofficial guidelines/rustfmt recommend.

Ah yes I mean the latter. Let's not change the style, let's follow the other thousands of lines of tests.

Is there a good way to deal with this?

Ah no unfortuntely not, and yeah let's not match exactly if it relies on rustc error messages.

[SergioBenitez]

Updated the style to match existing tests. Also squashed and rebased with master. :)

[alexcrichton]

@bors: r+

Thanks!

[bors]

📌 Commit 4adae9e has been approved by alexcrichton

[bors]

⌛ Testing commit 4adae9e with merge ec8e824...

[bors]

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing ec8e824 to master...