From 126d5d3ef5af37904f1d51d5a08f6c1b95403ea1 Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Fri, 30 Aug 2024 07:49:43 +0100 Subject: [PATCH] fix: create git graph log to after doing a fetch --- crates/cli/src/repo/handlers/validate_repo.rs | 40 +++++------ crates/cli/src/repo/mod.rs | 10 +-- crates/core/src/git/validation/positions.rs | 57 +++++++++------ crates/core/src/git/validation/tests.rs | 72 ++++--------------- 4 files changed, 71 insertions(+), 108 deletions(-) diff --git a/crates/cli/src/repo/handlers/validate_repo.rs b/crates/cli/src/repo/handlers/validate_repo.rs index 2e77d3f..a31572d 100644 --- a/crates/cli/src/repo/handlers/validate_repo.rs +++ b/crates/cli/src/repo/handlers/validate_repo.rs @@ -1,7 +1,7 @@ // use actix::prelude::*; -use tracing::{debug, info, instrument, Instrument as _}; +use tracing::{debug, instrument, Instrument as _}; use crate::{ repo::{ @@ -13,8 +13,8 @@ use crate::{ }; use git_next_core::git::{ - self, validation::positions::{validate, Error, Positions}, + UserNotification, }; impl Handler for RepoActor { @@ -63,26 +63,19 @@ impl Handler for RepoActor { }; logger(self.log.as_ref(), "have repo config"); - info!("collecting git graph log"); - let git_log = git::graph::log(&self.repo_details); - info!(?git_log, "collected git graph log"); - self.update_tui_log(git_log.clone()); - info!("sent to ui git graph log"); - - match validate( - &**open_repository, - &self.repo_details, - &repo_config, - git_log, - ) { - Ok(Positions { - main, - next, - dev, - dev_commit_history, - next_is_valid, - }) => { + match validate(&**open_repository, &self.repo_details, &repo_config) { + Ok(( + Positions { + main, + next, + dev, + dev_commit_history, + next_is_valid, + }, + git_log, + )) => { debug!(%main, %next, %dev, "positions"); + self.update_tui_log(git_log); if next_is_valid && next != main { do_send(&ctx.address(), CheckCIStatus::new(next), self.log.as_ref()); } else if next != dev { @@ -119,6 +112,11 @@ impl Handler for RepoActor { } Err(Error::UserIntervention(user_notification)) => { self.alert_tui(format!("[USER INTERVENTION: {user_notification}]")); + if let UserNotification::CICheckFailed { log, .. } + | UserNotification::DevNotBasedOnMain { log, .. } = &user_notification + { + self.update_tui_log(log.clone()); + } notify_user( self.notify_user_recipient.as_ref(), user_notification, diff --git a/crates/cli/src/repo/mod.rs b/crates/cli/src/repo/mod.rs index 7bcb502..32105a6 100644 --- a/crates/cli/src/repo/mod.rs +++ b/crates/cli/src/repo/mod.rs @@ -3,10 +3,7 @@ use actix::prelude::*; use crate::{ alerts::messages::NotifyUser, - server::{ - actor::messages::{RepoUpdate, ServerUpdate}, - ServerActor, - }, + server::{actor::messages::RepoUpdate, ServerActor}, }; use derive_more::Deref; use kxio::network::Network; @@ -114,6 +111,7 @@ impl RepoActor { } } + #[allow(unused_variables)] fn update_tui_log(&self, log: git::graph::Log) { #[cfg(feature = "tui")] { @@ -121,6 +119,7 @@ impl RepoActor { } } + #[allow(unused_variables)] fn alert_tui(&self, alert: impl Into) { #[cfg(feature = "tui")] { @@ -130,6 +129,7 @@ impl RepoActor { } } + #[allow(unused_variables)] fn update_tui(&self, repo_update: RepoUpdate) { #[cfg(feature = "tui")] { @@ -137,7 +137,7 @@ impl RepoActor { return; }; - let update = ServerUpdate::RepoUpdate { + let update = crate::server::actor::messages::ServerUpdate::RepoUpdate { forge_alias: self.repo_details.forge.forge_alias().clone(), repo_alias: self.repo_details.repo_alias.clone(), repo_update, diff --git a/crates/core/src/git/validation/positions.rs b/crates/core/src/git/validation/positions.rs index b264ab2..bdd953b 100644 --- a/crates/core/src/git/validation/positions.rs +++ b/crates/core/src/git/validation/positions.rs @@ -1,10 +1,9 @@ -use tracing::{debug, instrument}; - // use crate::{ git::{self, repository::open::OpenRepositoryLike, RepoDetails, UserNotification}, BranchName, RepoConfig, }; +use tracing::{debug, instrument}; pub type Result = core::result::Result; @@ -29,13 +28,14 @@ pub fn validate( open_repository: &dyn OpenRepositoryLike, repo_details: &git::RepoDetails, repo_config: &RepoConfig, - log: git::graph::Log, -) -> Result { +) -> Result<(Positions, git::graph::Log)> { let main_branch = repo_config.branches().main(); let next_branch = repo_config.branches().next(); let dev_branch = repo_config.branches().dev(); // Collect Commit Histories for `main`, `next` and `dev` branches open_repository.fetch()?; + let git_log = git::graph::log(repo_details); + let commit_histories = get_commit_histories(open_repository, repo_config)?; // branch tips let main = commit_histories @@ -64,7 +64,7 @@ pub fn validate( main_branch, dev_commit: dev, main_commit: main, - log, + log: git_log, }, )); } @@ -80,23 +80,38 @@ pub fn validate( &main, ) { tracing::info!("Main not on same commit as next, or it's parent - resetting next to main",); - return reset_next_to_main(open_repository, repo_details, &main, &next, &next_branch); + return Err(reset_next_to_main( + open_repository, + repo_details, + &main, + &next, + &next_branch, + )); } // verify that next is an ancestor of dev, else reset it back to main if dev not ahead of main if is_not_based_on(&commit_histories.dev, &next) && commit_histories.main.first() == commit_histories.dev.first() { tracing::info!("Next is not an ancestor of dev - resetting next to main"); - return reset_next_to_main(open_repository, repo_details, &main, &next, &next_branch); + return Err(reset_next_to_main( + open_repository, + repo_details, + &main, + &next, + &next_branch, + )); } let next_is_valid = is_based_on(&commit_histories.dev, &next); - Ok(git::validation::positions::Positions { - main, - next, - dev, - dev_commit_history: commit_histories.dev, - next_is_valid, - }) + Ok(( + git::validation::positions::Positions { + main, + next, + dev, + dev_commit_history: commit_histories.dev, + next_is_valid, + }, + git_log, + )) } #[allow(clippy::result_large_err)] @@ -106,22 +121,20 @@ fn reset_next_to_main( main: &git::Commit, next: &git::Commit, next_branch: &BranchName, -) -> Result { - git::push::reset( +) -> Error { + if let Err(err) = git::push::reset( open_repository, repo_details, next_branch, &main.clone().into(), &git::push::Force::From(next.clone().into()), - ) - .map_err(|err| { + ) { Error::NonRetryable(format!( "Failed to reset branch '{next_branch}' to commit '{next}': {err}" )) - })?; - Err(Error::Retryable(format!( - "Branch {next_branch} has been reset" - ))) + } else { + Error::Retryable(format!("Branch {next_branch} has been reset")) + } } fn is_not_based_on(commits: &[git::commit::Commit], needle: &git::Commit) -> bool { diff --git a/crates/core/src/git/validation/tests.rs b/crates/core/src/git/validation/tests.rs index 0b27a79..7b17545 100644 --- a/crates/core/src/git/validation/tests.rs +++ b/crates/core/src/git/validation/tests.rs @@ -75,12 +75,7 @@ mod positions { ); let repo_config = given::a_repo_config(); - let result = validate( - &*repository, - &repo_details, - &repo_config, - git::graph::Log::default(), - ); + let result = validate(&*repository, &repo_details, &repo_config); println!("{result:?}"); let_assert!(Err(err) = result, "validate"); @@ -120,12 +115,7 @@ mod positions { "open repo" ); - let result = validate( - &*open_repository, - &repo_details, - &repo_config, - git::graph::Log::default(), - ); + let result = validate(&*open_repository, &repo_details, &repo_config); println!("{result:?}"); assert!(matches!( @@ -164,12 +154,7 @@ mod positions { "open repo" ); - let result = validate( - &*open_repository, - &repo_details, - &repo_config, - git::graph::Log::default(), - ); + let result = validate(&*open_repository, &repo_details, &repo_config); println!("{result:?}"); assert!(matches!( @@ -208,12 +193,7 @@ mod positions { "open repo" ); - let result = validate( - &*open_repository, - &repo_details, - &repo_config, - git::graph::Log::default(), - ); + let result = validate(&*open_repository, &repo_details, &repo_config); println!("{result:?}"); assert!(matches!( @@ -260,12 +240,7 @@ mod positions { //when let_assert!( - Err(err) = validate( - &*open_repository, - &repo_details, - &repo_config, - git::graph::Log::default() - ), + Err(err) = validate(&*open_repository, &repo_details, &repo_config), "validate" ); @@ -350,12 +325,7 @@ mod positions { //when let_assert!( - Err(err) = validate( - &*open_repository, - &repo_details, - &repo_config, - git::graph::Log::default() - ), + Err(err) = validate(&*open_repository, &repo_details, &repo_config), "validate" ); @@ -424,12 +394,7 @@ mod positions { //when let_assert!( - Err(err) = validate( - &*open_repository, - &repo_details, - &repo_config, - git::graph::Log::default() - ), + Err(err) = validate(&*open_repository, &repo_details, &repo_config), "validate" ); @@ -516,12 +481,7 @@ mod positions { //when let_assert!( - Err(err) = validate( - &*open_repository, - &repo_details, - &repo_config, - git::graph::Log::default() - ), + Err(err) = validate(&*open_repository, &repo_details, &repo_config), "validate" ); @@ -607,12 +567,8 @@ mod positions { //when let_assert!( - Ok(positions) = validate( - &*open_repository, - &repo_details, - &repo_config, - git::graph::Log::default() - ), + Ok((positions, _git_log)) = + validate(&*open_repository, &repo_details, &repo_config), "validate" ); @@ -671,12 +627,8 @@ mod positions { //when let_assert!( - Ok(positions) = validate( - &*open_repository, - &repo_details, - &repo_config, - git::graph::Log::default() - ), + Ok((positions, _git_log)) = + validate(&*open_repository, &repo_details, &repo_config), "validate" );