-
Notifications
You must be signed in to change notification settings - Fork 19
CI build mode for WASM runtime artifact #1561
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
base: master
Are you sure you want to change the base?
Conversation
41ba796 to
6805964
Compare
6805964 to
86d9732
Compare
|
I believe we can remove
— and then we will be able to build WASM using the following command: This would reduce the number of dependency crates to build from the current 1270 to just 449. The only thing is, I haven't tested it yet to see if it'll break anything else. |
MOZGIII
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall! Small nits, but this is definitely on the right track
If we remove it it will trigger a wasm build while doing a wasm build - an infinite loop |
|
What build command should work just fine though Except it doesn't compress the built artifact |
|
After some research, I deem the way of building the runtime via the |
dmitrylavrenov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clear on my side.
Not sure, that I'm in context having a lot of commented code. Is it fully ready for merging or just some kind of checkpoint ?
I noticed that Polkadot itself sometime uses wasm-builder from helper scripts. I also believe it's easier to build from
I don’t yet know how to do this. |
We agreed with Mozgiii right away that launching CI here would require several attempts, and that running too many jobs would consume a significant portion of our CI budget. Therefore, in the first commit, I disabled all CI jobs except those necessary for demonstrating the new artifact type. I think we should first ensure that there are no comments on the Draft PR, and then I'll remove this commit in the final PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to have the syntax and formatting unified; I recommend using Bash IDE and shellcheck extensions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shellcheck — I had it installed,
BashIDE — I installed it, but it didn't offer any formatting corrections…
|
I removed the CLI arguments I will still see what can be done to avoid building so many crates. |
|
Maybe we could just structure the builds in such a way that we also publish a whole node with tracing support? I.e. so that the node would, like, also have the embedded tracing runtime initially, and the tracing variant of the on-chain runtime (code). |
28663d9 to
b7500ea
Compare
|
An experiment confirmed that the cargo flags Added Cargo target
I think a separate WASM artifact is more flexible to use. If it’s not needed, I can add node with EVM-tracing here. If another artifact (EVM-tracing node) is required, perhaps it would be best to create it in a separate task? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it, like this, will still build all the dependencies for the host triple before the actual runtime is built (for the wasm triple).
If we, instead, make another crate, and have the contents of the build.rs script in there (with slight changes to target a different crate) - we could essentially directly trigger the runtime build for wasm from a separate package - as opposed to doing it from the build script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we, instead, make another crate, and have the contents of the
build.rsscript in there (with slight changes to target a different crate) - we could essentially directly trigger the runtime build for wasm from a separate package - as opposed to doing it from the build script.
Tested this with 842c319 - the build script fails (substrate-wasm-builder panics at https://github.com/paritytech/substrate/blob/033d4e86cc7eff0066cd376b9375f815761d653c/utils/wasm-builder/src/wasm_project.rs#L837); the other variation where the build script it the bin itself also didn't work - but for other reason https://github.com/paritytech/substrate/blob/033d4e86cc7eff0066cd376b9375f815761d653c/utils/wasm-builder/src/builder.rs#L163.
cc8785e to
992422b
Compare
You can also review the draft release: https://github.com/humanode-network/humanode/releases/tag/untagged-c97d2543b7a7af640755I've tested releases as well 😁