search

rust-lang / rustwide

Execute your code on the Rust ecosystem.
Apache License 2.0
180 stars 41 forks source link

show stderr output on failed command #10

Closed jyn514 closed 4 years ago

jyn514 commented 4 years ago

this would have made debugging docs.rs much easier

pietroalbini commented 4 years ago

I'm not sure this is something we want to do: while for simple commands this is fine, when a build fails all the output would be duplicated, with a huge error message. I'd prefer for downstream users to avoid hiding the output when the command might fail.

jyn514 commented 4 years ago

What if we only printed the error if log_output is set to false? That wouldn't require any upstream changes and would also avoid duplicate output.

pietroalbini commented 4 years ago

Do you happen to remember which log_output(false) made debugging worse for you? It's supposed to be used only when the output is not relevant at all.

jyn514 commented 4 years ago

I think it might have been https://github.com/rust-lang/docs.rs/blob/master/src/docbuilder/rustwide_builder.rs#L118 and rustup was complaining about a toolchain that wasn't installed? Don't quote me on that.

I still this Rustwide is the right place to fix this so upstream doesn't have to guess which commands could ever fail.

pietroalbini commented 4 years ago

This solution is not consistent though. The output is collected only when run_capture is called, so the error message is not shown most of the times. If we fix that by always storing the output then we get into problems when a program outputs a bunch of stuff and we waste memory.

I don't think it's worth implementing it into rustwide.