jamesmunns / postcard

A no_std + serde compatible message library for Rust
Apache License 2.0
929 stars 88 forks source link

Check that target supports atomic pointer operations before defining max size for Arc #168

Closed sjudson closed 3 months ago

sjudson commented 3 months ago

Resolving #167 cc: @jamesmunns

When building with, e.g., --target riscv32i-unknown-none-elf and -F alloc on the current main branch, the build will fail with

error[E0432]: unresolved import `alloc::sync`
 --> source/postcard/src/max_size.rs:5:33
  |
5 | use alloc::{boxed::Box, rc::Rc, sync::Arc};
  |                                 ^^^^ could not find `sync` in `alloc`

due to the lack of support on that target for atomic pointer operations (see: https://doc.rust-lang.org/alloc/sync/index.html).

Following the recommendation from the Rust docs, this gates definition of POSTCARD_MAX_SIZE for alloc::sync::Arc behind a configuration check to confirm it is available on the target first.

netlify[bot] commented 3 months ago

Deploy Preview for cute-starship-2d9c9b canceled.

Name Link
Latest commit 774ed3aab1c65e331cdd7b158df4256f7b8c6fe8
Latest deploy log https://app.netlify.com/sites/cute-starship-2d9c9b/deploys/66c61f8ad7fd210007941ebf
jamesmunns commented 3 months ago

Gotcha! And you can confirm this resolves the issue you saw in #167? If so, I'll merge and release after CI is green.

jamesmunns commented 3 months ago

Also @sjudson if you'd like to add a test for this arch (rv32 + alloc) to the CI so it doesn't regress in the future: https://github.com/jamesmunns/postcard/blob/main/ci.sh feel free to add it to this PR!

jamesmunns commented 3 months ago

I went ahead and added it :)

sjudson commented 3 months ago

Thanks!