Fix a number of failing conflict situations

Adds further tests where the working tree is dirty (differs from
index) and where we have staged but uncommitted changes.

Fixed the test case 9 for file/directory conflicts.

Bug: 428819
Change-Id: Ie44a288b052abe936ebb74272d0fefef3b218a7a
Signed-off-by: Axel Richard <axel.richard@obeo.fr>
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
Signed-off-by: Christian Halstrick <christian.halstrick@sap.com>

1 files changed, 121 insertions, 45 deletions

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java
index fdf8c052fe..80dda8eb83 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java
@@ -562,7 +562,7 @@ public class DirCacheCheckout {
 		 * 6b   D        F       F       N         N       N       N           Conflict
 		 * 7    F        D       F       Y         Y       N       N           Update
 		 * 8    F        D       F       N         Y       N       N           Conflict
-		 * 9    F        D       F       Y         N       N       N           Update
+		 * 9    F        D       F                 N       N       N           Conflict
 		 * 10   F        D       D                 N       N       Y           Keep
 		 * 11   F        D       D                 N       N       N           Conflict
 		 * 12   F        F       D       Y         N       Y       N           Update
@@ -610,7 +610,7 @@ public class DirCacheCheckout {
 			// switch processes all relevant cases.
 			switch (ffMask) {
 			case 0xDDF: // 1 2
-				if (isModified(name)) {
+				if (f != null && isModifiedSubtree_IndexWorkingtree(name)) {
 					conflict(name, dce, h, m); // 1
 				} else {
 					update(name, mId, mMode); // 2
@@ -647,32 +647,29 @@ public class DirCacheCheckout {
 				break;
 			case 0xFDF: // 7 8 9
 				if (equalIdAndMode(hId, hMode, mId, mMode)) {
-					if (isModified(name))
+					if (isModifiedSubtree_IndexWorkingtree(name))
 						conflict(name, dce, h, m); // 8
 					else
 						update(name, mId, mMode); // 7
-				} else if (!isModified(name))
-					update(name, mId, mMode); // 9
-				else
-					// To be confirmed - this case is not in the table.
-					conflict(name, dce, h, m);
+				} else
+					conflict(name, dce, h, m); // 9
 				break;
 			case 0xFD0: // keep without a rule
 				keep(dce);
 				break;
 			case 0xFFD: // 12 13 14
 				if (equalIdAndMode(hId, hMode, iId, iMode))
-					if (f == null
-							|| f.isModified(dce, true,
+					if (f != null
+							&& f.isModified(dce, true,
 									this.walk.getObjectReader()))
-						conflict(name, dce, h, m);
+						conflict(name, dce, h, m); // 13
 					else
-						remove(name);
+						remove(name); // 12
 				else
-					conflict(name, dce, h, m);
+					conflict(name, dce, h, m); // 14
 				break;
 			case 0x0DF: // 16 17
-				if (!isModified(name))
+				if (!isModifiedSubtree_IndexWorkingtree(name))
 					update(name, mId, mMode);
 				else
 					conflict(name, dce, h, m);
@@ -684,12 +681,14 @@ public class DirCacheCheckout {
 		}
 
 		// if we have no file at all then there is nothing to do
-		if ((ffMask & 0x222) == 0)
+		if ((ffMask & 0x222) == 0
+				&& (f == null || FileMode.TREE.equals(f.getEntryFileMode())))
 			return;
 
 		if ((ffMask == 0x00F) && f != null && FileMode.TREE.equals(f.getEntryFileMode())) {
 			// File/Directory conflict case #20
 			conflict(name, null, h, m);
+			return;
 		}
 
 		if (i == null) {
@@ -768,7 +767,9 @@ public class DirCacheCheckout {
 				 * </pre>
 				 */
 
-				if (m == null || equalIdAndMode(mId, mMode, iId, iMode)) {
+				if (m == null
+						|| !isModified_IndexTree(name, iId, iMode, mId, mMode,
+								mergeCommitTree)) {
 					// Merge contains nothing or the same as Index
 					// Nothing in Head
 					// Something in Index
@@ -824,7 +825,7 @@ public class DirCacheCheckout {
 				 * 	           clean I==H  I==M       H        M        Result
 				 * 	         -----------------------------------------------------
 				 * 	        10 yes   yes   N/A     exists   nothing  remove path from index
-				 * 	        11 no    yes   N/A     exists   nothing  fail
+				 * 	        11 no    yes   N/A     exists   nothing  keep file
 				 * 	        12 yes   no    N/A     exists   nothing  fail
 				 * 	        13 no    no    N/A     exists   nothing  fail
 				 * </pre>
@@ -841,23 +842,31 @@ public class DirCacheCheckout {
 					// Something different from a submodule in Index
 					// Nothing in Merge
 					// Something in Head
-					if (equalIdAndMode(hId, hMode, iId, iMode)) {
+					if (!isModified_IndexTree(name, iId, iMode, hId, hMode,
+							headCommitTree)) {
 						// Index contains the same as Head
 						// Something different from a submodule in Index
 						// Nothing in Merge
 						// Something in Head
-						if (f == null
-								|| f.isModified(dce, true,
-										this.walk.getObjectReader()))
+						if (f != null
+								&& f.isModified(dce, true,
+										this.walk.getObjectReader())) {
 							// file is dirty
 							// Index contains the same as Head
 							// Something different from a submodule in Index
 							// Nothing in Merge
 							// Something in Head
-							// -> file is dirty but is should be removed. That's
-							// a conflict
-							conflict(name, dce, h, m);
-						else
+
+							if (!FileMode.TREE.equals(f.getEntryFileMode())
+									&& FileMode.TREE.equals(iMode))
+								// The workingtree contains a file and the index semantically contains a folder.
+								// Git considers the workingtree file as untracked. Just keep the untracked file.
+								return;
+							else
+								// -> file is dirty and tracked but is should be
+								// removed. That's a conflict
+								conflict(name, dce, h, m);
+						} else
 							// file doesn't exist or is clean
 							// Index contains the same as Head
 							// Something different from a submodule in Index
@@ -880,8 +889,10 @@ public class DirCacheCheckout {
 				// Something in Head
 				// Something in Index
 				if (!equalIdAndMode(hId, hMode, mId, mMode)
-						&& !equalIdAndMode(hId, hMode, iId, iMode)
-						&& !equalIdAndMode(mId, mMode, iId, iMode))
+						&& isModified_IndexTree(name, iId, iMode, hId, hMode,
+								headCommitTree)
+						&& isModified_IndexTree(name, iId, iMode, mId, mMode,
+								mergeCommitTree))
 					// All three contents in Head, Merge, Index differ from each
 					// other
 					// -> All contents differ. Report a conflict.
@@ -893,8 +904,10 @@ public class DirCacheCheckout {
 					// Something in Head
 					// Something in Index
 
-					if (equalIdAndMode(hId, hMode, iId, iMode)
-						&& !equalIdAndMode(mId, mMode, iId, iMode)) {
+				if (!isModified_IndexTree(name, iId, iMode, hId, hMode,
+						headCommitTree)
+						&& isModified_IndexTree(name, iId, iMode, mId, mMode,
+								mergeCommitTree)) {
 						// Head contains the same as Index. Merge differs
 						// Something in Merge
 
@@ -1036,25 +1049,88 @@ public class DirCacheCheckout {
 		}
 	}
 
-	private boolean isModified(String path) throws CorruptObjectException, IOException {
+	/**
+	 * Checks whether the subtree starting at a given path differs between Index and
+	 * workingtree.
+	 *
+	 * @param path
+	 * @return true if the subtrees differ
+	 * @throws CorruptObjectException
+	 * @throws IOException
+	 */
+	private boolean isModifiedSubtree_IndexWorkingtree(String path)
+			throws CorruptObjectException, IOException {
+		NameConflictTreeWalk tw = new NameConflictTreeWalk(repo);
+		try {
+			tw.addTree(new DirCacheIterator(dc));
+			tw.addTree(new FileTreeIterator(repo));
+			tw.setRecursive(true);
+			tw.setFilter(PathFilter.create(path));
+			DirCacheIterator dcIt;
+			WorkingTreeIterator wtIt;
+			while (tw.next()) {
+				dcIt = tw.getTree(0, DirCacheIterator.class);
+				wtIt = tw.getTree(1, WorkingTreeIterator.class);
+				if (dcIt == null || wtIt == null)
+					return true;
+				if (wtIt.isModified(dcIt.getDirCacheEntry(), true,
+						this.walk.getObjectReader())) {
+					return true;
+				}
+			}
+			return false;
+		} finally {
+			tw.release();
+		}
+	}
+
+	private boolean isModified_IndexTree(String path, ObjectId iId,
+			FileMode iMode, ObjectId tId, FileMode tMode, ObjectId rootTree)
+			throws CorruptObjectException, IOException {
+		if (iMode != tMode)
+			return true;
+		if (FileMode.TREE.equals(iMode)
+				&& (iId == null || ObjectId.zeroId().equals(iId)))
+			return isModifiedSubtree_IndexTree(path, rootTree);
+		else
+			return !equalIdAndMode(iId, iMode, tId, tMode);
+	}
+
+	/**
+	 * Checks whether the subtree starting at a given path differs between Index and
+	 * some tree.
+	 *
+	 * @param path
+	 * @param tree
+	 *            the tree to compare
+	 * @return true if the subtrees differ
+	 * @throws CorruptObjectException
+	 * @throws IOException
+	 */
+	private boolean isModifiedSubtree_IndexTree(String path, ObjectId tree)
+			throws CorruptObjectException, IOException {
 		NameConflictTreeWalk tw = new NameConflictTreeWalk(repo);
-		tw.addTree(new DirCacheIterator(dc));
-		tw.addTree(new FileTreeIterator(repo));
-		tw.setRecursive(true);
-		tw.setFilter(PathFilter.create(path));
-		DirCacheIterator dcIt;
-		WorkingTreeIterator wtIt;
-		while(tw.next()) {
-			dcIt = tw.getTree(0, DirCacheIterator.class);
-			wtIt = tw.getTree(1, WorkingTreeIterator.class);
-			if (dcIt == null || wtIt == null)
-				return true;
-			if (wtIt.isModified(dcIt.getDirCacheEntry(), true,
-					this.walk.getObjectReader())) {
-				return true;
+		try {
+			tw.addTree(new DirCacheIterator(dc));
+			tw.addTree(tree);
+			tw.setRecursive(true);
+			tw.setFilter(PathFilter.create(path));
+			while (tw.next()) {
+				AbstractTreeIterator dcIt = tw.getTree(0,
+						DirCacheIterator.class);
+				AbstractTreeIterator treeIt = tw.getTree(1,
+						AbstractTreeIterator.class);
+				if (dcIt == null || treeIt == null)
+					return true;
+				if (dcIt.getEntryRawMode() != treeIt.getEntryRawMode())
+					return true;
+				if (!dcIt.getEntryObjectId().equals(treeIt.getEntryObjectId()))
+					return true;
 			}
+			return false;
+		} finally {
+			tw.release();
 		}
-		return false;
 	}
 
 	/**