CLI API refactoring and improvement

bin/node-template/Cargo.toml

@@ -12,9 +12,7 @@ path = "src/main.rs"
 
 [dependencies]
 futures = "0.3.1"
-ctrlc = { version = "3.1.3", features = ["termination"] }
 log = "0.4.8"
-tokio = { version = "0.2", features = ["rt-threaded"] }
 sc-cli = { version = "0.8.0", path = "../../client/cli" }
 sp-core = { version = "2.0.0", path = "../../primitives/core" }
 sc-executor = { version = "0.8", path = "../../client/executor" }
@@ -31,7 +29,8 @@ grandpa-primitives = { version = "2.0.0", package = "sp-finality-grandpa", path
 sc-client = { version = "0.8", path = "../../client/" }
 node-template-runtime = { version = "2.0.0", path = "runtime" }
 sp-runtime = { version = "2.0.0", path = "../../primitives/runtime" }
-sc-basic-authorship = { path  = "../../client/basic-authorship" }
+sc-basic-authorship = { path = "../../client/basic-authorship" }
+structopt = "0.3.8"
 
 [build-dependencies]
 vergen = "3.0.4"

bin/node-template/src/chain_spec.rs

@@ -145,3 +145,10 @@ fn testnet_genesis(initial_authorities: Vec<(AuraId, GrandpaId)>,
 		}),
 	}
 }
+
+pub fn load_spec(id: &str) -> Result<Option<ChainSpec>, String> {
+	Ok(match Alternative::from(id) {
+		Some(spec) => Some(spec.load()?),
+		None => None,
+	})
+}

bin/node-template/src/cli.rs

@@ -1,121 +1,11 @@
-use crate::service;
-use futures::{future::{select, Map, Either}, FutureExt, channel::oneshot};
-use std::cell::RefCell;
-use tokio::runtime::Runtime;
-pub use sc_cli::{VersionInfo, IntoExit, error};
-use sc_cli::{display_role, informant, parse_and_prepare, ParseAndPrepare, NoCustom};
-use sc_service::{AbstractService, Roles as ServiceRoles, Configuration};
-use sp_consensus_aura::sr25519::{AuthorityPair as AuraPair};
-use crate::chain_spec;
-use log::info;
+use sc_cli::{RunCmd, Subcommand};
+use structopt::StructOpt;
 
-/// Parse command line arguments into service configuration.
-pub fn run<I, T, E>(args: I, exit: E, version: VersionInfo) -> error::Result<()> where
-	I: IntoIterator<Item = T>,
-	T: Into<std::ffi::OsString> + Clone,
-	E: IntoExit,
-{
-	type Config<T> = Configuration<(), T>;
-	match parse_and_prepare::<NoCustom, NoCustom, _>(&version, "substrate-node", args) {
-		ParseAndPrepare::Run(cmd) => cmd.run(load_spec, exit,
-		|exit, _cli_args, _custom_args, mut config: Config<_>| {
-			info!("{}", version.name);
-			info!("  version {}", config.full_version());
-			info!("  by {}, 2017, 2018", version.author);
-			info!("Chain specification: {}", config.chain_spec.name());
-			info!("Node name: {}", config.name);
-			info!("Roles: {}", display_role(&config));
-			let runtime = Runtime::new().map_err(|e| format!("{:?}", e))?;
-			config.tasks_executor = {
-				let runtime_handle = runtime.handle().clone();
-				Some(Box::new(move |fut| { runtime_handle.spawn(fut); }))
-			};
-			match config.roles {
-				ServiceRoles::LIGHT => run_until_exit(
-					runtime,
-					service::new_light(config)?,
-					exit
-				),
-				_ => run_until_exit(
-					runtime,
-					service::new_full(config)?,
-					exit
-				),
-			}
-		}),
-		ParseAndPrepare::BuildSpec(cmd) => cmd.run::<NoCustom, _, _, _>(load_spec),
-		ParseAndPrepare::ExportBlocks(cmd) => cmd.run_with_builder(|config: Config<_>|
-			Ok(new_full_start!(config).0), load_spec, exit),
-		ParseAndPrepare::ImportBlocks(cmd) => cmd.run_with_builder(|config: Config<_>|
-			Ok(new_full_start!(config).0), load_spec, exit),
-		ParseAndPrepare::CheckBlock(cmd) => cmd.run_with_builder(|config: Config<_>|
-			Ok(new_full_start!(config).0), load_spec, exit),
-		ParseAndPrepare::PurgeChain(cmd) => cmd.run(load_spec),
-		ParseAndPrepare::RevertChain(cmd) => cmd.run_with_builder(|config: Config<_>|
-			Ok(new_full_start!(config).0), load_spec),
-		ParseAndPrepare::CustomCommand(_) => Ok(())
-	}?;
+#[derive(Debug, StructOpt)]
+pub struct Cli {
+	#[structopt(subcommand)]
+	pub subcommand: Option<Subcommand>,
 
-	Ok(())
-}
-
-fn load_spec(id: &str) -> Result<Option<chain_spec::ChainSpec>, String> {
-	Ok(match chain_spec::Alternative::from(id) {
-		Some(spec) => Some(spec.load()?),
-		None => None,
-	})
-}
-
-fn run_until_exit<T, E>(
-	mut runtime: Runtime,
-	service: T,
-	e: E,
-) -> error::Result<()>
-where
-	T: AbstractService,
-	E: IntoExit,
-{
-	let (exit_send, exit) = oneshot::channel();
-
-	let informant = informant::build(&service);
-
-	let handle = runtime.spawn(select(exit, informant));
-
-	// we eagerly drop the service so that the internal exit future is fired,
-	// but we need to keep holding a reference to the global telemetry guard
-	let _telemetry = service.telemetry();
-
-	let exit = e.into_exit();
-	let service_res = runtime.block_on(select(service, exit));
-
-	let _ = exit_send.send(());
-
-	if let Err(e) = runtime.block_on(handle) {
-		log::error!("Error running node: {:?}", e);
-	}
-
-	match service_res {
-		Either::Left((res, _)) => res.map_err(error::Error::Service),
-		Either::Right((_, _)) => Ok(())
-	}
-}
-
-// handles ctrl-c
-pub struct Exit;
-impl IntoExit for Exit {
-	type Exit = Map<oneshot::Receiver<()>, fn(Result<(), oneshot::Canceled>) -> ()>;
-	fn into_exit(self) -> Self::Exit {
-		// can't use signal directly here because CtrlC takes only `Fn`.
-		let (exit_send, exit) = oneshot::channel();
-
-		let exit_send_cell = RefCell::new(Some(exit_send));
-		ctrlc::set_handler(move || {
-			let exit_send = exit_send_cell.try_borrow_mut().expect("signal handler not reentrant; qed").take();
-			if let Some(exit_send) = exit_send {
-				exit_send.send(()).expect("Error sending exit notification");
-			}
-		}).expect("Error setting Ctrl-C handler");
-
-		exit.map(drop)
-	}
+	#[structopt(flatten)]
+	pub run: RunCmd,
 }

bin/node-template/src/command.rs

@@ -0,0 +1,48 @@
+// Copyright 2017-2020 Parity Technologies (UK) Ltd.
+// This file is part of Substrate.
+
+// Substrate 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.
+
+// Substrate 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 Substrate.  If not, see <http://www.gnu.org/licenses/>.
+
+use sp_consensus_aura::sr25519::{AuthorityPair as AuraPair};
+use sc_cli::{VersionInfo, error};
+use crate::service;
+use crate::chain_spec;
+use crate::cli::Cli;
+
+/// Parse and run command line arguments
+pub fn run(version: VersionInfo) -> error::Result<()>
+{
+	let opt = sc_cli::from_args::<Cli>(&version);
+
+	let mut config = sc_service::Configuration::default();
+	config.impl_name = "node-template";
+
+	match opt.subcommand {
+		Some(subcommand) => sc_cli::run_subcommand(
+			config,
+			subcommand,
+			chain_spec::load_spec,
+			|config: _| Ok(new_full_start!(config).0),
+			&version,
+		),
+		None => sc_cli::run(
+			config,
+			opt.run,
+			service::new_light,
+			service::new_full,
+			chain_spec::load_spec,
+			&version,
+		)
+	}
+}

bin/node-template/src/main.rs

@@ -5,10 +5,11 @@ mod chain_spec;
 #[macro_use]
 mod service;
 mod cli;
+mod command;
 
-pub use sc_cli::{VersionInfo, IntoExit, error};
+pub use sc_cli::{VersionInfo, error};
 
-fn main() -> Result<(), cli::error::Error> {
+fn main() -> Result<(), error::Error> {
 	let version = VersionInfo {
 		name: "Substrate Node",
 		commit: env!("VERGEN_SHA_SHORT"),
@@ -17,7 +18,8 @@ fn main() -> Result<(), cli::error::Error> {
 		author: "Anonymous",
 		description: "Template Node",
 		support_url: "support.anonymous.an",
+		copyright_start_year: 2017,
 	};
 
-	cli::run(std::env::args(), cli::Exit, version)
+	command::run(version)
 }

bin/node-template/src/service.rs

@@ -77,7 +77,7 @@ macro_rules! new_full_start {
 }
 
 /// Builds a new service for a full client.
-pub fn new_full<C: Send + Default + 'static>(config: Configuration<C, GenesisConfig>)
+pub fn new_full(config: Configuration<GenesisConfig>)
 	-> Result<impl AbstractService, ServiceError>
 {
 	let is_authority = config.roles.is_authority();
@@ -192,7 +192,7 @@ pub fn new_full<C: Send + Default + 'static>(config: Configuration<C, GenesisCon
 }
 
 /// Builds a new service for a light client.
-pub fn new_light<C: Send + Default + 'static>(config: Configuration<C, GenesisConfig>)
+pub fn new_light(config: Configuration<GenesisConfig>)
 	-> Result<impl AbstractService, ServiceError>
 {
 	let inherent_data_providers = InherentDataProviders::new();

bin/node/cli/Cargo.toml

@@ -31,7 +31,7 @@ hex-literal = "0.2.1"
 jsonrpc-core = "14.0.3"
 log = "0.4.8"
 rand = "0.7.2"
-structopt = "=0.3.7"
+structopt = { version = "0.3.8", optional = true }
 
 # primitives
 sp-authority-discovery = { version = "2.0.0",  path = "../../../primitives/authority-discovery" }
@@ -81,9 +81,7 @@ node-primitives = { version = "2.0.0", path = "../primitives" }
 node-executor = { version = "2.0.0", path = "../executor" }
 
 # CLI-specific dependencies
-tokio = { version = "0.2", features = ["rt-threaded"], optional = true }
 sc-cli = { version = "0.8.0", optional = true, path = "../../../client/cli" }
-ctrlc = { version = "3.1.3", features = ["termination"], optional = true }
 node-transaction-factory = { version = "0.8.0", optional = true, path = "../transaction-factory" }
 
 # WASM-specific dependencies
@@ -99,10 +97,19 @@ futures = "0.3.1"
 tempfile = "3.1.0"
 
 [build-dependencies]
-sc-cli = { version = "0.8.0", package = "sc-cli", path = "../../../client/cli" }
 build-script-utils = { version = "2.0.0", package = "substrate-build-script-utils", path = "../../../utils/build-script-utils" }
-structopt = "=0.3.7"
-vergen = "3.0.4"
+structopt = { version = "0.3.8", optional = true }
+node-transaction-factory = { version = "0.8.0", optional = true, path = "../transaction-factory" }
+
+[build-dependencies.sc-cli]
+version = "0.8.0"
+package = "sc-cli"
+path = "../../../client/cli"
+optional = true
+
+[build-dependencies.vergen]
+version = "3.0.4"
+optional = true
 
 [features]
 default = ["cli", "wasmtime"]
@@ -114,10 +121,10 @@ browser = [
 cli = [
 	"sc-cli",
 	"node-transaction-factory",
-	"tokio",
-	"ctrlc",
 	"sc-service/rocksdb",
 	"node-executor/wasmi-errno",
+	"vergen",
+	"structopt",
 ]
 wasmtime = [
 	"cli",

bin/node/cli/bin/main.rs

@@ -18,31 +18,8 @@
 
 #![warn(missing_docs)]
 
-use futures::channel::oneshot;
-use futures::{future, FutureExt};
 use sc_cli::VersionInfo;
 
-use std::cell::RefCell;
-
-// handles ctrl-c
-struct Exit;
-impl sc_cli::IntoExit for Exit {
-	type Exit = future::Map<oneshot::Receiver<()>, fn(Result<(), oneshot::Canceled>) -> ()>;
-	fn into_exit(self) -> Self::Exit {
-		// can't use signal directly here because CtrlC takes only `Fn`.
-		let (exit_send, exit) = oneshot::channel();
-
-		let exit_send_cell = RefCell::new(Some(exit_send));
-		ctrlc::set_handler(move || {
-			if let Some(exit_send) = exit_send_cell.try_borrow_mut().expect("signal handler not reentrant; qed").take() {
-				exit_send.send(()).expect("Error sending exit notification");
-			}
-		}).expect("Error setting Ctrl-C handler");
-
-		exit.map(|_| ())
-	}
-}
-
 fn main() -> Result<(), sc_cli::error::Error> {
 	let version = VersionInfo {
 		name: "Substrate Node",
@@ -52,7 +29,8 @@ fn main() -> Result<(), sc_cli::error::Error> {
 		author: "Parity Technologies <[email protected]>",
 		description: "Generic substrate node",
 		support_url: "https://github.com/paritytech/substrate/issues/new",
+		copyright_start_year: 2017,
 	};
 
-	node_cli::run(std::env::args(), Exit, version)
+	node_cli::run(std::env::args(), version)
 }