ResolveMerger: Ignore merge conflicts if asked so

The recursive merge strategy builds a virtual ancestor merging
recursively the common bases (when more than one) between the
want-to-merge commits. While building this virtual ancestor, content
conflicts are ignored, but current code doesn't do so when a file is
removed.

This was spotted in [1], for example. Merging two commits to build the
virtual ancestor bumped into a conflict (modified in one side, deleted
in the other) that stopped the process.

Follow the "spec" and in case of conflict leave the unmerged content in
the index and working trees.

[1] https://android-review.googlesource.com/c/kernel/common/+/1228962

Change-Id: Ife9c32ae3ac3a87d3660fa1242e07854b65169d5
Signed-off-by: Ivan Frade <ifrade@google.com>

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/MergerTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/MergerTest.java
index 032349d5f8a855c9df3eaab301a4509f94deffd0..7a244e1d8b486a037fdb317686b8c6fffd5284a8 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/MergerTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/MergerTest.java
@@ -1254,6 +1254,94 @@ public class MergerTest extends RepositoryTestCase {
                }
        }
 
+       /**
+        * Merging two commits with a conflict in the virtual ancestor.
+        *
+        * Content conflicts while merging the virtual ancestor must be ignored.
+        *
+        * In the following tree, while merging A and B, the recursive algorithm
+        * finds as base commits X and Y and tries to merge them: X deletes file "a"
+        * and Y modifies it.
+        *
+        * Note: we delete "a" in (master) and (second-branch) to make avoid manual
+        * merges. The situation is the same without those deletions and fixing
+        * manually the merge of (merge-both-sides) on both branches.
+        *
+        * <pre>
+        * A  (second-branch) Merge branch 'merge-both-sides' into second-branch
+        * |\
+        * o | Delete modified a
+        * | |
+        * | | B (master) Merge branch 'merge-both-sides' (into master)
+        * | |/|
+        * | X | (merge-both-sides) Delete original a
+        * | | |
+        * | | o Delete modified a
+        * | |/
+        * |/|
+        * Y | Modify a
+        * |/
+        * o Initial commit
+        * </pre>
+        *
+        * @param strategy
+        * @throws Exception
+        */
+       @Theory
+       public void checkMergeConflictInVirtualAncestor(
+                       MergeStrategy strategy) throws Exception {
+               if (!strategy.equals(MergeStrategy.RECURSIVE)) {
+                       return;
+               }
+
+               Git git = Git.wrap(db);
+
+               // master
+               writeTrashFile("a", "aaaaaaaa");
+               writeTrashFile("b", "bbbbbbbb");
+               git.add().addFilepattern("a").addFilepattern("b").call();
+               RevCommit first = git.commit().setMessage("Initial commit").call();
+
+               writeTrashFile("a", "aaaaaaaaaaaaaaa");
+               git.add().addFilepattern("a").call();
+               RevCommit commitY = git.commit().setMessage("Modify a").call();
+
+               git.rm().addFilepattern("a").call();
+               // Do more in this commits, so it is not identical to the deletion in
+               // second-branch
+               writeTrashFile("c", "cccccccc");
+               git.add().addFilepattern("c").call();
+               git.commit().setMessage("Delete modified a").call();
+
+               // merge-both-sides: starts before "a" is modified and deletes it
+               git.checkout().setCreateBranch(true).setStartPoint(first)
+                               .setName("merge-both-sides").call();
+               git.rm().addFilepattern("a").call();
+               RevCommit commitX = git.commit().setMessage("Delete original a").call();
+
+               // second branch
+               git.checkout().setCreateBranch(true).setStartPoint(commitY)
+                               .setName("second-branch").call();
+               git.rm().addFilepattern("a").call();
+               git.commit().setMessage("Delete modified a").call();
+
+               // Merge merge-both-sides into second-branch
+               MergeResult mergeResult = git.merge().include(commitX)
+                               .setStrategy(strategy)
+                               .call();
+               ObjectId commitB = mergeResult.getNewHead();
+
+               // Merge merge-both-sides into master
+               git.checkout().setName("master").call();
+               mergeResult = git.merge().include(commitX).setStrategy(strategy)
+                               .call();
+
+               // Now, merge commit A and B (i.e. "master" and "second-branch").
+               // None of them have the file "a", so there is no conflict, BUT while
+               // building the virtual ancestor it will find a conflict between Y and X
+               git.merge().include(commitB).call();
+       }
+
        private void writeSubmodule(String path, ObjectId commit)
                        throws IOException, ConfigInvalidException {
                addSubmoduleToIndex(path, commit);

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java b/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java
index 575e7bd2850a285611afa2d20434d20ee049586d..506d333120a9ee2fcfeb2088d240dfb96a476865 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java
@@ -789,27 +789,37 @@ public class ResolveMerger extends ThreeWayMerger {
                                MergeResult<RawText> result = contentMerge(base, ours, theirs,
                                                attributes);
 
-                               add(tw.getRawPath(), base, DirCacheEntry.STAGE_1, EPOCH, 0);
-                               add(tw.getRawPath(), ours, DirCacheEntry.STAGE_2, EPOCH, 0);
-                               DirCacheEntry e = add(tw.getRawPath(), theirs,
-                                               DirCacheEntry.STAGE_3, EPOCH, 0);
+                               if (ignoreConflicts) {
+                                       // In case a conflict is detected the working tree file is
+                                       // again filled with new content (containing conflict
+                                       // markers). But also stage 0 of the index is filled with
+                                       // that content.
+                                       result.setContainsConflicts(false);
+                                       updateIndex(base, ours, theirs, result, attributes);
+                               } else {
+                                       add(tw.getRawPath(), base, DirCacheEntry.STAGE_1, EPOCH, 0);
+                                       add(tw.getRawPath(), ours, DirCacheEntry.STAGE_2, EPOCH, 0);
+                                       DirCacheEntry e = add(tw.getRawPath(), theirs,
+                                                       DirCacheEntry.STAGE_3, EPOCH, 0);
 
-                               // OURS was deleted checkout THEIRS
-                               if (modeO == 0) {
-                                       // Check worktree before checking out THEIRS
-                                       if (isWorktreeDirty(work, ourDce))
-                                               return false;
-                                       if (nonTree(modeT)) {
-                                               if (e != null) {
-                                                       addToCheckout(tw.getPathString(), e, attributes);
+                                       // OURS was deleted checkout THEIRS
+                                       if (modeO == 0) {
+                                               // Check worktree before checking out THEIRS
+                                               if (isWorktreeDirty(work, ourDce)) {
+                                                       return false;
+                                               }
+                                               if (nonTree(modeT)) {
+                                                       if (e != null) {
+                                                               addToCheckout(tw.getPathString(), e, attributes);
+                                                       }
                                                }
                                        }
-                               }
 
-                               unmergedPaths.add(tw.getPathString());
+                                       unmergedPaths.add(tw.getPathString());
 
-                               // generate a MergeResult for the deleted file
-                               mergeResults.put(tw.getPathString(), result);
+                                       // generate a MergeResult for the deleted file
+                                       mergeResults.put(tw.getPathString(), result);
+                               }
                        }
                }
                return true;