Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Commit

Permalink
CLI API refactoring and improvement (#4692)
Browse files Browse the repository at this point in the history
It changes the way we extended the CLI functionalities of substrate to allow more flexibility. (If this was not clear, here is another version: it changes the `sc_cli` API to allow more flexibility).

This touches a few important things:
 - the startup of the async task with tokei:
    This was in node and node-template and I moved it to substrate. The idea is to have 1 time the code that handles unix signals (SIGTERM and SIGINT) properly. It is however possible to make this more generic to wait for a future instead and provide only a helper for the basic handling of SIGTERM and SIGINT.
 - increased the version of structopt and tokei
 - no more use of structopt internal's API
 - less use of generics

Related to #4643 and paritytech/cumulus#42: the implementation of "into_configuration" and "get_config" are similar but with better flexibility so it is now possible in cumulus to have the command-line arguments only of the run command for polkadot if we want

Related to paritytech/cumulus#24 and paritytech/cumulus#34 : it will now be possible to make a configuration struct for polkadot with some overrides of the default parameters much more easily.
  • Loading branch information
cecton authored Jan 30, 2020
1 parent 1ed1cc5 commit 26e37ff
Show file tree
Hide file tree
Showing 28 changed files with 2,024 additions and 2,217 deletions.
1,435 changes: 714 additions & 721 deletions Cargo.lock

Large diffs are not rendered by default.

5 changes: 2 additions & 3 deletions bin/node-template/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand All @@ -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"
Expand Down
7 changes: 7 additions & 0 deletions bin/node-template/src/chain_spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
})
}
126 changes: 8 additions & 118 deletions bin/node-template/src/cli.rs
Original file line number Diff line number Diff line change
@@ -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,
}
48 changes: 48 additions & 0 deletions bin/node-template/src/command.rs
Original file line number Diff line number Diff line change
@@ -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,
)
}
}
8 changes: 5 additions & 3 deletions bin/node-template/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand All @@ -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)
}
4 changes: 2 additions & 2 deletions bin/node-template/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
23 changes: 15 additions & 8 deletions bin/node/cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down Expand Up @@ -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
Expand All @@ -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"]
Expand All @@ -114,10 +121,10 @@ browser = [
cli = [
"sc-cli",
"node-transaction-factory",
"tokio",
"ctrlc",
"sc-service/rocksdb",
"node-executor/wasmi-errno",
"vergen",
"structopt",
]
wasmtime = [
"cli",
Expand Down
26 changes: 2 additions & 24 deletions bin/node/cli/bin/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -52,7 +29,8 @@ fn main() -> Result<(), sc_cli::error::Error> {
author: "Parity Technologies <admin@parity.io>",
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)
}
Loading

0 comments on commit 26e37ff

Please sign in to comment.