Enable pushing partial merge commits

While merges of branches that where made in a filtered repo
where supported for a long time. This allows merges of branches
that where created on different versions of the canonical history.
The ambiguity is resolved by picking the parent that is a descendant
of the target branch tip.

Author: christian-schilling

josh-proxy/src/lib.rs

@@ -155,11 +155,8 @@ pub fn process_repo_update(repo_update: RepoUpdate) -> josh::JoshResult<String>
                 josh::UnapplyResult::BranchDoesNotExist => {
                     return Err(josh::josh_error("branch does not exist on remote"));
                 }
-                josh::UnapplyResult::RejectMerge(parent_count) => {
-                    return Err(josh::josh_error(&format!(
-                        "rejecting merge with {} parents",
-                        parent_count
-                    )));
+                josh::UnapplyResult::RejectMerge(msg) => {
+                    return Err(josh::josh_error(&msg));
                 }
                 josh::UnapplyResult::RejectAmend(msg) => {
                     return Err(josh::josh_error(&format!(

src/bin/josh-filter.rs

@@ -313,6 +313,10 @@ fn run_filter(args: Vec<String>) -> josh::JoshResult<i32> {
             josh::UnapplyResult::Done(rewritten) => {
                 repo.reference(&src, rewritten, true, "unapply_filter")?;
             }
+            josh::UnapplyResult::RejectMerge(msg) => {
+                println!("{}", msg);
+                return Ok(1);
+            }
             _ => {
                 return Ok(1);
             }

src/history.rs

@@ -281,7 +281,7 @@ pub fn unapply_filter(
 
         let commit_message = module_commit.summary().unwrap_or("NO COMMIT MESSAGE");
 
-        let new_trees: JoshResult<std::collections::HashSet<_>> = {
+        let new_trees: JoshResult<Vec<_>> = {
             let s = tracing::span!(
                 tracing::Level::TRACE,
                 "unapply filter",
@@ -311,27 +311,57 @@ pub fn unapply_filter(
             }
         };
 
-        let new_tree = match new_trees.len() {
-            1 => transaction
-                .repo()
-                .find_tree(*new_trees.iter().next().ok_or(josh_error("iter.next"))?)?,
+        let mut dedup = new_trees.clone();
+        dedup.sort();
+        dedup.dedup();
+
+        let new_tree = match dedup.len() {
+            // The normal case: Either there was only one parent or all of them where the same
+            // outside of the current filter in which case they collapse into one tree and that
+            // is the one we pick
+            1 => transaction.repo().find_tree(new_trees[0])?,
+
+            // 0 means the history is unrelated. Pushing it will fail if we are not
+            // dealing with either a force push or a push with the "merge" option set.
             0 => {
                 tracing::debug!("unrelated history");
-                // 0 means the history is unrelated. Pushing it will fail if we are not
-                // dealing with either a force push or a push with the "merge" option set.
                 filter::unapply(
                     transaction,
                     filterobj,
                     tree,
                     filter::tree::empty(transaction.repo()),
                 )?
             }
+
+            // This will typically be parent_count == 2 and mean we are dealing with a merge
+            // where the parents have differences outside of the filter. This is only possible
+            // if one of the parents is a descendant of the target branch and the other is not.
+            // In that case pick the tree of the one that is a descendant.
             parent_count => {
-                // This is a merge commit where the parents in the upstream repo
-                // have differences outside of the current filter.
-                // It is unclear what base tree to pick in this case.
-                tracing::warn!("rejecting merge");
-                return Ok(UnapplyResult::RejectMerge(parent_count));
+                let mut tid = git2::Oid::zero();
+                for i in 0..parent_count {
+                    if transaction
+                        .repo()
+                        .graph_descendant_of(original_parents_refs[i].id(), original_target)?
+                    {
+                        tid = new_trees[i];
+                        break;
+                    }
+                }
+
+                if tid != git2::Oid::zero() {
+                    transaction.repo().find_tree(tid)?
+                } else {
+                    // This used to be our only fallback for the parent_count > 1 case.
+                    // It should never happen anymore.
+                    tracing::warn!("rejecting merge");
+                    let msg = format!(
+                        "rejecting merge with {} parents:\n{:?}",
+                        parent_count,
+                        module_commit.summary().unwrap_or_default()
+                    );
+                    return Ok(UnapplyResult::RejectMerge(msg));
+                }
             }
         };
 

src/lib.rs

@@ -46,7 +46,7 @@ pub mod shell;
 #[derive(Clone)]
 pub enum UnapplyResult {
     Done(git2::Oid),
-    RejectMerge(usize),
+    RejectMerge(String),
     RejectAmend(String),
     BranchDoesNotExist,
 }

tests/filter/ambigous_merge.t

@@ -0,0 +1,155 @@
+  $ export TESTTMP=${PWD}
+
+  $ cd ${TESTTMP}
+  $ git init real_repo 1> /dev/null
+  $ cd real_repo
+
+  $ mkdir sub2
+  $ echo contents1 > sub2/file2
+  $ git add sub2
+  $ git commit -m "add sub2/file2" 1> /dev/null
+
+  $ mkdir sub1
+  $ echo contents1 > sub1/file1
+  $ git add sub1
+  $ git commit -m "add sub1/file1" 1> /dev/null
+  $ git branch branch1
+
+  $ echo contents1 > sub1/file2
+  $ echo contents2 > sub2/fileX
+  $ git add .
+  $ git commit -m "add sub1/file2 sub2/fileX" 1> /dev/null
+
+  $ git log --graph --oneline --decorate
+  * 0d0a43d (HEAD -> master) add sub1/file2 sub2/fileX
+  * 8a87424 (branch1) add sub1/file1
+  * 4d74643 add sub2/file2
+
+  $ git checkout branch1
+  Switched to branch 'branch1'
+  $ echo contents3 > sub2/fileY
+  $ echo contents6 > sub1/fileY
+  $ git add .
+  $ git commit -m "add sub_/fileY" 1> /dev/null
+  $ git log --graph --oneline --decorate
+  * 96cc30d (HEAD -> branch1) add sub_/fileY
+  * 8a87424 add sub1/file1
+  * 4d74643 add sub2/file2
+
+  $ josh-filter -s ::sub1/ branch1 --update refs/heads/hidden_branch1
+  [2] :prefix=sub1
+  [3] :/sub1
+  $ git checkout hidden_branch1
+  Switched to branch 'hidden_branch1'
+  $ git log --graph --oneline --decorate
+  * 81a8353 (HEAD -> hidden_branch1) add sub_/fileY
+  * 7671c2a add sub1/file1
+  $ echo contents3 > sub1/file3
+  $ git add .
+  $ git commit -m "add sub1/file3" 1> /dev/null
+
+  $ git checkout master
+  Switched to branch 'master'
+
+  $ josh-filter -s ::sub1/ master --update refs/heads/hidden_master
+  [3] :prefix=sub1
+  [4] :/sub1
+  $ git checkout hidden_master
+  Switched to branch 'hidden_master'
+  $ git log --graph --oneline --decorate
+  * 586737e (HEAD -> hidden_master) add sub1/file2 sub2/fileX
+  * 7671c2a add sub1/file1
+  $ echo contents4 > sub1/file4
+  $ git add sub1/file4
+  $ git commit -m "add sub1/file4" 1> /dev/null
+
+  $ git log hidden_master --graph --oneline --decorate
+  * 6f816ed (HEAD -> hidden_master) add sub1/file4
+  * 586737e add sub1/file2 sub2/fileX
+  * 7671c2a add sub1/file1
+  $ git log hidden_branch1 --graph --oneline --decorate
+  * 24a7e40 (hidden_branch1) add sub1/file3
+  * 81a8353 add sub_/fileY
+  * 7671c2a add sub1/file1
+
+  $ git merge hidden_branch1 --no-ff
+  Merge made by the 'recursive' strategy.
+   sub1/file3 | 1 +
+   sub1/fileY | 1 +
+   2 files changed, 2 insertions(+)
+   create mode 100644 sub1/file3
+   create mode 100644 sub1/fileY
+  $ git log --graph --oneline
+  *   2fcb6a4 Merge branch 'hidden_branch1' into hidden_master
+  |\  
+  | * 24a7e40 add sub1/file3
+  | * 81a8353 add sub_/fileY
+  * | 6f816ed add sub1/file4
+  * | 586737e add sub1/file2 sub2/fileX
+  |/  
+  * 7671c2a add sub1/file1
+
+  $ josh-filter -s ::sub1/ --reverse master --update refs/heads/hidden_master
+  [3] :prefix=sub1
+  [4] :/sub1
+
+  $ git checkout master
+  Switched to branch 'master'
+  $ git status
+  On branch master
+  nothing to commit, working tree clean
+
+  $ tree
+  .
+  |-- sub1
+  |   |-- file1
+  |   |-- file2
+  |   |-- file3
+  |   |-- file4
+  |   `-- fileY
+  `-- sub2
+      |-- file2
+      `-- fileX
+  
+  2 directories, 7 files
+
+  $ git log --graph --oneline --decorate master
+  *   8cbae19 (HEAD -> master) Merge branch 'hidden_branch1' into hidden_master
+  |\  
+  | * 7d3be00 add sub1/file3
+  | * 5e05ab3 add sub_/fileY
+  * | 4428b57 add sub1/file4
+  * | 0d0a43d add sub1/file2 sub2/fileX
+  |/  
+  * 8a87424 add sub1/file1
+  * 4d74643 add sub2/file2
+  $ git log --graph --oneline --decorate branch1
+  * 96cc30d (branch1) add sub_/fileY
+  * 8a87424 add sub1/file1
+  * 4d74643 add sub2/file2
+
+  $ git diff 4428b57..8cbae19 --stat
+   sub1/file3 | 1 +
+   sub1/fileY | 1 +
+   2 files changed, 2 insertions(+)
+  $ git diff 7d3be00..8cbae19 --stat
+   sub1/file2 | 1 +
+   sub1/file4 | 1 +
+   sub2/fileX | 1 +
+   3 files changed, 3 insertions(+)
+  $ git diff 5e05ab3..7d3be00 --stat
+   sub1/file3 | 1 +
+   1 file changed, 1 insertion(+)
+  $ git diff 5e05ab3 --stat
+   sub1/file2 | 1 +
+   sub1/file3 | 1 +
+   sub1/file4 | 1 +
+   sub2/fileX | 1 +
+   4 files changed, 4 insertions(+)
+  $ git diff 4428b57..7d3be00 --stat
+   sub1/file2 | 1 -
+   sub1/file3 | 1 +
+   sub1/file4 | 1 -
+   sub1/fileY | 1 +
+   sub2/fileX | 1 -
+   5 files changed, 2 insertions(+), 3 deletions(-)