From a8587f3890315b0dbd926b77abc9491f3a7a1376 Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Fri, 24 May 2024 08:47:34 +0100 Subject: [PATCH] WIP: refactor: get commit log from local repo Avoid using a forge-specific API to get a commit log when the information is already available locally in the cloned repo through a generic git command. --- crates/forge-forgejo/src/tests/mod.rs | 45 ++++++++++++- crates/git/src/commit.rs | 11 +++ crates/git/src/repository/mock.rs | 19 ++++++ crates/git/src/repository/open/mod.rs | 42 +++++++----- crates/git/src/repository/open/oreal.rs | 90 ++++++++++++++++++------- 5 files changed, 164 insertions(+), 43 deletions(-) diff --git a/crates/forge-forgejo/src/tests/mod.rs b/crates/forge-forgejo/src/tests/mod.rs index 30223ca..90e52d5 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 59693e1..e78e486 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 6975b3e..d6ac858 100644 --- a/crates/git/src/repository/mock.rs +++ b/crates/git/src/repository/mock.rs @@ -133,6 +133,17 @@ impl OpenRepositoryLike for MockOpenRepository { .map(|inner| inner.push(repo_details, branch_name, to_commit, force)) .unwrap() } + + fn commit_log( + &self, + branch_name: git_next_config::BranchName, + find_commits: Vec, + ) -> Vec { + 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 { @@ -155,4 +166,12 @@ impl OpenRepositoryLike for InnerMockOpenRepository { ) -> std::prelude::v1::Result<(), crate::push::Error> { todo!() } + + fn commit_log( + &self, + branch_name: git_next_config::BranchName, + find_commits: Vec, + ) -> Vec { + todo!() + } } diff --git a/crates/git/src/repository/open/mod.rs b/crates/git/src/repository/open/mod.rs index 2a86c43..d4ffa4d 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: Vec, + ) -> 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 b03f4b8..d5752a3 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,62 @@ 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: Vec, + ) -> Result, git::commit::log::Error> { + let repo = self.0.lock().map_err(|_| git::commit::log::Error::Lock)?; + 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()), + ) } }