Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

use runtime-wasm feature instead of assuming no_std to be wasm #7275

Closed
wants to merge 2 commits into from

Conversation

brenzi
Copy link
Contributor

@brenzi brenzi commented Oct 7, 2020

fixes #5547

Main steps done:

  1. replace #[cfg(feature = "std")] by #[cfg(not(feature = "runtime-wasm"))] and the logical opposite
find . -type f -name "*.rs" ! -path "./target/*" -print0 | xargs -0 sed -i 's/#\[cfg(feature = "std")\]/#\[cfg(not(feature = "runtime-wasm"))\]/g' 
find . -type f -name "*.rs" ! -path "./target/*" -print0 | xargs -0 sed -i 's/#\[cfg(not(feature = "std"))\]/#\[cfg(feature = "runtime-wasm")\]/g' 
  1. introduce runtime-wasm feature in all tomls (probably that's too many as not all are needed for runtime)
find . -type f -name "*.toml" -print0 | xargs -0 sed -i 's/default = \[ "std" \]/default = \[ "std" \]\nruntime-wasm = \[\]/g' 
find . -type f -name "*.toml" -print0 | xargs -0 sed -i 's/default = \["std"\]/default = \[ "std" \]\nruntime-wasm = \[\]/g' 
  1. let wasm-builder-runner add the runtime-wasm feature to rustc args
  2. manual fixes, i.e. to gate RuntimeDebug derivation

this is WIP. not yet sure if the approach makes sense

@cla-bot-2020
Copy link

cla-bot-2020 bot commented Oct 7, 2020

@brenzi it looks like you have not signed our contributor license aggreement yet. Please visit this link to sign our agreement. This pull request cannot be merged until the agrement is signed.

@cla-bot-2020
Copy link

cla-bot-2020 bot commented Oct 7, 2020

@brenzi, Your signature has been received.

Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

In general the runtime-wasm feature should only be used where it is required!

This means mostly primitives-io, because most other crates are still using the no_std "legally" by making themselves compilable for no-std.

Everywhere where we have special code that assumes no_std == runtime-wasm, we would need to switch it to runtime-wasm.

@@ -6,5 +6,6 @@ fn main() {
.with_wasm_builder_from_crates("2.0.0")
.export_heap_base()
.import_memory()
.append_to_rust_flags("--cfg feature=\\\"runtime-wasm\\\"")
Copy link
Member

Choose a reason for hiding this comment

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

Why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

from the errors I concluded that the feature runtime-wasm isn't applied automatically by the build script. Is that wrong?

Copy link
Member

Choose a reason for hiding this comment

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

When the feature is present in the cargo.toml, the wasm-builder should activate it automatically.

@@ -23,5 +23,6 @@ fn main() {
.with_wasm_builder_from_crates_or_path("2.0.0", "../../../utils/wasm-builder")
.export_heap_base()
.import_memory()
.append_to_rust_flags("--cfg feature=\\\"runtime-wasm\\\"")
Copy link
Member

Choose a reason for hiding this comment

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

?

@brenzi
Copy link
Contributor Author

brenzi commented Oct 7, 2020

I just tried to reset and try bottom up starting with sp-io only. However, the errors explode and I think it takes someone with deep knowledge of the code to distinguish between no_std == wasm assumptions and legit std switches.

Try and err will lead us nowhere because iterations are long as everything is rebuilt upon changes to fundamental crates.

@brenzi
Copy link
Contributor Author

brenzi commented Oct 7, 2020

I would assume we need to touch at least these:

  • sp-io
  • sp-core
  • sp-debug-derive
  • runtime-interface

TBC

@bkchr
Copy link
Member

bkchr commented Oct 7, 2020

I just tried to reset and try bottom up starting with sp-io only. However, the errors explode and I think it takes someone with deep knowledge of the code to distinguish between no_std == wasm assumptions and legit std switches.

Try and err will lead us nowhere because iterations are long as everything is rebuilt upon changes to fundamental crates.

I assume you desperately waiting for this feature? 🙈

@brenzi
Copy link
Contributor Author

brenzi commented Oct 7, 2020

well, it would make our lives a lot easier for sure 🙏

@brenzi brenzi force-pushed the runtime-wasm-feature-nostd branch from a880b53 to 971c20e Compare October 7, 2020 19:25
@brenzi brenzi requested a review from tomusdrw as a code owner October 7, 2020 19:25
@brenzi
Copy link
Contributor Author

brenzi commented Oct 7, 2020

well, I tried a little harder, but giving up for today....

@athei
Copy link
Member

athei commented Oct 27, 2020

Does this kind of "negative feature" even work with Cargo.toml? How do you express that you want to enable the std feature of some (non parity) dependency when runtime-wasm is not set?

@gnunicorn gnunicorn added A3-needsresolving A5-stale Pull request did not receive any updates in a long time. No review needed at this stage. Close it. labels Nov 25, 2020
@gnunicorn
Copy link
Contributor

Closing for a lack of progress.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A5-stale Pull request did not receive any updates in a long time. No review needed at this stage. Close it.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Don't assume wasm in no_std env in runtime-interface
4 participants