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

Chore(no_std): fix various compilation errors for no std crates #9564

Conversation

martinezjorge
Copy link
Contributor

@martinezjorge martinezjorge commented Jul 17, 2024

This PR closes #9478 Fix various compilation errors for no-std in crates.

After reading around, it seems that if a crate is going to support no_std, it should make std opt in. This is the official recommendation because it saves the work of having to make things support no_std after having developed everything with stable std features.

I went through all the crates running the command below:

cargo c --no-default-features -p <crate_name>

and the following crates had compilation errors:

Crates with compilation errors

  • reth-consensus
  • reth-evm-ethereum
  • reth-ethereum-forks
  • reth-evm
  • reth-execution-errors
  • reth-network-api
    • When running checking the crate dependencies with no default features, like so, cargo c --no-default-features -p reth-network-api, I'm getting the following error:
      • use of undeclared crate or module serde
    • In order to have serde support no_std, the std feature has to be configured to default-features = false.
    • Additionally, serde-json-core must be used instead of serde-json.
      • This is tricky because according to the serde docs:
      • Be aware that Cargo features are unioned together across your entire dependency graph. That means if any other crate you depend on has not opted out of Serde's default features, you will build Serde with the std feature enabled whether or not your direct dependency on Serde has default-features = false.
      • So it seems in order to fix the compiler errors in reth-network-api I may have to go through all the crates that reth-network-api depends on. I hope it doesn't include external crates reth depends on, as just making these changes seems like it could affect a large part of the codebase.
  • reth-primitives
  • reth-primitives-traits
  • reth-revm
  • reth-rpc-types
    • This one seems tough, not sure how to replace the ErrorObject
    • Its trying to convert an error into an a jsonrpsee ErrorObject
    • Could I emit the error without converting into an RPC error?
  • reth-storage-errors

Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ty,

can we do this in incremental steps, starting with the primitives-traits crate first?

Comment on lines 19 to +20
thiserror-no-std = {workspace = true, default-features = false }
thiserror.workspace = true
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this defeats the purpose of the thiserror-no-std crate, we only need thiserror-no-std here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh ok, I thought that due to the feature gates on the imports that thiserror-no-std would get used when std is not allowed and thiserror would get used when std is active.

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.

Fix various compilation errors for no-std in crates
2 participants