diff --git a/src/history.rs b/src/history.rs index 299d05bb3..b42cd791e 100644 --- a/src/history.rs +++ b/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)? } }; diff --git a/tests/filter/subtree_prefix.t b/tests/filter/subtree_prefix.t index 28510f24d..313322df8 100644 --- a/tests/filter/subtree_prefix.t +++ b/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 :at_commit=$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 :at_commit=$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 :at_commit=$SUBTREE_TIP[:prefix=subtree]:/subtree refs/heads/master --update refs/heads/subtree --reverse + [1] :prefix=subtree + [9] :/subtree + [9] :at_commit=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