-
Notifications
You must be signed in to change notification settings - Fork 27
Add no_std and no_alloc support
#61
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
Conversation
|
I believe the CI failure is due to the removal of the |
|
I think I've fixed the CI issue, and merging with main should fix the failure. Could you try to add your checks to I have a hokey copy of your branch that started doing this before I figured out I needed a larger overhaul of the coverage check (and then ran out of time this morning before the family woke up). You could glance at that for inspiration. |
Covers `portable-atomic` too
|
Thanks for working on this! I've merged in those changes you mentioned, and I believe I have a pair of |
| #[cfg(test)] | ||
| use std::println; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: Does this mean that std is required for testing?
Would it be possible to make at least some of the tests work for spin? Or am I wrong and you can run the tests without the std feature (but presumably with the std library used for testing)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately the way this library is setup right now it'd be a big change to run the tests on these platforms. The issue is once you compile tests, you bring in all of the dev-dependencies too, most of which aren't no_std friendly. And there is currently no way to mark a dev-dependency as optional. This also applies to examples.
A workaround could be to add a separate crate for no_std tests, or do the inverse and move the current tests into a std-tests crate. The important thing is no_std should be a subset of std, so as long as no_std compiles, and std passes tests, no_std should work as expected.
Objective
no_stdandno_allocinternmenton targets such asx86_64-unknown-noneandthumbv6m-none-eabiSolution
#[no_std]unconditionally 1stdandallocwhen features of the same names are enabled.stdandallocas required features of all features which fail to compile without them.coreand/orallocas required.spinfeature to provide aspinbased alternative tostd::sync::Mutex2portable-atomicandcritical-sectionfeatures to allowportable-atomicto provide compatibility to atomically challenged platforms such asthumbv6m-none-eabi3resolver = "2"to ensuredev-dependenciesdon't enablestdonserde4Testing
cargo check --no-default-features --target x86_64-unknown-none --features serde,arena,spin,intern,alloc,deepsizecargo check --no-default-features --target thumbv6m-none-eabi --features serde,arena,spin,intern,portable-atomic,critical-sectionRelease Notes
internmentcan now be used onno_stdplatforms. Simply disable default features, and enable any optional extras which themselves don't enable the newstdfeature:If you are compiling for a target without full support for Rust atomics, you may need to enable the
portable-atomicandcritical-sectionfeatures which aid in platform compatibility.Notes
cargo msrv, I believe these changes do not increase the MSRV of this crate.no_stdplatforms, so this is a prerequisite for our usage ofinternment. I also believe adding this support is generally beneficial for the broader Rust community without substantially increasing maintenance burden on crate maintainers.x86_64-unknown-noneandthumbv6m-none-eabitargets to your CI tests with the relevant features enabled/disabled to ensure compatibility does not regress. I am not familiar with this particular CI system so I'd rather leave that out of this PR, but I am happy to investigate adding it if that's desirable!Footnotes
Adding
#[no_std]using something like#![cfg_attr(not(feature = "std"), no_std)]will cause the implicit prelude to change based on the features used, which can make development harder than it needs to be. ↩If the library is compiled without
stdorspinacompile_error!is thrown indicating the user must enable one of those features. This allows thespindependency to only come into theCargo.lockif it is explicitly enabled. ↩Keeping this as an optional feature ensures it is only included in
Cargo.lockwhen explicitly enabled. ↩Without
resolver = "2", feature unification causes the features enabled bydev-dependenciesto be included in normaldependencies, which makesno_stdsupport impossible without removing examples, benches, and/or tests. ↩