From 5a515f9407c19eb6804ddedceb93e2ba52f0a3ed Mon Sep 17 00:00:00 2001 From: AlexD10S Date: Fri, 26 Apr 2024 15:00:09 +0200 Subject: [PATCH] refactor: improve code to manage different urls --- crates/pop-parachains/src/new_parachain.rs | 4 +- crates/pop-parachains/src/templates.rs | 33 +++++--------- crates/pop-parachains/src/utils/git.rs | 53 ++++++++++++++++++---- 3 files changed, 56 insertions(+), 34 deletions(-) diff --git a/crates/pop-parachains/src/new_parachain.rs b/crates/pop-parachains/src/new_parachain.rs index ed7422e2..a341c18a 100644 --- a/crates/pop-parachains/src/new_parachain.rs +++ b/crates/pop-parachains/src/new_parachain.rs @@ -24,7 +24,7 @@ pub fn instantiate_template_dir( if matches!(template, &Template::Base) { return instantiate_base_template(target, config, tag_version); } - let tag = Git::clone_and_degit(template, target, tag_version)?; + let tag = Git::clone_and_degit(template.repository_url()?, target, tag_version)?; Ok(tag) } @@ -37,7 +37,7 @@ pub fn instantiate_base_template( let source = temp_dir.path(); let template = crate::templates::Template::Base; - let tag = Git::clone_and_degit(&template, source, tag_version)?; + let tag = Git::clone_and_degit(template.repository_url()?, source, tag_version)?; for entry in WalkDir::new(&source) { let entry = entry?; diff --git a/crates/pop-parachains/src/templates.rs b/crates/pop-parachains/src/templates.rs index d353794f..5c73486e 100644 --- a/crates/pop-parachains/src/templates.rs +++ b/crates/pop-parachains/src/templates.rs @@ -76,7 +76,7 @@ pub enum Template { serialize = "base", message = "Standard", detailed_message = "A standard parachain", - props(Provider = "Pop", Repository = "r0gue-io/base-parachain") + props(Provider = "Pop", Repository = "https://github.com/r0gue-io/base-parachain") )] Base, // Parity @@ -84,14 +84,20 @@ pub enum Template { serialize = "cpt", message = "Contracts", detailed_message = "Minimal Substrate node configured for smart contracts via pallet-contracts.", - props(Provider = "Parity", Repository = "paritytech/substrate-contracts-node") + props( + Provider = "Parity", + Repository = "https://github.com/paritytech/substrate-contracts-node" + ) )] ParityContracts, #[strum( serialize = "fpt", message = "EVM", detailed_message = "Template node for a Frontier (EVM) based parachain.", - props(Provider = "Parity", Repository = "paritytech/frontier-parachain-template") + props( + Provider = "Parity", + Repository = "https://github.com/paritytech/frontier-parachain-template" + ) )] ParityFPT, } @@ -108,13 +114,8 @@ impl Template { self.get_str("Provider") == Some(provider.to_string().as_str()) } - pub fn repository_url(&self) -> Result { - Ok(["https://github.com/", self.get_str("Repository").ok_or(Error::RepositoryMissing)?] - .concat()) - } - pub fn ssh_repository_url(&self) -> Result { - Ok(["git@github.com:", self.get_str("Repository").ok_or(Error::RepositoryMissing)?, ".git"] - .concat()) + pub fn repository_url(&self) -> Result<&str, Error> { + self.get_str("Repository").ok_or(Error::RepositoryMissing) } } @@ -159,28 +160,16 @@ mod tests { template.repository_url().unwrap(), "https://github.com/r0gue-io/base-parachain" ); - assert_eq!( - template.ssh_repository_url().unwrap(), - "git@github.com:r0gue-io/base-parachain.git" - ); template = Template::ParityContracts; assert_eq!( template.repository_url().unwrap(), "https://github.com/paritytech/substrate-contracts-node" ); - assert_eq!( - template.ssh_repository_url().unwrap(), - "git@github.com:paritytech/substrate-contracts-node.git" - ); template = Template::ParityFPT; assert_eq!( template.repository_url().unwrap(), "https://github.com/paritytech/frontier-parachain-template" ); - assert_eq!( - template.ssh_repository_url().unwrap(), - "git@github.com:paritytech/frontier-parachain-template.git" - ); } #[test] diff --git a/crates/pop-parachains/src/utils/git.rs b/crates/pop-parachains/src/utils/git.rs index 982f2c04..83b9c8f7 100644 --- a/crates/pop-parachains/src/utils/git.rs +++ b/crates/pop-parachains/src/utils/git.rs @@ -1,6 +1,5 @@ // SPDX-License-Identifier: GPL-3.0 use crate::errors::Error; -use crate::Template; use anyhow::Result; use git2::{ build::RepoBuilder, FetchOptions, IndexAddOption, RemoteCallbacks, Repository, ResetType, @@ -13,8 +12,6 @@ use url::Url; pub struct Git; impl Git { - const GIT_SSH: &'static str = "git@github.com:"; - pub(crate) fn clone(url: &Url, working_dir: &Path, branch: Option<&str>) -> Result<()> { if !working_dir.exists() { let mut fo = FetchOptions::new(); @@ -31,8 +28,7 @@ impl Git { Ok(()) } pub(crate) fn ssh_clone(url: &Url, working_dir: &Path, branch: Option<&str>) -> Result<()> { - // Change the url to the ssh url with git@github.com: prefix, remove / from path and adding .git as suffix - let ssh_url = [Self::GIT_SSH, &url.path()[1..], ".git"].concat(); + let ssh_url = GitHub::convert_to_shh_url(url); if !working_dir.exists() { // Prepare callback and fetch options. let mut fo = FetchOptions::new(); @@ -49,13 +45,16 @@ impl Git { } /// Clone `url` into `target` and degit it pub fn clone_and_degit( - template: &Template, + url: &str, target: &Path, tag_version: Option, ) -> Result> { - let repo = match Repository::clone(&template.repository_url()?, target) { + let repo = match Repository::clone(url, target) { Ok(repo) => repo, - Err(_e) => Self::ssh_clone_and_degit(&template.ssh_repository_url()?, target)?, + Err(_e) => Self::ssh_clone_and_degit( + url::Url::parse(url).map_err(|err| Error::from(err))?, + target, + )?, }; if let Some(tag_version) = tag_version { @@ -84,14 +83,15 @@ impl Git { } /// For users that have ssh configuration for cloning repositories - fn ssh_clone_and_degit(url: &str, target: &Path) -> Result { + fn ssh_clone_and_degit(url: Url, target: &Path) -> Result { + let ssh_url = GitHub::convert_to_shh_url(&url); // Prepare callback and fetch options. let mut fo = FetchOptions::new(); Self::set_up_ssh_fetch_options(&mut fo)?; // Prepare builder and clone. let mut builder = RepoBuilder::new(); builder.fetch_options(fo); - let repo = builder.clone(url, target)?; + let repo = builder.clone(&ssh_url, target)?; Ok(repo) } @@ -218,6 +218,9 @@ impl GitHub { pub(crate) fn release(repo: &Url, tag: &str, artifact: &str) -> String { format!("{}/releases/download/{tag}/{artifact}", repo.as_str()) } + pub(crate) fn convert_to_shh_url(url: &Url) -> String { + format!("git@{}:{}.git", url.host_str().unwrap_or("github.com"), &url.path()[1..]) + } } #[derive(serde::Deserialize)] @@ -227,3 +230,33 @@ pub struct Release { pub prerelease: bool, pub commit: Option, } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_convert_to_shh_url() { + assert_eq!( + GitHub::convert_to_shh_url( + &Url::parse("https://github.com/r0gue-io/base-parachain") + .expect("valid repository url") + ), + "git@github.com:r0gue-io/base-parachain.git" + ); + assert_eq!( + GitHub::convert_to_shh_url( + &Url::parse("https://github.com/paritytech/substrate-contracts-node") + .expect("valid repository url") + ), + "git@github.com:paritytech/substrate-contracts-node.git" + ); + assert_eq!( + GitHub::convert_to_shh_url( + &Url::parse("https://github.com/paritytech/frontier-parachain-template") + .expect("valid repository url") + ), + "git@github.com:paritytech/frontier-parachain-template.git" + ); + } +}