allow pull that creates conflicts now

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<CommitId> {
+) -> Result<Option<CommitId>> {
     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);

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()));

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 =