Generate conflicts and index updates on file mode changes

Handle more cases for file mode changes. Especially make sure that the
following cases are handled correctly.

Case 1)

  An entry in the working tree, HEAD tree, and merge tree have
  different modes and different content.

  Prior Outcome:
    Dirty working tree content is replaced and file mode
    changes are lost.

  New Outcome:
    Conflict is generated.

Case 2)

  An entry in the index and merge tree have the same content
  but different modes but both modes are file type modes.

  Prior Outcome:
    File mode in working tree is not updated and the working
    directory is dirty.

  New Outcome:
    Index is updated and the working directory is clean.

Bug: 363772
Change-Id: I224602d68228eb419813986807f1eeab77e9c302
Signed-off-by: Christian Halstrick <christian.halstrick@sap.com>
Also-by: Kevin Sawicki <kevin@github.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

1 files changed, 33 insertions, 8 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 9b397e88c9..58b691228d 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java
@@ -473,6 +473,23 @@ public class DirCacheCheckout {
 	}
 
 	/**
+	 * Compares whether two pairs of ObjectId and FileMode are equal.
+	 *
+	 * @param id1
+	 * @param mode1
+	 * @param id2
+	 * @param mode2
+	 * @return <code>true</code> if FileModes and ObjectIds are equal.
+	 *         <code>false</code> otherwise
+	 */
+	private boolean equalIdAndMode(ObjectId id1, FileMode mode1, ObjectId id2,
+			FileMode mode2) {
+		if (!mode1.equals(mode2))
+			return false;
+		return id1 != null ? id1.equals(id2) : id2 == null;
+	}
+
+	/**
 	 * Here the main work is done. This method is called for each existing path
 	 * in head, index and merge. This method decides what to do with the
 	 * corresponding index entry: keep it, update it, remove it or mark a
@@ -508,6 +525,9 @@ public class DirCacheCheckout {
 		ObjectId iId = (i == null ? null : i.getEntryObjectId());
 		ObjectId mId = (m == null ? null : m.getEntryObjectId());
 		ObjectId hId = (h == null ? null : h.getEntryObjectId());
+		FileMode iMode = (i == null ? null : i.getEntryFileMode());
+		FileMode mMode = (m == null ? null : m.getEntryFileMode());
+		FileMode hMode = (h == null ? null : h.getEntryFileMode());
 
 		/**
 		 * <pre>
@@ -596,7 +616,7 @@ public class DirCacheCheckout {
 			case 0xFDD: // 10 11
 				// TODO: make use of tree extension as soon as available in jgit
 				// we would like to do something like
-				// if (!iId.equals(mId))
+				// if (!equalIdAndMode(iId, iMode, mId, mMode)
 				//   conflict(name, i.getDirCacheEntry(), h, m);
 				// But since we don't know the id of a tree in the index we do
 				// nothing here and wait that conflicts between index and merge
@@ -610,7 +630,7 @@ public class DirCacheCheckout {
 				conflict(name, (i != null) ? i.getDirCacheEntry() : null, h, m);
 				break;
 			case 0xFDF: // 7 8 9
-				if (hId.equals(mId)) {
+				if (equalIdAndMode(hId, hMode, mId, mMode)) {
 					if (isModified(name))
 						conflict(name, i.getDirCacheEntry(), h, m); // 8
 					else
@@ -625,7 +645,7 @@ public class DirCacheCheckout {
 				keep(i.getDirCacheEntry());
 				break;
 			case 0xFFD: // 12 13 14
-				if (hId.equals(iId)) {
+				if (equalIdAndMode(hId, hMode, iId, iMode)) {
 					dce = i.getDirCacheEntry();
 					if (f == null || f.isModified(dce, true))
 						conflict(name, dce, h, m);
@@ -662,7 +682,9 @@ public class DirCacheCheckout {
 				if (!FileMode.GITLINK.equals(m.getEntryFileMode())) {
 					// a dirty worktree: the index is empty but we have a
 					// workingtree-file
-					if (mId == null || !mId.equals(f.getEntryObjectId())) {
+					if (mId == null
+							|| !equalIdAndMode(mId, mMode,
+									f.getEntryObjectId(), f.getEntryFileMode())) {
 						conflict(name, null, h, m);
 						return;
 					}
@@ -703,7 +725,7 @@ public class DirCacheCheckout {
 				 * </pre>
 				 */
 
-				if (m == null || mId.equals(iId)) {
+				if (m == null || equalIdAndMode(mId, mMode, iId, iMode)) {
 					if (m==null && walk.isDirectoryFileConflict()) {
 						if (dce != null
 								&& (f == null || f.isModified(dce, true)))
@@ -732,7 +754,7 @@ public class DirCacheCheckout {
 					// be removed from the index, but not deleted from disk.
 					remove(name);
 				} else {
-					if (hId.equals(iId)) {
+					if (equalIdAndMode(hId, hMode, iId, iMode)) {
 						if (f == null || f.isModified(dce, true))
 							conflict(name, dce, h, m);
 						else
@@ -741,9 +763,12 @@ public class DirCacheCheckout {
 						conflict(name, dce, h, m);
 				}
 			} else {
-				if (!hId.equals(mId) && !hId.equals(iId) && !mId.equals(iId))
+				if (!equalIdAndMode(hId, hMode, mId, mMode)
+						&& !equalIdAndMode(hId, hMode, iId, iMode)
+						&& !equalIdAndMode(mId, mMode, iId, iMode))
 					conflict(name, dce, h, m);
-				else if (hId.equals(iId) && !mId.equals(iId)) {
+				else if (equalIdAndMode(hId, hMode, iId, iMode)
+						&& !equalIdAndMode(mId, mMode, iId, iMode)) {
 					// For submodules just update the index with the new SHA-1
 					if (dce != null
 							&& FileMode.GITLINK.equals(dce.getFileMode())) {