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 7 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
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
4 changes: 1 addition & 3 deletions client/cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,14 @@ structopt = "0.3.8"
sc-tracing = { version = "2.0.0-rc2", path = "../tracing" }
chrono = "0.4.10"
parity-util-mem = { version = "0.6.1", default-features = false, features = ["primitive-types"] }
tempfile = "3.1.0"

[target.'cfg(not(target_os = "unknown"))'.dependencies]
rpassword = "4.0.1"

[target.'cfg(target_family = "unix")'.dependencies]
nix = "0.17.0"

[dev-dependencies]
tempfile = "3.1.0"

[features]
wasmtime = [
"sc-service/wasmtime",
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
2 changes: 1 addition & 1 deletion client/cli/src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
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
35 changes: 32 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,26 @@ 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},
ChainSpec, Role,
};
use sc_telemetry::TelemetryEndpoints;
use std::net::{IpAddr, Ipv4Addr, SocketAddr};
use std::{
cell::RefCell,
net::{IpAddr, Ipv4Addr, SocketAddr},
path::PathBuf,
};
use structopt::StructOpt;
use tempfile::TempDir;

/// 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 +255,18 @@ pub struct RunCmd {
conflicts_with_all = &[ "sentry", "public-addr" ]
)]
pub sentry_nodes: Vec<MultiaddrWithPeerId>,

/// Run a temporary node.
///
/// A random temporary directory will be created to store the configuration and will be deleted
Copy link
Member

Choose a reason for hiding this comment

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

"random temporary" => "temporary"

Please also make the user aware that this is the "base-path" and what will be part of this path. Database, node key etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done in 55ed555

/// at the end of the process.
///
/// Note: the directory is random per process execution.
#[structopt(long)]
pub tmp: bool,

#[structopt(skip)]
temp_base_path: RefCell<Option<TempDir>>,
Copy link
Member

Choose a reason for hiding this comment

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

Nope. This is a really bad hack.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What part you don't like exactly? (so I can find another way)

Copy link
Member

Choose a reason for hiding this comment

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

Not having the temp base path as parameter here.

We could create some custom type that we return in base_path

Copy link
Contributor Author

@cecton cecton Jun 4, 2020

Choose a reason for hiding this comment

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

Ok done! Now the TempDir is hold in the configuration object instead of the SubstrateCli object, then it's move to the service and dropped with it. Also the core of the feature itself has moved from sc-cli to sc-service.

}

impl RunCmd {
Expand Down Expand Up @@ -446,6 +463,18 @@ 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<PathBuf>> {
Ok(if self.tmp {
let tmp = tempfile::Builder::new().prefix("substrate").tempdir()?;
let path = tmp.path().into();
*self.temp_base_path.borrow_mut() = Some(tmp);

Some(path)
} else {
self.shared_params().base_path()
})
}
}

/// Check whether a node name is considered as valid.
Expand Down
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
4 changes: 2 additions & 2 deletions client/cli/src/params/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ pub use crate::params::shared_params::*;
pub use crate::params::transaction_pool_params::*;

/// Wrapper type of `String` that holds an unsigned integer of arbitrary size, formatted as a decimal.
#[derive(Debug, Clone)]
#[derive(Debug)]
pub struct BlockNumber(String);

impl FromStr for BlockNumber {
Expand Down Expand Up @@ -72,7 +72,7 @@ impl BlockNumber {
}

/// Wrapper type that is either a `Hash` or the number of a `Block`.
#[derive(Debug, Clone)]
#[derive(Debug)]
pub struct BlockNumberOrHash(String);

impl FromStr for BlockNumberOrHash {
Expand Down
2 changes: 1 addition & 1 deletion client/cli/src/params/network_params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use std::path::PathBuf;
use structopt::StructOpt;

/// Parameters used to create the network configuration.
#[derive(Debug, StructOpt, Clone)]
#[derive(Debug, StructOpt)]
pub struct NetworkParams {
/// Specify a list of bootnodes.
#[structopt(long = "bootnodes", value_name = "ADDR")]
Expand Down
2 changes: 1 addition & 1 deletion client/cli/src/params/node_key_params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const NODE_KEY_ED25519_FILE: &str = "secret_ed25519";

/// Parameters used to create the `NodeKeyConfig`, which determines the keypair
/// used for libp2p networking.
#[derive(Debug, StructOpt, Clone)]
#[derive(Debug, StructOpt)]
pub struct NodeKeyParams {
/// The secret key to use for libp2p networking.
///
Expand Down
2 changes: 1 addition & 1 deletion client/cli/src/params/offchain_worker_params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use crate::OffchainWorkerEnabled;


/// Offchain worker related parameters.
#[derive(Debug, StructOpt, Clone)]
#[derive(Debug, StructOpt)]
pub struct OffchainWorkerParams {
/// Should execute offchain workers on every block.
///
Expand Down
2 changes: 1 addition & 1 deletion client/cli/src/params/pruning_params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use sc_service::{PruningMode, Role};
use structopt::StructOpt;

/// Parameters to define the pruning mode
#[derive(Debug, StructOpt, Clone)]
#[derive(Debug, StructOpt)]
pub struct PruningParams {
/// Specify the state pruning mode, a number of blocks to keep or 'archive'.
///
Expand Down
2 changes: 1 addition & 1 deletion client/cli/src/params/shared_params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use std::path::PathBuf;
use structopt::StructOpt;

/// Shared parameters used by all `CoreParams`.
#[derive(Debug, StructOpt, Clone)]
#[derive(Debug, StructOpt)]
pub struct SharedParams {
/// Specify the chain specification (one of dev, local, or staging).
#[structopt(long, value_name = "CHAIN_SPEC")]
Expand Down
2 changes: 1 addition & 1 deletion client/cli/src/params/transaction_pool_params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use sc_service::config::TransactionPoolOptions;
use structopt::StructOpt;

/// Parameters used to create the pool configuration.
#[derive(Debug, StructOpt, Clone)]
#[derive(Debug, StructOpt)]
pub struct TransactionPoolParams {
/// Maximum number of transactions in the transaction pool.
#[structopt(long = "pool-limit", value_name = "COUNT", default_value = "8192")]
Expand Down
2 changes: 1 addition & 1 deletion utils/frame/benchmarking-cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use sc_cli::{ExecutionStrategy, WasmExecutionMethod};
use std::fmt::Debug;

/// The `benchmark` command used to benchmark FRAME Pallets.
#[derive(Debug, structopt::StructOpt, Clone)]
#[derive(Debug, structopt::StructOpt)]
pub struct BenchmarkCmd {
/// Select a FRAME Pallet to benchmark, or `*` for all (in which case `extrinsic` must be `*`).
#[structopt(short, long)]
Expand Down