feat(up parachain): improve build ux (#123)

* build(deps): update cliclack

* feat(up parachain): improve build ux

* refactor: address warnings after dependency

* refactor: improve callback type

* docs: add doc comments

* refactor: improve types to better convey intent

Cargo.toml

@@ -53,7 +53,7 @@ git2_credentials = "0.13.0"
 
 # pop-cli
 clap = { version = "4.4", features = ["derive"] }
-cliclack = "0.1"
+cliclack = "0.2"
 console = "0.15"
 strum = "0.26"
 strum_macros = "0.26"

crates/pop-cli/src/commands/call/contract.rs

@@ -75,7 +75,7 @@ impl CallContractCommand {
 		.await?;
 
 		if !self.execute {
-			let mut spinner = cliclack::spinner();
+			let spinner = cliclack::spinner();
 			spinner.start("Calling the contract...");
 			let call_dry_run_result = dry_run_call(&call_exec).await?;
 			log::info(format!("Result: {}", call_dry_run_result))?;
@@ -90,7 +90,7 @@ impl CallContractCommand {
 				weight_limit =
 					Weight::from_parts(self.gas_limit.unwrap(), self.proof_size.unwrap());
 			} else {
-				let mut spinner = cliclack::spinner();
+				let spinner = cliclack::spinner();
 				spinner.start("Doing a dry run to estimate the gas...");
 				weight_limit = match dry_run_gas_estimate_call(&call_exec).await {
 					Ok(w) => {
@@ -104,7 +104,7 @@ impl CallContractCommand {
 					},
 				};
 			}
-			let mut spinner = cliclack::spinner();
+			let spinner = cliclack::spinner();
 			spinner.start("Calling the contract...");
 
 			let call_result = call_smart_contract(call_exec, weight_limit, &self.url)

crates/pop-cli/src/commands/new/contract.rs

@@ -47,7 +47,7 @@ impl NewContractCommand {
 			fs::remove_dir_all(contract_path.as_path())?;
 		}
 		fs::create_dir_all(contract_path.as_path())?;
-		let mut spinner = cliclack::spinner();
+		let spinner = cliclack::spinner();
 		spinner.start("Generating contract...");
 		create_smart_contract(&self.name, contract_path.as_path())?;
 

crates/pop-cli/src/commands/new/pallet.rs

@@ -45,7 +45,7 @@ impl NewPalletCommand {
 			}
 			fs::remove_dir_all(pallet_path)?;
 		}
-		let mut spinner = cliclack::spinner();
+		let spinner = cliclack::spinner();
 		spinner.start("Generating pallet...");
 		create_pallet_template(
 			self.path.clone(),

crates/pop-cli/src/commands/new/parachain.rs

@@ -156,7 +156,7 @@ fn generate_parachain_from_template(
 
 	let destination_path = check_destination_path(name_template)?;
 
-	let mut spinner = cliclack::spinner();
+	let spinner = cliclack::spinner();
 	spinner.start("Generating parachain...");
 	let tag = instantiate_template_dir(template, destination_path, tag_version, config)?;
 	if let Err(err) = Git::git_init(destination_path, "initialized parachain") {

crates/pop-cli/src/commands/up/contract.rs

@@ -73,7 +73,7 @@ impl UpContractCommand {
 		if self.gas_limit.is_some() && self.proof_size.is_some() {
 			weight_limit = Weight::from_parts(self.gas_limit.unwrap(), self.proof_size.unwrap());
 		} else {
-			let mut spinner = cliclack::spinner();
+			let spinner = cliclack::spinner();
 			spinner.start("Doing a dry run to estimate the gas...");
 			weight_limit = match dry_run_gas_estimate_instantiate(&instantiate_exec).await {
 				Ok(w) => {
@@ -87,7 +87,7 @@ impl UpContractCommand {
 				},
 			};
 		}
-		let mut spinner = cliclack::spinner();
+		let spinner = cliclack::spinner();
 		spinner.start("Uploading and instantiating the contract...");
 		let contract_address = instantiate_smart_contract(instantiate_exec, weight_limit)
 			.await

crates/pop-cli/src/commands/up/parachain.rs

@@ -2,9 +2,13 @@
 
 use crate::style::{style, Theme};
 use clap::Args;
-use cliclack::{clear_screen, confirm, intro, log, outro, outro_cancel, set_theme};
+use cliclack::{
+	clear_screen, confirm, intro, log, multi_progress, outro, outro_cancel, set_theme, ProgressBar,
+};
 use console::{Emoji, Style};
-use pop_parachains::{NetworkNode, Zombienet};
+use pop_parachains::{NetworkNode, Status, Zombienet};
+use std::time::Duration;
+use tokio::time::sleep;
 
 #[derive(Args)]
 pub(crate) struct ZombienetCommand {
@@ -46,28 +50,43 @@ impl ZombienetCommand {
 		let missing = zombienet.missing_binaries();
 		if missing.len() > 0 {
 			log::warning(format!(
-				"The following missing binaries are required: {}",
+				"⚠️ The following missing binaries are required: {}",
 				missing.iter().map(|b| b.name.as_str()).collect::<Vec<_>>().join(", ")
 			))?;
-			if !confirm("Would you like to source them automatically now?")
+			if !confirm("📦 Would you like to source them automatically now?")
 				.initial_value(true)
 				.interact()?
 			{
-				outro_cancel("Cannot deploy parachain to local network until all required binaries are available.")?;
+				outro_cancel("🚫 Cannot deploy parachain to local network until all required binaries are available.")?;
 				return Ok(());
 			}
-			log::info(format!("They will be cached at {}", &cache.to_str().unwrap()))?;
+			log::info(format!("ℹ️ They will be cached at {}", &cache.to_str().unwrap()))?;
 			// Source binaries
-			let mut spinner = cliclack::spinner();
 			for binary in missing {
-				spinner.start(format!("Sourcing {}...", binary.name));
-				binary.source(&cache).await?;
-				spinner.start(format!("Sourcing {} complete.", binary.name));
+				let multi = multi_progress(format!("📦 Sourcing {}...", binary.name));
+				let progress = multi.add(cliclack::spinner());
+				let progress_reporter = ProgressReporter(&progress);
+				for attempt in (0..=1).rev() {
+					if let Err(e) = binary.source(&cache, progress_reporter).await {
+						match attempt {
+							0 => {
+								progress.error(format!("🚫 Sourcing failed: {e}"));
+								multi.stop();
+								return Ok(());
+							},
+							_ => {
+								progress.error("🚫 Sourcing attempt failed, retrying...");
+								sleep(Duration::from_secs(1)).await;
+							},
+						}
+					}
+				}
+				progress.stop(format!("✅ Sourcing {} complete.", binary.name));
+				multi.stop();
 			}
-			spinner.stop("Sourcing complete.");
 		}
 		// Finally spawn network and wait for signal to terminate
-		let mut spinner = cliclack::spinner();
+		let spinner = cliclack::spinner();
 		spinner.start("🚀 Launching local network...");
 		//tracing_subscriber::fmt().init();
 		match zombienet.spawn().await {
@@ -131,3 +150,13 @@ impl ZombienetCommand {
 		Ok(())
 	}
 }
+
+/// Reports any observed status updates to a progress bar.
+#[derive(Copy, Clone)]
+struct ProgressReporter<'a>(&'a ProgressBar);
+
+impl Status for ProgressReporter<'_> {
+	fn update(&self, status: &str) {
+		self.0.start(status.replace("   Compiling", "Compiling"))
+	}
+}

crates/pop-parachains/src/lib.rs

@@ -12,7 +12,7 @@ pub use build::build_parachain;
 pub use new_pallet::{create_pallet_template, TemplatePalletConfig};
 pub use new_parachain::instantiate_template_dir;
 pub use templates::{Config, Provider, Template};
-pub use up::Zombienet;
+pub use up::{Status, Zombienet};
 pub use utils::git::{Git, GitHub, Release};
 pub use utils::pallet_helpers::resolve_pallet_path;
 // External exports

crates/pop-parachains/src/up.rs

@@ -6,7 +6,7 @@ use indexmap::IndexMap;
 use std::{
 	env::current_dir,
 	fs::{copy, metadata, remove_dir_all, write, File},
-	io::Write,
+	io::{BufRead, Write},
 	os::unix::fs::PermissionsExt,
 	path::{Path, PathBuf},
 };
@@ -382,17 +382,26 @@ impl Zombienet {
 	}
 }
 
+/// A binary used to launch a node.
 pub struct Binary {
+	/// The name of a binary.
 	pub name: String,
 	version: String,
 	path: PathBuf,
 	sources: Vec<Source>,
 }
 
 impl Binary {
-	pub async fn source(&self, cache: &PathBuf) -> Result<(), Error> {
+	/// Sources the binary by either downloading from a url or by cloning a git repository and
+	/// building locally from the resulting source code.
+	///
+	/// # Arguments
+	///
+	/// * `cache` - path to the local cache
+	/// * `status` - used to observe status updates
+	pub async fn source(&self, cache: &PathBuf, status: impl Status) -> Result<(), Error> {
 		for source in &self.sources {
-			source.process(cache).await?;
+			source.process(cache, status).await?;
 		}
 		Ok(())
 	}
@@ -426,20 +435,17 @@ impl Source {
 		path: &Path,
 		package: &str,
 		names: impl Iterator<Item = (&'b String, PathBuf)>,
+		status: impl Status,
 	) -> Result<(), Error> {
 		// Build binaries and then copy to cache and target
-		cmd(
-			"cargo",
-			vec![
-				"build",
-				"--release",
-				"-p",
-				package,
-				//     "--quiet"
-			],
-		)
-		.dir(path)
-		.run()?;
+		let reader = cmd("cargo", vec!["build", "--release", "-p", package])
+			.dir(path)
+			.stderr_to_stdout()
+			.reader()?;
+		let mut output = std::io::BufReader::new(reader).lines();
+		while let Some(Ok(line)) = output.next() {
+			status.update(&line);
+		}
 		for (name, dest) in names {
 			copy(path.join(format!("target/release/{name}")), dest)?;
 		}
@@ -458,7 +464,18 @@ impl Source {
 		Ok(())
 	}
 
-	pub async fn process(&self, cache: &Path) -> Result<Option<Vec<PathBuf>>, Error> {
+	/// Processes the binary source, by either downloading the binary from a url or by cloning a
+	/// git repository and building locally from the resulting source code.
+	///
+	/// # Arguments
+	///
+	/// * `cache` - path to the local cache
+	/// * `status` - used to observe status updates
+	pub async fn process(
+		&self,
+		cache: &Path,
+		status: impl Status,
+	) -> Result<Option<Vec<PathBuf>>, Error> {
 		// Download or clone and build from source
 		match self {
 			Source::Url { name, version, url } => {
@@ -469,6 +486,7 @@ impl Source {
 				}
 
 				// Download required version of binaries
+				status.update(&format!("Downloading from {url}..."));
 				Self::download(&url, &cache.join(&versioned_name)).await?;
 				Ok(None)
 			},
@@ -483,24 +501,37 @@ impl Source {
 				}
 
 				let repository_name = GitHub::name(url)?;
-				// Clone repository into working directory
 				let working_dir = cache.join(".src").join(repository_name);
 				let working_dir = Path::new(&working_dir);
-				if let Err(e) = Git::clone(url, working_dir, branch.as_deref()) {
-					if working_dir.exists() {
-						Self::remove(working_dir)?;
+
+				// Clone repository into working directory
+				if !working_dir.exists() {
+					status.update(&format!("Cloning {url}..."));
+					if let Err(e) = Git::clone(url, working_dir, branch.as_deref()) {
+						if working_dir.exists() {
+							// Preserve original error
+							let _ = Self::remove(working_dir);
+						}
+						return Err(e.into());
 					}
-					return Err(e.into());
 				}
 				// Build binaries and finally remove working directory
-				Self::build_binaries(
+				if let Err(e) = Self::build_binaries(
 					working_dir,
 					package,
 					versioned_names
 						.iter()
 						.map(|(binary, versioned)| (*binary, cache.join(versioned))),
+					status,
 				)
-				.await?;
+				.await
+				{
+					if working_dir.exists() {
+						// Preserve original error
+						let _ = Self::remove(working_dir);
+					}
+					return Err(e.into());
+				}
 				Self::remove(working_dir)?;
 				Ok(None)
 			},
@@ -519,6 +550,10 @@ impl Source {
 	}
 
 	/// A versioned name of a binary.
+	///
+	/// # Arguments
+	///
+	/// * `version` - an optional version to be appended to the binary name
 	pub fn versioned_name(name: &str, version: Option<&str>) -> String {
 		match version {
 			Some(version) => format!("{name}-{version}"),
@@ -527,6 +562,17 @@ impl Source {
 	}
 }
 
+/// Trait for observing status updates.
+pub trait Status: Copy {
+	/// Update the observer with the provided `status`.
+	fn update(&self, status: &str);
+}
+
+impl Status for () {
+	// no-op: status updates are ignored
+	fn update(&self, _: &str) {}
+}
+
 #[cfg(test)]
 mod tests {
 
@@ -802,7 +848,7 @@ mod tests {
 		.await?;
 		let missing_binaries = zombienet.missing_binaries();
 		for binary in missing_binaries {
-			binary.source(&cache).await?;
+			binary.source(&cache, ()).await?;
 		}
 
 		let spawn = zombienet.spawn().await;
@@ -841,7 +887,7 @@ mod tests {
 			version: TESTING_POLKADOT_VERSION.to_string(),
 			url: "https://github.com/paritytech/polkadot-sdk/releases/download/polkadot-v1.7.0/polkadot".to_string()
 		};
-		let result = source.process(&cache).await;
+		let result = source.process(&cache, ()).await;
 		assert!(result.is_ok());
 		assert!(temp_dir.path().join(POLKADOT_BINARY).exists());
 
@@ -867,7 +913,7 @@ mod tests {
 			version: Some(version),
 		};
 
-		let result = source.process(&cache).await;
+		let result = source.process(&cache, ()).await;
 		assert!(result.is_ok());
 		assert!(temp_dir.path().join(POLKADOT_BINARY).exists());