refactor: improve implementation and test coverage

Cargo.toml

@@ -26,6 +26,7 @@ duct = "0.13"
 env_logger = "0.11.1"
 flate2 = "1.0.30"
 git2 = { version = "0.18", features = ["vendored-openssl"] }
+glob = "0.3.1"
 log = "0.4.20"
 mockito = "1.4.0"
 predicates = "3.1.0"

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

@@ -321,13 +321,8 @@ async fn choose_release(template: &Template) -> Result<Option<String>> {
 }
 
 async fn get_latest_3_releases(repo: &GitHub) -> Result<Vec<Release>> {
-	let mut latest_3_releases: Vec<Release> = repo
-		.get_latest_releases()
-		.await?
-		.into_iter()
-		.filter(|r| !r.prerelease)
-		.take(3)
-		.collect();
+	let mut latest_3_releases: Vec<Release> =
+		repo.releases().await?.into_iter().filter(|r| !r.prerelease).take(3).collect();
 	repo.get_repo_license().await?;
 	// Get the commit sha for the releases
 	for release in latest_3_releases.iter_mut() {

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

@@ -1,5 +1,4 @@
 // SPDX-License-Identifier: GPL-3.0
-
 use crate::style::{style, Theme};
 use clap::Args;
 use cliclack::{
@@ -8,8 +7,8 @@ use cliclack::{
 };
 use console::{Emoji, Style, Term};
 use duct::cmd;
-use pop_parachains::{Binary, Error, NetworkNode, Source, Status, Zombienet};
-use std::{fs::remove_dir_all, path::PathBuf, time::Duration};
+use pop_parachains::{Error, IndexSet, NetworkNode, Status, Zombienet};
+use std::{path::PathBuf, time::Duration};
 use tokio::time::sleep;
 
 #[derive(Args)]
@@ -25,8 +24,8 @@ pub(crate) struct ZombienetCommand {
 	/// "v1.11.0"). Defaults to the relay chain version if not specified.
 	#[arg(short, long)]
 	system_parachain: Option<String>,
-	/// The url of the git repository of a parachain to be used, with branch/release tag specified as #fragment (e.g. 'https://github.com/org/repository#tag').
-	/// A specific binary name can also be optionally specified via query string parameter (e.g. 'https://github.com/org/repository?binaryname#tag'), defaulting to the name of the repository when not specified.
+	/// The url of the git repository of a parachain to be used, with branch/release tag/commit specified as #fragment (e.g. 'https://github.com/org/repository#ref').
+	/// A specific binary name can also be optionally specified via query string parameter (e.g. 'https://github.com/org/repository?binaryname#ref'), defaulting to the name of the repository when not specified.
 	#[arg(short, long)]
 	parachain: Option<Vec<String>>,
 	/// The command to run after the network has been launched.
@@ -45,10 +44,10 @@ impl ZombienetCommand {
 		// Parse arguments
 		let cache = crate::cache()?;
 		let mut zombienet = match Zombienet::new(
-			cache.clone(),
+			&cache,
 			&self.file,
-			self.relay_chain.as_ref(),
-			self.system_parachain.as_ref(),
+			self.relay_chain.as_ref().map(|v| v.as_str()),
+			self.system_parachain.as_ref().map(|v| v.as_str()),
 			self.parachain.as_ref(),
 		)
 		.await
@@ -65,30 +64,14 @@ impl ZombienetCommand {
 			},
 		};
 
-		// Check if any missing binaries
-		let missing: Vec<_> = zombienet
-			.missing_binaries()
-			.into_iter()
-			.filter_map(|b| match &b.source {
-				Source::None | Source::Artifact => None,
-				_ => Some(b),
-			})
-			.collect();
-		if !missing.is_empty() {
-			let list = style(format!(
-				"> {}",
-				missing.iter().map(|b| b.name.clone()).collect::<Vec<_>>().join(", ")
-			))
-			.dim()
-			.to_string();
-			log::warning(format!("⚠️ The following binaries specified in the network configuration file cannot be found locally:\n   {list}"))?;
-			Self::source_binaries(missing, &cache, self.verbose).await?;
+		// Source any missing/stale binaries
+		if Self::source_binaries(&mut zombienet, &cache, self.verbose).await? {
+			return Ok(());
 		}
 
 		// Finally spawn network and wait for signal to terminate
 		let spinner = cliclack::spinner();
 		spinner.start("🚀 Launching local network...");
-		//tracing_subscriber::fmt().init();
 		match zombienet.spawn().await {
 			Ok(network) => {
 				let mut result =
@@ -155,106 +138,165 @@ impl ZombienetCommand {
 	}
 
 	async fn source_binaries(
-		missing: Vec<&Binary>,
+		zombienet: &mut Zombienet,
 		cache: &PathBuf,
 		verbose: bool,
-	) -> anyhow::Result<()> {
-		// Prompt for automatic sourcing of binaries
-		let list = style(format!(
-			"> {}",
-			missing
-				.iter()
-				.map(|binary| {
-					let version = binary.version();
-					if version != "" {
-						format!("{} {}", binary.name, binary.version())
-					} else {
-						binary.name.clone()
-					}
-				})
-				.collect::<Vec<_>>()
-				.join(", ")
-		))
-		.dim()
-		.to_string();
-		if !confirm(format!(
-			"📦 Would you like to source them automatically now? It may take some time...\n   {list}"))
-		.initial_value(true)
-		.interact()?
-		{
-			outro_cancel(
-				"🚫 Cannot launch the specified network until all required binaries are available.",
-			)?;
-			return Ok(());
+	) -> anyhow::Result<bool> {
+		// Check for any missing or stale binaries
+		let binaries: Vec<_> = zombienet.binaries().filter(|b| !b.exists() || b.stale()).collect();
+		if binaries.is_empty() {
+			return Ok(false);
 		}
 
-		log::info(format!(
-			"ℹ️ Binaries will be cached at {}",
-			&cache.to_str().expect("expected local cache is invalid")
-		))?;
+		// Check if any missing binaries
+		let missing: IndexSet<_> = binaries
+			.iter()
+			.filter_map(|b| (!b.exists()).then_some((b.name(), b.version())))
+			.collect();
+		if !missing.is_empty() {
+			let list = style(format!(
+				"> {}",
+				missing.iter().map(|(name, _)| name.to_string()).collect::<Vec<_>>().join(", ")
+			))
+			.dim()
+			.to_string();
+			log::warning(format!("⚠️ The following binaries specified in the network configuration file cannot be found locally:\n   {list}"))?;
 
-		// Check for pre-existing working directory
-		let working_dir = cache.join(".src");
-		if working_dir.exists() && confirm(
-			"📦 A previous working directory has been detected. Would you like to remove it now?",
-		)
+			// Prompt for automatic sourcing of binaries
+			let list = style(format!(
+				"> {}",
+				missing
+					.iter()
+					.map(|(name, version)| {
+						if let Some(version) = version {
+							format!("{name} {version}")
+						} else {
+							name.to_string()
+						}
+					})
+					.collect::<Vec<_>>()
+					.join(", ")
+			))
+			.dim()
+			.to_string();
+			if !confirm(format!(
+				"📦 Would you like to source them automatically now? It may take some time...\n   {list}"))
 			.initial_value(true)
-			.interact()? {
-			remove_dir_all(&working_dir)?;
+			.interact()?
+			{
+				outro_cancel(
+					"🚫 Cannot launch the specified network until all required binaries are available.",
+				)?;
+				return Ok(true);
+			}
+		}
+
+		// Check if any stale binaries
+		let stale: IndexSet<_> = binaries
+			.iter()
+			.filter_map(|b| (b.stale()).then_some((b.name(), b.version(), b.latest())))
+			.collect();
+		let mut latest = false;
+		if !stale.is_empty() {
+			let list = style(format!(
+				"> {}",
+				stale
+					.iter()
+					.map(|(name, version, latest)| {
+						format!(
+							"{name} {} -> {}",
+							version.unwrap_or("None"),
+							latest.unwrap_or("None")
+						)
+					})
+					.collect::<Vec<_>>()
+					.join(", ")
+			))
+			.dim()
+			.to_string();
+			log::warning(format!(
+				"ℹ️ The following binaries have newer versions available:\n   {list}"
+			))?;
+
+			latest = confirm(
+				"📦 Would you like to source them automatically now? It may take some time..."
+					.to_string(),
+			)
+			.initial_value(true)
+			.interact()?;
+		}
+
+		let binaries: Vec<_> = binaries
+			.into_iter()
+			.filter(|b| !b.exists() || (latest && b.stale()))
+			.map(|b| {
+				if latest && b.stale() {
+					b.use_latest()
+				}
+				b
+			})
+			.collect();
+
+		if binaries.is_empty() {
+			return Ok(false);
+		}
+
+		if binaries.iter().any(|b| !b.local()) {
+			log::info(format!(
+				"ℹ️ Binaries will be cached at {}",
+				&cache.to_str().expect("expected local cache is invalid")
+			))?;
 		}
 
 		// Source binaries
+		let release = true;
 		match verbose {
 			true => {
 				let reporter = VerboseReporter;
-				for binary in missing {
-					log::info(format!("📦 Sourcing {}...", binary.name))?;
+				for binary in binaries {
+					log::info(format!("📦 Sourcing {}...", binary.name()))?;
 					Term::stderr().clear_last_lines(1)?;
-					if let Err(e) = binary.source(&working_dir, reporter, verbose).await {
+					if let Err(e) = binary.source(release, &reporter, verbose).await {
 						reporter.update(&format!("Sourcing failed: {e}"));
 						outro_cancel(
 							"🚫 Cannot launch the network until all required binaries are available.",
 						)?;
-						return Ok(());
+						return Ok(true);
 					}
 				}
 				reporter.update("");
 			},
 			false => {
 				let multi = multi_progress("📦 Sourcing binaries...".to_string());
-				let queue: Vec<_> = missing
-					.iter()
+				let queue: Vec<_> = binaries
+					.into_iter()
 					.map(|binary| {
 						let progress = multi.add(cliclack::spinner());
-						progress.start(format!("{}: waiting...", binary.name));
+						progress.start(format!("{}: waiting...", binary.name()));
 						(binary, progress)
 					})
 					.collect();
 				let mut error = false;
-				for (binary, progress) in &queue {
-					let prefix = format!("{}: ", binary.name);
-					let progress_reporter = ProgressReporter(&prefix, &progress);
-					if let Err(e) = binary.source(&working_dir, progress_reporter, verbose).await {
-						progress.error(format!("🚫 {}: {e}", binary.name));
+				for (binary, progress) in queue {
+					let prefix = format!("{}: ", binary.name());
+					let progress_reporter = ProgressReporter(prefix, progress);
+					if let Err(e) = binary.source(release, &progress_reporter, verbose).await {
+						progress_reporter.1.error(format!("🚫 {}: {e}", binary.name()));
 						error = true;
 					}
-					progress.stop(format!("✅  {}", binary.name));
+					progress_reporter.1.stop(format!("✅  {}", binary.name()));
 				}
 				multi.stop();
 				if error {
 					outro_cancel(
 						"🚫 Cannot launch the network until all required binaries are available.",
 					)?;
-					return Ok(());
+					return Ok(true);
 				}
 			},
 		};
 
-		// Remove working directory once completed successfully
-		if working_dir.exists() {
-			remove_dir_all(working_dir)?
-		}
-		return Ok(());
+		return Ok(false);
 	}
 }
 
@@ -278,10 +320,9 @@ pub(crate) async fn run_custom_command(
 }
 
 /// Reports any observed status updates to a progress bar.
-#[derive(Copy, Clone)]
-struct ProgressReporter<'a>(&'a str, &'a ProgressBar);
+struct ProgressReporter(String, ProgressBar);
 
-impl Status for ProgressReporter<'_> {
+impl Status for ProgressReporter {
 	fn update(&self, status: &str) {
 		self.1
 			.start(&format!("{}{}", self.0, status.replace("   Compiling", "Compiling")))

crates/pop-parachains/Cargo.toml

@@ -13,6 +13,7 @@ duct.workspace = true
 flate2.workspace = true
 git2.workspace = true
 git2_credentials.workspace = true
+glob.workspace = true
 strum.workspace = true
 strum_macros.workspace = true
 tar.workspace = true

crates/pop-parachains/src/lib.rs

@@ -11,10 +11,11 @@ mod utils;
 
 pub use build::build_parachain;
 pub use errors::Error;
+pub use indexmap::IndexSet;
 pub use new_pallet::{create_pallet_template, TemplatePalletConfig};
 pub use new_parachain::instantiate_template_dir;
 pub use templates::{Config, Provider, Template};
-pub use up::{Binary, Source, Status, Zombienet};
+pub use up::{Binary, Status, Zombienet};
 pub use utils::git::{Git, GitHub, Release};
 pub use utils::helpers::is_initial_endowment_valid;
 pub use utils::pallet_helpers::resolve_pallet_path;

crates/pop-parachains/src/templates.rs

@@ -1,7 +1,5 @@
 // SPDX-License-Identifier: GPL-3.0
-use strum::{
-	EnumMessage as EnumMessageT, EnumProperty as EnumPropertyT, VariantArray as VariantArrayT,
-};
+use strum::{EnumMessage as _, EnumProperty as _, VariantArray as _};
 use strum_macros::{AsRefStr, Display, EnumMessage, EnumProperty, EnumString, VariantArray};
 use thiserror::Error;