From fdb0a34abfd8b105a08887821d857fcdda2e22c4 Mon Sep 17 00:00:00 2001 From: Philipp Thun Date: Fri, 18 Mar 2011 13:33:36 +0100 Subject: [PATCH] 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 --- .../eclipse/jgit/api/MergeCommandTest.java | 4 + .../org/eclipse/jgit/merge/ResolveMerger.java | 76 ++++++++++++------- 2 files changed, 52 insertions(+), 28 deletions(-) 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 0ef24bd8a..987e64731 100644 --- 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 525e4b352..a055131d6 100644 --- 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 {