Print & color build script warnings, even when build script panics

[mystor]

Currently, when the build script panics, the stdout of the build script is written out to show what happened. However, this means that any warnings emitted during the build script (using cargo:warning= from #2630) won't be printed nicely like they are when the build script succeeds.

It would be nice if these warnings could be printed prettily even when the build script fails to execute due to a panic.

[alexcrichton]

Seems reasonable to me! Do you have a preference either before or after the standard error output?

[mystor]

I feel like putting the warning before the panic dump is a good idea, because conceptually the warnings were "emitted" by the build script before the build script "emitted" the panic. I'm open to other options though.

[alexcrichton]

Seems reasonable to me. Unfortunately I probably won't have time to implement this in the near future, but I believe the relevant loop is this one. We likely just need to listen for more messages coming on self.rx and print them out in the same fashion.

[aleksator]

I did some investigation on this and here's what I've found:

The warnings get printed in emit_warnings() from build_script_outputs variable, but when build script panics it is empty. Since it does not contain warnings or anything else, nothing gets printed at this point.

As to why that happens:
build_script_outputs gets filled in build_work() function, but when a panic happens an error is thrown in exec_with_streaming(), so build_work() quits early on question mark here and filling of build_script_outputs isn't reached.

The output with warnings is still contained inside the error returned by exec_with_streaming() and is printed later in stack trace in a raw form of --- stdout cargo:warning=*warning from build script*.

At this point I would like to get the team's input on what would be the cleanest way to push warnings into build_script_outputs in unhappy path, and whether we would need to solve the resulting duplication of warnings in colored and raw forms.

[ehuss]

Yea, that's a little awkward. I think what I would do is inside the stdout callback, check for lines that start with cargo:warning= and stash the line into a Vec of warnings. If exec_with_streaming returns an error, it can take those warnings and generate an empty BuildOutput, and set the warnings field and insert the BuildOutput into the map. I think that should then get picked up by emit_warnings, but not interfere with anything else.

Also, an unrelated thing: it might be nice for build script warnings to be prefixed with the package name/version (the same way it is prefixed with -vv). I notice that the warning doesn't indicate where it comes from, so that might be useful.

[aleksator]

@ehuss I've done the suggested implementation

Also, an unrelated thing: it might be nice for build script warnings to be prefixed with the package name/version (the same way it is prefixed with -vv). I notice that the warning doesn't indicate where it comes from, so that might be useful.

Maybe worth creating a separate issue for this?

[ehuss]

Maybe worth creating a separate issue for this?

Sure, posted #8018.