Cache whether RUST_BACKTRACE is enabled in a relaxed atomic static.

[eddyb]

Fixes #207. Inspired by libfringe (thanks, @edef1c!).

[eddyb]

Apparently there's a test that changes RUST_BACKTRACE at runtime.
The intention seems to be to check a few potential RUST_BACKTRACE values, shouldn't it be done by running a different executable via std::process?

[Yamakaky]

It's an edge case, I think a subprocess would be fine.

[Arnavion]

Perhaps split it up into separate tests (one for each RUST_BACKTRACE value) rather than actual subprocesses? nvm, I see the flag is static, so it will apply to all tests in the binary.

[faern]

Would it not make sense to implement this with std::sync::Once instead of manually having to encode whether or not the environment variable has been loaded? But maybe that has noticeably worse performance?

[faern]

I have created a PR on @eddyb 's fork that fixes the failing test by running it as a subprocess. eddyb#1

I have verified it works on Linux, but I have not tested other platforms. I wrote the code with a special case for Windows, but I have not verified that it actually runs.

[eddyb]

Thanks, @faern! ping @Yamakaky

[Yamakaky]

Maybe add that this bin should not be used outside of the tests?

[eddyb]

I would use cargo run and maybe put it somewhere cargo wouldn't pick up by default (maybe --example?). I can try that if it sounds good.

[Yamakaky]

Nice! Could you update the changelog?

[faern]

I added PR for updating change log: eddyb#2

[eddyb]

I still have to play with cargo run, unless @faern gets to it first 😆

[faern]

What is it that you are trying to solve? Moving has_backtrace.rs out of bin/ so it is not automatically picked up and built by cargo test? examples/ does not sound like an optimal place to put it IMO. examples/ is also automatically picked up by cargo test, plus users looking there expect to see examples of how to use the library.

[faern]

Manually compiling it has the downside of us not knowing how the user has configured their toolchain. Just running rustc ... might not be on the PATH or might not be what the user want to run. Maybe they have invoked the tests with cargo +beta test which is something a hardcoded cargo .../rustc ... would miss.

[eddyb]

Hmm in those cases I don't know how to actually have a separate process. Then again, this is a test so if cargo run doesn't work...

[faern]

I still didn't get what the problem we are trying to solve is? Is it too ugly to have it in bin/? Even if we rename it to test_has_backtrace.rs or internal_test_helper__do_not_use__has_backtrace.rs? :D

[eddyb]

It doesn't work. Look at the test failure. It worked for you only because you built it manually.

[faern]

Hmm. What? cargo clean && cargo test works for me. I didn't build anything manually. There is only one build failing, and it looks like that is because of some syntax error in the backtrace crate.

[eddyb]

@faern Huh, my bad, guess it... fixed itself? Really weird since the last failure I saw was not finding that executable. I'll try to rebase then.

[faern]

It looks like we have to bump the minimum required version from 1.13.0 to 1.14.0. Looks like backtrace 0.3.3 won't build on stable Rust 1.13.0. I verified locally that it builds on 1.14.0

[faern]

Ping @Yamakaky :)

[Yamakaky]

Yeah, it's good !

Good coop btw, keeping an eye on it ;)

[Yamakaky]

Are you good for merge?

[eddyb]

I think so.

[Yamakaky]

I'm just thinking this piece of code could be reused in others libs. Something like LazyBool or something like that.

[faern]

I agree that this could me generalized. Or at least refactored into a separate function, since it's fairly large.

[Yamakaky]

Manually merged. Thank you two!