From cb9c1818f198b946b402652d173bbd76e8fbe7d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 21 Jan 2020 12:33:28 +0100 Subject: [PATCH] Make debug builds more usable (#4683) * Make debug builds more usable This pr makes debug builds more usable in terms of `cargo run -- --dev`. 1. `--dev` activates `--execution native`, iff `--execution` is not given or no sub `--execution-*` is given. 2. It was probably a mistake to compile WASM in debug for a debug build. So, we now build the WASM binary always as `release` (if not requested differently by the user). So, we trade compilation time for a better debug experience. * Make sure we only overwrite default values * Make it work * Apply suggestion --- Cargo.lock | 2 +- bin/node-template/runtime/build.rs | 2 +- bin/node/cli/src/cli.rs | 7 +++- bin/node/runtime/build.rs | 2 +- client/cli/src/execution_strategy.rs | 24 +++++++++++- client/cli/src/lib.rs | 39 ++++++++++++++----- client/cli/src/params.rs | 11 +++--- client/executor/runtime-test/build.rs | 2 +- .../runtime-interface/test-wasm/build.rs | 2 +- test-utils/runtime/build.rs | 2 +- utils/wasm-builder/Cargo.toml | 2 +- utils/wasm-builder/src/wasm_project.rs | 2 +- 12 files changed, 73 insertions(+), 24 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 630e372dff82e..8030a4cd13ad2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6973,7 +6973,7 @@ version = "2.0.0" [[package]] name = "substrate-wasm-builder" -version = "1.0.8" +version = "1.0.9" dependencies = [ "atty 0.2.13 (registry+https://github.com/rust-lang/crates.io-index)", "build-helper 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/bin/node-template/runtime/build.rs b/bin/node-template/runtime/build.rs index ed0e28ec0d140..dc7e949d809f8 100644 --- a/bin/node-template/runtime/build.rs +++ b/bin/node-template/runtime/build.rs @@ -19,7 +19,7 @@ use wasm_builder_runner::{build_current_project_with_rustflags, WasmBuilderSourc fn main() { build_current_project_with_rustflags( "wasm_binary.rs", - WasmBuilderSource::Crates("1.0.8"), + WasmBuilderSource::Crates("1.0.9"), // This instructs LLD to export __heap_base as a global variable, which is used by the // external memory allocator. "-Clink-arg=--export=__heap_base", diff --git a/bin/node/cli/src/cli.rs b/bin/node/cli/src/cli.rs index 8156cd766cc79..6a2e02efb6209 100644 --- a/bin/node/cli/src/cli.rs +++ b/bin/node/cli/src/cli.rs @@ -141,7 +141,12 @@ pub fn run(args: I, exit: E, version: sc_cli::VersionInfo) -> error::Re &version, None, )?; - sc_cli::fill_import_params(&mut config, &cli_args.import_params, ServiceRoles::FULL)?; + sc_cli::fill_import_params( + &mut config, + &cli_args.import_params, + ServiceRoles::FULL, + cli_args.shared_params.dev, + )?; match ChainSpec::from(config.chain_spec.id()) { Some(ref c) if c == &ChainSpec::Development || c == &ChainSpec::LocalTestnet => {}, diff --git a/bin/node/runtime/build.rs b/bin/node/runtime/build.rs index 1ed2fa43e685c..9c81ea6f38b8d 100644 --- a/bin/node/runtime/build.rs +++ b/bin/node/runtime/build.rs @@ -21,7 +21,7 @@ fn main() { "wasm_binary.rs", WasmBuilderSource::CratesOrPath { path: "../../../utils/wasm-builder", - version: "1.0.8", + version: "1.0.9", }, // This instructs LLD to export __heap_base as a global variable, which is used by the // external memory allocator. diff --git a/client/cli/src/execution_strategy.rs b/client/cli/src/execution_strategy.rs index fe353da80ddef..888d7b6c4a096 100644 --- a/client/cli/src/execution_strategy.rs +++ b/client/cli/src/execution_strategy.rs @@ -20,7 +20,7 @@ use structopt::clap::arg_enum; arg_enum! { /// How to execute blocks - #[derive(Debug, Clone, Copy)] + #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum ExecutionStrategy { // Execute with native build (if available, WebAssembly otherwise). Native, @@ -33,3 +33,25 @@ arg_enum! { } } +impl ExecutionStrategy { + /// Returns the variant as `'&static str`. + pub fn as_str(&self) -> &'static str { + match self { + Self::Native => "Native", + Self::Wasm => "Wasm", + Self::Both => "Both", + Self::NativeElseWasm => "NativeElseWasm", + } + } +} + +/// Default value for the `--execution-syncing` parameter. +pub const DEFAULT_EXECUTION_SYNCING: ExecutionStrategy = ExecutionStrategy::NativeElseWasm; +/// Default value for the `--execution-import-block` parameter. +pub const DEFAULT_EXECUTION_IMPORT_BLOCK: ExecutionStrategy = ExecutionStrategy::NativeElseWasm; +/// Default value for the `--execution-block-construction` parameter. +pub const DEFAULT_EXECUTION_BLOCK_CONSTRUCTION: ExecutionStrategy = ExecutionStrategy::Wasm; +/// Default value for the `--execution-offchain-worker` parameter. +pub const DEFAULT_EXECUTION_OFFCHAIN_WORKER: ExecutionStrategy = ExecutionStrategy::Native; +/// Default value for the `--execution-other` parameter. +pub const DEFAULT_EXECUTION_OTHER: ExecutionStrategy = ExecutionStrategy::Native; diff --git a/client/cli/src/lib.rs b/client/cli/src/lib.rs index 52a3fc46ec726..fc5025952860b 100644 --- a/client/cli/src/lib.rs +++ b/client/cli/src/lib.rs @@ -40,6 +40,7 @@ use sc_network::{ }, }; use sp_core::H256; +use execution_strategy::*; use std::{ io::{Write, Read, Seek, Cursor, stdin, stdout, ErrorKind}, iter, fmt::Debug, fs::{self, File}, @@ -556,7 +557,12 @@ impl<'a> ParseAndPrepareImport<'a> { self.version, None, )?; - fill_import_params(&mut config, &self.params.import_params, sc_service::Roles::FULL)?; + fill_import_params( + &mut config, + &self.params.import_params, + sc_service::Roles::FULL, + self.params.shared_params.dev, + )?; let file: Box = match self.params.input { Some(filename) => Box::new(File::open(filename)?), @@ -620,7 +626,12 @@ impl<'a> CheckBlock<'a> { self.version, None, )?; - fill_import_params(&mut config, &self.params.import_params, sc_service::Roles::FULL)?; + fill_import_params( + &mut config, + &self.params.import_params, + sc_service::Roles::FULL, + self.params.shared_params.dev, + )?; fill_config_keystore_in_memory(&mut config)?; let input = if self.params.input.starts_with("0x") { &self.params.input[2..] } else { &self.params.input[..] }; @@ -904,6 +915,7 @@ pub fn fill_import_params( config: &mut Configuration, cli: &ImportParams, role: sc_service::Roles, + is_dev: bool, ) -> error::Result<()> where C: Default, @@ -943,13 +955,22 @@ pub fn fill_import_params( config.wasm_method = cli.wasm_method.into(); let exec = &cli.execution_strategies; - let exec_all_or = |strat: ExecutionStrategy| exec.execution.unwrap_or(strat).into(); + let exec_all_or = |strat: ExecutionStrategy, default: ExecutionStrategy| { + exec.execution.unwrap_or(if strat == default && is_dev { + ExecutionStrategy::Native + } else { + strat + }).into() + }; + config.execution_strategies = ExecutionStrategies { - syncing: exec_all_or(exec.execution_syncing), - importing: exec_all_or(exec.execution_import_block), - block_construction: exec_all_or(exec.execution_block_construction), - offchain_worker: exec_all_or(exec.execution_offchain_worker), - other: exec_all_or(exec.execution_other), + syncing: exec_all_or(exec.execution_syncing, DEFAULT_EXECUTION_SYNCING), + importing: exec_all_or(exec.execution_import_block, DEFAULT_EXECUTION_IMPORT_BLOCK), + block_construction: + exec_all_or(exec.execution_block_construction, DEFAULT_EXECUTION_BLOCK_CONSTRUCTION), + offchain_worker: + exec_all_or(exec.execution_offchain_worker, DEFAULT_EXECUTION_OFFCHAIN_WORKER), + other: exec_all_or(exec.execution_other, DEFAULT_EXECUTION_OTHER), }; Ok(()) } @@ -987,7 +1008,7 @@ where sc_service::Roles::FULL }; - fill_import_params(&mut config, &cli.import_params, role)?; + fill_import_params(&mut config, &cli.import_params, role, is_dev)?; config.impl_name = impl_name; config.impl_commit = version.commit; diff --git a/client/cli/src/params.rs b/client/cli/src/params.rs index 07fbe9b73885c..57b90c2e73d5c 100644 --- a/client/cli/src/params.rs +++ b/client/cli/src/params.rs @@ -18,6 +18,7 @@ use crate::traits::GetSharedParams; use std::{str::FromStr, path::PathBuf}; use structopt::{StructOpt, StructOptInternal, clap::{arg_enum, App, AppSettings, SubCommand, Arg}}; +use crate::execution_strategy::*; pub use crate::execution_strategy::ExecutionStrategy; @@ -321,7 +322,7 @@ pub struct ExecutionStrategies { value_name = "STRATEGY", possible_values = &ExecutionStrategy::variants(), case_insensitive = true, - default_value = "NativeElseWasm" + default_value = DEFAULT_EXECUTION_SYNCING.as_str(), )] pub execution_syncing: ExecutionStrategy, @@ -331,7 +332,7 @@ pub struct ExecutionStrategies { value_name = "STRATEGY", possible_values = &ExecutionStrategy::variants(), case_insensitive = true, - default_value = "NativeElseWasm" + default_value = DEFAULT_EXECUTION_IMPORT_BLOCK.as_str(), )] pub execution_import_block: ExecutionStrategy, @@ -341,7 +342,7 @@ pub struct ExecutionStrategies { value_name = "STRATEGY", possible_values = &ExecutionStrategy::variants(), case_insensitive = true, - default_value = "Wasm" + default_value = DEFAULT_EXECUTION_BLOCK_CONSTRUCTION.as_str(), )] pub execution_block_construction: ExecutionStrategy, @@ -351,7 +352,7 @@ pub struct ExecutionStrategies { value_name = "STRATEGY", possible_values = &ExecutionStrategy::variants(), case_insensitive = true, - default_value = "Native" + default_value = DEFAULT_EXECUTION_OFFCHAIN_WORKER.as_str(), )] pub execution_offchain_worker: ExecutionStrategy, @@ -361,7 +362,7 @@ pub struct ExecutionStrategies { value_name = "STRATEGY", possible_values = &ExecutionStrategy::variants(), case_insensitive = true, - default_value = "Native" + default_value = DEFAULT_EXECUTION_OTHER.as_str(), )] pub execution_other: ExecutionStrategy, diff --git a/client/executor/runtime-test/build.rs b/client/executor/runtime-test/build.rs index 1ed2fa43e685c..9c81ea6f38b8d 100644 --- a/client/executor/runtime-test/build.rs +++ b/client/executor/runtime-test/build.rs @@ -21,7 +21,7 @@ fn main() { "wasm_binary.rs", WasmBuilderSource::CratesOrPath { path: "../../../utils/wasm-builder", - version: "1.0.8", + version: "1.0.9", }, // This instructs LLD to export __heap_base as a global variable, which is used by the // external memory allocator. diff --git a/primitives/runtime-interface/test-wasm/build.rs b/primitives/runtime-interface/test-wasm/build.rs index 8e1d17e821195..9c81ea6f38b8d 100644 --- a/primitives/runtime-interface/test-wasm/build.rs +++ b/primitives/runtime-interface/test-wasm/build.rs @@ -21,7 +21,7 @@ fn main() { "wasm_binary.rs", WasmBuilderSource::CratesOrPath { path: "../../../utils/wasm-builder", - version: "1.0.6", + version: "1.0.9", }, // This instructs LLD to export __heap_base as a global variable, which is used by the // external memory allocator. diff --git a/test-utils/runtime/build.rs b/test-utils/runtime/build.rs index a90270d85d55f..8a9ee642c4fee 100644 --- a/test-utils/runtime/build.rs +++ b/test-utils/runtime/build.rs @@ -21,7 +21,7 @@ fn main() { "wasm_binary.rs", WasmBuilderSource::CratesOrPath { path: "../../utils/wasm-builder", - version: "1.0.8", + version: "1.0.9", }, // Note that we set the stack-size to 1MB explicitly even though it is set // to this value by default. This is because some of our tests (`restoration_of_globals`) diff --git a/utils/wasm-builder/Cargo.toml b/utils/wasm-builder/Cargo.toml index 66d94c32ab330..5941a39ffb6a6 100644 --- a/utils/wasm-builder/Cargo.toml +++ b/utils/wasm-builder/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "substrate-wasm-builder" -version = "1.0.8" +version = "1.0.9" authors = ["Parity Technologies "] description = "Utility for building WASM binaries" edition = "2018" diff --git a/utils/wasm-builder/src/wasm_project.rs b/utils/wasm-builder/src/wasm_project.rs index a028d00930a4e..c9b573ff1940f 100644 --- a/utils/wasm-builder/src/wasm_project.rs +++ b/utils/wasm-builder/src/wasm_project.rs @@ -357,7 +357,7 @@ fn is_release_build() -> bool { ), } } else { - !build_helper::debug() + true } }