From bfa240115cb497ec2ad9cacb40f0fc21481fb0ae Mon Sep 17 00:00:00 2001 From: Stephan Dilly Date: Thu, 11 Mar 2021 11:49:04 +0100 Subject: [PATCH] support rebase merge (conflict free only) (#567) --- CHANGELOG.md | 1 + asyncgit/src/sync/branch/merge_rebase.rs | 226 +++++++++++++++++++++++ asyncgit/src/sync/branch/mod.rs | 15 ++ asyncgit/src/sync/mod.rs | 7 +- src/app.rs | 4 +- src/components/pull.rs | 32 +++- src/components/reset.rs | 6 +- src/queue.rs | 2 +- src/strings.rs | 18 +- 9 files changed, 289 insertions(+), 22 deletions(-) create mode 100644 asyncgit/src/sync/branch/merge_rebase.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 21045147..55dbb9ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `[s]` key repurposed to trigger line based (un)stage ### Added +- support pull via rebase (using config `pull.rebase`) ([#566](https://github.com/extrawurst/gitui/issues/566)) - support stage/unstage selected lines ([#59](https://github.com/extrawurst/gitui/issues/59)) - support discarding selected lines ([#59](https://github.com/extrawurst/gitui/issues/59)) - support for pushing tags ([#568](https://github.com/extrawurst/gitui/issues/568)) diff --git a/asyncgit/src/sync/branch/merge_rebase.rs b/asyncgit/src/sync/branch/merge_rebase.rs new file mode 100644 index 00000000..0b1c56f7 --- /dev/null +++ b/asyncgit/src/sync/branch/merge_rebase.rs @@ -0,0 +1,226 @@ +//! merging from upstream (rebase) + +use crate::{ + error::{Error, Result}, + sync::utils, +}; +use git2::BranchType; +use scopetime::scope_time; + +/// trys merging current branch with its upstrema using rebase +pub fn merge_upstream_rebase( + repo_path: &str, + branch_name: &str, +) -> Result<()> { + scope_time!("merge_upstream_rebase"); + + let repo = utils::repo(repo_path)?; + let branch = repo.find_branch(branch_name, BranchType::Local)?; + let upstream = branch.upstream()?; + let upstream_commit = upstream.get().peel_to_commit()?; + let annotated_upstream = + repo.find_annotated_commit(upstream_commit.id())?; + + let branch_commit = branch.get().peel_to_commit()?; + let annotated_branch = + repo.find_annotated_commit(branch_commit.id())?; + + let rebase = repo.rebase( + Some(&annotated_branch), + Some(&annotated_upstream), + None, + None, + )?; + + let signature = + crate::sync::commit::signature_allow_undefined_name(&repo)?; + + for e in rebase { + let _op = e?; + // dbg!(op.id()); + // dbg!(op.kind()); + } + + let mut rebase = repo.open_rebase(None)?; + + if repo.index()?.has_conflicts() { + rebase.abort()?; + + Err(Error::Generic(String::from("conflicts while merging"))) + } else { + rebase.commit(None, &signature, None)?; + + rebase.finish(Some(&signature))?; + + Ok(()) + } +} + +#[cfg(test)] +mod test { + use super::*; + use crate::sync::{ + branch_compare_upstream, get_commits_info, + remotes::{fetch_origin, push::push}, + tests::{ + debug_cmd_print, get_commit_ids, repo_clone, + repo_init_bare, write_commit_file, + }, + RepoState, + }; + use git2::Repository; + + fn get_commit_msgs(r: &Repository) -> Vec { + let commits = get_commit_ids(r, 10); + get_commits_info( + r.workdir().unwrap().to_str().unwrap(), + &commits, + 10, + ) + .unwrap() + .into_iter() + .map(|c| c.message) + .collect() + } + + #[test] + fn test_merge_normal() { + let (r1_dir, _repo) = repo_init_bare().unwrap(); + + let (clone1_dir, clone1) = + repo_clone(r1_dir.path().to_str().unwrap()).unwrap(); + + let clone1_dir = clone1_dir.path().to_str().unwrap(); + + // clone1 + + let _commit1 = + write_commit_file(&clone1, "test.txt", "test", "commit1"); + + push(clone1_dir, "origin", "master", false, None, None) + .unwrap(); + + // clone2 + + let (clone2_dir, clone2) = + repo_clone(r1_dir.path().to_str().unwrap()).unwrap(); + + let clone2_dir = clone2_dir.path().to_str().unwrap(); + + let _commit2 = write_commit_file( + &clone2, + "test2.txt", + "test", + "commit2", + ); + + push(clone2_dir, "origin", "master", false, None, None) + .unwrap(); + + // clone1 + + let _commit3 = write_commit_file( + &clone1, + "test3.txt", + "test", + "commit3", + ); + + //lets fetch from origin + let bytes = + fetch_origin(clone1_dir, "master", None, None).unwrap(); + assert!(bytes > 0); + + //we should be one commit behind + assert_eq!( + branch_compare_upstream(clone1_dir, "master") + .unwrap() + .behind, + 1 + ); + + // debug_cmd_print(clone1_dir, "git log"); + + merge_upstream_rebase(clone1_dir, "master").unwrap(); + + debug_cmd_print(clone1_dir, "git log"); + + let state = crate::sync::repo_state(clone1_dir).unwrap(); + + assert_eq!(state, RepoState::Clean); + + let commits = get_commit_msgs(&clone1); + assert_eq!( + commits, + vec![ + String::from("commit3"), + String::from("commit2"), + String::from("commit1") + ] + ); + } + + #[test] + fn test_merge_conflict() { + let (r1_dir, _repo) = repo_init_bare().unwrap(); + + let (clone1_dir, clone1) = + repo_clone(r1_dir.path().to_str().unwrap()).unwrap(); + + let clone1_dir = clone1_dir.path().to_str().unwrap(); + + // clone1 + + let _commit1 = + write_commit_file(&clone1, "test.txt", "test", "commit1"); + + push(clone1_dir, "origin", "master", false, None, None) + .unwrap(); + + // clone2 + + let (clone2_dir, clone2) = + repo_clone(r1_dir.path().to_str().unwrap()).unwrap(); + + let clone2_dir = clone2_dir.path().to_str().unwrap(); + + let _commit2 = write_commit_file( + &clone2, + "test2.txt", + "test", + "commit2", + ); + + push(clone2_dir, "origin", "master", false, None, None) + .unwrap(); + + // clone1 + + let _commit3 = + write_commit_file(&clone1, "test2.txt", "foo", "commit3"); + + let bytes = + fetch_origin(clone1_dir, "master", None, None).unwrap(); + assert!(bytes > 0); + + assert_eq!( + branch_compare_upstream(clone1_dir, "master") + .unwrap() + .behind, + 1 + ); + + let res = merge_upstream_rebase(clone1_dir, "master"); + assert!(res.is_err()); + + let state = crate::sync::repo_state(clone1_dir).unwrap(); + + assert_eq!(state, RepoState::Clean); + + let commits = get_commit_msgs(&clone1); + assert_eq!( + commits, + vec![String::from("commit3"), String::from("commit1")] + ); + } +} diff --git a/asyncgit/src/sync/branch/mod.rs b/asyncgit/src/sync/branch/mod.rs index ac7d81ed..73b8f411 100644 --- a/asyncgit/src/sync/branch/mod.rs +++ b/asyncgit/src/sync/branch/mod.rs @@ -2,6 +2,7 @@ pub mod merge_commit; pub mod merge_ff; +pub mod merge_rebase; pub mod rename; use super::{ @@ -108,6 +109,20 @@ pub(crate) fn branch_set_upstream( Ok(()) } +/// returns whether the pull merge strategy is set to rebase +pub fn config_is_pull_rebase(repo_path: &str) -> Result { + let repo = utils::repo(repo_path)?; + let config = repo.config()?; + + if let Ok(rebase) = config.get_entry("pull.rebase") { + let value = + rebase.value().map(String::from).unwrap_or_default(); + return Ok(value == "true"); + }; + + Ok(false) +} + /// pub fn branch_compare_upstream( repo_path: &str, diff --git a/asyncgit/src/sync/mod.rs b/asyncgit/src/sync/mod.rs index 519c87bb..71977edd 100644 --- a/asyncgit/src/sync/mod.rs +++ b/asyncgit/src/sync/mod.rs @@ -25,11 +25,12 @@ mod tags; pub mod utils; pub use branch::{ - branch_compare_upstream, checkout_branch, create_branch, - delete_branch, get_branches_info, + branch_compare_upstream, checkout_branch, config_is_pull_rebase, + create_branch, delete_branch, get_branches_info, merge_commit::merge_upstream_commit, merge_ff::branch_merge_upstream_fastforward, - rename::rename_branch, BranchCompare, BranchInfo, + merge_rebase::merge_upstream_rebase, rename::rename_branch, + BranchCompare, BranchInfo, }; pub use commit::{amend, commit, tag}; pub use commit_details::{ diff --git a/src/app.rs b/src/app.rs index 9a18608b..390f2f72 100644 --- a/src/app.rs +++ b/src/app.rs @@ -524,8 +524,8 @@ impl App { .queue .borrow_mut() .push_back(InternalEvent::Push(branch, force)), - Action::PullMerge(_) => { - self.pull_popup.try_conflict_free_merge(); + Action::PullMerge { rebase, .. } => { + self.pull_popup.try_conflict_free_merge(rebase); flags.insert(NeedsUpdate::ALL); } }, diff --git a/src/components/pull.rs b/src/components/pull.rs index 08764d05..c68fed9d 100644 --- a/src/components/pull.rs +++ b/src/components/pull.rs @@ -151,12 +151,12 @@ impl PullComponent { let branch_compare = sync::branch_compare_upstream(CWD, &self.branch)?; if branch_compare.behind > 0 { - let merge_res = sync::branch_merge_upstream_fastforward( + let ff_res = sync::branch_merge_upstream_fastforward( CWD, &self.branch, ); - if let Err(err) = merge_res { - log::trace!("ff merge failed: {}", err); + if let Err(err) = ff_res { + log::trace!("ff failed: {}", err); self.confirm_merge(branch_compare.behind); } } @@ -166,17 +166,29 @@ impl PullComponent { Ok(()) } - pub fn try_conflict_free_merge(&self) { - try_or_popup!( - self, - "merge failed:", - sync::merge_upstream_commit(CWD, &self.branch) - ); + pub fn try_conflict_free_merge(&self, rebase: bool) { + if rebase { + try_or_popup!( + self, + "rebase failed:", + sync::merge_upstream_rebase(CWD, &self.branch) + ); + } else { + try_or_popup!( + self, + "merge failed:", + sync::merge_upstream_commit(CWD, &self.branch) + ); + } } fn confirm_merge(&mut self, incoming: usize) { self.queue.borrow_mut().push_back( - InternalEvent::ConfirmAction(Action::PullMerge(incoming)), + InternalEvent::ConfirmAction(Action::PullMerge { + incoming, + rebase: sync::config_is_pull_rebase(CWD) + .unwrap_or_default(), + }), ); self.hide(); } diff --git a/src/components/reset.rs b/src/components/reset.rs index 6c969e18..0db0fa30 100644 --- a/src/components/reset.rs +++ b/src/components/reset.rs @@ -173,9 +173,9 @@ impl ResetComponent { branch.rsplit('/').next().expect("There was no / in the head reference which is impossible in git"), ), ), - Action::PullMerge(incoming) => ( - strings::confirm_title_merge(&self.key_config), - strings::confirm_msg_merge(&self.key_config,*incoming), + Action::PullMerge{incoming,rebase} => ( + strings::confirm_title_merge(&self.key_config,*rebase), + strings::confirm_msg_merge(&self.key_config,*incoming,*rebase), ), }; } diff --git a/src/queue.rs b/src/queue.rs index 21da03fc..9df50d7b 100644 --- a/src/queue.rs +++ b/src/queue.rs @@ -31,7 +31,7 @@ pub enum Action { StashDrop(CommitId), DeleteBranch(String), ForcePush(String, bool), - PullMerge(usize), + PullMerge { incoming: usize, rebase: bool }, } /// diff --git a/src/strings.rs b/src/strings.rs index f7a615b6..260990a5 100644 --- a/src/strings.rs +++ b/src/strings.rs @@ -89,14 +89,26 @@ pub fn confirm_title_stashdrop( ) -> String { "Drop".to_string() } -pub fn confirm_title_merge(_key_config: &SharedKeyConfig) -> String { - "Merge".to_string() +pub fn confirm_title_merge( + _key_config: &SharedKeyConfig, + rebase: bool, +) -> String { + if rebase { + "Merge (via rebase)".to_string() + } else { + "Merge (via commit)".to_string() + } } pub fn confirm_msg_merge( _key_config: &SharedKeyConfig, incoming: usize, + rebase: bool, ) -> String { - format!("confirm merge of {} incoming commits? ", incoming) + if rebase { + format!("Rebase onto {} incoming commits?", incoming) + } else { + format!("Merge of {} incoming commits?", incoming) + } } pub fn confirm_msg_reset(_key_config: &SharedKeyConfig) -> String { "confirm file reset?".to_string()