Trim noise from build script errors

[kornelski]

This is an alternative solution to #10159, because #11312 is stuck in review for a long time due to open questions about some implementation details and a concern about #11461.

• 02557c3 enables a behavior that was basically already meant to be there, and it helps the rest of the changes.

• 69cacfa is the main part.

• 2c2c6d6 makes error messages even nicer. This may be a bit controversial, because it parses output assuming a particular text format of Rust panics. I think it's pretty safe to do this, but I'd be ok landing the rest of the changes without this one.

[rustbot]

r? @weihanglo

(rustbot has picked a reviewer for you, use r? to override)

[weihanglo]

Haven't walked through all changes, but we need to bump a breaking version for cargo-util crate.

[epage]

First, this should be discussed in an issue and approved before posting a PR.

Second, I do feel uncomfortable doing text re-formatting of output from other operations.

[kornelski]

I've explained the need for improvement in this area already in #10159. The particular error+= PR has been killed due to bikeshed about syntax and a concern about backwards-compatibility that I've already mitigated in that PR.

I still feel the overall issue is important and needs addressing, so I went as far as writing the code again just to avoid being blocked on that other PR.

There's no reformatting of other text, other than the natural consequence of adding a note that there are more errors to show behind --verbose flag. This is from a very sensible 1-line change that was almost a TODO in the existing code. If I made this change a different way to limit it strictly to the build script error, I'd need to be more complicated and IMHO needlessly add more special-case code than it needs to be. The note is universal and works correctly for other cases too.

[epage]

I've explained the need for improvement in this area already in #10159. The particular error+= PR has been killed due to bikeshed about syntax and a concern about backwards-compatibility that I've already mitigated in that PR.

Yes, the need is addressed there but our contributing document asks people to discuss their design first before creating a PR.

Also, the other PR was not killed those reasons. We provided a path forward around the backwards compatibility and I do not see any syntax bike shedding going on. We wanted to further discuss += syntax but that doesn't "kill it" or block the other improvements.

I still feel the overall issue is important and needs addressing, so I went as far as writing the code again just to avoid being blocked on that other PR.

There's no reformatting of other text, other than the natural consequence of adding a note that there are more errors to show behind --verbose flag. This is from a very sensible 1-line change that was almost a TODO in the existing code. If I made this change a different way to limit it strictly to the build script error, I'd need to be more complicated and IMHO needlessly add more special-case code than it needs to be. The note is universal and works correctly for other cases too.

Sorry, I mispoke / misunderstood. You are parsing non-programmatic output of a command which you admitted would be controversial in your PR. That is what I'm concerned about. Generally, I view non-programmatic output as opaque. In taking on parsing, we at least have the benefit of (usually) shipping with the cause of that panic (I say usually because of cargo-as-a-library). Ensuring everything stays in sync does add extra maintenance cost, so we need to make sure we are solid that this is a route to go. Considering we had another solution that sounded really promising, I don't see why to even go down this route. It doesn't help that the PR message doesn't even explain how this solves any of the problems, forcing people to dig into the commits to find out how (all of which should have been discussed in the issue first).

[bors]

☔ The latest upstream changes (presumably #11252) made this pull request unmergeable. Please resolve the merge conflicts.

[epage]

Closing due to inactivity. I'd recommend creating an issue to further discuss the underlying need if someone wants to move this forward.

[kornelski]

This would have helped a lot here:

https://users.rust-lang.org/t/in-alpine-rust-can-not-link-ssl-library/117576/5?u=kornel

Users are struggling to see the error. Printing of cargo:rerun-if-env-changed is awful, and recently the output has become even worse due to cargo:rustc-check-cfg spam.

I unfortunately don't have patience to see these features through, because opening issues and going through the process takes months or years, and this could have been fixed with this PR in a day.