diff --git a/crates/forge-forgejo/src/branch/validate_positions.rs b/crates/forge-forgejo/src/branch/validate_positions.rs index 286fcaa0..bc3bc0c8 100644 --- a/crates/forge-forgejo/src/branch/validate_positions.rs +++ b/crates/forge-forgejo/src/branch/validate_positions.rs @@ -14,7 +14,8 @@ pub async fn validate_positions( ) -> git::validation::Result { let repo_details = &forge.repo_details; // Collect Commit Histories for `main`, `next` and `dev` branches - let commit_histories = get_commit_histories(repo_details, &repo_config, &forge.net).await; + let commit_histories = + get_commit_histories(repository, repo_details, &repo_config, &forge.net).await; let commit_histories = match commit_histories { Ok(commit_histories) => commit_histories, Err(err) => { @@ -147,25 +148,34 @@ pub async fn validate_positions( } async fn get_commit_histories( + repository: &git::repository::OpenRepository, repo_details: &git::RepoDetails, repo_config: &config::RepoConfig, net: &network::Network, ) -> Result { - let main = - (get_commit_history(repo_details, &repo_config.branches().main(), vec![], net).await)?; + let main = (get_commit_history( + repository, + repo_details, + &repo_config.branches().main(), + &[], + net, + ) + .await)?; let main_head = main[0].clone(); let next = (get_commit_history( + repository, repo_details, &repo_config.branches().next(), - vec![main_head.clone()], + &[main_head.clone()], net, ) .await)?; let next_head = next[0].clone(); let dev = (get_commit_history( + repository, repo_details, &repo_config.branches().dev(), - vec![next_head, main_head], + &[next_head, main_head], net, ) .await)?; @@ -179,11 +189,45 @@ async fn get_commit_histories( Ok(histories) } -#[tracing::instrument(fields(%branch_name),skip_all)] async fn get_commit_history( + repository: &git::repository::OpenRepository, repo_details: &git::RepoDetails, branch_name: &config::BranchName, - find_commits: Vec, + find_commits: &[git::Commit], + net: &kxio::network::Network, +) -> Result, network::NetworkError> { + let original = original_get_commit_history(repo_details, branch_name, find_commits, net).await; + let updated = repository.commit_log(branch_name, find_commits); + match (original, updated) { + (Ok(original), Ok(updated)) => { + if updated == original { + info!("new version matches"); + Ok(updated) + } else { + error!(?updated, ?original, "new version doesn't match original"); + Ok(original) + } + } + (Ok(original), Err(err_updated)) => { + warn!(?err_updated, "original ok, updated error"); + Ok(original) + } + (Err(err_original), Ok(updated)) => { + warn!(?err_original, "original err, updated ok"); + Ok(updated) + } + (Err(err_orignal), Err(err_updated)) => { + error!(?err_orignal, ?err_updated, "original err, updated err"); + Err(err_orignal) + } + } +} + +#[tracing::instrument(fields(%branch_name),skip_all)] +async fn original_get_commit_history( + repo_details: &git::RepoDetails, + branch_name: &config::BranchName, + find_commits: &[git::Commit], net: &kxio::network::Network, ) -> Result, network::NetworkError> { let hostname = &repo_details.forge.hostname(); @@ -223,9 +267,8 @@ async fn get_commit_history( let found = find_commits.is_empty() || find_commits - .clone() - .into_iter() - .any(|find_commit| commits.iter().any(|commit| commit == &find_commit)); + .iter() + .any(|find_commit| commits.iter().any(|commit| commit == find_commit)); let at_end = commits.len() < limit; all_commits.extend(commits); if found || at_end { diff --git a/crates/forge-forgejo/src/tests/mod.rs b/crates/forge-forgejo/src/tests/mod.rs index 30223ca6..90e52d51 100644 --- a/crates/forge-forgejo/src/tests/mod.rs +++ b/crates/forge-forgejo/src/tests/mod.rs @@ -20,7 +20,7 @@ mod branch { let mut net = MockNetwork::new(); //given given_forgejo_has_branches(&mut net, 1); - let repo_details = given_a_repo(fs.base(), 1); + let repo_details = given_repo_details(fs.base(), 1); let net = Network::from(net); let (repo, _reality) = git::repository::mock(); @@ -35,6 +35,47 @@ mod branch { assert_eq!(branches, vec![config::BranchName::new("string")]); } } + // mod validate_positions { + // + // use git::ForgeLike; + // + // use super::*; + // + // #[test] + // fn should_ok_all_branches_on_same_commit() { + // let_assert!(Ok(fs) = kxio::fs::temp()); + // let mut net = MockNetwork::new(); + // //given + // let repo_details = given_repo_details(fs.base(), 1); + // let net = Network::from(net); + // let (repo, _reality) = git::repository::mock(); + // let forge = given_a_forge(repo_details, &net, repo); + // let open_repository = given_an_open_repository(); + // let repo_config = given_a_repo_config(); + // + // let forge = forgejo::ForgeJo::new(repo_details, net.clone(), repo); + // + // let_assert!( + // Ok(positions) = forge + // .branches_validate_positions(open_repository, repo_config) + // .await + // ); + // } + // + // fn given_a_forge( + // repo_details: git::RepoDetails, + // net: &Network, + // repo: git::Repository, + // ) -> forgejo::ForgeJo { + // forgejo::ForgeJo::new(repo_details, net.clone(), repo) + // } + // fn given_an_open_repository() -> git::OpenRepository { + // todo!() + // } + // fn given_a_repo_config() -> config::RepoConfig { + // todo!() + // } + // } } fn given_forgejo_has_branches(net: &mut MockNetwork, i: u32) { @@ -48,7 +89,7 @@ fn given_forgejo_has_branches(net: &mut MockNetwork, i: u32) { net.add_get_response(&url, StatusCode::OK, body); } -fn given_a_repo(path: &Path, i: u32) -> git::RepoDetails { +fn given_repo_details(path: &Path, i: u32) -> git::RepoDetails { git::common::repo_details( i, git::Generation::new(), diff --git a/crates/git/src/commit.rs b/crates/git/src/commit.rs index 59693e15..e78e4868 100644 --- a/crates/git/src/commit.rs +++ b/crates/git/src/commit.rs @@ -32,3 +32,14 @@ pub struct Histories { pub next: Vec, pub dev: Vec, } + +pub mod log { + use derive_more::{Display, From}; + + #[derive(Debug, Display, From)] + pub enum Error { + Gix(String), + Lock, + } + impl std::error::Error for Error {} +} diff --git a/crates/git/src/repository/mock.rs b/crates/git/src/repository/mock.rs index 6975b3e6..fc266151 100644 --- a/crates/git/src/repository/mock.rs +++ b/crates/git/src/repository/mock.rs @@ -6,8 +6,8 @@ use std::{ sync::{Arc, Mutex}, }; -use git_next_config::GitDir; - +use super::Error; +use crate as git; use crate::{ repository::{ open::{OpenRepository, OpenRepositoryLike}, @@ -15,8 +15,7 @@ use crate::{ }, GitRemote, RepoDetails, }; - -use super::Error; +use git_next_config::GitDir; #[derive(Debug, Default, Clone)] pub struct MockRepository(Arc>); @@ -117,7 +116,7 @@ impl OpenRepositoryLike for MockOpenRepository { .unwrap() } - fn fetch(&self) -> std::prelude::v1::Result<(), crate::fetch::Error> { + fn fetch(&self) -> core::result::Result<(), crate::fetch::Error> { self.inner.lock().map(|inner| inner.fetch()).unwrap() } @@ -127,12 +126,23 @@ impl OpenRepositoryLike for MockOpenRepository { branch_name: git_next_config::BranchName, to_commit: crate::GitRef, force: crate::push::Force, - ) -> std::prelude::v1::Result<(), crate::push::Error> { + ) -> core::result::Result<(), crate::push::Error> { self.inner .lock() .map(|inner| inner.push(repo_details, branch_name, to_commit, force)) .unwrap() } + + fn commit_log( + &self, + branch_name: &git_next_config::BranchName, + find_commits: &[git::Commit], + ) -> core::result::Result, git::commit::log::Error> { + self.inner + .lock() + .map(|inner| inner.commit_log(branch_name, find_commits)) + .unwrap() + } } impl OpenRepositoryLike for InnerMockOpenRepository { fn find_default_remote(&self, direction: Direction) -> Option { @@ -142,7 +152,7 @@ impl OpenRepositoryLike for InnerMockOpenRepository { } } - fn fetch(&self) -> std::prelude::v1::Result<(), crate::fetch::Error> { + fn fetch(&self) -> core::result::Result<(), crate::fetch::Error> { todo!() } @@ -152,7 +162,15 @@ impl OpenRepositoryLike for InnerMockOpenRepository { _branch_name: git_next_config::BranchName, _to_commit: crate::GitRef, _force: crate::push::Force, - ) -> std::prelude::v1::Result<(), crate::push::Error> { + ) -> core::result::Result<(), crate::push::Error> { + todo!() + } + + fn commit_log( + &self, + _branch_name: &git_next_config::BranchName, + _find_commits: &[git::Commit], + ) -> core::result::Result, crate::commit::log::Error> { todo!() } } diff --git a/crates/git/src/repository/open/mod.rs b/crates/git/src/repository/open/mod.rs index 2a86c43b..b2fcd65b 100644 --- a/crates/git/src/repository/open/mod.rs +++ b/crates/git/src/repository/open/mod.rs @@ -6,38 +6,44 @@ pub mod oreal; use std::sync::{Arc, Mutex}; -use git_next_config::BranchName; - -use crate::{ - fetch, push, - repository::{mock::MockOpenRepository, open::oreal::RealOpenRepository, Direction}, - GitRef, GitRemote, RepoDetails, -}; +use crate as git; +use git::repository::open::oreal::RealOpenRepository; +use git::repository::Direction; +use git_next_config as config; #[derive(Clone, Debug)] pub enum OpenRepository { - Real(oreal::RealOpenRepository), - Mock(MockOpenRepository), // TODO: (#38) contain a mock model of a repo + Real(RealOpenRepository), + Mock(git::repository::mock::MockOpenRepository), // TODO: (#38) contain a mock model of a repo } impl OpenRepository { pub fn real(gix_repo: gix::Repository) -> Self { - Self::Real(RealOpenRepository::new(Arc::new(Mutex::new(gix_repo)))) + Self::Real(oreal::RealOpenRepository::new(Arc::new(Mutex::new( + gix_repo, + )))) } #[cfg(not(tarpaulin_include))] // don't test mocks - pub const fn mock(mock: MockOpenRepository) -> Self { + pub const fn mock(mock: git::repository::mock::MockOpenRepository) -> Self { Self::Mock(mock) } } pub trait OpenRepositoryLike { - fn find_default_remote(&self, direction: Direction) -> Option; - fn fetch(&self) -> Result<(), fetch::Error>; + fn find_default_remote(&self, direction: Direction) -> Option; + fn fetch(&self) -> Result<(), git::fetch::Error>; fn push( &self, - repo_details: &RepoDetails, - branch_name: BranchName, - to_commit: GitRef, - force: push::Force, - ) -> Result<(), push::Error>; + repo_details: &git::RepoDetails, + branch_name: config::BranchName, + to_commit: git::GitRef, + force: git::push::Force, + ) -> Result<(), git::push::Error>; + + /// List of commits in a branch, optionally up-to any specified commit. + fn commit_log( + &self, + branch_name: &config::BranchName, + find_commits: &[git::Commit], + ) -> Result, git::commit::log::Error>; } impl std::ops::Deref for OpenRepository { type Target = dyn OpenRepositoryLike; diff --git a/crates/git/src/repository/open/oreal.rs b/crates/git/src/repository/open/oreal.rs index b03f4b87..61c5e8d3 100644 --- a/crates/git/src/repository/open/oreal.rs +++ b/crates/git/src/repository/open/oreal.rs @@ -1,15 +1,16 @@ // + +use crate as git; +use git_next_config as config; +use gix::bstr::BStr; + use std::sync::{Arc, Mutex}; - -use git_next_config::{BranchName, Hostname, RepoPath}; use tracing::{info, warn}; -use crate::{fetch, push, repository::Direction, GitRef, GitRemote, RepoDetails}; - #[derive(Clone, Debug, derive_more::Constructor)] pub struct RealOpenRepository(Arc>); impl super::OpenRepositoryLike for RealOpenRepository { - fn find_default_remote(&self, direction: Direction) -> Option { + fn find_default_remote(&self, direction: git::repository::Direction) -> Option { let Ok(repository) = self.0.lock() else { #[cfg(not(tarpaulin_include))] // don't test mutex lock failure return None; @@ -21,14 +22,16 @@ impl super::OpenRepositoryLike for RealOpenRepository { remote.url(direction.into()).map(Into::into) } - fn fetch(&self) -> Result<(), fetch::Error> { + fn fetch(&self) -> Result<(), git::fetch::Error> { let Ok(repository) = self.0.lock() else { #[cfg(not(tarpaulin_include))] // don't test mutex lock failure - return Err(fetch::Error::Lock); + return Err(git::fetch::Error::Lock); }; - let Some(Ok(remote)) = repository.find_default_remote(Direction::Fetch.into()) else { + let Some(Ok(remote)) = + repository.find_default_remote(git::repository::Direction::Fetch.into()) + else { #[cfg(not(tarpaulin_include))] // test is on local repo - should always have remotes - return Err(fetch::Error::NoFetchRemoteFound); + return Err(git::fetch::Error::NoFetchRemoteFound); }; let prepared_fetch = remote .connect(gix::remote::Direction::Fetch)? @@ -37,9 +40,9 @@ impl super::OpenRepositoryLike for RealOpenRepository { Ok(fetch) => { fetch .receive(gix::progress::Discard, &Default::default()) - .map_err(|e| fetch::Error::Fetch(e.to_string()))?; + .map_err(|e| git::fetch::Error::Fetch(e.to_string()))?; } - Err(e) => Err(fetch::Error::Fetch(e.to_string()))?, + Err(e) => Err(git::fetch::Error::Fetch(e.to_string()))?, } Ok(()) @@ -49,15 +52,17 @@ impl super::OpenRepositoryLike for RealOpenRepository { #[cfg(not(tarpaulin_include))] // would require writing to external service fn push( &self, - repo_details: &RepoDetails, - branch_name: BranchName, - to_commit: GitRef, - force: push::Force, - ) -> Result<(), push::Error> { + repo_details: &git::RepoDetails, + branch_name: config::BranchName, + to_commit: git::GitRef, + force: git::push::Force, + ) -> Result<(), git::push::Error> { let origin = repo_details.origin(); let force = match force { - push::Force::No => "".to_string(), - push::Force::From(old_ref) => format!("--force-with-lease={branch_name}:{old_ref}"), + git::push::Force::No => "".to_string(), + git::push::Force::From(old_ref) => { + format!("--force-with-lease={branch_name}:{old_ref}") + } }; // INFO: never log the command as it contains the API token within the 'origin' use secrecy::ExposeSecret; @@ -70,7 +75,7 @@ impl super::OpenRepositoryLike for RealOpenRepository { let git_dir = self .0 .lock() - .map_err(|_| push::Error::Lock) + .map_err(|_| git::push::Error::Lock) .map(|r| r.git_dir().to_path_buf())?; let ctx = gix::diff::command::Context { @@ -91,23 +96,66 @@ impl super::OpenRepositoryLike for RealOpenRepository { } Err(err) => { warn!(?err, ?git_dir, "Failed (wait)"); - Err(push::Error::Push) + Err(git::push::Error::Push) } }, Err(err) => { warn!(?err, ?git_dir, "Failed (spawn)"); - Err(push::Error::Push) + Err(git::push::Error::Push) } } } + + fn commit_log( + &self, + branch_name: &config::BranchName, + find_commits: &[git::Commit], + ) -> Result, git::commit::log::Error> { + self.0 + .lock() + .map_err(|_| git::commit::log::Error::Lock) + .map(|repo| { + let branch_name = branch_name.to_string(); + let branch_name = BStr::new(&branch_name); + let branch_head = repo + .rev_parse_single(branch_name) + .map_err(|e| e.to_string())?; + let object = branch_head.object().map_err(|e| e.to_string())?; + let commit = object.try_into_commit().map_err(|e| e.to_string())?; + let walk = repo + .rev_walk([commit.id]) + .all() + .map_err(|e| e.to_string())?; + let mut commits = vec![]; + for item in walk { + let item = item.map_err(|e| e.to_string())?; + let commit = item.object().map_err(|e| e.to_string())?; + let id = commit.id().to_string(); + let message = commit.message_raw().map_err(|e| e.to_string())?.to_string(); + let commit = git::Commit::new( + git::commit::Sha::new(id), + git::commit::Message::new(message), + ); + if find_commits.contains(&commit) { + commits.push(commit); + break; + } + commits.push(commit); + } + Ok(commits) + })? + } } -impl From<&gix::Url> for GitRemote { +impl From<&gix::Url> for git::GitRemote { fn from(url: &gix::Url) -> Self { let host = url.host().unwrap_or_default(); let path = url.path.to_string(); let path = path.strip_prefix('/').map_or(path.as_str(), |path| path); let path = path.strip_suffix(".git").map_or(path, |path| path); - Self::new(Hostname::new(host), RepoPath::new(path.to_string())) + Self::new( + config::Hostname::new(host), + config::RepoPath::new(path.to_string()), + ) } }