Build WASM binaries as part of cargo build #2868

bkchr · 2019-06-14T14:03:42Z

So yeah, this replaces all the build.sh files with some build.rs files and a little bit of magic :)

Why?

Yeah, while working a Cumulus, I came to the point that I want to use something that depends on polkadot-service and this depends on polkadot-executor. The executor needs the WASM binary and this does not work when the executor is compiled as git-dependency (the binary can not be found in this case).
Other reasons are that people often forget to rebuild the WASM binaries and end up with weird errors.

How does it work?

Every crate that should be compiled as a WASM binary, needs to add a build.rs. This build.rs calls wasm-builder-runner. We can not directly use wasm-builder, because as always, cargo depedency/feature resolution is broken. For more information, see: rust-lang/cargo#5730
The wasm-builder-runner does not use any dependencies and create a new crate that depends on wasm-builder. The runner will cargo run the newly created crate.
wasm-builder itself also creates a new crate. This crate imports the project that should be compiled to WASM. The crate is build by wasm-builder into WASM and is being compacted by wasm-gc. Finally a file is created with a single constant WASM_BINARY, that includes the full compacted binary.

A project that should be compiled to WASM, needs to have a feature no_std, which is activated by the wasm-builder while compiling it to WASM.

There are also some env variables to control the build process:

SKIP_WASM_BUILD can be set to anything to skip building the WASM files. (This should be done after the files are build for the first time)
BUILD_DUMMY_WASM_BINARY can be set to anything to build some dummy WASM binaries (will just be &[]). This is useful for cargo check runs to prevent requiring the full WASM files.

TODO

Fix node-template release
Check if we need more fine-grained rerun-if in build.rs files.
Make sure that we always rebuild the WASM files when it is necessary

For the last two bulletpoints, I still have some ideas in my Köcher (funny if you understand german).

Compilation times

With wasm-builder:
cargo build --all --release: 7868.39s user 129.82s system 1006% cpu 13:14.50 total

Without:
./scripts/build.sh && cargo build --all --release: 6078.73s user 107.65s system 693% cpu 14:52.53 total

Make use of `wasm-builder` in `test-runtime`.

core/utils/wasm-builder/src/wasm_project.rs

tomaka · 2019-06-14T14:23:24Z

core/utils/wasm-builder/src/wasm_project.rs

+fn find_cargo_lock() -> PathBuf {
+	let mut out_dir = build_helper::out_dir();
+
+	while out_dir.pop() {


This is not great, as the target directory could be totally unrelated to where the Cargo.lock is (if you do cargo b --target-dir=/tmp/foo for example).

I don't think there's a way to detect that, except by depending on the cargo crate of crates.io, but that's a pretty big dependency to pull in the tree.

Hmm :( Then the only solution I see, is to make people specify the correct dependency version in their Cargo.toml and I don't copy the Cargo.lock.

This is not great, as the target directory could be totally unrelated to where the Cargo.lock is (if you do cargo b --target-dir=/tmp/foo for example).

I don't think there's a way to detect that, except by depending on the cargo crate of crates.io, but that's a pretty big dependency to pull in the tree.

Could we implement a subset of Cargo’s logic ourselves, or get the info we need from environment variables?

TriplEight · 2019-06-14T18:39:07Z

This definitely looks more graceful than where we are now, but:
Since these wasm binaries were separate and were compiled in the same state (with build.sh) for several jobs in CI (right now it's 4 and yet more to come), it appears 4x faster to precompile them once and dispense to the needy jobs.

TriplEight · 2019-06-14T18:43:22Z

Also, for CI I can use my own way to make it faster, but at some point it can become inconsistent.

Demi-Marie

A neat idea

core/utils/wasm-builder/src/lib.rs

core/utils/wasm-builder/src/prerequisites.rs

core/utils/wasm-builder/src/wasm_project.rs

bkchr · 2019-06-14T19:10:48Z

This definitely looks more graceful than where we are now, but:
Since these wasm binaries were separate and were compiled in the same state (with build.sh) for several jobs in CI (right now it's 4 and yet more to come), it appears 4x faster to precompile them once and dispense to the needy jobs.

As I understand your problems with the CI. This is mainly for making the development more easier + supporting use-cases we could not support up to this point (Like the example I have given in the description).

For the CI, why do we need to run everything in different pipelines? Is there any advantage? I would propose to run everything that needs the full WASM binaries, in one pipeline.
Everything that is just cargo check or something different can be run in a separate pipeline.
For the cargo check use-case I added BUILD_DUMMY_WASM_BINARY to skip building the binaries completely.

TriplEight · 2019-06-14T20:11:19Z

why do we need to run everything in different pipelines? Is there any advantage? I would propose to run everything that needs the full WASM binaries, in one pipeline.

CI is not only jobs running in parallel. It can be some stages that reuse the results of previous ones.
Running everything in one job will mean it runs consequentially on one host. And I do not talk about the release build pipeline, just test ones.
Which would make sense if:

we had only one runner, and no need to scale
we didn't make use of caching
different test jobs had the same flags.

In our case precompilation of at least "somewhat static" parts of code would make CI much faster.

I'd propose a more clean way: to store unchanged compiled dependencies/binaries/pieces in i.e. our own registry, but for now it doesn't seem safe.

TriplEight · 2019-06-14T20:22:27Z

TLDR: I want less compilations of the same stuff in CI

bkchr · 2019-06-14T20:33:29Z

TLDR: I want less compilations of the same stuff in CI

Yeah, I understand this :D But that is sadly something that needs to be done by sccache.
And I also understand that you want to reuse as much as possible, but for that you should run as much as possible in the same pipeline. test-linux-stable and test-linux-stable-int would be much faster in the same pipeline, as the integration test could use the precompiled files of the test-linux-stable run.

Rust sadly does not support to split the compilation across so many machines, even with sccache. (while getting a significant speedup of the compilation)

pepyakin · 2019-07-03T16:54:00Z

We have noticed some problems when running the runtime built with old nightly (rustc 1.36.0-nightly (a9ec99f42 2019-05-13)), namely running

cargo run -- purge-chain -y --dev && time cargo run -- factory --dev --mode MasterTo1 --num 1500

failed with the following error:


Thread 'main' panicked at 'Failed to create inherent extrinsics: VersionInvalid', src/libcore/result.rs:997

Upgrading the nightly to

rustc 1.37.0-nightly (0beb2ba16 2019-07-02)

solved the issue.

README.adoc

core/utils/wasm-builder-runner/src/lib.rs

core/utils/wasm-builder/src/lib.rs

Co-Authored-By: Shawn Tabrizi <[email protected]>

joepetrowski

Should decide if we use WASM/Wasm/wasm in our docs. I like "Wasm" myself.

core/utils/wasm-builder-runner/README.md

core/utils/wasm-builder-runner/src/lib.rs

core/utils/wasm-builder/src/lib.rs

core/utils/wasm-builder/src/wasm_project.rs

Co-Authored-By: joe petrowski <[email protected]>

…into bkchr-wasm-builder

* Introduce `wasm-builder` and `wasm-builder-runner` to retire `build.sh` Make use of `wasm-builder` in `test-runtime`. * Add build script and remove the wasm project * Port `node-runtime` to new wasm-builder * Make `substrate-executor` tests work with `wasm-builder` * Move `node-template` to `wasm-builder` * Remove `build.sh` :) * Remove the last include_bytes * Adds the missing build.rs files * Remove `build.sh` from CI * Debug CI * Make it work in CI * CI attempt 3 * Make `substrate-runtime-test` compile on stable * Ahhh, some missed `include_bytes!` * AHH * Add suggestions * Improve search for `Cargo.lock` and don't panic if it is not found * Searching from manifest path was no good idea * Make the `wasm-builder` source better configurable * Expose the bloaty wasm binary as well * Make sure to rerun WASM recompilation on changes in dependencies * Introduce new `WASM_BUILD_TYPE` env and make sure to call `build.rs` on changes to env variables * Remove `build.sh` from READMEs * Rename the projects * Fixes CI * Update lock file * Fixes merge-conflict * Apply suggestions from code review Co-Authored-By: TriplEight <[email protected]> * Try to make windows happy * Replace all back slashes in paths with slashes * Apply suggestions from code review Co-Authored-By: Pierre Krieger <[email protected]> * Use cargo from `CARGO` env variable * Fix compilation * Use `rustup` for running the nightly build * Make individual projects skipable * Fix compilation * Fixes compilation * Build all WASM projects in one workspace * Replace more back slashes! * Remove `inlcude_bytes!` * Adds some documentation * Apply suggestions from code review Co-Authored-By: Shawn Tabrizi <[email protected]> * Apply suggestions from code review Co-Authored-By: Shawn Tabrizi <[email protected]> * More review comments * Update `Cargo.lock` * Set license * Apply suggestions from code review Co-Authored-By: joe petrowski <[email protected]> * More review comments + adds `TRIGGER_WASM_BUILD` env * Fix doc tests * Increase version + update README * Switch crates.io version of `wasm-builder` * Update README * Switch to released version of `wasm-builder-runner`

bkchr added 8 commits June 13, 2019 18:17

Introduce wasm-builder and wasm-builder-runner to retire build.sh

1ed68f2

Make use of `wasm-builder` in `test-runtime`.

Add build script and remove the wasm project

96b1fb6

Port node-runtime to new wasm-builder

47dd047

Make substrate-executor tests work with wasm-builder

468bbee

Move node-template to wasm-builder

3e6108f

Remove build.sh :)

cdd2619

Remove the last include_bytes

fb80a34

Adds the missing build.rs files

8a61c2f

bkchr added the A0-please_review Pull request needs code review. label Jun 14, 2019

devops-parity added the A4-gotissues label Jun 14, 2019

Remove build.sh from CI

f4ceedc

tomaka reviewed Jun 14, 2019

View reviewed changes

core/utils/wasm-builder/src/wasm_project.rs Show resolved Hide resolved

tomaka reviewed Jun 14, 2019

View reviewed changes

bkchr force-pushed the bkchr-wasm-builder branch from 80ac3cc to b9ec757 Compare June 14, 2019 14:43

Debug CI

c9e4094

bkchr force-pushed the bkchr-wasm-builder branch from b9ec757 to c9e4094 Compare June 14, 2019 14:46

Make it work in CI

4269b6f

Demi-Marie reviewed Jun 14, 2019

View reviewed changes

bkchr added 2 commits June 14, 2019 21:27

CI attempt 3

98c3dad

Make substrate-runtime-test compile on stable

3140240

Ahhh, some missed include_bytes!

ee690d4

bkchr added 3 commits June 14, 2019 22:35

AHH

cd62ba8

Add suggestions

4daabf1

Improve search for Cargo.lock and don't panic if it is not found

1b334b2

Adds some documentation

efb977b