From 7fbed1b9ec2c83037db7e016c26dc1f0df4d9655 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Tue, 2 Jun 2020 18:58:19 +0200 Subject: [PATCH 01/21] Initial commit Forked at: 342caad3074076a4fde0472719f6a473df839e42 Parent branch: origin/master From aa518e410765afb8c4a8a3294f5676e87447dbe4 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Tue, 2 Jun 2020 19:23:01 +0200 Subject: [PATCH 02/21] Add a feature to create automatically a temporary directory for base path --- bin/node/cli/src/cli.rs | 2 +- client/cli/Cargo.toml | 4 +--- client/cli/src/commands/run_cmd.rs | 27 +++++++++++++++++++++++++-- 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/bin/node/cli/src/cli.rs b/bin/node/cli/src/cli.rs index 0156faf47ee4a..399aaac4e9464 100644 --- a/bin/node/cli/src/cli.rs +++ b/bin/node/cli/src/cli.rs @@ -20,7 +20,7 @@ use sc_cli::RunCmd; use structopt::StructOpt; /// An overarching CLI command definition. -#[derive(Clone, Debug, StructOpt)] +#[derive(Debug, StructOpt)] pub struct Cli { /// Possible subcommand with parameters. #[structopt(subcommand)] diff --git a/client/cli/Cargo.toml b/client/cli/Cargo.toml index 8c19da95c493b..a3369617f67b4 100644 --- a/client/cli/Cargo.toml +++ b/client/cli/Cargo.toml @@ -44,6 +44,7 @@ structopt = "0.3.8" sc-tracing = { version = "2.0.0-rc2", path = "../tracing" } chrono = "0.4.10" parity-util-mem = { version = "0.6.1", default-features = false, features = ["primitive-types"] } +tempfile = "3.1.0" [target.'cfg(not(target_os = "unknown"))'.dependencies] rpassword = "4.0.1" @@ -51,9 +52,6 @@ rpassword = "4.0.1" [target.'cfg(target_family = "unix")'.dependencies] nix = "0.17.0" -[dev-dependencies] -tempfile = "3.1.0" - [features] wasmtime = [ "sc-service/wasmtime", diff --git a/client/cli/src/commands/run_cmd.rs b/client/cli/src/commands/run_cmd.rs index 23a410d679b8f..74a6a59674a48 100644 --- a/client/cli/src/commands/run_cmd.rs +++ b/client/cli/src/commands/run_cmd.rs @@ -31,11 +31,12 @@ use sc_service::{ ChainSpec, Role, }; use sc_telemetry::TelemetryEndpoints; -use std::net::{IpAddr, Ipv4Addr, SocketAddr}; +use std::{net::{IpAddr, Ipv4Addr, SocketAddr}, path::PathBuf, sync::Mutex}; use structopt::StructOpt; +use tempfile::TempDir; /// The `run` command used to run a node. -#[derive(Debug, StructOpt, Clone)] +#[derive(Debug, StructOpt)] pub struct RunCmd { /// Enable validator mode. /// @@ -250,6 +251,18 @@ pub struct RunCmd { conflicts_with_all = &[ "sentry", "public-addr" ] )] pub sentry_nodes: Vec, + + /// Run a temporary node. + /// + /// A random temporary will be created to store the configuration and will be deleted at the end + /// of the process. + /// + /// Note: the directory is random per process execution + #[structopt(long)] + pub tmp: bool, + + #[structopt(skip)] + temp_base_path: Mutex>, } impl RunCmd { @@ -446,6 +459,16 @@ impl CliConfiguration for RunCmd { fn max_runtime_instances(&self) -> Result> { Ok(self.max_runtime_instances.map(|x| x.min(256))) } + + fn base_path(&self) -> Result> { + Ok(if self.tmp { + *self.temp_base_path.lock().unwrap() = Some(tempfile::Builder::new().prefix("substrate").tempdir()?); + + self.temp_base_path.lock().unwrap().as_ref().map(|x| x.path().into()) + } else { + self.shared_params().base_path() + }) + } } /// Check whether a node name is considered as valid. From 5338633758797b2a97d0ced0e87fd247b1ed6b30 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Tue, 2 Jun 2020 19:27:12 +0200 Subject: [PATCH 03/21] doc fix and todos --- client/cli/src/commands/run_cmd.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/client/cli/src/commands/run_cmd.rs b/client/cli/src/commands/run_cmd.rs index 74a6a59674a48..18cee15f89ce9 100644 --- a/client/cli/src/commands/run_cmd.rs +++ b/client/cli/src/commands/run_cmd.rs @@ -35,6 +35,7 @@ use std::{net::{IpAddr, Ipv4Addr, SocketAddr}, path::PathBuf, sync::Mutex}; use structopt::StructOpt; use tempfile::TempDir; +// TODO: check why all the structopt need clone? /// The `run` command used to run a node. #[derive(Debug, StructOpt)] pub struct RunCmd { @@ -254,10 +255,10 @@ pub struct RunCmd { /// Run a temporary node. /// - /// A random temporary will be created to store the configuration and will be deleted at the end - /// of the process. + /// A random temporary directory will be created to store the configuration and will be deleted + /// at the end of the process. /// - /// Note: the directory is random per process execution + /// Note: the directory is random per process execution. #[structopt(long)] pub tmp: bool, @@ -462,6 +463,7 @@ impl CliConfiguration for RunCmd { fn base_path(&self) -> Result> { Ok(if self.tmp { + // TODO: use parking_lot to avoid the unwrap() *self.temp_base_path.lock().unwrap() = Some(tempfile::Builder::new().prefix("substrate").tempdir()?); self.temp_base_path.lock().unwrap().as_ref().map(|x| x.path().into()) From baf6ad2c8617d4671ad7ea49cbf505b6d4286e45 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Wed, 3 Jun 2020 08:15:24 +0200 Subject: [PATCH 04/21] use parking_lot instead --- Cargo.lock | 1 + client/cli/Cargo.toml | 1 + client/cli/src/commands/run_cmd.rs | 14 +++++++++----- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5a13d30be1ef6..8442868a6e27a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5957,6 +5957,7 @@ dependencies = [ "names", "nix", "parity-util-mem", + "parking_lot 0.10.2", "regex", "rpassword", "sc-client-api", diff --git a/client/cli/Cargo.toml b/client/cli/Cargo.toml index a3369617f67b4..84292d57c2146 100644 --- a/client/cli/Cargo.toml +++ b/client/cli/Cargo.toml @@ -45,6 +45,7 @@ sc-tracing = { version = "2.0.0-rc2", path = "../tracing" } chrono = "0.4.10" parity-util-mem = { version = "0.6.1", default-features = false, features = ["primitive-types"] } tempfile = "3.1.0" +parking_lot = "0.10.2" [target.'cfg(not(target_os = "unknown"))'.dependencies] rpassword = "4.0.1" diff --git a/client/cli/src/commands/run_cmd.rs b/client/cli/src/commands/run_cmd.rs index 18cee15f89ce9..3a3939771e8ac 100644 --- a/client/cli/src/commands/run_cmd.rs +++ b/client/cli/src/commands/run_cmd.rs @@ -21,17 +21,21 @@ use crate::error::{Error, Result}; use crate::params::ImportParams; use crate::params::KeystoreParams; use crate::params::NetworkParams; +use crate::params::OffchainWorkerParams; use crate::params::SharedParams; use crate::params::TransactionPoolParams; -use crate::params::OffchainWorkerParams; use crate::CliConfiguration; +use parking_lot::Mutex; use regex::Regex; use sc_service::{ config::{MultiaddrWithPeerId, PrometheusConfig, TransactionPoolOptions}, ChainSpec, Role, }; use sc_telemetry::TelemetryEndpoints; -use std::{net::{IpAddr, Ipv4Addr, SocketAddr}, path::PathBuf, sync::Mutex}; +use std::{ + net::{IpAddr, Ipv4Addr, SocketAddr}, + path::PathBuf, +}; use structopt::StructOpt; use tempfile::TempDir; @@ -463,10 +467,10 @@ impl CliConfiguration for RunCmd { fn base_path(&self) -> Result> { Ok(if self.tmp { - // TODO: use parking_lot to avoid the unwrap() - *self.temp_base_path.lock().unwrap() = Some(tempfile::Builder::new().prefix("substrate").tempdir()?); + *self.temp_base_path.lock() = + Some(tempfile::Builder::new().prefix("substrate").tempdir()?); - self.temp_base_path.lock().unwrap().as_ref().map(|x| x.path().into()) + self.temp_base_path.lock().as_ref().map(|x| x.path().into()) } else { self.shared_params().base_path() }) From 179dda32e60a539501edc9405e8d04ad14a03d9e Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Wed, 3 Jun 2020 08:54:19 +0200 Subject: [PATCH 05/21] use refcell instead since we stay in the main thread --- Cargo.lock | 1 - client/cli/Cargo.toml | 1 - client/cli/src/commands/run_cmd.rs | 11 ++++++----- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8442868a6e27a..5a13d30be1ef6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5957,7 +5957,6 @@ dependencies = [ "names", "nix", "parity-util-mem", - "parking_lot 0.10.2", "regex", "rpassword", "sc-client-api", diff --git a/client/cli/Cargo.toml b/client/cli/Cargo.toml index 84292d57c2146..a3369617f67b4 100644 --- a/client/cli/Cargo.toml +++ b/client/cli/Cargo.toml @@ -45,7 +45,6 @@ sc-tracing = { version = "2.0.0-rc2", path = "../tracing" } chrono = "0.4.10" parity-util-mem = { version = "0.6.1", default-features = false, features = ["primitive-types"] } tempfile = "3.1.0" -parking_lot = "0.10.2" [target.'cfg(not(target_os = "unknown"))'.dependencies] rpassword = "4.0.1" diff --git a/client/cli/src/commands/run_cmd.rs b/client/cli/src/commands/run_cmd.rs index 3a3939771e8ac..bddcfc98f5eaf 100644 --- a/client/cli/src/commands/run_cmd.rs +++ b/client/cli/src/commands/run_cmd.rs @@ -25,7 +25,6 @@ use crate::params::OffchainWorkerParams; use crate::params::SharedParams; use crate::params::TransactionPoolParams; use crate::CliConfiguration; -use parking_lot::Mutex; use regex::Regex; use sc_service::{ config::{MultiaddrWithPeerId, PrometheusConfig, TransactionPoolOptions}, @@ -33,6 +32,7 @@ use sc_service::{ }; use sc_telemetry::TelemetryEndpoints; use std::{ + cell::RefCell, net::{IpAddr, Ipv4Addr, SocketAddr}, path::PathBuf, }; @@ -267,7 +267,7 @@ pub struct RunCmd { pub tmp: bool, #[structopt(skip)] - temp_base_path: Mutex>, + temp_base_path: RefCell>, } impl RunCmd { @@ -467,10 +467,11 @@ impl CliConfiguration for RunCmd { fn base_path(&self) -> Result> { Ok(if self.tmp { - *self.temp_base_path.lock() = - Some(tempfile::Builder::new().prefix("substrate").tempdir()?); + let tmp = tempfile::Builder::new().prefix("substrate").tempdir()?; + let path = tmp.path().into(); + *self.temp_base_path.borrow_mut() = Some(tmp); - self.temp_base_path.lock().as_ref().map(|x| x.path().into()) + Some(path) } else { self.shared_params().base_path() }) From 2aea88f3e45e1f4a1227de6caecffa9c7a5016c4 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Wed, 3 Jun 2020 09:23:51 +0200 Subject: [PATCH 06/21] remove Clone derives --- bin/node/cli/src/cli.rs | 2 +- bin/node/inspect/src/cli.rs | 4 ++-- client/cli/src/commands/build_spec_cmd.rs | 2 +- client/cli/src/commands/check_block_cmd.rs | 2 +- client/cli/src/commands/export_blocks_cmd.rs | 2 +- client/cli/src/commands/export_state_cmd.rs | 4 ++-- client/cli/src/commands/import_blocks_cmd.rs | 2 +- client/cli/src/commands/mod.rs | 2 +- client/cli/src/commands/purge_chain_cmd.rs | 2 +- client/cli/src/commands/revert_cmd.rs | 2 +- client/cli/src/commands/run_cmd.rs | 1 - client/cli/src/params/database_params.rs | 2 +- client/cli/src/params/import_params.rs | 4 ++-- client/cli/src/params/keystore_params.rs | 2 +- client/cli/src/params/mod.rs | 4 ++-- client/cli/src/params/network_params.rs | 2 +- client/cli/src/params/node_key_params.rs | 2 +- client/cli/src/params/offchain_worker_params.rs | 2 +- client/cli/src/params/pruning_params.rs | 2 +- client/cli/src/params/shared_params.rs | 2 +- client/cli/src/params/transaction_pool_params.rs | 2 +- utils/frame/benchmarking-cli/src/lib.rs | 2 +- 22 files changed, 25 insertions(+), 26 deletions(-) diff --git a/bin/node/cli/src/cli.rs b/bin/node/cli/src/cli.rs index 399aaac4e9464..29e916fe0180e 100644 --- a/bin/node/cli/src/cli.rs +++ b/bin/node/cli/src/cli.rs @@ -31,7 +31,7 @@ pub struct Cli { } /// Possible subcommands of the main binary. -#[derive(Clone, Debug, StructOpt)] +#[derive(Debug, StructOpt)] pub enum Subcommand { /// A set of base subcommands handled by `sc_cli`. #[structopt(flatten)] diff --git a/bin/node/inspect/src/cli.rs b/bin/node/inspect/src/cli.rs index 4475d31755fdc..d66644bab52fa 100644 --- a/bin/node/inspect/src/cli.rs +++ b/bin/node/inspect/src/cli.rs @@ -23,7 +23,7 @@ use sc_cli::{ImportParams, SharedParams}; use structopt::StructOpt; /// The `inspect` command used to print decoded chain data. -#[derive(Debug, StructOpt, Clone)] +#[derive(Debug, StructOpt)] pub struct InspectCmd { #[allow(missing_docs)] #[structopt(flatten)] @@ -39,7 +39,7 @@ pub struct InspectCmd { } /// A possible inspect sub-commands. -#[derive(Debug, StructOpt, Clone)] +#[derive(Debug, StructOpt)] pub enum InspectSubCmd { /// Decode block with native version of runtime and print out the details. Block { diff --git a/client/cli/src/commands/build_spec_cmd.rs b/client/cli/src/commands/build_spec_cmd.rs index d2e2ef3a5467c..23626359ff131 100644 --- a/client/cli/src/commands/build_spec_cmd.rs +++ b/client/cli/src/commands/build_spec_cmd.rs @@ -27,7 +27,7 @@ use structopt::StructOpt; use std::io::Write; /// The `build-spec` command used to build a specification. -#[derive(Debug, StructOpt, Clone)] +#[derive(Debug, StructOpt)] pub struct BuildSpecCmd { /// Force raw genesis storage output. #[structopt(long = "raw")] diff --git a/client/cli/src/commands/check_block_cmd.rs b/client/cli/src/commands/check_block_cmd.rs index d1241f010d597..c000ea7fb11ee 100644 --- a/client/cli/src/commands/check_block_cmd.rs +++ b/client/cli/src/commands/check_block_cmd.rs @@ -25,7 +25,7 @@ use std::{fmt::Debug, str::FromStr}; use structopt::StructOpt; /// The `check-block` command used to validate blocks. -#[derive(Debug, StructOpt, Clone)] +#[derive(Debug, StructOpt)] pub struct CheckBlockCmd { /// Block hash or number #[structopt(value_name = "HASH or NUMBER")] diff --git a/client/cli/src/commands/export_blocks_cmd.rs b/client/cli/src/commands/export_blocks_cmd.rs index 2fdc408250bce..7c523c0555d55 100644 --- a/client/cli/src/commands/export_blocks_cmd.rs +++ b/client/cli/src/commands/export_blocks_cmd.rs @@ -31,7 +31,7 @@ use std::path::PathBuf; use structopt::StructOpt; /// The `export-blocks` command used to export blocks. -#[derive(Debug, StructOpt, Clone)] +#[derive(Debug, StructOpt)] pub struct ExportBlocksCmd { /// Output file name or stdout if unspecified. #[structopt(parse(from_os_str))] diff --git a/client/cli/src/commands/export_state_cmd.rs b/client/cli/src/commands/export_state_cmd.rs index 33111e7737b1c..23a43a178abe5 100644 --- a/client/cli/src/commands/export_state_cmd.rs +++ b/client/cli/src/commands/export_state_cmd.rs @@ -27,7 +27,7 @@ use structopt::StructOpt; /// The `export-state` command used to export the state of a given block into /// a chain spec. -#[derive(Debug, StructOpt, Clone)] +#[derive(Debug, StructOpt)] pub struct ExportStateCmd { /// Block hash or number. #[structopt(value_name = "HASH or NUMBER")] @@ -59,7 +59,7 @@ impl ExportStateCmd { { info!("Exporting raw state..."); let mut input_spec = config.chain_spec.cloned_box(); - let block_id = self.input.clone().map(|b| b.parse()).transpose()?; + let block_id = self.input.as_ref().map(|b| b.parse()).transpose()?; let raw_state = builder(config)?.export_raw_state(block_id)?; input_spec.set_storage(raw_state); diff --git a/client/cli/src/commands/import_blocks_cmd.rs b/client/cli/src/commands/import_blocks_cmd.rs index a74f4d524c95b..8e178c4b97964 100644 --- a/client/cli/src/commands/import_blocks_cmd.rs +++ b/client/cli/src/commands/import_blocks_cmd.rs @@ -29,7 +29,7 @@ use std::path::PathBuf; use structopt::StructOpt; /// The `import-blocks` command used to import blocks. -#[derive(Debug, StructOpt, Clone)] +#[derive(Debug, StructOpt)] pub struct ImportBlocksCmd { /// Input file or stdin if unspecified. #[structopt(parse(from_os_str))] diff --git a/client/cli/src/commands/mod.rs b/client/cli/src/commands/mod.rs index 62757890ef01d..ea884508d240e 100644 --- a/client/cli/src/commands/mod.rs +++ b/client/cli/src/commands/mod.rs @@ -40,7 +40,7 @@ use structopt::StructOpt; /// The core commands are split into multiple subcommands and `Run` is the default subcommand. From /// the CLI user perspective, it is not visible that `Run` is a subcommand. So, all parameters of /// `Run` are exported as main executable parameters. -#[derive(Debug, Clone, StructOpt)] +#[derive(Debug, StructOpt)] pub enum Subcommand { /// Build a spec.json file, outputs to stdout. BuildSpec(BuildSpecCmd), diff --git a/client/cli/src/commands/purge_chain_cmd.rs b/client/cli/src/commands/purge_chain_cmd.rs index 9d364a45f7d09..053f427309828 100644 --- a/client/cli/src/commands/purge_chain_cmd.rs +++ b/client/cli/src/commands/purge_chain_cmd.rs @@ -26,7 +26,7 @@ use std::io::{self, Write}; use structopt::StructOpt; /// The `purge-chain` command used to remove the whole chain. -#[derive(Debug, StructOpt, Clone)] +#[derive(Debug, StructOpt)] pub struct PurgeChainCmd { /// Skip interactive prompt by answering yes automatically. #[structopt(short = "y")] diff --git a/client/cli/src/commands/revert_cmd.rs b/client/cli/src/commands/revert_cmd.rs index 6117eaf4880bf..1b5489df708a7 100644 --- a/client/cli/src/commands/revert_cmd.rs +++ b/client/cli/src/commands/revert_cmd.rs @@ -25,7 +25,7 @@ use std::fmt::Debug; use structopt::StructOpt; /// The `revert` command used revert the chain to a previous state. -#[derive(Debug, StructOpt, Clone)] +#[derive(Debug, StructOpt)] pub struct RevertCmd { /// Number of blocks to revert. #[structopt(default_value = "256")] diff --git a/client/cli/src/commands/run_cmd.rs b/client/cli/src/commands/run_cmd.rs index bddcfc98f5eaf..1697e25d7f17a 100644 --- a/client/cli/src/commands/run_cmd.rs +++ b/client/cli/src/commands/run_cmd.rs @@ -39,7 +39,6 @@ use std::{ use structopt::StructOpt; use tempfile::TempDir; -// TODO: check why all the structopt need clone? /// The `run` command used to run a node. #[derive(Debug, StructOpt)] pub struct RunCmd { diff --git a/client/cli/src/params/database_params.rs b/client/cli/src/params/database_params.rs index 3ff8eb01d0643..24b23f6076a02 100644 --- a/client/cli/src/params/database_params.rs +++ b/client/cli/src/params/database_params.rs @@ -20,7 +20,7 @@ use crate::arg_enums::Database; use structopt::StructOpt; /// Parameters for block import. -#[derive(Debug, StructOpt, Clone)] +#[derive(Debug, StructOpt)] pub struct DatabaseParams { /// Select database backend to use. #[structopt( diff --git a/client/cli/src/params/import_params.rs b/client/cli/src/params/import_params.rs index fb683df6d3be9..101189bec4448 100644 --- a/client/cli/src/params/import_params.rs +++ b/client/cli/src/params/import_params.rs @@ -27,7 +27,7 @@ use sc_client_api::execution_extensions::ExecutionStrategies; use structopt::StructOpt; /// Parameters for block import. -#[derive(Debug, StructOpt, Clone)] +#[derive(Debug, StructOpt)] pub struct ImportParams { #[allow(missing_docs)] #[structopt(flatten)] @@ -130,7 +130,7 @@ impl ImportParams { } /// Execution strategies parameters. -#[derive(Debug, StructOpt, Clone)] +#[derive(Debug, StructOpt)] pub struct ExecutionStrategiesParams { /// The means of execution used when calling into the runtime while syncing blocks. #[structopt( diff --git a/client/cli/src/params/keystore_params.rs b/client/cli/src/params/keystore_params.rs index 2fd610377d793..840cc51dff33f 100644 --- a/client/cli/src/params/keystore_params.rs +++ b/client/cli/src/params/keystore_params.rs @@ -26,7 +26,7 @@ use structopt::StructOpt; const DEFAULT_KEYSTORE_CONFIG_PATH: &'static str = "keystore"; /// Parameters of the keystore -#[derive(Debug, StructOpt, Clone)] +#[derive(Debug, StructOpt)] pub struct KeystoreParams { /// Specify custom keystore path. #[structopt(long = "keystore-path", value_name = "PATH", parse(from_os_str))] diff --git a/client/cli/src/params/mod.rs b/client/cli/src/params/mod.rs index 3a66e5f05588a..f648337ed0af6 100644 --- a/client/cli/src/params/mod.rs +++ b/client/cli/src/params/mod.rs @@ -39,7 +39,7 @@ pub use crate::params::shared_params::*; pub use crate::params::transaction_pool_params::*; /// Wrapper type of `String` that holds an unsigned integer of arbitrary size, formatted as a decimal. -#[derive(Debug, Clone)] +#[derive(Debug)] pub struct BlockNumber(String); impl FromStr for BlockNumber { @@ -72,7 +72,7 @@ impl BlockNumber { } /// Wrapper type that is either a `Hash` or the number of a `Block`. -#[derive(Debug, Clone)] +#[derive(Debug)] pub struct BlockNumberOrHash(String); impl FromStr for BlockNumberOrHash { diff --git a/client/cli/src/params/network_params.rs b/client/cli/src/params/network_params.rs index c1639ad2b4370..2e0a6f1973064 100644 --- a/client/cli/src/params/network_params.rs +++ b/client/cli/src/params/network_params.rs @@ -26,7 +26,7 @@ use std::path::PathBuf; use structopt::StructOpt; /// Parameters used to create the network configuration. -#[derive(Debug, StructOpt, Clone)] +#[derive(Debug, StructOpt)] pub struct NetworkParams { /// Specify a list of bootnodes. #[structopt(long = "bootnodes", value_name = "ADDR")] diff --git a/client/cli/src/params/node_key_params.rs b/client/cli/src/params/node_key_params.rs index 7d19971ad6470..689cc6c681c83 100644 --- a/client/cli/src/params/node_key_params.rs +++ b/client/cli/src/params/node_key_params.rs @@ -31,7 +31,7 @@ const NODE_KEY_ED25519_FILE: &str = "secret_ed25519"; /// Parameters used to create the `NodeKeyConfig`, which determines the keypair /// used for libp2p networking. -#[derive(Debug, StructOpt, Clone)] +#[derive(Debug, StructOpt)] pub struct NodeKeyParams { /// The secret key to use for libp2p networking. /// diff --git a/client/cli/src/params/offchain_worker_params.rs b/client/cli/src/params/offchain_worker_params.rs index ca99ab506e484..f8d48edc4729d 100644 --- a/client/cli/src/params/offchain_worker_params.rs +++ b/client/cli/src/params/offchain_worker_params.rs @@ -32,7 +32,7 @@ use crate::OffchainWorkerEnabled; /// Offchain worker related parameters. -#[derive(Debug, StructOpt, Clone)] +#[derive(Debug, StructOpt)] pub struct OffchainWorkerParams { /// Should execute offchain workers on every block. /// diff --git a/client/cli/src/params/pruning_params.rs b/client/cli/src/params/pruning_params.rs index 36179359063e7..7db808e6d8f2f 100644 --- a/client/cli/src/params/pruning_params.rs +++ b/client/cli/src/params/pruning_params.rs @@ -21,7 +21,7 @@ use sc_service::{PruningMode, Role}; use structopt::StructOpt; /// Parameters to define the pruning mode -#[derive(Debug, StructOpt, Clone)] +#[derive(Debug, StructOpt)] pub struct PruningParams { /// Specify the state pruning mode, a number of blocks to keep or 'archive'. /// diff --git a/client/cli/src/params/shared_params.rs b/client/cli/src/params/shared_params.rs index e9440f38a1fad..1127435915f97 100644 --- a/client/cli/src/params/shared_params.rs +++ b/client/cli/src/params/shared_params.rs @@ -20,7 +20,7 @@ use std::path::PathBuf; use structopt::StructOpt; /// Shared parameters used by all `CoreParams`. -#[derive(Debug, StructOpt, Clone)] +#[derive(Debug, StructOpt)] pub struct SharedParams { /// Specify the chain specification (one of dev, local, or staging). #[structopt(long, value_name = "CHAIN_SPEC")] diff --git a/client/cli/src/params/transaction_pool_params.rs b/client/cli/src/params/transaction_pool_params.rs index 2283c0f39f9c7..3ad278426922e 100644 --- a/client/cli/src/params/transaction_pool_params.rs +++ b/client/cli/src/params/transaction_pool_params.rs @@ -20,7 +20,7 @@ use sc_service::config::TransactionPoolOptions; use structopt::StructOpt; /// Parameters used to create the pool configuration. -#[derive(Debug, StructOpt, Clone)] +#[derive(Debug, StructOpt)] pub struct TransactionPoolParams { /// Maximum number of transactions in the transaction pool. #[structopt(long = "pool-limit", value_name = "COUNT", default_value = "8192")] diff --git a/utils/frame/benchmarking-cli/src/lib.rs b/utils/frame/benchmarking-cli/src/lib.rs index 7704f032b87fa..149b971577fab 100644 --- a/utils/frame/benchmarking-cli/src/lib.rs +++ b/utils/frame/benchmarking-cli/src/lib.rs @@ -21,7 +21,7 @@ use sc_cli::{ExecutionStrategy, WasmExecutionMethod}; use std::fmt::Debug; /// The `benchmark` command used to benchmark FRAME Pallets. -#[derive(Debug, structopt::StructOpt, Clone)] +#[derive(Debug, structopt::StructOpt)] pub struct BenchmarkCmd { /// Select a FRAME Pallet to benchmark, or `*` for all (in which case `extrinsic` must be `*`). #[structopt(short, long)] From b6358e84d34ca3e98e90f52eb9d54978b7f62539 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Wed, 3 Jun 2020 09:41:59 +0200 Subject: [PATCH 07/21] add test --- bin/node/cli/tests/temp_base_path_works.rs | 72 ++++++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 bin/node/cli/tests/temp_base_path_works.rs diff --git a/bin/node/cli/tests/temp_base_path_works.rs b/bin/node/cli/tests/temp_base_path_works.rs new file mode 100644 index 0000000000000..9351568d87955 --- /dev/null +++ b/bin/node/cli/tests/temp_base_path_works.rs @@ -0,0 +1,72 @@ +// This file is part of Substrate. + +// Copyright (C) 2020 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0 + +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +#![cfg(unix)] + +use assert_cmd::cargo::cargo_bin; +use nix::sys::signal::{kill, Signal::SIGINT}; +use nix::unistd::Pid; +use regex::Regex; +use std::convert::TryInto; +use std::io::Read; +use std::path::PathBuf; +use std::process::{Command, Stdio}; +use std::thread; +use std::time::Duration; + +pub mod common; + +#[test] +fn temp_base_path_works() { + let mut cmd = Command::new(cargo_bin("substrate")); + + let mut cmd = cmd + .args(&["--dev", "--tmp"]) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + + // Let it produce some blocks. + thread::sleep(Duration::from_secs(30)); + assert!( + cmd.try_wait().unwrap().is_none(), + "the process should still be running" + ); + + // Stop the process + kill(Pid::from_raw(cmd.id().try_into().unwrap()), SIGINT).unwrap(); + assert!(common::wait_for(&mut cmd, 40) + .map(|x| x.success()) + .unwrap_or_default()); + + // Ensure the database has been deleted + let mut stderr = String::new(); + cmd.stderr.unwrap().read_to_string(&mut stderr).unwrap(); + let re = Regex::new(r"Database: .+ at (\S+)").unwrap(); + let db_path = PathBuf::from( + re.captures(stderr.as_str()) + .unwrap() + .get(1) + .unwrap() + .as_str() + .to_string(), + ); + + assert!(!db_path.exists()); +} From f81522d89902a00682b062b46530431f3d537f27 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Wed, 3 Jun 2020 10:18:27 +0200 Subject: [PATCH 08/21] solving dependency issue --- client/cli/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/cli/Cargo.toml b/client/cli/Cargo.toml index a3369617f67b4..79dfa089aff56 100644 --- a/client/cli/Cargo.toml +++ b/client/cli/Cargo.toml @@ -44,10 +44,10 @@ structopt = "0.3.8" sc-tracing = { version = "2.0.0-rc2", path = "../tracing" } chrono = "0.4.10" parity-util-mem = { version = "0.6.1", default-features = false, features = ["primitive-types"] } -tempfile = "3.1.0" [target.'cfg(not(target_os = "unknown"))'.dependencies] rpassword = "4.0.1" +tempfile = "3.1.0" [target.'cfg(target_family = "unix")'.dependencies] nix = "0.17.0" From 55ed555d4965f615a702b2ce991023eb4bc49fd5 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Wed, 3 Jun 2020 10:18:41 +0200 Subject: [PATCH 09/21] clarifying doc --- client/cli/src/commands/run_cmd.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/client/cli/src/commands/run_cmd.rs b/client/cli/src/commands/run_cmd.rs index 1697e25d7f17a..8902455278c6a 100644 --- a/client/cli/src/commands/run_cmd.rs +++ b/client/cli/src/commands/run_cmd.rs @@ -258,10 +258,11 @@ pub struct RunCmd { /// Run a temporary node. /// - /// A random temporary directory will be created to store the configuration and will be deleted + /// A temporary directory will be created to store the configuration and will be deleted /// at the end of the process. /// - /// Note: the directory is random per process execution. + /// Note: the directory is random per process execution. This directory is used as base path + /// which includes: database, node key and keystore. #[structopt(long)] pub tmp: bool, From cfc6fa44d3f62f961ab123125c4017e0aa12164d Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Wed, 3 Jun 2020 14:12:38 +0200 Subject: [PATCH 10/21] conflict argument with base-path --- client/cli/src/commands/run_cmd.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/cli/src/commands/run_cmd.rs b/client/cli/src/commands/run_cmd.rs index 8902455278c6a..796b586b8a24f 100644 --- a/client/cli/src/commands/run_cmd.rs +++ b/client/cli/src/commands/run_cmd.rs @@ -263,7 +263,7 @@ pub struct RunCmd { /// /// Note: the directory is random per process execution. This directory is used as base path /// which includes: database, node key and keystore. - #[structopt(long)] + #[structopt(long, conflicts_with = "base-path")] pub tmp: bool, #[structopt(skip)] From 5a5c546cdf61621f8a632ceab7c8962218e868f7 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Thu, 4 Jun 2020 11:32:30 +0200 Subject: [PATCH 11/21] WIP Forked at: 342caad3074076a4fde0472719f6a473df839e42 Parent branch: origin/master --- Cargo.lock | 4 +- client/cli/Cargo.toml | 2 - client/cli/src/commands/mod.rs | 5 +-- client/cli/src/commands/run_cmd.rs | 20 ++-------- client/cli/src/config.rs | 23 ++++++----- client/cli/src/params/shared_params.rs | 12 ++---- client/service/Cargo.toml | 4 ++ client/service/src/config.rs | 53 +++++++++++++++++++++++++- 8 files changed, 79 insertions(+), 44 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5a13d30be1ef6..139cf6a0abcd0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5948,7 +5948,6 @@ dependencies = [ "atty", "chrono", "derive_more", - "directories", "env_logger 0.7.1", "fdlimit", "futures 0.3.4", @@ -5976,7 +5975,6 @@ dependencies = [ "sp-version", "structopt", "substrate-prometheus-endpoint", - "tempfile", "time", "tokio 0.2.18", ] @@ -6706,6 +6704,7 @@ name = "sc-service" version = "0.8.0-rc2" dependencies = [ "derive_more", + "directories", "exit-future", "futures 0.1.29", "futures 0.3.4", @@ -6758,6 +6757,7 @@ dependencies = [ "substrate-prometheus-endpoint", "substrate-test-runtime-client", "sysinfo", + "tempfile", "tracing", "wasm-timer", ] diff --git a/client/cli/Cargo.toml b/client/cli/Cargo.toml index 79dfa089aff56..61a4b4e29dcff 100644 --- a/client/cli/Cargo.toml +++ b/client/cli/Cargo.toml @@ -20,7 +20,6 @@ regex = "1.3.1" time = "0.1.42" ansi_term = "0.12.1" lazy_static = "1.4.0" -directories = "2.0.2" tokio = { version = "0.2.9", features = [ "signal", "rt-core", "rt-threaded" ] } futures = "0.3.4" fdlimit = "0.1.4" @@ -47,7 +46,6 @@ parity-util-mem = { version = "0.6.1", default-features = false, features = ["pr [target.'cfg(not(target_os = "unknown"))'.dependencies] rpassword = "4.0.1" -tempfile = "3.1.0" [target.'cfg(target_family = "unix")'.dependencies] nix = "0.17.0" diff --git a/client/cli/src/commands/mod.rs b/client/cli/src/commands/mod.rs index ea884508d240e..ff405a753fd31 100644 --- a/client/cli/src/commands/mod.rs +++ b/client/cli/src/commands/mod.rs @@ -27,11 +27,11 @@ mod run_cmd; pub use self::build_spec_cmd::BuildSpecCmd; pub use self::check_block_cmd::CheckBlockCmd; pub use self::export_blocks_cmd::ExportBlocksCmd; +pub use self::export_state_cmd::ExportStateCmd; pub use self::import_blocks_cmd::ImportBlocksCmd; pub use self::purge_chain_cmd::PurgeChainCmd; pub use self::revert_cmd::RevertCmd; pub use self::run_cmd::RunCmd; -pub use self::export_state_cmd::ExportStateCmd; use std::fmt::Debug; use structopt::StructOpt; @@ -162,7 +162,7 @@ macro_rules! substrate_cli_subcommands { } } - fn base_path(&self) -> $crate::Result<::std::option::Option<::std::path::PathBuf>> { + fn base_path(&self) -> $crate::Result<::std::option::Option<::sc_service::config::BasePath>> { match self { $($enum::$variant(cmd) => cmd.base_path()),* } @@ -409,4 +409,3 @@ macro_rules! substrate_cli_subcommands { substrate_cli_subcommands!( Subcommand => BuildSpec, ExportBlocks, ImportBlocks, CheckBlock, Revert, PurgeChain, ExportState ); - diff --git a/client/cli/src/commands/run_cmd.rs b/client/cli/src/commands/run_cmd.rs index 796b586b8a24f..82d40e6a73f07 100644 --- a/client/cli/src/commands/run_cmd.rs +++ b/client/cli/src/commands/run_cmd.rs @@ -27,17 +27,12 @@ use crate::params::TransactionPoolParams; use crate::CliConfiguration; use regex::Regex; use sc_service::{ - config::{MultiaddrWithPeerId, PrometheusConfig, TransactionPoolOptions}, + config::{BasePath, MultiaddrWithPeerId, PrometheusConfig, TransactionPoolOptions}, ChainSpec, Role, }; use sc_telemetry::TelemetryEndpoints; -use std::{ - cell::RefCell, - net::{IpAddr, Ipv4Addr, SocketAddr}, - path::PathBuf, -}; +use std::net::{IpAddr, Ipv4Addr, SocketAddr}; use structopt::StructOpt; -use tempfile::TempDir; /// The `run` command used to run a node. #[derive(Debug, StructOpt)] @@ -265,9 +260,6 @@ pub struct RunCmd { /// which includes: database, node key and keystore. #[structopt(long, conflicts_with = "base-path")] pub tmp: bool, - - #[structopt(skip)] - temp_base_path: RefCell>, } impl RunCmd { @@ -465,13 +457,9 @@ impl CliConfiguration for RunCmd { Ok(self.max_runtime_instances.map(|x| x.min(256))) } - fn base_path(&self) -> Result> { + fn base_path(&self) -> Result> { Ok(if self.tmp { - let tmp = tempfile::Builder::new().prefix("substrate").tempdir()?; - let path = tmp.path().into(); - *self.temp_base_path.borrow_mut() = Some(tmp); - - Some(path) + Some(BasePath::new_temp_dir()?) } else { self.shared_params().base_path() }) diff --git a/client/cli/src/config.rs b/client/cli/src/config.rs index a1ee1b0cc1da9..a594e56ae2821 100644 --- a/client/cli/src/config.rs +++ b/client/cli/src/config.rs @@ -27,9 +27,9 @@ use crate::{ use names::{Generator, Name}; use sc_client_api::execution_extensions::ExecutionStrategies; use sc_service::config::{ - Configuration, DatabaseConfig, ExtTransport, KeystoreConfig, NetworkConfiguration, - NodeKeyConfig, OffchainWorkerConfig, PrometheusConfig, PruningMode, Role, RpcMethods, - TaskType, TelemetryEndpoints, TransactionPoolOptions, WasmExecutionMethod, + BasePath, Configuration, DatabaseConfig, ExtTransport, KeystoreConfig, NetworkConfiguration, + NodeKeyConfig, OffchainWorkerConfig, PrometheusConfig, PruningMode, Role, RpcMethods, TaskType, + TelemetryEndpoints, TransactionPoolOptions, WasmExecutionMethod, }; use sc_service::{ChainSpec, TracingReceiver}; use std::future::Future; @@ -87,7 +87,7 @@ pub trait CliConfiguration: Sized { /// Get the base path of the configuration (if any) /// /// By default this is retrieved from `SharedParams`. - fn base_path(&self) -> Result> { + fn base_path(&self) -> Result> { Ok(self.shared_params().base_path()) } @@ -402,14 +402,12 @@ pub trait CliConfiguration: Sized { let is_dev = self.is_dev()?; let chain_id = self.chain_id(is_dev)?; let chain_spec = cli.load_spec(chain_id.as_str())?; - let config_dir = self + let base_path = self .base_path()? - .unwrap_or_else(|| { - directories::ProjectDirs::from("", "", C::executable_name()) - .expect("app directories exist on all supported platforms; qed") - .data_local_dir() - .into() - }) + .unwrap_or_else(|| BasePath::from_project("", "", C::executable_name())); + let config_dir = base_path + .path() + .to_path_buf() .join("chains") .join(chain_spec.id()); let net_config_dir = config_dir.join(DEFAULT_NETWORK_CONFIG_PATH); @@ -464,6 +462,7 @@ pub trait CliConfiguration: Sized { max_runtime_instances, announce_block: self.announce_block()?, role, + base_path, }) } @@ -507,5 +506,5 @@ pub fn generate_node_name() -> String { if count < NODE_NAME_MAX_LENGTH { return node_name; } - }; + } } diff --git a/client/cli/src/params/shared_params.rs b/client/cli/src/params/shared_params.rs index 1127435915f97..b6cef406343ac 100644 --- a/client/cli/src/params/shared_params.rs +++ b/client/cli/src/params/shared_params.rs @@ -16,6 +16,7 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . +use sc_service::config::BasePath; use std::path::PathBuf; use structopt::StructOpt; @@ -31,12 +32,7 @@ pub struct SharedParams { pub dev: bool, /// Specify custom base path. - #[structopt( - long, - short = "d", - value_name = "PATH", - parse(from_os_str) - )] + #[structopt(long, short = "d", value_name = "PATH", parse(from_os_str))] pub base_path: Option, /// Sets a custom logging filter. Syntax is =, e.g. -lsync=debug. @@ -49,8 +45,8 @@ pub struct SharedParams { impl SharedParams { /// Specify custom base path. - pub fn base_path(&self) -> Option { - self.base_path.clone() + pub fn base_path(&self) -> Option { + self.base_path.clone().map(|x| x.into()) } /// Specify the development chain. diff --git a/client/service/Cargo.toml b/client/service/Cargo.toml index fc5991bc3f131..1ac04e37fad74 100644 --- a/client/service/Cargo.toml +++ b/client/service/Cargo.toml @@ -81,6 +81,10 @@ netstat2 = "0.8.1" [target.'cfg(target_os = "linux")'.dependencies] procfs = '0.7.8' +[target.'cfg(not(target_os = "unknown"))'.dependencies] +tempfile = "3.1.0" +directories = "2.0.2" + [dev-dependencies] substrate-test-runtime-client = { version = "2.0.0-rc2", path = "../../test-utils/runtime/client" } diff --git a/client/service/src/config.rs b/client/service/src/config.rs index cc9c742ed68db..aba91a8987067 100644 --- a/client/service/src/config.rs +++ b/client/service/src/config.rs @@ -24,12 +24,13 @@ pub use sc_network::config::{ExtTransport, MultiaddrWithPeerId, NetworkConfigura pub use sc_executor::WasmExecutionMethod; use sc_client_api::execution_extensions::ExecutionStrategies; -use std::{future::Future, path::{PathBuf, Path}, pin::Pin, net::SocketAddr, sync::Arc}; +use std::{io, future::Future, path::{PathBuf, Path}, pin::Pin, net::SocketAddr, sync::Arc}; pub use sc_transaction_pool::txpool::Options as TransactionPoolOptions; use sc_chain_spec::ChainSpec; use sp_core::crypto::Protected; pub use sc_telemetry::TelemetryEndpoints; use prometheus_endpoint::Registry; +use tempfile::TempDir; /// Service configuration. pub struct Configuration { @@ -102,6 +103,8 @@ pub struct Configuration { pub max_runtime_instances: usize, /// Announce block automatically after they have been imported pub announce_block: bool, + /// Base path of the configuration + pub base_path: BasePath, } /// Type for tasks spawned by the executor. @@ -191,3 +194,51 @@ impl Default for RpcMethods { RpcMethods::Auto } } + +/// The base path that is used for everything that needs to be write on disk to run a node. +pub enum BasePath { + /// A temporary directory is used as base path and will be deleted when dropped. + #[cfg(not(target_os = "unknown"))] + Temporary(TempDir), + /// An path existing path on the disk. + Permanenent(PathBuf), +} + +impl BasePath { + /// Create a temporary directory prefixed with "substrate" and use it as base path. + #[cfg(not(target_os = "unknown"))] + pub fn new_temp_dir() -> io::Result { + Ok(BasePath::Temporary( + tempfile::Builder::new().prefix("substrate").tempdir()?, + )) + } + + /// Create a base path based on an existing path on disk. + pub fn new>(path: P) -> BasePath { + BasePath::Permanenent(path.as_ref().to_path_buf()) + } + + /// Create a base path from values describing the project. + #[cfg(not(target_os = "unknown"))] + pub fn from_project(qualifier: &str, organization: &str, application: &str) -> BasePath { + BasePath::new( + directories::ProjectDirs::from(qualifier, organization, application) + .expect("app directories exist on all supported platforms; qed") + .data_local_dir(), + ) + } + + /// Retrieve the base path. + pub fn path(&self) -> &Path { + match self { + BasePath::Temporary(temp_dir) => temp_dir.path(), + BasePath::Permanenent(path) => path.as_path(), + } + } +} + +impl std::convert::From for BasePath { + fn from(path: PathBuf) -> Self { + BasePath::new(path) + } +} From 68dfe898a18425827f33111da3b6df294f3990b0 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Thu, 4 Jun 2020 11:41:50 +0200 Subject: [PATCH 12/21] revert dep deletion --- Cargo.lock | 1 + client/cli/Cargo.toml | 3 +++ 2 files changed, 4 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index cdf097e763875..288135bf8d37f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5837,6 +5837,7 @@ dependencies = [ "sp-version", "structopt", "substrate-prometheus-endpoint", + "tempfile", "time", "tokio 0.2.18", ] diff --git a/client/cli/Cargo.toml b/client/cli/Cargo.toml index 61a4b4e29dcff..f7e94f36d7473 100644 --- a/client/cli/Cargo.toml +++ b/client/cli/Cargo.toml @@ -50,6 +50,9 @@ rpassword = "4.0.1" [target.'cfg(target_family = "unix")'.dependencies] nix = "0.17.0" +[dev-dependencies] +tempfile = "3.1.0" + [features] wasmtime = [ "sc-service/wasmtime", From 898dcd67a37b6de118c6c645fd5d6211461c35ac Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Thu, 4 Jun 2020 11:48:44 +0200 Subject: [PATCH 13/21] fixing test and making base_path optional --- client/cli/src/config.rs | 2 +- client/service/src/config.rs | 2 +- client/service/test/src/lib.rs | 3 ++- utils/browser/src/lib.rs | 1 + 4 files changed, 5 insertions(+), 3 deletions(-) diff --git a/client/cli/src/config.rs b/client/cli/src/config.rs index a594e56ae2821..1374e75daf9c3 100644 --- a/client/cli/src/config.rs +++ b/client/cli/src/config.rs @@ -462,7 +462,7 @@ pub trait CliConfiguration: Sized { max_runtime_instances, announce_block: self.announce_block()?, role, - base_path, + base_path: Some(base_path), }) } diff --git a/client/service/src/config.rs b/client/service/src/config.rs index aba91a8987067..f307c868d4001 100644 --- a/client/service/src/config.rs +++ b/client/service/src/config.rs @@ -104,7 +104,7 @@ pub struct Configuration { /// Announce block automatically after they have been imported pub announce_block: bool, /// Base path of the configuration - pub base_path: BasePath, + pub base_path: Option, } /// Type for tasks spawned by the executor. diff --git a/client/service/test/src/lib.rs b/client/service/test/src/lib.rs index 63c7e0795dc1a..ac4ec4c151746 100644 --- a/client/service/test/src/lib.rs +++ b/client/service/test/src/lib.rs @@ -34,7 +34,7 @@ use sc_service::{ GenericChainSpec, ChainSpecExtension, Configuration, - config::{DatabaseConfig, KeystoreConfig}, + config::{BasePath, DatabaseConfig, KeystoreConfig}, RuntimeGenesis, Role, Error, @@ -210,6 +210,7 @@ fn node_config Date: Thu, 4 Jun 2020 12:00:39 +0200 Subject: [PATCH 14/21] hold basepath while the service is running --- client/service/src/builder.rs | 3 ++- client/service/src/lib.rs | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/client/service/src/builder.rs b/client/service/src/builder.rs index d0cdaac5b7b68..d527baa3683f9 100644 --- a/client/service/src/builder.rs +++ b/client/service/src/builder.rs @@ -1350,7 +1350,8 @@ ServiceBuilder< _telemetry_on_connect_sinks: telemetry_connection_sinks.clone(), keystore, marker: PhantomData::, - prometheus_registry: config.prometheus_config.map(|config| config.registry) + prometheus_registry: config.prometheus_config.map(|config| config.registry), + _base_path: config.base_path, }) } } diff --git a/client/service/src/lib.rs b/client/service/src/lib.rs index 4f2be23f877ba..af862376768ed 100644 --- a/client/service/src/lib.rs +++ b/client/service/src/lib.rs @@ -66,7 +66,7 @@ pub use self::builder::{ ServiceBuilder, ServiceBuilderCommand, TFullClient, TLightClient, TFullBackend, TLightBackend, TFullCallExecutor, TLightCallExecutor, RpcExtensionBuilder, }; -pub use config::{Configuration, DatabaseConfig, PruningMode, Role, RpcMethods, TaskType}; +pub use config::{BasePath, Configuration, DatabaseConfig, PruningMode, Role, RpcMethods, TaskType}; pub use sc_chain_spec::{ ChainSpec, GenericChainSpec, Properties, RuntimeGenesis, Extension as ChainSpecExtension, NoExtension, ChainType, @@ -127,6 +127,7 @@ pub struct Service { keystore: sc_keystore::KeyStorePtr, marker: PhantomData, prometheus_registry: Option, + _base_path: Option, } impl Unpin for Service {} From 9c6f71a223a60e0d80ef6081c3435f514c88ff6a Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Thu, 4 Jun 2020 16:21:14 +0200 Subject: [PATCH 15/21] fixes --- client/service/src/config.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/client/service/src/config.rs b/client/service/src/config.rs index f307c868d4001..ee073a1d788f2 100644 --- a/client/service/src/config.rs +++ b/client/service/src/config.rs @@ -30,6 +30,7 @@ use sc_chain_spec::ChainSpec; use sp_core::crypto::Protected; pub use sc_telemetry::TelemetryEndpoints; use prometheus_endpoint::Registry; +#[cfg(not(target_os = "unknown"))] use tempfile::TempDir; /// Service configuration. @@ -231,6 +232,7 @@ impl BasePath { /// Retrieve the base path. pub fn path(&self) -> &Path { match self { + #[cfg(not(target_os = "unknown"))] BasePath::Temporary(temp_dir) => temp_dir.path(), BasePath::Permanenent(path) => path.as_path(), } From 9e989de92cad431915c49c62cc11bdfa48c04b73 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Fri, 5 Jun 2020 05:51:37 +0200 Subject: [PATCH 16/21] Update client/cli/src/params/shared_params.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- client/cli/src/params/shared_params.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/cli/src/params/shared_params.rs b/client/cli/src/params/shared_params.rs index b6cef406343ac..ad9ab04070563 100644 --- a/client/cli/src/params/shared_params.rs +++ b/client/cli/src/params/shared_params.rs @@ -46,7 +46,7 @@ pub struct SharedParams { impl SharedParams { /// Specify custom base path. pub fn base_path(&self) -> Option { - self.base_path.clone().map(|x| x.into()) + self.base_path.clone().map(Into::into) } /// Specify the development chain. From c811cb6152f73f3e23f9d54ffcc2ae488b742731 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Fri, 5 Jun 2020 05:51:50 +0200 Subject: [PATCH 17/21] Update client/service/Cargo.toml MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- client/service/Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/client/service/Cargo.toml b/client/service/Cargo.toml index fcc58aca1fab1..1eda92d735716 100644 --- a/client/service/Cargo.toml +++ b/client/service/Cargo.toml @@ -86,7 +86,6 @@ procfs = '0.7.8' tempfile = "3.1.0" directories = "2.0.2" - [dev-dependencies] substrate-test-runtime-client = { version = "2.0.0-rc2", path = "../../test-utils/runtime/client" } sp-consensus-babe = { version = "0.8.0-rc2", path = "../../primitives/consensus/babe" } From 65cb074ea11ff637be3378afc99b2a856393b847 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Fri, 5 Jun 2020 05:52:09 +0200 Subject: [PATCH 18/21] Update client/cli/src/commands/mod.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- client/cli/src/commands/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/cli/src/commands/mod.rs b/client/cli/src/commands/mod.rs index ff405a753fd31..a4d5c8ca7f20b 100644 --- a/client/cli/src/commands/mod.rs +++ b/client/cli/src/commands/mod.rs @@ -162,7 +162,7 @@ macro_rules! substrate_cli_subcommands { } } - fn base_path(&self) -> $crate::Result<::std::option::Option<::sc_service::config::BasePath>> { + fn base_path(&self) -> $crate::Result<::std::option::Option> { match self { $($enum::$variant(cmd) => cmd.base_path()),* } From fdb3f20e801673705e69454f7856f82b336c4ceb Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Fri, 5 Jun 2020 05:53:49 +0200 Subject: [PATCH 19/21] Update client/service/src/config.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- client/service/src/config.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/service/src/config.rs b/client/service/src/config.rs index ee073a1d788f2..a217f6c79aa7a 100644 --- a/client/service/src/config.rs +++ b/client/service/src/config.rs @@ -201,7 +201,7 @@ pub enum BasePath { /// A temporary directory is used as base path and will be deleted when dropped. #[cfg(not(target_os = "unknown"))] Temporary(TempDir), - /// An path existing path on the disk. + /// A path on the disk. Permanenent(PathBuf), } From 39bc0228a3e1ce3ad1e15ae5da524e488d05069a Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Fri, 5 Jun 2020 06:15:05 +0200 Subject: [PATCH 20/21] WIP Forked at: 342caad3074076a4fde0472719f6a473df839e42 Parent branch: origin/master --- client/cli/src/runner.rs | 4 ++++ client/service/src/builder.rs | 2 +- client/service/src/lib.rs | 23 ++++++++++++++++------- 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/client/cli/src/runner.rs b/client/cli/src/runner.rs index 9286031589892..1f3cdf6f16dd4 100644 --- a/client/cli/src/runner.rs +++ b/client/cli/src/runner.rs @@ -274,6 +274,10 @@ impl Runner { // and drop the runtime first. let _telemetry = service.telemetry(); + // we hold a reference to the base path so if the base path is a temporary directory it will + // not be deleted before the tokio runtime finish to clean up + let _base_path = service.base_path(); + { let f = service.fuse(); self.tokio_runtime diff --git a/client/service/src/builder.rs b/client/service/src/builder.rs index d527baa3683f9..9dde0bc5eb75c 100644 --- a/client/service/src/builder.rs +++ b/client/service/src/builder.rs @@ -1351,7 +1351,7 @@ ServiceBuilder< keystore, marker: PhantomData::, prometheus_registry: config.prometheus_config.map(|config| config.registry), - _base_path: config.base_path, + _base_path: config.base_path.map(Arc::new), }) } } diff --git a/client/service/src/lib.rs b/client/service/src/lib.rs index af862376768ed..bceaf8f3aed09 100644 --- a/client/service/src/lib.rs +++ b/client/service/src/lib.rs @@ -110,14 +110,14 @@ pub struct Service { task_manager: TaskManager, select_chain: Option, network: Arc, - /// Sinks to propagate network status updates. - /// For each element, every time the `Interval` fires we push an element on the sender. + // Sinks to propagate network status updates. + // For each element, every time the `Interval` fires we push an element on the sender. network_status_sinks: Arc>>, transaction_pool: Arc, - /// Send a signal when a spawned essential task has concluded. The next time - /// the service future is polled it should complete with an error. + // Send a signal when a spawned essential task has concluded. The next time + // the service future is polled it should complete with an error. essential_failed_tx: TracingUnboundedSender<()>, - /// A receiver for spawned essential-tasks concluding. + // A receiver for spawned essential-tasks concluding. essential_failed_rx: TracingUnboundedReceiver<()>, rpc_handlers: sc_rpc_server::RpcHandler, _rpc: Box, @@ -127,7 +127,9 @@ pub struct Service { keystore: sc_keystore::KeyStorePtr, marker: PhantomData, prometheus_registry: Option, - _base_path: Option, + // The base path is kept here because it can be a temporary directory which will be deleted + // when dropped + _base_path: Option>, } impl Unpin for Service {} @@ -211,6 +213,9 @@ pub trait AbstractService: Future> + Send + Unpin + S /// Get the prometheus metrics registry, if available. fn prometheus_registry(&self) -> Option; + + /// Get a clone of the base_path + fn base_path(&self) -> Option>; } impl AbstractService for @@ -245,7 +250,7 @@ where } fn telemetry(&self) -> Option { - self._telemetry.as_ref().map(|t| t.clone()) + self._telemetry.clone() } fn keystore(&self) -> sc_keystore::KeyStorePtr { @@ -311,6 +316,10 @@ where fn prometheus_registry(&self) -> Option { self.prometheus_registry.clone() } + + fn base_path(&self) -> Option> { + self._base_path.clone() + } } impl Future for From cb140b6eee63c209339552280a8fb77909cb24fa Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Mon, 8 Jun 2020 09:28:41 +0200 Subject: [PATCH 21/21] improve doc --- client/service/src/config.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/client/service/src/config.rs b/client/service/src/config.rs index a217f6c79aa7a..2d4dc9dc2e90a 100644 --- a/client/service/src/config.rs +++ b/client/service/src/config.rs @@ -206,7 +206,11 @@ pub enum BasePath { } impl BasePath { - /// Create a temporary directory prefixed with "substrate" and use it as base path. + /// Create a `BasePath` instance using a temporary directory prefixed with "substrate" and use + /// it as base path. + /// + /// Note: the temporary directory will be created automatically and deleted when the `BasePath` + /// instance is dropped. #[cfg(not(target_os = "unknown"))] pub fn new_temp_dir() -> io::Result { Ok(BasePath::Temporary( @@ -214,7 +218,10 @@ impl BasePath { )) } - /// Create a base path based on an existing path on disk. + /// Create a `BasePath` instance based on an existing path on disk. + /// + /// Note: this function will not ensure that the directory exist nor create the directory. It + /// will also not delete the directory when the instance is dropped. pub fn new>(path: P) -> BasePath { BasePath::Permanenent(path.as_ref().to_path_buf()) }