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

Implement Cargo feature: prohibit_backtrace #256

Closed
wants to merge 1 commit into from

Conversation

vi
Copy link

@vi vi commented Aug 11, 2022

Resolves #239.

Copy link
Owner

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A cfg(all(…, not(feature = "prohibit_backtrace"))) feature gate is not the right implementation strategy for this. Cargo features are required to be additive; a Cargo feature is not supposed to remove something from the API. The Cargo feature added in this PR removes anyhow::Error::backtrace when enabled. That means if one crate enables feature = "backtrace" and uses the backtrace method, and a different crate enables feature = "prohibit_backtrace", each one will compile in isolation but will break when any project tries to introduce a transitive dependency on both those crates. This is harmful for the crates.io ecosystem because it breaks composability.

From reading your rationale in #239, it sounds like the PR is motivated by wanting to use anyhow in projects that build with -Zbuild-std-features=panic_immediate_abort. (Because anyone not using -Zbuild-std-features=panic_immediate_abort is gonna get the backtrace infrastructure regardless of your PR, because the standard library uses it.) I think I would recommend that those projects just use a different library. I am not really interested in scrutinizing the rest of this library for code size (the vtables, the use of core::fmt) at the level that would make it truly good for code size constrained use cases.

@vi
Copy link
Author

vi commented Aug 13, 2022

each one will compile in isolation but will break when any project tries to introduce a transitive dependency on both those crates

Doesn't anyhow have a stub empty backtrace class for cases where backtrace support is not enabled? I supposed it gets activated, so that libraries speaking to each other in terms of anyhow can continue to rely on existance of Backtrace - it will just never be actually capturable.

@vi
Copy link
Author

vi commented Aug 13, 2022

I think I would recommend that those projects just use a different library.

Libraries (or project components) can talk to each other in terms of anyhow::Error (when thinking about proper error handling by thiserror is skipped) and the min_sized_rust-style builds can be only one of building modes, with default being debug-friendly build with backtraces enabled.

@vi
Copy link
Author

vi commented Aug 13, 2022

Is anyhow in general intended to have optional backtrace support (even when native backtracing will be stable)?

Or is anyhow is designed to have backtracing as a core, unconditional feature in mind - cfgs are only a temporary measure? And once native backtracing gets stable, anyhow would remove dependency on backtrace crate and all those cfgs would go away?

@dtolnay
Copy link
Owner

dtolnay commented Aug 29, 2022

It will be unconditional on toolchain versions that have a stable backtrace implementation.

@purplesyringa
Copy link

purplesyringa commented Feb 7, 2024

I am not really interested in scrutinizing the rest of this library for code size (the vtables, the use of core::fmt) at the level that would make it truly good for code size constrained use cases.

Actually, I've got a few projects where disabling backtrace reduces the binary size two-fold. So perhaps that would be a reasonable avenue to pursue, if anyone is interested

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

Successfully merging this pull request may close these issues.

Cannot opt out of backtrace feature on nigthly.
3 participants