NameConflictTreeWalk: respect git order on multi-tree iteration

The NameConflictTreeWalk class is used in 3-way merge for iterating over
entries in 3 different commits. The class provides information about a
current entry and a state of the entry in commits (e.g entry is file,
entry is directory, entry is missing). In rare cases, the tree walker
can mix information about entries with different name.

The problem appears, because git uses unusual sorting order for
files. Example (this is a simplified real-life example):
Commit 1:
* gradle.properties - file
* gradle - directory (with nested files)
*   gradle/file - file in gradle directory
Commit 2:
* gradle.properties - file
* no entry with the name gradle
Commit 3:
* gradle.properties - file
* gradle - file
Here the names are ordered like this:
"gradle" file <"gradle.properties" file < "gradle/file" file.

NameConflictTreeWalk iterator already have code for processing
git sorting order, however in the example above the code doesn't
work correctly. Before the fix, NameConflictTreeWalk returns:
#next()
"gradle - directory" | "gradle.properties" | "gradle - file" - which is
wrong. The expected result is
#next()
"gradle - directory | MISSED_FILE | "gradle - file"
#next()
"gradle.properties"|"gradle.properties"|"gradle.properties"

Ensure that the "matches" field of tree iterators (which contains the
current path) is kept in sync in the case above.

Change-Id: Ief5aa06d80b358f4080043c8694aa0fd7c60045b
Signed-off-by: Dmitrii Filippov <dmfilippov@google.com>

1 files changed, 22 insertions, 0 deletions

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/NameConflictTreeWalk.java b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/NameConflictTreeWalk.java
index 28b7423817..3c8553c74f 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/NameConflictTreeWalk.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/NameConflictTreeWalk.java
@@ -268,6 +268,28 @@ public class NameConflictTreeWalk extends TreeWalk {
 			}
 		}
 
+		// When the combineDF is called, the t.matches field stores other
+		// entry (i.e. tree iterator) with an equal path. However, the
+		// previous loop moves each iterator independently. As a result,
+		// iterators which have had equals path at the start of the
+		// method can have different paths at this point.
+		// Reevaluate existing matches.
+		// The NameConflictTreeWalkTest.testDF_specialFileNames test
+		// cover this situation.
+		for (AbstractTreeIterator t : trees) {
+			// The previous loop doesn't touch tree iterator if it matches
+			// minRef. Skip it here
+			if (t.eof() || t.matches == null || t.matches == minRef)
+				continue;
+			// The t.pathCompare takes into account the entry type (file
+			// or directory) and returns non-zero value if names match
+			// but entry type don't match.
+			// We want to keep such matches (file/directory conflict),
+			// so reset matches only if names are not equal.
+			if (!nameEqual(t, t.matches))
+				t.matches = null;
+		}
+
 		if (treeMatch != null) {
 			// If we do have a conflict use one of the directory
 			// matching iterators instead of the file iterator.