From 4af37e43616498c02d44d9cfa06632619dda3b52 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bastian=20K=C3=B6cher?= <info@kchr.de>
Date: Fri, 2 Sep 2022 09:27:00 +0200
Subject: [PATCH] Do not drop the `task_manager` for benchmarking stuff
 (#12147)

We can not drop the `task_manager` for benchmarking stuff, because otherwise stuff that may needs
this feature (like background signature verification) will fail. Besides the base path setup is
moved to `SharedParams` directly. Meaning any call to `base_path` will now directly return a tmp
path when `--dev` is given.
---
 bin/node/cli/src/command.rs            | 30 +++++++++++++++-----------
 client/cli/src/commands/insert_key.rs  |  2 +-
 client/cli/src/commands/run_cmd.rs     |  2 +-
 client/cli/src/config.rs               |  2 +-
 client/cli/src/params/shared_params.rs |  9 ++++++--
 5 files changed, 27 insertions(+), 18 deletions(-)

diff --git a/bin/node/cli/src/command.rs b/bin/node/cli/src/command.rs
index 85e5415dbe139..1d88e9b539421 100644
--- a/bin/node/cli/src/command.rs
+++ b/bin/node/cli/src/command.rs
@@ -115,35 +115,39 @@ pub fn run() -> Result<()> {
 						cmd.run::<Block, ExecutorDispatch>(config)
 					},
 					BenchmarkCmd::Block(cmd) => {
-						let PartialComponents { client, .. } = new_partial(&config)?;
-						cmd.run(client)
+						// ensure that we keep the task manager alive
+						let partial = new_partial(&config)?;
+						cmd.run(partial.client)
 					},
 					BenchmarkCmd::Storage(cmd) => {
-						let PartialComponents { client, backend, .. } = new_partial(&config)?;
-						let db = backend.expose_db();
-						let storage = backend.expose_storage();
+						// ensure that we keep the task manager alive
+						let partial = new_partial(&config)?;
+						let db = partial.backend.expose_db();
+						let storage = partial.backend.expose_storage();
 
-						cmd.run(config, client, db, storage)
+						cmd.run(config, partial.client, db, storage)
 					},
 					BenchmarkCmd::Overhead(cmd) => {
-						let PartialComponents { client, .. } = new_partial(&config)?;
-						let ext_builder = RemarkBuilder::new(client.clone());
+						// ensure that we keep the task manager alive
+						let partial = new_partial(&config)?;
+						let ext_builder = RemarkBuilder::new(partial.client.clone());
 
-						cmd.run(config, client, inherent_benchmark_data()?, &ext_builder)
+						cmd.run(config, partial.client, inherent_benchmark_data()?, &ext_builder)
 					},
 					BenchmarkCmd::Extrinsic(cmd) => {
-						let PartialComponents { client, .. } = service::new_partial(&config)?;
+						// ensure that we keep the task manager alive
+						let partial = service::new_partial(&config)?;
 						// Register the *Remark* and *TKA* builders.
 						let ext_factory = ExtrinsicFactory(vec![
-							Box::new(RemarkBuilder::new(client.clone())),
+							Box::new(RemarkBuilder::new(partial.client.clone())),
 							Box::new(TransferKeepAliveBuilder::new(
-								client.clone(),
+								partial.client.clone(),
 								Sr25519Keyring::Alice.to_account_id(),
 								ExistentialDeposit::get(),
 							)),
 						]);
 
-						cmd.run(client, inherent_benchmark_data()?, &ext_factory)
+						cmd.run(partial.client, inherent_benchmark_data()?, &ext_factory)
 					},
 					BenchmarkCmd::Machine(cmd) =>
 						cmd.run(&config, SUBSTRATE_REFERENCE_HARDWARE.clone()),
diff --git a/client/cli/src/commands/insert_key.rs b/client/cli/src/commands/insert_key.rs
index 68201d7b4bffc..5029ecd29248c 100644
--- a/client/cli/src/commands/insert_key.rs
+++ b/client/cli/src/commands/insert_key.rs
@@ -60,7 +60,7 @@ impl InsertKeyCmd {
 		let suri = utils::read_uri(self.suri.as_ref())?;
 		let base_path = self
 			.shared_params
-			.base_path()
+			.base_path()?
 			.unwrap_or_else(|| BasePath::from_project("", "", &C::executable_name()));
 		let chain_id = self.shared_params.chain_id(self.shared_params.is_dev());
 		let chain_spec = cli.load_spec(&chain_id)?;
diff --git a/client/cli/src/commands/run_cmd.rs b/client/cli/src/commands/run_cmd.rs
index fd236aa0211dd..11cfb9d0ff651 100644
--- a/client/cli/src/commands/run_cmd.rs
+++ b/client/cli/src/commands/run_cmd.rs
@@ -485,7 +485,7 @@ impl CliConfiguration for RunCmd {
 		Ok(if self.tmp {
 			Some(BasePath::new_temp_dir()?)
 		} else {
-			match self.shared_params().base_path() {
+			match self.shared_params().base_path()? {
 				Some(r) => Some(r),
 				// If `dev` is enabled, we use the temp base path.
 				None if self.shared_params().is_dev() => Some(BasePath::new_temp_dir()?),
diff --git a/client/cli/src/config.rs b/client/cli/src/config.rs
index 92091ea118ff1..bc5941914de89 100644
--- a/client/cli/src/config.rs
+++ b/client/cli/src/config.rs
@@ -125,7 +125,7 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
 	///
 	/// By default this is retrieved from `SharedParams`.
 	fn base_path(&self) -> Result<Option<BasePath>> {
-		Ok(self.shared_params().base_path())
+		self.shared_params().base_path()
 	}
 
 	/// Returns `true` if the node is for development or not
diff --git a/client/cli/src/params/shared_params.rs b/client/cli/src/params/shared_params.rs
index 67b18aa8b09e2..4a8fc4bcfffdd 100644
--- a/client/cli/src/params/shared_params.rs
+++ b/client/cli/src/params/shared_params.rs
@@ -82,8 +82,13 @@ pub struct SharedParams {
 
 impl SharedParams {
 	/// Specify custom base path.
-	pub fn base_path(&self) -> Option<BasePath> {
-		self.base_path.clone().map(Into::into)
+	pub fn base_path(&self) -> Result<Option<BasePath>, crate::Error> {
+		match &self.base_path {
+			Some(r) => Ok(Some(r.clone().into())),
+			// If `dev` is enabled, we use the temp base path.
+			None if self.is_dev() => Ok(Some(BasePath::new_temp_dir()?)),
+			None => Ok(None),
+		}
 	}
 
 	/// Specify the development chain.