Handle one more corner case in push (#994)

Change: push-corner-case

Author: christian-schilling

src/history.rs

@@ -465,12 +465,12 @@ pub fn unapply_filter(
             }
 
             // 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.
+            // where the parents have differences outside of the filter.
             parent_count => {
                 let mut tid = git2::Oid::zero();
                 for i in 0..parent_count {
+                    // If one of the parents is a descendant of the target branch and the other is
+                    // not, pick the tree of the one that is a descendant.
                     if (original_parents_refs[i].id() == original_target)
                         || transaction
                             .repo()
@@ -481,19 +481,36 @@ pub fn unapply_filter(
                     }
                 }
 
-                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.
+                if tid == git2::Oid::zero() && parent_count == 2 {
+                    // If we could not select one of the parents, try to merge them.
+
+                    if let Ok(mut merged_index) = transaction.repo().merge_commits(
+                        original_parents_refs[0],
+                        original_parents_refs[1],
+                        None,
+                    ) {
+                        // If we can auto merge without conflicts, take the result.
+                        if !merged_index.has_conflicts() {
+                            tid = merged_index.write_tree_to(transaction.repo())?;
+                        }
+                    }
+                }
+
+                if tid == git2::Oid::zero() {
+                    // We give up. If we see this message again we need to investigate once
+                    // more and maybe consider allowing a manual override as last resort.
                     tracing::warn!("rejecting merge");
                     let msg = format!(
-                        "rejecting merge with {} parents:\n{:?}",
+                        "rejecting merge with {} parents:\n{:?}\n1) {:?}\n2) {:?}",
                         parent_count,
-                        module_commit.summary().unwrap_or_default()
+                        module_commit.summary().unwrap_or_default(),
+                        original_parents_refs[0].summary().unwrap_or_default(),
+                        original_parents_refs[1].summary().unwrap_or_default(),
                     );
                     return Err(josh_error(&msg));
                 }
+
+                transaction.repo().find_tree(tid)?
             }
         };
 

tests/filter/subtree_prefix.t

@@ -106,3 +106,104 @@ And then re-extract, which should re-construct the same subtree.
   [5] :/subtree
   [5] :rev(c036f944faafb865e0585e4fa5e005afa0aeea3f:prefix=subtree)
   $ test $(git rev-parse subtree) = $(git rev-parse subtree2)
+
+Simulate a feature branch on the main repo that crosses subtree changes
+  $ git checkout master 2>/dev/null
+  $ git checkout -b feature1 2>/dev/null
+  $ git reset --hard $SUBTREE_TIP >/dev/null
+  $ echo work > feature1
+  $ git add feature1 >/dev/null
+  $ git commit -m feature1 >/dev/null
+  $ git checkout master 2>/dev/null
+  $ git merge feature1 --no-ff >/dev/null
+
+On the subtree, simulate some independent work, and then a sync, then some more work.
+  $ git checkout subtree 2>/dev/null
+  $ echo work > subfeature1
+  $ git add subfeature1 >/dev/null
+  $ git commit -m subfeature1 >/dev/null
+  $ josh-filter -s ":rev($SUBTREE_TIP:prefix=subtree):/subtree" refs/heads/master --update refs/heads/subtree-sync >/dev/null
+  $ git merge subtree-sync --no-ff >/dev/null
+  $ echo work > subfeature2
+  $ git add subfeature2 >/dev/null
+  $ git commit -m subfeature2 >/dev/null
+
+And another main tree feature off of SUBTREE_TIP
+  $ git checkout -b feature2 2>/dev/null
+  $ git reset --hard $SUBTREE_TIP >/dev/null
+  $ echo work > feature2
+  $ git add feature2 >/dev/null
+  $ git commit -m feature2 >/dev/null
+  $ git checkout master 2>/dev/null
+  $ git merge feature2 --no-ff >/dev/null
+
+And finally, sync first from main to sub and then back.
+  $ git checkout subtree 2>/dev/null
+  $ josh-filter -s ":rev($SUBTREE_TIP:prefix=subtree):/subtree" refs/heads/master --update refs/heads/subtree-sync >/dev/null
+  $ git merge subtree-sync --no-ff >/dev/null
+
+  $ git log --graph --pretty=%s refs/heads/master
+  *   Merge branch 'feature2'
+  |\  
+  | * feature2
+  * |   Merge branch 'feature1'
+  |\ \  
+  | * | feature1
+  | |/  
+  * | add even more content
+  * | subtree edit from main repo
+  * | subtree merge
+  |\| 
+  | * add file2 (in subtree)
+  * add file1
+  $ git log --graph --pretty=%s refs/heads/subtree
+  *   Merge branch 'subtree-sync' into subtree
+  |\  
+  | *   Merge branch 'feature2'
+  | |\  
+  | | * feature2
+  * | | subfeature2
+  * | | Merge branch 'subtree-sync' into subtree
+  |\| | 
+  | * |   Merge branch 'feature1'
+  | |\ \  
+  | | * | feature1
+  | | |/  
+  * | / subfeature1
+  |/ /  
+  * | add even more content
+  * | subtree edit from main repo
+  |/  
+  * add file2 (in subtree)
+  $ josh-filter -s ":rev($SUBTREE_TIP:prefix=subtree):/subtree" refs/heads/master --update refs/heads/subtree --reverse
+  [1] :prefix=subtree
+  [9] :/subtree
+  [9] :rev(c036f944faafb865e0585e4fa5e005afa0aeea3f:prefix=subtree)
+
+  $ git log --graph --pretty=%H:%s refs/heads/master
+  *   6ac0ba56575859cfaacd5818084333e532ffc442:Merge branch 'subtree-sync' into subtree
+  |\  
+  | *   38a6d753c8b17b4c6721050befbccff012dfde85:Merge branch 'feature2'
+  | |\  
+  | | * 221f5ceab31209c3d3b16d5b2485ea54c465eca6:feature2
+  * | | 75e90f7f1b54cc343f2f75dcdee33650654a52a6:subfeature2
+  * | | 3fa497039e5b384cb44b704e6e96f52e0ae599c9:Merge branch 'subtree-sync' into subtree
+  |\| | 
+  | * |   2739fb8f0b3f6d5a264fb89ea20674fe34790321:Merge branch 'feature1'
+  | |\ \  
+  | | * | dbfaf5dd32fc39ce3c0ebe61864406bb7e2ad113:feature1
+  | | |/  
+  * | / 59b5c1623da3f89229c6dd36f8baf2e5868d0288:subfeature1
+  |/ /  
+  * | 103bfec17c47adbe70a95fca90caefb989b6cda6:add even more content
+  * | 41130c5d66736545562212f820cdbfbb3d3779c4:subtree edit from main repo
+  * | 0642c36d6b53f7e829531aed848e3ceff0762c64:subtree merge
+  |\| 
+  | * c036f944faafb865e0585e4fa5e005afa0aeea3f:add file2 (in subtree)
+  * 0b4cf6c9efbbda1eada39fa9c1d21d2525b027bb:add file1
+
+  $ git ls-tree --name-only -r 3fa497039e5b384cb44b704e6e96f52e0ae599c9
+  feature1
+  file1
+  subtree/file2
+  subtree/subfeature1