rust-lang / pkg-config-rs

Build library for invoking pkg-config for Rust
https://docs.rs/pkg-config
Apache License 2.0
173 stars 79 forks source link

Add an API that can be compatible with cargo:error #167

Open kornelski opened 1 month ago

kornelski commented 1 month ago

Currently this crate recommends .unwrap():

pkg_config::Config::new().atleast_version("1.2.3").probe("foo").unwrap();

but the problem with this is that Rust and Cargo present unwrapped error in an ugly and noisy way. Users often struggle to find the relevant messge among all the cargo:rerun-if-env-changed= lines. If they have RUST_BACKTRACE=1 enabled, the error is also buried under an irrelevant backtrace.

I hoped that this could be fixed by emitting cargo:error=reason that would replace the noisy dump with a clear user-facing message. However, the current cargo:error implementation is incompatible with the .probe() call, because any printed error will fail the whole build script as a side effect, even if the caller handles the probe error gracefully and the script succeeds. It can't even be hacked by sneaking in a cargo:error= into the unwrapped pkg_config::Error's message, because unwrap prints to stderr, and cargo:error must be in stdout.

Therefore, I suggest adding a new API like .probe_or_exit(), that will print cargo:error=message on failure, and exit(101) the build script cleanly instead of panicking, so that Rust doesn't append unnecessary info about panicking and backtraces.

pkg_config users can do something like that themselves, here's an example, but 700+ crates are unlikely to make such improvements when .unwrap() is easier, and recommended by pkg_config.

kornelski commented 1 month ago

Even without cargo:error support, such API could improve output significantly. That's because the noisy rerun-if-env-changed directives are not needed when the build fails (a failed build will always be retried, whether envs changed or not). This would let pkg-config remove the log noise that cargo won't.