-
Notifications
You must be signed in to change notification settings - Fork 105
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
feat(wasm-builder): New approach to build demos #3602
Conversation
plz check this #3588 (comment) btw |
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.
Good job! 🥇
gclient/binaries/build.rs
Outdated
@@ -0,0 +1,21 @@ | |||
// This file is part of Gear. | |||
// | |||
// Copyright (C) 2023 Gear Technologies Inc. |
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 suggest removing the year number from the copyright since it is not consistent across the source code and we will not update it later
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 see most of files have year when just a few don't
@@ -18,6 +18,8 @@ | |||
|
|||
//! Module for memory pages. | |||
|
|||
#![allow(clippy::assertions_on_constants)] |
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.
merge master and remove this
@@ -0,0 +1 @@ | |||
|
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.
why examples/signal-wait/**
is empty?
@@ -213,6 +213,7 @@ gear-service = { path = "node/service", default-features = false } | |||
gear-stack-buffer = { path = "stack-buffer" } | |||
gear-utils = { path = "utils/utils" } | |||
gear-wasm-builder = { path = "utils/wasm-builder", default-features = false } | |||
wasm-dep-builder = { path = "utils/wasm-dep-builder" } |
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.
Do we want to prefix it with gear
as for the others?
[dependencies] | ||
gcore.workspace = true | ||
parity-scale-codec.workspace = true | ||
gstd = { workspace = true } |
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.
gstd = { workspace = true } | |
gstd.workspace = true |
|
||
[dependencies] |
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.
redundant
|
||
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html |
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.
redundant
|
||
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html |
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.
redundant
gmeta = { workspace = true } | ||
gstd = { workspace = true, features = ["debug"] } | ||
demo-wat.workspace = true | ||
gclient-binaries = { path = "binaries" } |
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 it be better to call it demo-binaries?
@@ -42,6 +42,7 @@ rand = { workspace = true, optional = true } | |||
|
|||
[dev-dependencies] | |||
gsdk = { path = ".", features = ["testing"] } | |||
gsdk-binaries = { path = "binaries" } |
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.
-> demo-binaries
?
demo-meta-io.workspace = true | ||
gtest-binaries = { path = "binaries" } |
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.
-> demo-binaries
?
@@ -41,7 +41,9 @@ sp-inherents = { workspace = true, features = ["std"] } | |||
prometheus-endpoint.workspace = true | |||
|
|||
[dev-dependencies] | |||
gear-authorship-binaries = { path = "binaries" } |
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.
-> demo-binaries
?
repository.workspace = true | ||
|
||
[dependencies] | ||
cargo_metadata = "0.18.1" |
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.
Do you want to move them into the workspace deps?
|
Can you describe the approach in the PR outline? |
Can we maybe put "dependency builder" inside gear-wasm-builder itself? |
It will replace gear_wasm_builder some day, so is there any reason to merge them now? Just for notice: as it was mentioned, there are a few problems with these approach, and as far as we discussed with @ark0f they're already solved, but not pushed |
Done |
Temporary closing in favour of opened referencing issue as won't be implemented |
This is a new approach to build Gear programs which can replace
gear-wasm-builder
.Schematic architecture:
Pipeline:
Purpose of program build script is just write features to lock file and don't use any
cargo:rerun-if
instructions so default cargo heuristics are enabled.Binaries crate build script list its build dependencies and corresponding reads lock files to obtain program features then it runs
cargo build -p program1 -p program2
socargo
builds WASM binaries effectively and concurrently. Then it writes to lock files to skipcargo
invocation in next or in another binaries crate to avoid few seconds cache verificationPros:
Features are tracked automatically so we don't need to re-run build script each time like in
gear-wasm-builder
(can be mitigated bycargo-metadata
andCargo.toml
tracking)gear-wasm-builder
builds program via newcargo
command in the same target directory so eachcargo
process blocks other.wasm-dep-builder
collects list of programs so only onecargo
process runs.Cons:
It is important to create binaries create that will list and build programs
Current problem:
When crate includes itself as dev dependency (any integration tests) with different feature set then cargo build and runs 2 different versions of build script - for workspace and for tests.
gear-wasm-builder
case is thatcargo
builds and runs script just once with merged feature set.So, how to resolve these features? Should we deliver different WASM binaries? Or just merge feature sets?
I vote for merging but we need some mechanism to obtain all of feature sets cargo resolved for each build script invocation of program before build script invocation of binaries crate if we don't want to waste time on WASM build with incomplete feature set