Skip to content

fix: features #353

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.

Sign up for GitHub

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

Merged
weichweich merged 8 commits into from
Apr 11, 2022
Merged

fix: features #353

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c2ed756
fix: even more features
weichweich Apr 7, 2022
5e54aa7
fix: more features
weichweich Apr 7, 2022
597799c
feat: add script for testing features
weichweich Apr 7, 2022
3493d97
fix: benchmark in try-runtime !!
weichweich Apr 8, 2022
677e9b4
update script
weichweich Apr 8, 2022
c92232d
doc: mention cargo-workspaces
weichweich Apr 11, 2022
c9be8e6
Merge branch 'develop' into aw-fix-features
weichweich Apr 11, 2022
38018cb
check default features
weichweich Apr 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 4 additions & 5 deletions nodes/standalone/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ mashnet-node-runtime = {path = "../../runtimes/standalone"}
runtime-common = {path = "../../runtimes/common"}

# External dependencies
clap = { version = "3.1", features = ["derive"] }
clap = {version = "3.1", features = ["derive"]}
futures = "0.3"
hex-literal = "0.3.4"
log = "0.4"
Expand Down Expand Up @@ -65,9 +65,8 @@ frame-benchmarking = {git = "https://github.com/paritytech/substrate", branch =
frame-benchmarking-cli = {git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.17"}

# Runtime tests
node-executor = {git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.17", optional = true}
pallet-conviction-voting = {git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.17", optional = true}
pallet-referenda = {git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.17", optional = true}
pallet-conviction-voting = {git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.17", optional = true}
pallet-referenda = {git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.17", optional = true}
try-runtime-cli = {git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.17", optional = true}

[features]
Expand All @@ -76,9 +75,9 @@ runtime-benchmarks = [
"mashnet-node-runtime/runtime-benchmarks",
"pallet-conviction-voting/runtime-benchmarks",
"pallet-referenda/runtime-benchmarks",
"runtime-common/runtime-benchmarks"
]
try-runtime = [
"mashnet-node-runtime/try-runtime",
"node-executor",
"try-runtime-cli",
]
5 changes: 1 addition & 4 deletions nodes/standalone/src/command.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,6 @@ use mashnet_node_runtime::opaque::Block;
use sc_cli::{ChainSpec, RuntimeVersion, SubstrateCli};
use sc_service::PartialComponents;

#[cfg(feature = "try-runtime")]
use node_executor::ExecutorDispatch;

impl SubstrateCli for Cli {
fn impl_name() -> String {
"KILT Node".to_string()
Expand Down Expand Up @@ -155,7 +152,7 @@ pub fn run() -> sc_cli::Result<()> {
let task_manager = sc_service::TaskManager::new(config.tokio_handle.clone(), registry)
.map_err(|e| sc_cli::Error::Service(sc_service::Error::Prometheus(e)))?;

Ok((cmd.run::<Block, ExecutorDispatch>(config), task_manager))
Ok((cmd.run::<Block, service::ExecutorDispatch>(config), task_manager))
})
}
#[cfg(not(feature = "try-runtime"))]
Expand Down
1 change: 1 addition & 0 deletions pallets/delegation/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ runtime-benchmarks = [
"sp-io",
]
std = [
"attestation/std",
"codec/std",
"ctype/std",
"frame-support/std",
Expand Down
3 changes: 2 additions & 1 deletion pallets/pallet-did-lookup/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ targets = ["x86_64-unknown-linux-gnu"]
kilt-support = {features = ["mock"], path = "../../support"}

pallet-balances = {branch = "polkadot-v0.9.17", default-features = false, git = "https://github.com/paritytech/substrate"}
sp-core = {branch = "polkadot-v0.9.17", default-features = false, git = "https://github.com/paritytech/substrate"}
sp-io = {branch = "polkadot-v0.9.17", default-features = false, git = "https://github.com/paritytech/substrate"}
sp-keystore = {branch = "polkadot-v0.9.17", default-features = false, git = "https://github.com/paritytech/substrate"}
sp-core = {branch = "polkadot-v0.9.17", default-features = false, git = "https://github.com/paritytech/substrate"}

[dependencies]
codec = {default-features = false, features = ["derive"], package = "parity-scale-codec", version = "2.3.1"}
Expand All @@ -42,6 +42,7 @@ runtime-benchmarks = [
"frame-benchmarking",
"sp-io",
"kilt-support/runtime-benchmarks",
"frame-support/runtime-benchmarks",
"frame-system/runtime-benchmarks",
]

Expand Down
2 changes: 2 additions & 0 deletions pallets/pallet-inflation/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ default = ["std"]

runtime-benchmarks = [
"frame-benchmarking",
"frame-support/runtime-benchmarks",
"frame-system/runtime-benchmarks",
]

std = [
Expand Down
7 changes: 5 additions & 2 deletions pallets/pallet-web3-names/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ runtime-benchmarks = [
"frame-benchmarking",
"sp-io",
"kilt-support/runtime-benchmarks",
"frame-support/runtime-benchmarks",
"frame-system/runtime-benchmarks",
]

Expand All @@ -54,5 +55,7 @@ std = [
"sp-std/std",
]


try-runtime = []
try-runtime = [
"frame-support/try-runtime",
"frame-system/try-runtime",
]
2 changes: 0 additions & 2 deletions pallets/pallet-web3-names/src/web3_name.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@

// If you feel like getting in touch with us, you can do so at info@botlabs.org

#![cfg_attr(not(feature = "std"), no_std)]

use sp_std::{fmt::Debug, marker::PhantomData, vec::Vec};

use codec::{Decode, Encode, MaxEncodedLen};
Expand Down
3 changes: 3 additions & 0 deletions runtimes/common/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ sp-std = {git = "https://github.com/paritytech/substrate", default-features = fa
default = ["std"]
fast-gov = []
runtime-benchmarks = [
"attestation/runtime-benchmarks",
"frame-support/runtime-benchmarks",

@ntn-x2 ntn-x2 Apr 8, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why does runtime common enable these features upon runtime benchmarks? I'm talking specifically about attestation and parachain-staking.

@weichweich weichweich Apr 11, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

we need to pass this feature down to all dependencies. If runtime-benchmarks is enabled, some method in some trait that is guarded by this feature might be enabled too. If attestation and frame-support implement that trait and we don't enable the feature, this method from the trait would be missing and compilation would fail. So we always need to pass down all features that we use to all the dependencies. Either runtime-benchmarks is active for all dependencies or for none. Mixing this would create terrible errors.

"parachain-staking/runtime-benchmarks",
"sp-runtime/runtime-benchmarks",
]
std = [
Expand Down
2 changes: 1 addition & 1 deletion runtimes/standalone/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ try-runtime = [
"pallet-session/try-runtime",
"pallet-sudo/try-runtime",
"pallet-timestamp/try-runtime",
"pallet-web3-names/runtime-benchmarks",
"pallet-web3-names/try-runtime",
"pallet-utility/try-runtime",
"pallet-transaction-payment/try-runtime",
"pallet-vesting/try-runtime",
Expand Down
11 changes: 11 additions & 0 deletions scripts/all.fish
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
for features in "--all-features" "--features runtime-benchmarks" "--features try-runtime"

@wischli wischli Apr 8, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When I try to execute this, I receive an error: no such subcommand: workspaces`.

@weichweich weichweich Apr 11, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You have to install cargo-workspaces. I will add a comment for that.

@ntn-x2 ntn-x2 Apr 8, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would it also make sense to test with no (default) features, and with the mock feature, where specified? Anyway very good solution to the problem!

@weichweich weichweich Apr 11, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes that's a good idea

@weichweich weichweich Apr 11, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

default features are a good idea. But mock doesn'T work since not all of the crates have that feature

for package in (cargo workspaces list)
cargo build -p $package (echo $features | string split " ") > /dev/null ^ /dev/null
if [ "$status" = "0" ]
echo -n "[ok] "
else
echo -n "[fail] "
end
echo cargo build -p $package (echo $features | string split " ")
end
end