CLI API refactoring and improvement

bin/node-template/Cargo.toml

@@ -12,7 +12,6 @@ path = "src/main.rs"
 [dependencies]
 futures = "0.3.1"
 futures01 = { package = "futures", version = "0.1.29" }
-ctrlc = { version = "3.1.3", features = ["termination"] }
 log = "0.4.8"
 tokio = "0.1.22"
 parking_lot = "0.9.0"
@@ -36,6 +35,7 @@ sc-client = { version = "2.0.0", path = "../../client/" }
 node-template-runtime = { version = "2.0.0", path = "runtime" }
 sp-runtime = { version = "2.0.0", path = "../../primitives/runtime" }
 sc-basic-authority = { path  = "../../client/basic-authorship" }
+structopt = "=0.3.8"
 
 [build-dependencies]
 vergen = "3.0.4"

bin/node-template/src/cli.rs

@@ -1,58 +1,49 @@
 use crate::service;
-use futures::{future::{select, Map}, FutureExt, TryFutureExt, channel::oneshot, compat::Future01CompatExt};
-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};
+pub use sc_cli::{VersionInfo, error};
+use sc_cli::{CoreParams, RunCmd};
 use sc_service::{AbstractService, Roles as ServiceRoles, Configuration};
+use structopt::StructOpt;
 use sp_consensus_aura::sr25519::{AuthorityPair as AuraPair};
 use crate::chain_spec;
 use log::info;
 
+#[derive(Clone, Debug, StructOpt)]
+#[structopt(settings = &[structopt::clap::AppSettings::GlobalVersion, structopt::clap::AppSettings::ArgsNegateSubcommands, structopt::clap::AppSettings::SubcommandsNegateReqs])]
+struct Cli {
+	#[structopt(subcommand)]
+	subcommand: Option<CoreParams>,
+}
+
 /// 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>,
+pub fn run<I, T>(args: I, version: VersionInfo) -> error::Result<()>
+where
+	I: Iterator<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, 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))?;
-			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(())
-	}?;
-
-	Ok(())
+	type Config<A, B> = Configuration<A, B>;
+
+	let args: Vec<_> = args.collect();
+	let opt = sc_cli::from_iter::<Cli, _>(args.clone(), &version);
+	let subcommand = opt.subcommand.unwrap_or_else(|| {
+		let opt = sc_cli::from_iter::<RunCmd, _>(args, &version);
+
+		eprintln!(
+			"WARNING: running this command without the subcommand `run` is deprecated, please \
+			use run:\n{} run [node_arguments]",
+			version.executable_name,
+		);
+		CoreParams::Run(opt)
+	});
+
+	sc_cli::run(
+		subcommand,
+		service::new_light,
+		service::new_full,
+		load_spec,
+		|config: Config<_, _>| Ok(new_full_start!(config).0),
+		"substrate-node",
+		&version,
+	)
 }
 
 fn load_spec(id: &str) -> Result<Option<chain_spec::ChainSpec>, String> {
@@ -61,68 +52,3 @@ fn load_spec(id: &str) -> Result<Option<chain_spec::ChainSpec>, String> {
 		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 future = select(exit, informant)
-		.map(|_| Ok(()))
-		.compat();
-
-	runtime.executor().spawn(future);
-
-	// 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 service_res = {
-		let exit = e.into_exit();
-		let service = service
-			.map_err(|err| error::Error::Service(err))
-			.compat();
-		let select = select(service, exit)
-			.map(|_| Ok(()))
-			.compat();
-		runtime.block_on(select)
-	};
-
-	let _ = exit_send.send(());
-
-	// TODO [andre]: timeout this future #1318
-
-	use futures01::Future;
-
-	let _ = runtime.shutdown_on_idle().wait();
-
-	service_res
-}
-
-// 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)
-	}
-}

bin/node-template/src/main.rs

@@ -8,7 +8,7 @@ mod chain_spec;
 mod service;
 mod cli;
 
-pub use sc_cli::{VersionInfo, IntoExit, error};
+pub use sc_cli::{VersionInfo, error};
 
 fn main() -> Result<(), cli::error::Error> {
 	let version = VersionInfo {
@@ -21,5 +21,5 @@ fn main() -> Result<(), cli::error::Error> {
 		support_url: "support.anonymous.an",
 	};
 
-	cli::run(std::env::args(), cli::Exit, version)
+	cli::run(std::env::args(), version)
 }

bin/node-template/src/service.rs

@@ -80,7 +80,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();
@@ -195,7 +195,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 = "=0.3.8"
 
 # primitives
 sp-authority-discovery = { version = "2.0.0",  path = "../../../primitives/authority-discovery" }
@@ -83,7 +83,6 @@ node-executor = { version = "2.0.0", path = "../executor" }
 # CLI-specific dependencies
 tokio = { version = "0.1.22", optional = true }
 sc-cli = { version = "2.0.0", optional = true, path = "../../../client/cli" }
-ctrlc = { version = "3.1.3", features = ["termination"], optional = true }
 node-transaction-factory = { version = "2.0.0", optional = true, path = "../transaction-factory" }
 
 # WASM-specific dependencies
@@ -101,7 +100,7 @@ tempfile = "3.1.0"
 [build-dependencies]
 sc-cli = { version = "2.0.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"
+structopt = "=0.3.8"
 vergen = "3.0.4"
 
 [features]
@@ -115,7 +114,6 @@ cli = [
 	"sc-cli",
 	"node-transaction-factory",
 	"tokio",
-	"ctrlc",
 	"sc-service/rocksdb",
 	"node-executor/wasmi-errno",
 ]

bin/node/cli/bin/main.rs

@@ -22,27 +22,6 @@ 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",
@@ -54,5 +33,5 @@ fn main() -> Result<(), sc_cli::error::Error> {
 		support_url: "https://github.com/paritytech/substrate/issues/new",
 	};
 
-	node_cli::run(std::env::args(), Exit, version)
+	node_cli::run(std::env::args(), version)
 }

bin/node/cli/build.rs

@@ -16,7 +16,7 @@
 
 use std::{fs, env, path::Path};
 use structopt::{StructOpt, clap::Shell};
-use sc_cli::{NoCustom, CoreParams};
+use sc_cli::CoreParams;
 use vergen::{ConstantsFlags, generate_cargo_keys};
 
 fn main() {
@@ -48,5 +48,5 @@ fn build_completion(shell: &Shell) {
 
 	fs::create_dir(&path).ok();
 
-	CoreParams::<NoCustom, NoCustom>::clap().gen_completions("substrate-node", *shell, &path);
+	CoreParams::clap().gen_completions("substrate-node", *shell, &path);
 }