Refactor ResolveMerger

1. Perform an explicit check for untracked files.
2. Extract 'dirty checks' into separate methods
3. Clean up comments.
4. Tests: also check contents of files not affected by merge.

Change-Id: Ieb089668834d0a395c9ab192c555538917dfdc47
Signed-off-by: Philipp Thun <philipp.thun@sap.com>

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/MergeCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/MergeCommandTest.java
index 0ef24bd8a5d4168b349319bba61c1afdeed7ea83..987e64731e4926166613ab9ebf7f0c3549943b1a 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/MergeCommandTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/MergeCommandTest.java
@@ -411,6 +411,7 @@ public class MergeCommandTest extends RepositoryTestCase {
                assertEquals("1\nb(side)\n3\n", read(new File(db.getWorkTree(), "b")));
                assertEquals("1\nc(main)\n3\n", read(new File(db.getWorkTree(),
                                "c/c/c")));
+               assertEquals("--- dirty ---", read(new File(db.getWorkTree(), "d")));
 
                assertEquals(null, result.getConflicts());
 
@@ -468,6 +469,7 @@ public class MergeCommandTest extends RepositoryTestCase {
                assertFalse(new File(db.getWorkTree(), "b").exists());
                assertEquals("1\nc(main)\n3\n",
                                read(new File(db.getWorkTree(), "c/c/c")));
+               assertEquals("1\nd\n3\n", read(new File(db.getWorkTree(), "d")));
 
                // Do the opposite, be on a branch where we have deleted a file and
                // merge in a old commit where this file was not deleted
@@ -482,6 +484,7 @@ public class MergeCommandTest extends RepositoryTestCase {
                assertFalse(new File(db.getWorkTree(), "b").exists());
                assertEquals("1\nc(main)\n3\n",
                                read(new File(db.getWorkTree(), "c/c/c")));
+               assertEquals("1\nd\n3\n", read(new File(db.getWorkTree(), "d")));
        }
 
        @Test
@@ -554,6 +557,7 @@ public class MergeCommandTest extends RepositoryTestCase {
                assertFalse(new File(db.getWorkTree(), "b").exists());
                assertEquals("1\nc(main)\n3\n",
                                read(new File(db.getWorkTree(), "c/c/c")));
+               assertEquals("1\nd\n3\n", read(new File(db.getWorkTree(), "d")));
        }
 
        @Test

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 525e4b352a5a111e676f055e2191cc94917665e5..a055131d61c0989ea2230f54830f7b4b8aa96ffd 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java
@@ -59,10 +59,10 @@ import java.util.Map;
 
 import org.eclipse.jgit.JGitText;
 import org.eclipse.jgit.diff.DiffAlgorithm;
+import org.eclipse.jgit.diff.DiffAlgorithm.SupportedAlgorithm;
 import org.eclipse.jgit.diff.RawText;
 import org.eclipse.jgit.diff.RawTextComparator;
 import org.eclipse.jgit.diff.Sequence;
-import org.eclipse.jgit.diff.DiffAlgorithm.SupportedAlgorithm;
 import org.eclipse.jgit.dircache.DirCache;
 import org.eclipse.jgit.dircache.DirCacheBuildIterator;
 import org.eclipse.jgit.dircache.DirCacheBuilder;
@@ -364,42 +364,42 @@ public class ResolveMerger extends ThreeWayMerger {
                        CorruptObjectException, IOException {
                enterSubtree = true;
                final int modeO = tw.getRawMode(T_OURS);
-               final int modeI = tw.getRawMode(T_INDEX);
+               final int modeT = tw.getRawMode(T_THEIRS);
+               final int modeB = tw.getRawMode(T_BASE);
+
+               if (modeO == 0 && modeT == 0 && modeB == 0)
+                       // File is either untracked or new, staged but uncommitted
+                       return true;
 
-               // Each index entry has to match ours, means: it has to be clean
-               if (nonTree(modeI)
-                               && !(tw.idEqual(T_INDEX, T_OURS) && modeO == modeI)) {
-                       failingPaths.put(tw.getPathString(), MergeFailureReason.DIRTY_INDEX);
+               if (isIndexDirty())
                        return false;
-               }
 
-               final int modeT = tw.getRawMode(T_THEIRS);
                if (nonTree(modeO) && modeO == modeT && tw.idEqual(T_OURS, T_THEIRS)) {
-                       // ours and theirs are equal: it doesn'nt matter
-                       // which one we choose. OURS is choosen here.
+                       // OURS and THEIRS are equal: it doesn't matter which one we choose.
+                       // OURS is chosen.
                        add(tw.getRawPath(), ours, DirCacheEntry.STAGE_0);
                        // no checkout needed!
                        return true;
                }
 
-               final int modeB = tw.getRawMode(T_BASE);
                if (nonTree(modeO) && modeB == modeT && tw.idEqual(T_BASE, T_THEIRS)) {
-                       // THEIRS was not changed compared to base. All changes must be in
-                       // OURS. Choose OURS.
+                       // THEIRS was not changed compared to BASE. All changes must be in
+                       // OURS. OURS is chosen.
                        add(tw.getRawPath(), ours, DirCacheEntry.STAGE_0);
+                       // no checkout needed!
                        return true;
                }
 
                if (modeB == modeO && tw.idEqual(T_BASE, T_OURS)) {
-                       // OURS was not changed compared to base. All changes must be in
-                       // THEIRS. Choose THEIRS.
+                       // OURS was not changed compared to BASE. All changes must be in
+                       // THEIRS. THEIRS is chosen.
                        if (nonTree(modeT)) {
                                DirCacheEntry e = add(tw.getRawPath(), theirs,
                                                DirCacheEntry.STAGE_0);
                                if (e != null)
                                        toBeCheckedOut.put(tw.getPathString(), e);
                                return true;
-                       } else if ((modeT == 0) && (modeB != 0)) {
+                       } else if (modeT == 0 && modeB != 0) {
                                // we want THEIRS ... but THEIRS contains the deletion of the
                                // file
                                toBeCheckedOut.put(tw.getPathString(), null);
@@ -441,18 +441,9 @@ public class ResolveMerger extends ThreeWayMerger {
                }
 
                if (nonTree(modeO) && nonTree(modeT)) {
-                       if (!inCore) {
-                               // We are going to update the worktree. Make sure the worktree
-                               // is not modified
-                               if (work != null
-                                               && (!nonTree(work.getEntryRawMode()) || work
-                                                               .isModified(index.getDirCacheEntry(), true))) {
-                                       failingPaths.put(tw.getPathString(),
-                                                       MergeFailureReason.DIRTY_WORKTREE);
-                                       return false;
-                               }
-                       }
-
+                       // Check worktree before modifying files
+                       if (isWorktreeDirty())
+                               return false;
                        if (!contentMerge(base, ours, theirs)) {
                                unmergedPaths.add(tw.getPathString());
                        }
@@ -461,6 +452,35 @@ public class ResolveMerger extends ThreeWayMerger {
                return true;
        }
 
+       private boolean isIndexDirty() {
+               final int modeI = tw.getRawMode(T_INDEX);
+               final int modeO = tw.getRawMode(T_OURS);
+
+               // Index entry has to match ours to be considered clean
+               final boolean isDirty = nonTree(modeI)
+                               && !(tw.idEqual(T_INDEX, T_OURS) && modeO == modeI);
+               if (isDirty)
+                       failingPaths
+                                       .put(tw.getPathString(), MergeFailureReason.DIRTY_INDEX);
+               return isDirty;
+       }
+
+       private boolean isWorktreeDirty() {
+               if (inCore)
+                       return false;
+
+               final int modeF = tw.getRawMode(T_FILE);
+               final int modeO = tw.getRawMode(T_OURS);
+
+               // Worktree entry has to match ours to be considered clean
+               final boolean isDirty = nonTree(modeF)
+                               && !(tw.idEqual(T_FILE, T_OURS) && modeO == modeF);
+               if (isDirty)
+                       failingPaths.put(tw.getPathString(),
+                                       MergeFailureReason.DIRTY_WORKTREE);
+               return isDirty;
+       }
+
        private boolean contentMerge(CanonicalTreeParser base,
                        CanonicalTreeParser ours, CanonicalTreeParser theirs)
                        throws FileNotFoundException, IllegalStateException, IOException {