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

feat: support no_std #266

Merged
merged 8 commits into from
Feb 13, 2024
Merged

feat: support no_std #266

merged 8 commits into from
Feb 13, 2024

Conversation

GZTimeWalker
Copy link
Contributor

This is only a early pr for support no_std.

All tests are passed. Use core and alloc instead of std.

There should be some better ways to disable serialization tests when no_std.

For issue #265

@GZTimeWalker GZTimeWalker marked this pull request as ready for review November 26, 2023 05:22
@jf2048
Copy link

jf2048 commented Jan 20, 2024

Instead of having all these separate #[cfg(...)] in the tests, the entire file can be disabled by putting #![cfg(feature = "std")] at the top.

I noticed bytemuck and byteorder are only used for serialization, and serde depends on the serialization. So IMO the Cargo.toml can be changed to this:

[dependencies]
bytemuck = { version = "1.7.3", optional = true }
byteorder = { version = "1.4.3", optional = true }
serde = { version = "1.0.139", optional = true }

[features]
default = ["std"]
serde = ["dep:serde", "std"]
simd = []
std = ["dep:bytemuck", "dep:byteorder"]

So a build with no default features will have no dependencies besides the alloc crate 😃

@GZTimeWalker
Copy link
Contributor Author

Since no one has responded to this PR for a long time, I wonder if there is any plan to continue support no_std?

@jf2048
Copy link

jf2048 commented Jan 20, 2024

No commit has been made in 5 months, probably the maintainers are busy.

@GZTimeWalker
Copy link
Contributor Author

No commit has been made in 5 months, probably the maintainers are busy.

ok, I'll take your suggestions, thanks!

@GZTimeWalker
Copy link
Contributor Author

@Kerollmops Hello! Will no_std support be in the plans?

Copy link
Member

@Kerollmops Kerollmops left a comment

Choose a reason for hiding this comment

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

Hey @jf2048 and @GZTimeWalker 👋

Sorry for the long response time. I was, indeed, swamped working on meilisearch/heed and meilisearch/Meilisearch.

Your changes look good to me overall! However, you'll probably encounter formatting issues in the CI. I advise you to do a small cargo +nightly fmt before pushing to this PR again 😃

Thank you very much 🎉

Cargo.toml Outdated Show resolved Hide resolved
note: all tests passed
@GZTimeWalker
Copy link
Contributor Author

GZTimeWalker commented Feb 13, 2024

I tested the latest version for all dependencies, all works fine and all tests passed, so I locked them to latest.

note that this pr has already run cargo +nightly fmt

@Kerollmops
Copy link
Member

Thank you very much @GZTimeWalker.

I am in the process of bumping the MSRV and fixing the SIMD feature in #267. Would it be possible to rebase on the main once I merge it, please? After that, I will merge yours and #264 to create a new release.

@GZTimeWalker
Copy link
Contributor Author

Thank you very much @GZTimeWalker.

I am in the process of bumping the MSRV and fixing the SIMD feature in #267. Would it be possible to rebase on the main once I merge it, please? After that, I will merge yours and #264 to create a new release.

Sure.

@Kerollmops
Copy link
Member

Can you rebase on main, please? 😃

@GZTimeWalker
Copy link
Contributor Author

Can you rebase on main, please? 😃

Ok, wait a second

@GZTimeWalker
Copy link
Contributor Author

Merged main into no_std.

@Kerollmops
Copy link
Member

Thank you very much for this addition 🎉

@Kerollmops Kerollmops merged commit a2aa2a5 into RoaringBitmap:main Feb 13, 2024
4 checks passed
@GZTimeWalker GZTimeWalker deleted the no-std branch February 13, 2024 17:36
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.

3 participants