Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
kornelski opened this issue Sep 11, 2024 · 1 comment
Open

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

kornelski opened this issue Sep 11, 2024 · 1 comment

Comments

@kornelski
Copy link
Contributor

kornelski commented Sep 11, 2024

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
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant