diff --git a/asyncgit/src/sync/branch/merge_commit.rs b/asyncgit/src/sync/branch/merge_commit.rs index 368d3228..2084bd12 100644 --- a/asyncgit/src/sync/branch/merge_commit.rs +++ b/asyncgit/src/sync/branch/merge_commit.rs @@ -3,16 +3,18 @@ use super::BranchType; use crate::{ error::{Error, Result}, - sync::{utils, CommitId}, + sync::{merge_msg, utils, CommitId}, }; -use git2::{Commit, MergeOptions}; +use git2::Commit; use scopetime::scope_time; -/// merge upstream using a merge commit without conflicts. fails if not possible without conflicts +/// merge upstream using a merge commit if we did not create conflicts. +/// if we did not create conflicts we create a merge commit and return the commit id. +/// Otherwise we return `None` pub fn merge_upstream_commit( repo_path: &str, branch_name: &str, -) -> Result { +) -> Result> { scope_time!("merge_upstream_commit"); let repo = utils::repo(repo_path)?; @@ -22,10 +24,10 @@ pub fn merge_upstream_commit( let upstream_commit = upstream.get().peel_to_commit()?; - let annotated_upstream = - repo.find_annotated_commit(upstream_commit.id())?; + let annotated_upstream = repo + .reference_to_annotated_commit(&upstream.into_reference())?; - let (analysis, _) = + let (analysis, pref) = repo.merge_analysis(&[&annotated_upstream])?; if !analysis.is_normal() { @@ -34,42 +36,29 @@ pub fn merge_upstream_commit( )); } - //TODO: support merge on unborn + if analysis.is_fast_forward() && pref.is_fastforward_only() { + return Err(Error::Generic( + "ff merge would be possible".into(), + )); + } + + //TODO: support merge on unborn? if analysis.is_unborn() { return Err(Error::Generic("head is unborn".into())); } - let mut opt = MergeOptions::default(); - opt.fail_on_conflict(true); + repo.merge(&[&annotated_upstream], None, None)?; - repo.merge(&[&annotated_upstream], Some(&mut opt), None)?; + if !repo.index()?.has_conflicts() { + let msg = merge_msg(repo_path)?; - if repo.index()?.has_conflicts() { - return Err(Error::Generic("creates conflicts".into())); + let commit_id = + commit_merge_with_head(&repo, &[upstream_commit], &msg)?; + + return Ok(Some(commit_id)); } - let remote_url = { - let branch_refname = - branch.get().name().ok_or_else(|| { - Error::Generic(String::from( - "branch refname not found", - )) - })?; - let buf = repo.branch_upstream_remote(branch_refname)?; - let remote = - repo.find_remote(buf.as_str().ok_or_else(|| { - Error::Generic(String::from("remote name not found")) - })?)?; - remote.url().unwrap_or_default().to_string() - }; - - let commit_id = commit_merge_with_head( - &repo, - &[upstream_commit], - format!("Merge '{}' of {}", branch_name, remote_url).as_str(), - )?; - - Ok(commit_id) + Ok(None) } pub(crate) fn commit_merge_with_head( @@ -171,7 +160,9 @@ mod test { ); let merge_commit = - merge_upstream_commit(clone2_dir, "master").unwrap(); + merge_upstream_commit(clone2_dir, "master") + .unwrap() + .unwrap(); let state = crate::sync::repo_state(clone2_dir).unwrap(); assert_eq!(state, RepoState::Clean); @@ -190,15 +181,12 @@ mod test { .unwrap(); assert_eq!( details.message.unwrap().combine(), - format!( - "Merge 'master' of {}", - r1_dir.path().to_str().unwrap() - ) + String::from("Merge remote-tracking branch 'refs/remotes/origin/master'") ); } #[test] - fn test_merge_normal_conflict() { + fn test_merge_normal_non_ff() { let (r1_dir, _repo) = repo_init_bare().unwrap(); let (clone1_dir, clone1) = @@ -209,7 +197,12 @@ mod test { // clone1 - write_commit_file(&clone1, "test.bin", "test", "commit1"); + write_commit_file( + &clone1, + "test.bin", + "test\nfooo", + "commit1", + ); debug_cmd_print( clone2_dir.path().to_str().unwrap(), @@ -228,7 +221,12 @@ mod test { // clone2 - write_commit_file(&clone2, "test.bin", "foobar", "commit2"); + write_commit_file( + &clone2, + "test.bin", + "foobar\ntest", + "commit2", + ); let bytes = fetch( clone2_dir.path().to_str().unwrap(), @@ -242,18 +240,19 @@ mod test { let res = merge_upstream_commit( clone2_dir.path().to_str().unwrap(), "master", - ); + ) + .unwrap(); - //this should have failed cause it would create a conflict - assert!(res.is_err()); + //this should not have commited cause we left conflicts behind + assert_eq!(res, None); let state = crate::sync::repo_state( clone2_dir.path().to_str().unwrap(), ) .unwrap(); - //make sure we left the repo not in some merging state - assert_eq!(state, RepoState::Clean); + //validate the repo is in a merge state now + assert_eq!(state, RepoState::Merge); //check that we still only have the first commit let commits = get_commit_ids(&clone1, 10); diff --git a/asyncgit/src/sync/branch/merge_ff.rs b/asyncgit/src/sync/branch/merge_ff.rs index f5265a70..fb45fe67 100644 --- a/asyncgit/src/sync/branch/merge_ff.rs +++ b/asyncgit/src/sync/branch/merge_ff.rs @@ -25,7 +25,7 @@ pub fn branch_merge_upstream_fastforward( let annotated = repo.find_annotated_commit(upstream_commit.id())?; - let (analysis, _) = repo.merge_analysis(&[&annotated])?; + let (analysis, pref) = repo.merge_analysis(&[&annotated])?; if !analysis.is_fast_forward() { return Err(Error::Generic( @@ -33,6 +33,10 @@ pub fn branch_merge_upstream_fastforward( )); } + if pref.is_no_fast_forward() { + return Err(Error::Generic("fast forward not wanted".into())); + } + //TODO: support merge on unborn if analysis.is_unborn() { return Err(Error::Generic("head is unborn".into())); diff --git a/asyncgit/src/sync/merge.rs b/asyncgit/src/sync/merge.rs index e7f9609f..bb6e5d2f 100644 --- a/asyncgit/src/sync/merge.rs +++ b/asyncgit/src/sync/merge.rs @@ -7,7 +7,7 @@ use crate::{ reset_workdir, utils, CommitId, }, }; -use git2::{BranchType, Commit, MergeOptions}; +use git2::{BranchType, Commit, MergeOptions, Repository}; use scopetime::scope_time; /// @@ -48,6 +48,16 @@ pub fn merge_branch(repo_path: &str, branch: &str) -> Result<()> { let repo = utils::repo(repo_path)?; + merge_branch_repo(&repo, branch)?; + + Ok(()) +} + +/// +pub fn merge_branch_repo( + repo: &Repository, + branch: &str, +) -> Result<()> { let branch = repo.find_branch(branch, BranchType::Local)?; let annotated =