Re-add deprecated --execution arg on benchmark pallet (#14567)

* Add DeferGuard::new

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Re-add deprecated 'execution' arg to benchmark pallet cmd.

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Extend tests

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Remove from tests

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

---------

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

substrate/bin/node/cli/tests/benchmark_block_works.rs

@@ -39,7 +39,7 @@ async fn benchmark_block_works() {
 		.arg(base_dir.path())
 		.args(["--from", "1", "--to", "1"])
 		.args(["--repeat", "1"])
-		.args(["--wasm-execution", "compiled"])
+		.args(["--wasm-execution=compiled"])
 		.status()
 		.unwrap();
 

substrate/bin/node/cli/tests/benchmark_extrinsic_works.rs

@@ -39,6 +39,7 @@ fn benchmark_extrinsic(pallet: &str, extrinsic: &str) {
 		.args(&["--pallet", pallet, "--extrinsic", extrinsic])
 		// Run with low repeats for faster execution.
 		.args(["--warmup=10", "--repeat=10", "--max-ext-per-block=10"])
+		.args(["--wasm-execution=compiled"])
 		.status()
 		.unwrap();
 

substrate/bin/node/cli/tests/benchmark_overhead_works.rs

@@ -36,6 +36,7 @@ fn benchmark_overhead_works() {
 		.args(["--warmup", "10", "--repeat", "10"])
 		.args(["--add", "100", "--mul", "1.2", "--metric", "p75"])
 		.args(["--max-ext-per-block", "10"])
+		.args(["--wasm-execution=compiled"])
 		.status()
 		.unwrap();
 	assert!(status.success());

substrate/bin/node/cli/tests/benchmark_pallet_works.rs

@@ -34,16 +34,20 @@ fn benchmark_pallet_works() {
 }
 
 fn benchmark_pallet(steps: u32, repeat: u32, should_work: bool) {
-	let output = Command::new(cargo_bin("substrate"))
+	let status = Command::new(cargo_bin("substrate"))
 		.args(["benchmark", "pallet", "--dev"])
 		// Use the `addition` benchmark since is the fastest.
 		.args(["--pallet", "frame-benchmarking", "--extrinsic", "addition"])
 		.args(["--steps", &format!("{}", steps), "--repeat", &format!("{}", repeat)])
-		.output()
+		.args([
+			"--wasm-execution=compiled",
+			"--no-storage-info",
+			"--no-median-slopes",
+			"--no-min-squares",
+			"--heap-pages=4096",
+		])
+		.status()
 		.unwrap();
 
-	if output.status.success() != should_work {
+	assert_eq!(status.success(), should_work);
-		let log = String::from_utf8_lossy(&output.stderr).to_string();
-		panic!("Test failed:\n{}", log);
-	}
 }

substrate/primitives/core/src/defer.rs

@@ -25,6 +25,13 @@
 #[must_use]
 pub struct DeferGuard<F: FnOnce()>(pub Option<F>);
 
+impl<F: FnOnce()> DeferGuard<F> {
+	/// Creates a new `DeferGuard` with the given closure.
+	pub fn new(f: F) -> Self {
+		Self(Some(f))
+	}
+}
+
 impl<F: FnOnce()> Drop for DeferGuard<F> {
 	fn drop(&mut self) {
 		self.0.take().map(|f| f());

substrate/utils/frame/benchmarking-cli/src/pallet/command.rs

@@ -147,6 +147,16 @@ impl PalletCmd {
 		<<<BB as BlockT>::Header as HeaderT>::Number as std::str::FromStr>::Err: std::fmt::Debug,
 		ExtraHostFunctions: sp_wasm_interface::HostFunctions,
 	{
+		let _d = self.execution.as_ref().map(|exec| {
+			// We print the warning at the end, since there is often A LOT of output.
+			sp_core::defer::DeferGuard::new(move || {
+				log::warn!(
+					target: LOG_TARGET,
+					"⚠️  Argument `--execution` is deprecated. Its value of `{exec}` has on effect.",
+				)
+			})
+		});
+
 		if let Some(output_path) = &self.output {
 			if !output_path.is_dir() && output_path.file_name().is_none() {
 				return Err("Output file or path is invalid!".into())

substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs

@@ -150,6 +150,10 @@ pub struct PalletCmd {
 	)]
 	pub wasmtime_instantiation_strategy: WasmtimeInstantiationStrategy,
 
+	/// DEPRECATED: This argument has no effect.
+	#[arg(long = "execution")]
+	pub execution: Option<String>,
+
 	/// Limit the memory the database cache can use.
 	#[arg(long = "db-cache", value_name = "MiB", default_value_t = 1024)]
 	pub database_cache_size: u32,