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

No way to enable serde for no-std builds #904

Closed
ass3rt opened this issue Mar 24, 2022 · 5 comments
Closed

No way to enable serde for no-std builds #904

ass3rt opened this issue Mar 24, 2022 · 5 comments

Comments

@ass3rt
Copy link

ass3rt commented Mar 24, 2022

Since Serde depends on std/alloc by default, and conditional features are not possible in the manifest, there is presently no way to enable the serde feature for no-std builds.

Note: if you enable 'serde', it will attempt to compile std, even if you specified a no-std build. This will outright fail on targets which do not have std available. Secretly pulling in std sounds like an issue of its own, but not the focus of this ticket.

@tcharding had a suggestion (#896 (comment)) to let a dependent crate disable default features, and explicitly enable the necessary sub features; e.g. serde/alloc in this case. Unfortunately, cargo does not allow specifying sub-crate features, unless those features are explicitly reexported 🤮 :

error: failed to parse manifest at `/home/user/repos/embedded-dev/test-cortex-m/Cargo.toml`

Caused by:
  feature `serde/alloc` in dependency `bitcoin` is not allowed to contain slashes
  If you want to enable features of a transitive dependency, the direct dependency needs to re-export those features from the `[features]` table.

Instead of exhaustively reexporting sub-crate features, I think it would make sense in this case (and possibly others?) to replace the use-serde feature with two variants for std and no-std builds:

serde-std =    ["serde", "bitcoin_hashes/serde", "secp256k1/serde", "serde/std"]
serde-no-std = ["serde", "bitcoin_hashes/serde", "secp256k1/serde", "serde/alloc"]

I have made this change on my fork, if anyone would like to see it in action: ass3rt@cc35a6e

Does this sound like a reasonable change?
Would it make sense to do this for other features as well? (e.g. #830)
Does anyone have a better idea how serde can be enabled on a pure no-std build?

@tcharding
Copy link
Member

tcharding commented Mar 24, 2022

I think we can solve your problem by disabling default-features in the serde dependency of rust-bitcoin. Then you can enable std/alloc in the serde dependency of the crate using rust-bitcoin and because features are cumulative [1] you get the features you want.

For example this toy crate builds, Cargo.toml:

[package]
name = "play"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
bitcoin = { path = "/home/tobin/build/github.com/tcharding/rust-bitcoin", default-features = false, features = ["no-std"] }
serde = { version = "1", default-features = false, features = [ "derive", "alloc" ] }
serde_json = "1.0.45"

And main.rs

use serde::Serialize;

#[derive(Debug, Copy, Clone, Serialize)]
struct Dummy{
    x: u32
}

fn main() {
    let d = Dummy {
        x: 42,
    };

    let json = serde_json::to_string(&d).unwrap();
    println!("{}", json);
}

Does that work for you?

[1] - I don't know the correct term for this but I mean features enabled in one place are enabled for the whole build.

@ass3rt
Copy link
Author

ass3rt commented Mar 24, 2022

Ah yes, this would work 👍

The trade-off is that other projects will lose serde/std unless they do a similar trick. I suspect most projects would be unaware, and just continue using use-serde without knowing they lost serde/std, but its worth noting.

@ass3rt
Copy link
Author

ass3rt commented Mar 24, 2022

I've opened a PR to disable Serde's default-features: #905

@DanGould
Copy link
Contributor

I believe this issue may be closed

@tcharding
Copy link
Member

Thanks Dan! Had a quick squiz at our toml and a few serde tomls - all looks sane from my brief look.

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

No branches or pull requests

4 participants
@DanGould @tcharding @ass3rt and others