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

Add a feature to create automatically a random temporary directory for base path & remove Clone #6221

Merged
merged 25 commits into from
Jun 10, 2020
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
7fbed1b
Initial commit
cecton Jun 2, 2020
aa518e4
Add a feature to create automatically a temporary directory for base …
cecton Jun 2, 2020
5338633
doc fix and todos
cecton Jun 2, 2020
baf6ad2
use parking_lot instead
cecton Jun 3, 2020
179dda3
use refcell instead since we stay in the main thread
cecton Jun 3, 2020
2aea88f
remove Clone derives
cecton Jun 3, 2020
b6358e8
add test
cecton Jun 3, 2020
f81522d
solving dependency issue
cecton Jun 3, 2020
55ed555
clarifying doc
cecton Jun 3, 2020
cfc6fa4
conflict argument with base-path
cecton Jun 3, 2020
5a5c546
WIP
cecton Jun 4, 2020
bf1da7e
Merge commit f028a509789289a34c468f42b4361c49279893f2 (no conflict)
cecton Jun 4, 2020
68dfe89
revert dep deletion
cecton Jun 4, 2020
898dcd6
fixing test and making base_path optional
cecton Jun 4, 2020
d26da87
hold basepath while the service is running
cecton Jun 4, 2020
9c6f71a
fixes
cecton Jun 4, 2020
9e989de
Update client/cli/src/params/shared_params.rs
cecton Jun 5, 2020
c811cb6
Update client/service/Cargo.toml
cecton Jun 5, 2020
65cb074
Update client/cli/src/commands/mod.rs
cecton Jun 5, 2020
fdb3f20
Update client/service/src/config.rs
cecton Jun 5, 2020
39bc022
WIP
cecton Jun 5, 2020
cb140b6
improve doc
cecton Jun 8, 2020
ca42803
Merge commit 252b146b43e3877aefae6afd5c9a9e6093c5d7a9 (no conflict)
cecton Jun 8, 2020
468f16a
Merge commit 9ce077465ac46a93ebaced4b47863952b3d63d33 (conflicts)
cecton Jun 8, 2020
49cfe00
Merge commit d68cfd7cd5c64cb0965b49d9868aff02849e077c (no conflict)
cecton Jun 8, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions bin/node/cli/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand All @@ -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)]
Expand Down
72 changes: 72 additions & 0 deletions bin/node/cli/tests/temp_base_path_works.rs
Original file line number Diff line number Diff line change
@@ -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 <https://www.gnu.org/licenses/>.

#![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());
}
4 changes: 2 additions & 2 deletions bin/node/inspect/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand All @@ -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 {
Expand Down
1 change: 0 additions & 1 deletion client/cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion client/cli/src/commands/build_spec_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down
2 changes: 1 addition & 1 deletion client/cli/src/commands/check_block_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down
2 changes: 1 addition & 1 deletion client/cli/src/commands/export_blocks_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))]
Expand Down
4 changes: 2 additions & 2 deletions client/cli/src/commands/export_state_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion client/cli/src/commands/import_blocks_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))]
Expand Down
7 changes: 3 additions & 4 deletions client/cli/src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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),
Expand Down Expand Up @@ -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>> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fn base_path(&self) -> $crate::Result<::std::option::Option<::sc_service::config::BasePath>> {
fn base_path(&self) -> $crate::Result<::std::option::Option<sc_service::config::BasePath>> {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I think these are all wrong. sc-cli should probably make a hidden re-export of sc_service and the macro should get it from there.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that is right.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will make another PR for that for the sake of having isolated commits per change

match self {
$($enum::$variant(cmd) => cmd.base_path()),*
}
Expand Down Expand Up @@ -409,4 +409,3 @@ macro_rules! substrate_cli_subcommands {
substrate_cli_subcommands!(
Subcommand => BuildSpec, ExportBlocks, ImportBlocks, CheckBlock, Revert, PurgeChain, ExportState
);

2 changes: 1 addition & 1 deletion client/cli/src/commands/purge_chain_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down
2 changes: 1 addition & 1 deletion client/cli/src/commands/revert_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down
24 changes: 21 additions & 3 deletions client/cli/src/commands/run_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,21 +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 regex::Regex;
use sc_service::{
config::{MultiaddrWithPeerId, PrometheusConfig, TransactionPoolOptions},
config::{BasePath, MultiaddrWithPeerId, PrometheusConfig, TransactionPoolOptions},
ChainSpec, Role,
};
use sc_telemetry::TelemetryEndpoints;
use std::net::{IpAddr, Ipv4Addr, SocketAddr};
use structopt::StructOpt;

/// The `run` command used to run a node.
#[derive(Debug, StructOpt, Clone)]
#[derive(Debug, StructOpt)]
pub struct RunCmd {
/// Enable validator mode.
///
Expand Down Expand Up @@ -250,6 +250,16 @@ pub struct RunCmd {
conflicts_with_all = &[ "sentry", "public-addr" ]
)]
pub sentry_nodes: Vec<MultiaddrWithPeerId>,

/// Run a temporary node.
///
/// 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. This directory is used as base path
/// which includes: database, node key and keystore.
#[structopt(long, conflicts_with = "base-path")]
pub tmp: bool,
}

impl RunCmd {
Expand Down Expand Up @@ -446,6 +456,14 @@ impl CliConfiguration for RunCmd {
fn max_runtime_instances(&self) -> Result<Option<usize>> {
Ok(self.max_runtime_instances.map(|x| x.min(256)))
}

fn base_path(&self) -> Result<Option<BasePath>> {
Ok(if self.tmp {
Some(BasePath::new_temp_dir()?)
} else {
self.shared_params().base_path()
})
}
}

/// Check whether a node name is considered as valid.
Expand Down
23 changes: 11 additions & 12 deletions client/cli/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Option<PathBuf>> {
fn base_path(&self) -> Result<Option<BasePath>> {
Ok(self.shared_params().base_path())
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -464,6 +462,7 @@ pub trait CliConfiguration: Sized {
max_runtime_instances,
announce_block: self.announce_block()?,
role,
base_path: Some(base_path),
})
}

Expand Down Expand Up @@ -507,5 +506,5 @@ pub fn generate_node_name() -> String {
if count < NODE_NAME_MAX_LENGTH {
return node_name;
}
};
}
}
2 changes: 1 addition & 1 deletion client/cli/src/params/database_params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
4 changes: 2 additions & 2 deletions client/cli/src/params/import_params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion client/cli/src/params/keystore_params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))]
Expand Down
Loading