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 no_std support and CI check for embedded #6

Merged
merged 8 commits into from
Apr 18, 2020

Conversation

PTaylor-us
Copy link
Contributor

I am pretty new to Rust and am using it for embedded. I was unable to compile this library for an embedded target even when supplying the --no-default-features build option as seen below:

(using rustc 1.44.0-nightly (94d346360 2020-04-09))

> cargo +nightly build --target thumbv7em-none-eabihf --no-default-features
   Compiling standback v0.2.2 (C:\Users\Peter\.cargo\registry\src\github.com1ecc6299db9ec823\standback-0.2.2)
error[E0463]: can't find crate for `std`
  |
  = note: the `thumbv7em-none-eabihf` target may not be installed
error: aborting due to previous error
For more information about this error, try `rustc --explain E0463`.
error: could not compile `standback`.
To learn more, run the command again with --verbose.

I added a conditional #![no_std] in the crate root (not(std)) and also as well as in 1_42.rs where needed. In addition, I added an embedded target for the check performed with the --no-default-features flag. It is my understanding that this is one option to ensure that the check actually fails if not no_std. It does appear that rustc versions before 1.42 will still fail. I'm not sure what would be required to get those working.

@jhpratt
Copy link
Owner

jhpratt commented Apr 17, 2020

Looks like it's failing for a simple reason — #[cfg_attr(not(std), no_std)] should be replaced with #[cfg(std)]. #![no_std] (note the !) is something used at the crate level to make it work without the standard library. What we need to do here is conditionally enable the API — in this case if the standard library is enabled.

As for CI, I'm actually going to opt just have a single check for an embedded environment, rather than check everything on that alone. This is especially important given it's a tier 2 target.

Would you prefer to make these changes yourself, or would you like me to do them? It's mostly trivial replacement, but it's still a learning experience 🙂 CI can be tricky, though.

@PTaylor-us
Copy link
Contributor Author

PTaylor-us commented Apr 18, 2020

Looks like I got a little copy/paste happy. I would love to make the changes. Thanks.

@PTaylor-us
Copy link
Contributor Author

I'm going to keep working on this.

@PTaylor-us
Copy link
Contributor Author

Ok, I think that is it. You said you wanted just a single test with the embedded target, but I just added a duplicate cargo check --no-default-features with the --target argument using the matrix. It was definitely helpful to have it test all the versions with the embedded target, but may be a bit redundant to run it on all the OSs.

@PTaylor-us
Copy link
Contributor Author

These changes are required to resolve time-rs/time#246

@jhpratt
Copy link
Owner

jhpratt commented Apr 18, 2020

Thanks! I'll take a look at the CI bit later today; that's not a big deal.

@jhpratt
Copy link
Owner

jhpratt commented Apr 18, 2020

Superfluous CI failure, given that commit was only bumping version. Merging.

@jhpratt jhpratt changed the title Add no_std support and CI check Add no_std support and CI check for embedded Apr 18, 2020
@jhpratt jhpratt merged commit 4f219e2 into jhpratt:master Apr 18, 2020
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.

2 participants