Update SubstrateCli to return String (#6550)

* Update SubstrateCli to return String

* Add default implementation for executable_name()

* Use display instead of PathBuf

* Get file_name in default impl of executable_name

* Remove String::from and use .into()

* Use default impl for executable_name()

* Use .as_str() and remove useless .to_string()

* Update only sp-io when running companion build

* Remove unneeded update of sp-io in CI

Co-authored-by: Cecile Tonglet <cecile@parity.io>

substrate/.maintain/gitlab/check_polkadot_companion_build.sh

@@ -111,9 +111,5 @@ echo "paths = [ \"$SUBSTRATE_PATH\" ]" > .cargo/config
 mkdir -p target/debug/wbuild/.cargo
 cp .cargo/config target/debug/wbuild/.cargo/config
 
-# package, others are updated along the way.
-cargo update
-
 # Test Polkadot pr or master branch with this Substrate commit.
 time cargo test --all --release --verbose
-

substrate/bin/node-template/node/src/command.rs

@@ -21,34 +21,30 @@ use crate::service;
 use sc_cli::{SubstrateCli, RuntimeVersion, Role, ChainSpec};
 
 impl SubstrateCli for Cli {
-	fn impl_name() -> &'static str {
-		"Substrate Node"
+	fn impl_name() -> String {
+		"Substrate Node".into()
 	}
 
-	fn impl_version() -> &'static str {
-		env!("SUBSTRATE_CLI_IMPL_VERSION")
+	fn impl_version() -> String {
+		env!("SUBSTRATE_CLI_IMPL_VERSION").into()
 	}
 
-	fn description() -> &'static str {
-		env!("CARGO_PKG_DESCRIPTION")
+	fn description() -> String {
+		env!("CARGO_PKG_DESCRIPTION").into()
 	}
 
-	fn author() -> &'static str {
-		env!("CARGO_PKG_AUTHORS")
+	fn author() -> String {
+		env!("CARGO_PKG_AUTHORS").into()
 	}
 
-	fn support_url() -> &'static str {
-		"support.anonymous.an"
+	fn support_url() -> String {
+		"support.anonymous.an".into()
 	}
 
 	fn copyright_start_year() -> i32 {
 		2017
 	}
 
-	fn executable_name() -> &'static str {
-		env!("CARGO_PKG_NAME")
-	}
-
 	fn load_spec(&self, id: &str) -> Result<Box<dyn sc_service::ChainSpec>, String> {
 		Ok(match id {
 			"dev" => Box::new(chain_spec::development_config()),

substrate/bin/node/cli/src/command.rs

@@ -22,34 +22,30 @@ use node_runtime::{Block, RuntimeApi};
 use sc_cli::{Result, SubstrateCli, RuntimeVersion, Role, ChainSpec};
 
 impl SubstrateCli for Cli {
-	fn impl_name() -> &'static str {
-		"Substrate Node"
+	fn impl_name() -> String {
+		"Substrate Node".into()
 	}
 
-	fn impl_version() -> &'static str {
-		env!("SUBSTRATE_CLI_IMPL_VERSION")
+	fn impl_version() -> String {
+		env!("SUBSTRATE_CLI_IMPL_VERSION").into()
 	}
 
-	fn description() -> &'static str {
-		env!("CARGO_PKG_DESCRIPTION")
+	fn description() -> String {
+		env!("CARGO_PKG_DESCRIPTION").into()
 	}
 
-	fn author() -> &'static str {
-		env!("CARGO_PKG_AUTHORS")
+	fn author() -> String {
+		env!("CARGO_PKG_AUTHORS").into()
 	}
 
-	fn support_url() -> &'static str {
-		"https://github.com/paritytech/substrate/issues/new"
+	fn support_url() -> String {
+		"https://github.com/paritytech/substrate/issues/new".into()
 	}
 
 	fn copyright_start_year() -> i32 {
 		2017
 	}
 
-	fn executable_name() -> &'static str {
-		"substrate"
-	}
-
 	fn load_spec(&self, id: &str) -> std::result::Result<Box<dyn sc_service::ChainSpec>, String> {
 		Ok(match id {
 			"dev" => Box::new(chain_spec::development_config()),

substrate/client/cli/src/config.rs

@@ -413,7 +413,7 @@ pub trait CliConfiguration: Sized {
 		let chain_spec = cli.load_spec(chain_id.as_str())?;
 		let base_path = self
 			.base_path()?
-			.unwrap_or_else(|| BasePath::from_project("", "", C::executable_name()));
+			.unwrap_or_else(|| BasePath::from_project("", "", &C::executable_name()));
 		let config_dir = base_path
 			.path()
 			.to_path_buf()
@@ -498,7 +498,7 @@ pub trait CliConfiguration: Sized {
 	fn init<C: SubstrateCli>(&self) -> Result<()> {
 		let logger_pattern = self.log_filters()?;
 
-		sp_panic_handler::set(C::support_url(), C::impl_version());
+		sp_panic_handler::set(&C::support_url(), &C::impl_version());
 
 		fdlimit::raise_fd_limit();
 		init_logger(&logger_pattern);

substrate/client/cli/src/lib.rs

@@ -57,25 +57,33 @@ use structopt::{
 /// its own implementation that will fill the necessary field based on the trait's functions.
 pub trait SubstrateCli: Sized {
 	/// Implementation name.
-	fn impl_name() -> &'static str;
+	fn impl_name() -> String;
 
 	/// Implementation version.
 	///
 	/// By default this will look like this: 2.0.0-b950f731c-x86_64-linux-gnu where the hash is the
 	/// short commit hash of the commit of in the Git repository.
-	fn impl_version() -> &'static str;
+	fn impl_version() -> String;
 
 	/// Executable file name.
-	fn executable_name() -> &'static str;
+	///
+	/// Extracts the file name from `std::env::current_exe()`.
+	/// Resorts to the env var `CARGO_PKG_NAME` in case of Error.
+	fn executable_name() -> String {
+		std::env::current_exe().ok()
+			.and_then(|e| e.file_name().map(|s| s.to_os_string()))
+			.and_then(|w| w.into_string().ok())
+			.unwrap_or_else(|| env!("CARGO_PKG_NAME").into())
+	}
 
 	/// Executable file description.
-	fn description() -> &'static str;
+	fn description() -> String;
 
 	/// Executable file author.
-	fn author() -> &'static str;
+	fn author() -> String;
 
 	/// Support URL.
-	fn support_url() -> &'static str;
+	fn support_url() -> String;
 
 	/// Copyright starting year (x-current year)
 	fn copyright_start_year() -> i32;
@@ -116,13 +124,16 @@ pub trait SubstrateCli: Sized {
 	{
 		let app = <Self as StructOpt>::clap();
 
-		let mut full_version = Self::impl_version().to_string();
+		let mut full_version = Self::impl_version();
 		full_version.push_str("\n");
 
+		let name = Self::executable_name();
+		let author = Self::author();
+		let about = Self::description();
 		let app = app
-			.name(Self::executable_name())
-			.author(Self::author())
-			.about(Self::description())
+			.name(name)
+			.author(author.as_str())
+			.about(about.as_str())
 			.version(full_version.as_str())
 			.settings(&[
 				AppSettings::GlobalVersion,
@@ -175,13 +186,16 @@ pub trait SubstrateCli: Sized {
 	{
 		let app = <Self as StructOpt>::clap();
 
-		let mut full_version = Self::impl_version().to_string();
+		let mut full_version = Self::impl_version();
 		full_version.push_str("\n");
 
+		let name = Self::executable_name();
+		let author = Self::author();
+		let about = Self::description();
 		let app = app
-			.name(Self::executable_name())
-			.author(Self::author())
-			.about(Self::description())
+			.name(name)
+			.author(author.as_str())
+			.about(about.as_str())
 			.version(full_version.as_str());
 
 		let matches = app.get_matches_from_safe(iter)?;

substrate/client/service/src/builder.rs

@@ -1203,8 +1203,8 @@ fn build_telemetry<TBl: BlockT>(
 	let is_authority = config.role.is_authority();
 	let network_id = network.local_peer_id().to_base58();
 	let name = config.network.node_name.clone();
-	let impl_name = config.impl_name.to_owned();
-	let version = config.impl_version;
+	let impl_name = config.impl_name.clone();
+	let impl_version = config.impl_version.clone();
 	let chain_name = config.chain_spec.name().to_owned();
 	let telemetry = sc_telemetry::init_telemetry(sc_telemetry::TelemetryConfig {
 		endpoints,
@@ -1221,7 +1221,7 @@ fn build_telemetry<TBl: BlockT>(
 			telemetry!(SUBSTRATE_INFO; "system.connected";
 				"name" => name.clone(),
 				"implementation" => impl_name.clone(),
-				"version" => version,
+				"version" => impl_version.clone(),
 				"config" => "",
 				"chain" => chain_name.clone(),
 				"genesis_hash" => ?genesis_hash,
@@ -1270,8 +1270,8 @@ fn gen_handler<TBl, TBackend, TExPool, TRpc, TCl>(
 
 	let system_info = sc_rpc::system::SystemInfo {
 		chain_name: config.chain_spec.name().into(),
-		impl_name: config.impl_name.into(),
-		impl_version: config.impl_version.into(),
+		impl_name: config.impl_name.clone(),
+		impl_version: config.impl_version.clone(),
 		properties: config.chain_spec.properties(),
 		chain_type: config.chain_spec.chain_type(),
 	};

substrate/client/service/src/config.rs

@@ -37,9 +37,9 @@ use tempfile::TempDir;
 #[derive(Debug)]
 pub struct Configuration {
 	/// Implementation name
-	pub impl_name: &'static str,
+	pub impl_name: String,
 	/// Implementation version (see sc-cli to see an example of format)
-	pub impl_version: &'static str,
+	pub impl_version: String,
 	/// Node role.
 	pub role: Role,
 	/// How to spawn background tasks. Mandatory, otherwise creating a `Service` will error.

substrate/client/service/test/src/lib.rs

@@ -229,8 +229,8 @@ fn node_config<G: RuntimeGenesis + 'static, E: ChainSpecExtension + Clone + 'sta
 	};
 
 	Configuration {
-		impl_name: "network-test-impl",
-		impl_version: "0.1",
+		impl_name: String::from("network-test-impl"),
+		impl_version: String::from("0.1"),
 		role,
 		task_executor,
 		transaction_pool: Default::default(),

substrate/primitives/panic-handler/src/lib.rs

@@ -52,11 +52,12 @@ enum OnPanic {
 ///
 /// The `bug_url` parameter is an invitation for users to visit that URL to submit a bug report
 /// in the case where a panic happens.
-pub fn set(bug_url: &'static str, version: &str) {
+pub fn set(bug_url: &str, version: &str) {
 	panic::set_hook(Box::new({
 		let version = version.to_string();
+		let bug_url = bug_url.to_string();
 		move |c| {
-			panic_hook(c, bug_url, &version)
+			panic_hook(c, &bug_url, &version)
 		}
 	}));
 }
@@ -130,7 +131,7 @@ impl Drop for AbortGuard {
 }
 
 /// Function being called when a panic happens.
-fn panic_hook(info: &PanicInfo, report_url: &'static str, version: &str) {
+fn panic_hook(info: &PanicInfo, report_url: &str, version: &str) {
 	let location = info.location();
 	let file = location.as_ref().map(|l| l.file()).unwrap_or("<unknown>");
 	let line = location.as_ref().map(|l| l.line()).unwrap_or(0);

substrate/utils/browser/src/lib.rs

@@ -79,8 +79,8 @@ where
 		disable_grandpa: Default::default(),
 		execution_strategies: Default::default(),
 		force_authoring: Default::default(),
-		impl_name: "parity-substrate",
-		impl_version: "0.0.0",
+		impl_name: String::from("parity-substrate"),
+		impl_version: String::from("0.0.0"),
 		offchain_worker: Default::default(),
 		prometheus_config: Default::default(),
 		pruning: Default::default(),