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

no_std support #563

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

no_std support #563

wants to merge 1 commit into from

Conversation

olanod
Copy link

@olanod olanod commented Jul 28, 2022

This PR turns the crate into no_std compatible replacing std references with core + alloc and adding the std feature where needed(std::error::Error support).
Also tried to make the dependency on HashMap optional making extensions an optional feature, the ahash dependency shouldn't be an issue since it's already a dependency of hashbrown which is the the std HashMap implementation.

Extensions are also made an optional feature to not require depending on HashMap
@jplatte
Copy link
Contributor

jplatte commented Sep 8, 2022

TypeId implements Ord, so how about using BTreeMap for extensions if there is no std, rather than not having it?

@algesten
Copy link

This looks really good. Wonder if the crate authors will get time to consider it?

@algesten
Copy link

algesten commented Oct 1, 2023

Hm. I guess it doesn't get rid of allocations cause it relies on the extern crate alloc

@seanmonstar seanmonstar added this to the 1.0 milestone Nov 10, 2023
@seanmonstar seanmonstar added the B-breaking Blocked: breaking change. This requires a breaking change. label Nov 10, 2023
@seanmonstar
Copy link
Member

I'm not sure about getting this into 1.0, but I do want to allow for it. I think the only breaking change would be to add a std feature that is default-on, yea? Then we could eventually fix up support with it off.

@algesten
Copy link

Yes. I got a feeling i've seen more std feature flags than the opposite no_std. Either way works though.

@seanmonstar
Copy link
Member

I created #637 which does that, allowing us to eventually add support later.

@seanmonstar seanmonstar removed this from the 1.0 milestone Nov 10, 2023
@seanmonstar seanmonstar removed the B-breaking Blocked: breaking change. This requires a breaking change. label Nov 10, 2023
@olanod
Copy link
Author

olanod commented Jul 26, 2024

What's the current feeling about this feature, any must haves besides the basic support I initially added?

@bstrie
Copy link

bstrie commented Jul 29, 2024

What's the current feeling about this feature, any must haves besides the basic support I initially added?

@olanod I see that this PR takes into account that error::Error is only in std, not in core. However, core::error::Error will be stable in Rust 1.81 (releasing September 5), so that might make this easier to land... although then we have to consider bumping the MSRV, and this crate's MSRV is currently an extremely conservative 1.49 (based on a former MSRV of Tokio).

So it may be worth asking @seanmonstar what he thinks about either 1) accepting this as-is with Error-in-std-but-not-in-core, or 2) thinking about waiting until Tokio bumps their MSRV to at least 1.81 (which will take at least 6 months). In the event of the latter, maybe we could get no_std support merged on a beta release branch of the HTTP crate? no_std users presumably don't care about the entire Hyper/Tokio ecosystem, so it would be useful to them to have this crate available on no_std even if it's a parallel release branch from that being used by Hyper/Tokio.

@seanmonstar
Copy link
Member

If I remember correctly, we made it so currently with feature disabled nothing compiles. So if we make it so some of it compiles without the feature, that's still just an addition. And adding the error impls back eventually would still be just an addition. Right?

@bstrie
Copy link

bstrie commented Jul 30, 2024

So if we make it so some of it compiles without the feature, that's still just an addition. And adding the error impls back eventually would still be just an addition. Right?

@seanmonstar That seems reasonable, yes. Although, if you're willing to allow the crate's MSRV to vary based upon whether or not the std feature is enabled, then another option would be to allow this feature to land while just conditionally importing either core::error::Error or std::error::Error, which seems reasonable at first blush.

sunfishcode added a commit to sunfishcode/http that referenced this pull request Jan 2, 2025
This PR is similar to other no-std PRs, however it takes the approach of
using the new [`core::error`] module in Rust 1.81. This means that no-std
mode has an MSRV of Rust 1.81, while the existing MSRV of 1.49 is still
supported for existing users, as suggested [here].

This PR also preserves semver compatibility, and avoids adding any new
dependencies or required features for existing users. And it avoids
modifying the tests and benchmark sources, as those don't need to be no-std.
And it avoids making any unrelated changes.

And, it adds CI coverage and README.md documentation.

[here]: hyperium#563 (comment)
[`core::error`]: https://doc.rust-lang.org/stable/core/error/index.html

Fixes hyperium#551.
@sunfishcode sunfishcode mentioned this pull request Jan 2, 2025
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.

5 participants