From 9c73f8934285f86e7a1b7d917ca96d8d1aee9953 Mon Sep 17 00:00:00 2001 From: Vincenzo Palazzo Date: Fri, 12 Apr 2024 12:52:33 +0200 Subject: [PATCH 1/2] core: remove repo path from url Signed-off-by: Vincenzo Palazzo --- coffee_core/src/coffee.rs | 23 +++++------ coffee_core/src/nurse/strategy.rs | 4 +- coffee_github/src/repository.rs | 54 ++++++++++++++++++-------- coffee_github/src/utils.rs | 8 ++-- coffee_lib/src/repository.rs | 2 + coffee_lib/src/url.rs | 19 ++++----- coffee_storage/src/model/repository.rs | 4 ++ 7 files changed, 71 insertions(+), 43 deletions(-) diff --git a/coffee_core/src/coffee.rs b/coffee_core/src/coffee.rs index 1f0f01ca..dbb0a1f3 100644 --- a/coffee_core/src/coffee.rs +++ b/coffee_core/src/coffee.rs @@ -2,7 +2,6 @@ use std::collections::HashMap; use std::fmt::Debug; use std::vec::Vec; -use tokio::fs; use async_trait::async_trait; use clightningrpc_common::client::Client; @@ -12,6 +11,7 @@ use log; use serde::de::DeserializeOwned; use serde::{Deserialize, Serialize}; use serde_json::json; +use tokio::fs; use tokio::process::Command; use coffee_github::repository::Github; @@ -108,11 +108,11 @@ impl CoffeeManager { .storage .load::>("repositories") .await - .map(|item| { + .map(|mut item| { log::debug!("repositories in store {:?}", item); - item.iter().for_each(|repo| match repo.1.kind { + item.iter_mut().for_each(|(_, info)| match info.kind { Kind::Git => { - let repo = Github::from(repo.1); + let repo = Github::from(info); self.repos.insert(repo.name(), Box::new(repo)); } }); @@ -432,9 +432,10 @@ impl PluginManager for CoffeeManager { if self.repos.contains_key(name) { return Err(error!("repository with name: {name} already exists")); } - let url = URL::new(&self.config.root_path, url, name); + let url = URL::new(url, name); log::debug!("remote adding: {} {}", name, &url.url_string); - let mut repo = Github::new(name, &url); + let repo_path = format!("{}/repositories/{name}", self.config.root_path); + let mut repo = Github::new(name, &repo_path, &url); repo.init().await?; self.repos.insert(repo.name(), Box::new(repo)); log::debug!("remote added: {} {}", name, &url.url_string); @@ -447,7 +448,7 @@ impl PluginManager for CoffeeManager { match self.repos.get(name) { Some(repo) => { let remote_repo = repo.list().await?; - let repo_path = repo.url().path_string; + let repo_home = repo.home(); let plugins = self.config.plugins.clone(); for plugin in &remote_repo { if let Some(ind) = plugins @@ -461,7 +462,7 @@ impl PluginManager for CoffeeManager { } } } - fs::remove_dir_all(repo_path).await?; + fs::remove_dir_all(repo_home).await?; self.repos.remove(name); log::debug!("remote removed: {}", name); self.flush().await?; @@ -476,8 +477,8 @@ impl PluginManager for CoffeeManager { async fn list_remotes(&mut self) -> Result { let mut remote_list = Vec::new(); for repo in self.repos.values() { - let repository = git2::Repository::open(repo.url().path_string.as_str()) - .map_err(|err| error!("{}", err.message()))?; + let repository = + git2::Repository::open(repo.home()).map_err(|err| error!("{}", err.message()))?; let (commit, date) = get_repo_info!(repository); remote_list.push(CoffeeListRemote { local_name: repo.name(), @@ -587,7 +588,7 @@ impl PluginManager for CoffeeManager { // Maybe when trying to recover the repository, // we have created the folder but we were not able // to clone the repository. - let repo_path = repo.url().path_string; + let repo_path = repo.home(); // This shouldn't return an error if the repository // is not present locally. let _ = fs::remove_dir_all(repo_path).await; diff --git a/coffee_core/src/nurse/strategy.rs b/coffee_core/src/nurse/strategy.rs index a1e85d4d..9777414e 100644 --- a/coffee_core/src/nurse/strategy.rs +++ b/coffee_core/src/nurse/strategy.rs @@ -56,9 +56,9 @@ impl Handler for GitRepositoryLocallyAbsentStrategy { let mut repos: Vec = Vec::new(); let coffee_repos = &coffee.repos; for repo in coffee_repos.values() { + let repo_home = repo.home(); log::debug!("Checking if repository {} exists locally", repo.name()); - let repo_path = repo.url().path_string; - let repo_path = Path::new(&repo_path); + let repo_path = Path::new(&repo_home); if !repo_path.exists() { log::debug!("Repository {} is missing locally", repo.name()); repos.push(repo.name().to_string()); diff --git a/coffee_github/src/repository.rs b/coffee_github/src/repository.rs index be237e65..8166bbde 100644 --- a/coffee_github/src/repository.rs +++ b/coffee_github/src/repository.rs @@ -27,6 +27,8 @@ pub struct Github { /// the url of the repository to be able /// to get all the plugin information. url: URL, + /// The root path of the repository + root_path: Option, /// the name of the repository that can be used /// by coffee as repository key. name: String, @@ -54,11 +56,12 @@ fn is_hidden(entry: &DirEntry) -> bool { impl Github { /// Create a new instance of the Repository /// with a name and a url - pub fn new(name: &str, url: &URL) -> Self { + pub fn new(name: &str, path: &str, url: &URL) -> Self { debug!("creating repository: {} {}", name, url.url_string); Github { name: name.to_owned(), url: url.clone(), + root_path: Some(path.to_owned()), plugins: vec![], branch: "".to_owned(), git_head: None, @@ -69,8 +72,7 @@ impl Github { /// Index the repository to store information /// related to the plugins pub async fn index_repository(&mut self) -> Result<(), CoffeeError> { - let repo_path = &self.url.path_string; - let target_dirs = WalkDir::new(repo_path) + let target_dirs = WalkDir::new(self.home()) .max_depth(1) .into_iter() .filter_entry(|dir_entry| !is_hidden(dir_entry)); @@ -203,17 +205,25 @@ impl Github { #[async_trait] impl Repository for Github { + fn home(&self) -> String { + // SAFETY: this is safe bause the optional + // value is used only for retro-compatibility + self.root_path.clone().unwrap() + } + /// Init the repository where it is required to index /// all the plugin contained, and store somewhere the index. /// /// Where to store the index is an implementation /// details. async fn init(&mut self) -> Result<(), CoffeeError> { - debug!( + log::debug!( "initializing repository: {} {} > {}", - self.name, &self.url.url_string, &self.url.path_string, + self.name, + self.url.url_string, + self.home(), ); - let res = git2::Repository::clone(&self.url.url_string, &self.url.path_string); + let res = git2::Repository::clone(&self.url.url_string, &self.home()); match res { Ok(repo) => { self.branch = if repo.find_branch("master", git2::BranchType::Local).is_ok() { @@ -225,7 +235,7 @@ impl Repository for Github { self.git_head = Some(commit.clone()); self.last_activity = Some(date.clone()); - let clone = clone_recursive_fix(repo, &self.url).await; + let clone = clone_recursive_fix(repo, &self.home()).await; self.index_repository().await?; clone } @@ -259,7 +269,7 @@ impl Repository for Github { } } // pull the changes from the repository - let status = git_upgrade(&self.url.path_string, &self.branch, verbose).await?; + let status = git_upgrade(&self.home(), &self.branch, verbose).await?; self.git_head = Some(status.commit_id()); self.last_activity = Some(status.date()); Ok(CoffeeUpgrade { @@ -275,11 +285,11 @@ impl Repository for Github { log::debug!( "recovering repository: {} {} > {}", self.name, - &self.url.url_string, - &self.url.path_string, + self.url.url_string, + self.home(), ); // recursively clone the repository - let res = git2::Repository::clone(&self.url.url_string, &self.url.path_string); + let res = git2::Repository::clone(&self.url.url_string, &self.home()); match res { Ok(repo) => { // get the commit id @@ -298,8 +308,7 @@ impl Repository for Github { // retrieve the submodules let submodules = repo.submodules().unwrap_or_default(); for (_, sub) in submodules.iter().enumerate() { - let path = - format!("{}/{}", &self.url.path_string, sub.path().to_str().unwrap()); + let path = format!("{}/{}", &self.home(), sub.path().to_str().unwrap()); if let Err(err) = git2::Repository::clone(sub.url().unwrap(), &path) { return Err(error!("{}", err.message())); } @@ -344,7 +353,8 @@ impl Repository for Github { impl From for Github { fn from(value: StorageRepository) -> Self { Github { - url: value.url, + url: value.url.clone(), + root_path: Some(value.root_path.unwrap_or(value.url.path_string.unwrap())), name: value.name, plugins: value.plugins, branch: value.branch, @@ -354,10 +364,16 @@ impl From for Github { } } -impl From<&StorageRepository> for Github { - fn from(value: &StorageRepository) -> Self { +impl From<&mut StorageRepository> for Github { + fn from(value: &mut StorageRepository) -> Self { + let root_path = value + .root_path + .clone() + .unwrap_or(value.url.path_string.clone().unwrap()); + value.url.path_string = None; // make sure that we store a none value Github { url: value.url.to_owned(), + root_path: Some(root_path), name: value.name.to_owned(), plugins: value.plugins.to_owned(), branch: value.branch.to_owned(), @@ -377,6 +393,9 @@ impl From for StorageRepository { branch: value.branch, git_head: value.git_head, last_activity: value.last_activity, + root_path: Some(value.root_path.expect( + "this should be alway not None, otherwise there is a bug in the migration logic", + )), } } } @@ -391,6 +410,9 @@ impl From<&Github> for StorageRepository { branch: value.branch.to_owned(), git_head: value.git_head.to_owned(), last_activity: value.last_activity.to_owned(), + root_path: Some(value.root_path.clone().expect( + "this should be alway not None, otherwise there is a bug in the migration logic", + )), } } } diff --git a/coffee_github/src/utils.rs b/coffee_github/src/utils.rs index 269354a5..4a9bb7c3 100644 --- a/coffee_github/src/utils.rs +++ b/coffee_github/src/utils.rs @@ -1,17 +1,19 @@ use coffee_lib::errors::CoffeeError; use coffee_lib::macros::error; -use coffee_lib::url::URL; use coffee_lib::{commit_id, get_repo_info, sh}; use log::debug; use coffee_lib::types::response::UpgradeStatus; -pub async fn clone_recursive_fix(repo: git2::Repository, url: &URL) -> Result<(), CoffeeError> { +pub async fn clone_recursive_fix( + repo: git2::Repository, + root_path: &str, +) -> Result<(), CoffeeError> { let repository = repo.submodules().unwrap_or_default(); debug!("submodule count: {}", repository.len()); for (index, sub) in repository.iter().enumerate() { debug!("url {}: {}", index + 1, sub.url().unwrap()); - let path = format!("{}/{}", &url.path_string, sub.path().to_str().unwrap()); + let path = format!("{}/{}", root_path, sub.path().to_str().unwrap()); match git2::Repository::clone(sub.url().unwrap(), &path) { // Fix error handling Ok(_) => { diff --git a/coffee_lib/src/repository.rs b/coffee_lib/src/repository.rs index dbb9bec7..7e8d44d4 100644 --- a/coffee_lib/src/repository.rs +++ b/coffee_lib/src/repository.rs @@ -41,4 +41,6 @@ pub trait Repository: Any { fn url(&self) -> URL; fn as_any(&self) -> &dyn Any; + + fn home(&self) -> String; } diff --git a/coffee_lib/src/url.rs b/coffee_lib/src/url.rs index e5172647..f7fbc1e5 100644 --- a/coffee_lib/src/url.rs +++ b/coffee_lib/src/url.rs @@ -11,10 +11,12 @@ pub struct URL { pub name: String, /// the url string pub url_string: String, - /// the coffee path associated with the url - pub path_string: String, /// the repo name associated with the url pub repo_name: String, + #[deprecated( + note = "this value is used only for migration from db, a caller you never use this value" + )] + pub path_string: Option, } /// Handle GitHub HTTP links @@ -48,23 +50,19 @@ fn get_repo_name_from_url(url: &str) -> String { impl URL { /// Build a new URL and initialize its fields - pub fn new(local_path: &str, url: &str, remote_name: &str) -> Self { + pub fn new(url: &str, remote_name: &str) -> Self { URL { name: remote_name.to_owned(), url_string: handle_incorrect_url(url), - path_string: format!("{local_path}/repositories/{remote_name}"), repo_name: get_repo_name_from_url(url), + path_string: None, } } } impl fmt::Display for URL { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!( - f, - "repo_name: {}, url: {}, path: {}", - self.repo_name, self.url_string, self.path_string - ) + write!(f, "repo_name: {}, url: {}", self.repo_name, self.url_string) } } @@ -75,9 +73,8 @@ mod tests { #[test] fn test_remote() { let u = "https://github.com/lightningd/plugins"; - let url = URL::new("/tmp/", u, "lightningd_plugins"); + let url = URL::new(u, "lightningd_plugins"); assert_eq!(url.repo_name, "plugins"); assert_eq!(url.url_string, u); - println!("{}", &url); } } diff --git a/coffee_storage/src/model/repository.rs b/coffee_storage/src/model/repository.rs index 190e545d..7e4203dd 100644 --- a/coffee_storage/src/model/repository.rs +++ b/coffee_storage/src/model/repository.rs @@ -12,6 +12,10 @@ pub enum Kind { pub struct Repository { pub kind: Kind, pub name: String, + #[deprecated( + note = "make this not optional, the optional value is good just for db migration" + )] + pub root_path: Option, pub url: URL, pub plugins: Vec, pub branch: String, From 1c2382fef6f5186a496933760e9ed1a568342832 Mon Sep 17 00:00:00 2001 From: Vincenzo Palazzo Date: Fri, 12 Apr 2024 13:10:36 +0200 Subject: [PATCH 2/2] core: add the home plugins and repository home path This is adding the repository home and the pligin home method inside the coffee manager, because this is where we store the repositories and plugins on disk. Signed-off-by: Vincenzo Palazzo --- coffee_core/src/coffee.rs | 32 ++++++++++++++++++-------------- coffee_lib/src/plugin_manager.rs | 11 +++++++++++ 2 files changed, 29 insertions(+), 14 deletions(-) diff --git a/coffee_core/src/coffee.rs b/coffee_core/src/coffee.rs index dbb0a1f3..865f4e54 100644 --- a/coffee_core/src/coffee.rs +++ b/coffee_core/src/coffee.rs @@ -247,6 +247,21 @@ impl CoffeeManager { #[async_trait] impl PluginManager for CoffeeManager { + /// The repositories home directory, where all the dirs + /// will be copied and cloned + fn repositories_home(&self) -> String { + format!("{}/repositories", self.config.root_path) + } + + /// The plugins home directory + /// + /// When we install a plugin we will have to copy it in + /// another plugin directory and this is the home + /// (aka root path for it). + fn plugins_home(&self) -> String { + unimplemented!() + } + async fn configure(&mut self) -> Result<(), CoffeeError> { log::debug!("plugin configured"); Ok(()) @@ -269,8 +284,6 @@ impl PluginManager for CoffeeManager { // keep track if the plugin is successfully installed for repo in self.repos.values() { if let Some(mut plugin) = repo.get_plugin_by_name(plugin) { - log::trace!("{:?}", plugin); - if try_dynamic && plugin.important() { return Err(error!( "plugin is important, can't be dynamically installed" @@ -289,20 +302,11 @@ impl PluginManager for CoffeeManager { plugin.name() ); - log::debug!( - "Start! copying directory from {} inside the new one {}", - old_root_path, - new_root_path - ); + // FIXME: does this with pure rust let script = format!("cp -r {old_root_path} {new_root_path}"); sh!(self.config.root_path.clone(), script, verbose); - log::debug!( - "Done! copying directory from {} inside the new one {}", - old_root_path, - new_root_path - ); - let old_exec_path = plugin.exec_path.clone(); + let old_exec_path = plugin.exec_path.clone(); let plugin_conf_key = if plugin.important() { "important-plugin" } else { @@ -434,7 +438,7 @@ impl PluginManager for CoffeeManager { } let url = URL::new(url, name); log::debug!("remote adding: {} {}", name, &url.url_string); - let repo_path = format!("{}/repositories/{name}", self.config.root_path); + let repo_path = format!("{}/{name}", self.repositories_home()); let mut repo = Github::new(name, &repo_path, &url); repo.init().await?; self.repos.insert(repo.name(), Box::new(repo)); diff --git a/coffee_lib/src/plugin_manager.rs b/coffee_lib/src/plugin_manager.rs index 103b77e1..c2ad5ef0 100644 --- a/coffee_lib/src/plugin_manager.rs +++ b/coffee_lib/src/plugin_manager.rs @@ -75,4 +75,15 @@ pub trait PluginManager { /// enable a plugin by name async fn enable(&mut self, plugin: &str) -> Result<(), CoffeeError>; + + /// The repositories home directory, where all the dirs + /// will be copied and cloned + fn repositories_home(&self) -> String; + + /// The plugins home directory + /// + /// When we install a plugin we will have to copy it in + /// another plugin directory and this is the home + /// (aka root path for it). + fn plugins_home(&self) -> String; }