jonasbb / serde_with

This crate provides custom de/serialization helpers to use in combination with serde's `with`-annotation and with the improved `serde_as`-annotation.
https://docs.rs/serde_with
Apache License 2.0
665 stars 72 forks source link

no_std support #435

Closed pwnorbitals closed 2 years ago

pwnorbitals commented 2 years ago

It looks like this crate doesn’t support no_std, making it harder to serialize long byte arrays in embedded environments. Is this on the roadmap ?

jonasbb commented 2 years ago

Hi @pwnorbitals, no_std support is currently not on the roadmap, since nobody requested it before. However, I gladly review PRs adding support and helping implement that feature. I never used no_std, so I could really use some help setting up features or autodetecting the target support and testing.

Generally, I don't see a problem with no_std support. serde fully supports no_std and rustversion can be replaced with a build script to check the compiler version. The newly stabilized weak dependency features should also help here. All other crates are already feature gated, so that those implementations should not cause any problems. Adding a std feature and gating existing functionality behind it is as far as I know a breaking change.


Could you maybe expand a bit on what exactly you need or in general expect from a no_std crate? Do you need core, core+alloc or both? Are there any SerializeAs implementations more important? I guess, each implementation could be made no_std compatible one by one.

jonasbb commented 2 years ago

I started working on this in #444. So far, I only changed the imports and feature gated imports and implementations. The changes so far seem relatively unproblematic to me and can probably be merged anyway as future preparations. To really support no_std properly are two main questions to answer and some more work updating macros and dependencies.

  1. What is the target where to use no_std? Is it only core or core+alloc.
  2. I need help from someone who understands no_std how to test it. Otherwise, it is not possible to ensure that macros or dependencies do not depend on std in some way. I looked at serde, but they only have minimal tests for no_std and also no dependencies which could interfere.
cbeck88 commented 2 years ago

I ran into this today too.

Core only is best, Core + alloc will still be fine for many no_std projects. Generally the more you can minimize your dependencies the better it is for your users.

I can try to offer a PR to help with this, I have a lot of experience moving stuff to no_std

cbeck88 commented 2 years ago

It looks like you already did a lot of work on this in this commit in master, thank you! https://github.com/jonasbb/serde_with/commit/c9d92269fd493b467ae39ee44b1de92c325669a1

At this point you are still bringing in extern crate std unconditionally? https://github.com/jonasbb/serde_with/blob/7d9aebc1096903ea4e29ccdb6ae2f95e897cafde/serde_with/src/lib.rs#L267

For my 2 cents, I think it is worth it to have an std feature and allow to turn off std for serde_with, even if the macros don't fully work that when it is configured that way yet. Because the crate is quite useful even without the macros, and we could make the macros work in a later release of the macros crate. That would have worked for my use-case anyways.

jonasbb commented 2 years ago

Thanks for offering to help @garbageslam The most helpful would be to assist with testing. I can only support no_std in as much as it is covered by tests. PRs would of course be welcome, but even just blog post / projects which show good no_std testing would be extremely useful. I already looked at serde, but it is minimal and in no way shows how to extend it for external crates. Ideally, the existing test suite could be used at least for cargo build/check.

Without some testing, I am not sure how much can be supported. Maybe no optional dependencies and no macros could be supported. That might be similar enough to serde to copy the testing. Optional dependencies would then enable the std feature and macros be gated by it.

Core only is best, Core + alloc will still be fine for many no_std projects. Generally the more you can minimize your dependencies the better it is for your users.

This crate only depends on serde. All other dependencies are optional. Having an alloc and a std feature, like serde, is likely the most useful option.

At this point you are still bringing in extern crate std unconditionally?

Yes, removing the std dependency or putting it behind a feature flag is a breaking change. This combination of no_std + extren crate was the best to use the core prelude without introducing a breaking change.

cbeck88 commented 2 years ago

projects which show good no_std testing would be extremely useful

The strategy they used for testing no_std support in serde-cbor is here: Basically, they pick a supported rustc target that doesn't have an std and try to build for it. Then, they also try running the tests with default-features = false.

https://github.com/pyfisch/cbor/blob/347a3f096d5a1e8e380b9968e74838c9037a5b14/.travis.yml#L22

That's probably the easiest way to get coverage.

An alternative might be like, try to compile things with the nightly compiler, and try to define a lang item like "alloc error handler": https://github.com/mobilecoinfoundation/mobilecoin/blob/fc7936fc960471654f9015e8d6f49799e8588289/sgx/alloc/src/lib.rs#L34 This will cause a build failure if anything in your build is pulling in std, because std also supplies this lang item, and the compiler enforces that there is at most one definition of this lang item in the whole program. (That's usually the build failure I hit when something pulls in std that's not supposed to.) But this can only be done with the nightly compiler. I'm not sure any of these lang items are considered "stable", so that may deter you. The serde-cbor approach is probably simpler.

Yes, removing the std dependency or putting it behind a feature flag is a breaking change. This combination of no_std + extren crate was the best to use the core prelude without introducing a breaking change.

So, I would double tap on this:

Dropping a dependency is never a breaking change. The world we are going to as an ecosystem is, std is just another dependency. However, it could be a breaking change if we gate std on a feature, and some of the types that were previously part of the public API become gated on the std feature. If that feature becomes off-by-default, then everyone who currently depends on you might not get some of the types your crate was exporting unless they reconfigure their Cargo.toml to turn the feature on.

A lot of crates introduce no_std support in the following way:

This is exactly what serde did when they first added no_std support: https://github.com/serde-rs/serde/blob/2e38e2bf2fb718dda4418ac1998b8441c3bc1d30/serde/Cargo.toml#L33

And a lot of other crates copied that pattern: https://github.com/pyfisch/cbor/blob/347a3f096d5a1e8e380b9968e74838c9037a5b14/Cargo.toml#L27 https://github.com/tokio-rs/prost/blob/8576e5d5504412705877a02ae6eecaa8851dc90a/Cargo.toml#L44 https://github.com/tokio-rs/bytes/blob/b8d27c016f53f0c1fea920223bc5a92f329d47df/Cargo.toml#L21

Then, people like me would import your crate like

serde_with = { version = "0.14", default-features = false }

so that I opt in to turning off std.

So, I think a viable way to evolve serde_with would be, first do this same pattern with an std feature and gating of things on it, and make the feature on by default. Then in a second step, make the macro crates reference appropriate types in serde_with and/or use feature gating on various macros, so that that crate is also no_std compatible.

jonasbb commented 2 years ago

Thanks for the cbor reference. That seems quite useful. That works as a starting point: testing --no-default-features and --no-default-features --features=alloc. Some extra testing for all the features would also be good. Probably time to look into cargo hack and see if that makes sense here.

So the steps I see which need to be done are:

  1. Add another CI job to ci.yml testing with both --no-default-features and --no-default-features --features=alloc. The job uses a target without std.
  2. Add alloc and std features. Enable std by default. std should probably also enable alloc. Both features also need to be forwarded to serde.
  3. Feature gate all imports and trait implementations on either alloc or std.

After this, the CI job should pass.

Each optional feature/dependency also needs a check. This means more feature gating where possible and forwarding alloc/std to dependencies. Where dependencies are not no_std compatible including the dependency should just enable the std feature.

Dropping a dependency is never a breaking change.

Dropping a dependency might not be a breaking change, but loosing trait implementations for alloc and std types is.

cbeck88 commented 2 years ago

Dropping a dependency might not be a breaking change, but loosing trait implementations for alloc and std types is.

Yes true. But you can put std feature on by default, and then it's not a breaking change

jonasbb commented 2 years ago

I created a new PR with the changes as explained above https://github.com/jonasbb/serde_with/pull/469

Currently, this is very bare bones, but ready for testing. I would be very happy to get some feedback if this works or if there is some problem using it in a no_std project @garbageslam This is still missing updates to all optional dependencies and macros are gated behind std for now. But these can be changed separately.


Edit: I merged the initial PR and also a PR to enable support for dependencies, as good as possible. Implementation wise, this should be the bulk of changes. I would still like some feedback if the implementation works for you or if there are any problems.

jonasbb commented 2 years ago

I released new version of serde_with with the changes to support no_std. I am still hoping for some feedback, either positive (i.e, everything works) or negative (i.e., something incompatible with no_std). If you have any problems, please open a new issue.