AddCommand: Use NameConflictTreeWalk to identify file-dir changes

Adding a path that already exists but is changing type such as
from symlink to subdirectory requires a NameConflictTreeWalk to
match up the two different entry types that share the same name.

NameConflictTreeWalk needs a bug fix to pop conflicting entries
when PathFilterGroup aborts the walk early so that it does not
allow DirCacheBuilderIterator to copy conflicting entries into
the output cache.

Change-Id: I61b49cbe949ca8b4b98f9eb6dbe7b1f82eabb724

org.eclipse.jgit.test/tst/org/eclipse/jgit/api/AddCommandTest.java

@@ -43,6 +43,7 @@
  */
 package org.eclipse.jgit.api;
 
+import static org.eclipse.jgit.util.FileUtils.RECURSIVE;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
@@ -777,12 +778,107 @@ public class AddCommandTest extends RepositoryTestCase {
 
 		assertEquals("[a.txt, mode:100644, content:more content,"
 				+ " assume-unchanged:false][b.txt, mode:100644,"
- + "" + ""
 				+ " content:content, assume-unchanged:true]",
 				indexState(CONTENT
 				| ASSUME_UNCHANGED));
 	}
 
+	@Test
+	public void testReplaceFileWithDirectory()
+			throws IOException, NoFilepatternException, GitAPIException {
+		try (Git git = new Git(db)) {
+			writeTrashFile("df", "before replacement");
+			git.add().addFilepattern("df").call();
+			assertEquals("[df, mode:100644, content:before replacement]",
+					indexState(CONTENT));
+			FileUtils.delete(new File(db.getWorkTree(), "df"));
+			writeTrashFile("df/f", "after replacement");
+			git.add().addFilepattern("df").call();
+			assertEquals("[df/f, mode:100644, content:after replacement]",
+					indexState(CONTENT));
+		}
+	}
+
+	@Test
+	public void testReplaceDirectoryWithFile()
+			throws IOException, NoFilepatternException, GitAPIException {
+		try (Git git = new Git(db)) {
+			writeTrashFile("df/f", "before replacement");
+			git.add().addFilepattern("df").call();
+			assertEquals("[df/f, mode:100644, content:before replacement]",
+					indexState(CONTENT));
+			FileUtils.delete(new File(db.getWorkTree(), "df"), RECURSIVE);
+			writeTrashFile("df", "after replacement");
+			git.add().addFilepattern("df").call();
+			assertEquals("[df, mode:100644, content:after replacement]",
+					indexState(CONTENT));
+		}
+	}
+
+	@Test
+	public void testReplaceFileByPartOfDirectory()
+			throws IOException, NoFilepatternException, GitAPIException {
+		try (Git git = new Git(db)) {
+			writeTrashFile("src/main", "df", "before replacement");
+			writeTrashFile("src/main", "z", "z");
+			writeTrashFile("z", "z2");
+			git.add().addFilepattern("src/main/df")
+				.addFilepattern("src/main/z")
+				.addFilepattern("z")
+				.call();
+			assertEquals(
+					"[src/main/df, mode:100644, content:before replacement]" +
+					"[src/main/z, mode:100644, content:z]" +
+					"[z, mode:100644, content:z2]",
+					indexState(CONTENT));
+			FileUtils.delete(new File(db.getWorkTree(), "src/main/df"));
+			writeTrashFile("src/main/df", "a", "after replacement");
+			writeTrashFile("src/main/df", "b", "unrelated file");
+			git.add().addFilepattern("src/main/df/a").call();
+			assertEquals(
+					"[src/main/df/a, mode:100644, content:after replacement]" +
+					"[src/main/z, mode:100644, content:z]" +
+					"[z, mode:100644, content:z2]",
+					indexState(CONTENT));
+		}
+	}
+
+	@Test
+	public void testReplaceDirectoryConflictsWithFile()
+			throws IOException, NoFilepatternException, GitAPIException {
+		DirCache dc = db.lockDirCache();
+		try (ObjectInserter oi = db.newObjectInserter()) {
+			DirCacheBuilder builder = dc.builder();
+			File f = writeTrashFile("a", "df", "content");
+			addEntryToBuilder("a", f, oi, builder, 1);
+
+			f = writeTrashFile("a", "df", "other content");
+			addEntryToBuilder("a/df", f, oi, builder, 3);
+
+			f = writeTrashFile("a", "df", "our content");
+			addEntryToBuilder("a/df", f, oi, builder, 2);
+
+			f = writeTrashFile("z", "z");
+			addEntryToBuilder("z", f, oi, builder, 0);
+			builder.commit();
+		}
+		assertEquals(
+				"[a, mode:100644, stage:1, content:content]" +
+				"[a/df, mode:100644, stage:2, content:our content]" +
+				"[a/df, mode:100644, stage:3, content:other content]" +
+				"[z, mode:100644, content:z]",
+				indexState(CONTENT));
+
+		try (Git git = new Git(db)) {
+			FileUtils.delete(new File(db.getWorkTree(), "a"), RECURSIVE);
+			writeTrashFile("a", "merged");
+			git.add().addFilepattern("a").call();
+			assertEquals("[a, mode:100644, content:merged]" +
+					"[z, mode:100644, content:z]",
+					indexState(CONTENT));
+		}
+	}
+
 	@Test
 	public void testExecutableRetention() throws Exception {
 		StoredConfig config = db.getConfig();

org.eclipse.jgit/src/org/eclipse/jgit/api/AddCommand.java

@@ -45,6 +45,7 @@ package org.eclipse.jgit.api;
 
 import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
 import static org.eclipse.jgit.lib.FileMode.GITLINK;
+import static org.eclipse.jgit.lib.FileMode.TYPE_TREE;
 
 import java.io.IOException;
 import java.io.InputStream;
@@ -66,7 +67,7 @@ import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectInserter;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.treewalk.FileTreeIterator;
-import org.eclipse.jgit.treewalk.TreeWalk;
+import org.eclipse.jgit.treewalk.NameConflictTreeWalk;
 import org.eclipse.jgit.treewalk.TreeWalk.OperationType;
 import org.eclipse.jgit.treewalk.WorkingTreeIterator;
 import org.eclipse.jgit.treewalk.filter.PathFilterGroup;
@@ -141,7 +142,7 @@ public class AddCommand extends GitCommand<DirCache> {
 		boolean addAll = filepatterns.contains("."); //$NON-NLS-1$
 
 		try (ObjectInserter inserter = repo.newObjectInserter();
-				final TreeWalk tw = new TreeWalk(repo)) {
+				NameConflictTreeWalk tw = new NameConflictTreeWalk(repo)) {
 			tw.setOperationType(OperationType.CHECKIN_OP);
 			dc = repo.lockDirCache();
 
@@ -151,7 +152,6 @@ public class AddCommand extends GitCommand<DirCache> {
 				workingTreeIterator = new FileTreeIterator(repo);
 			workingTreeIterator.setDirCacheIterator(tw, 0);
 			tw.addTree(workingTreeIterator);
-			tw.setRecursive(true);
 			if (!addAll)
 				tw.setFilter(PathFilterGroup.createFromStrings(filepatterns));
 
@@ -180,9 +180,14 @@ public class AddCommand extends GitCommand<DirCache> {
 					continue;
 				}
 
+				if (tw.isSubtree() && !tw.isDirectoryFileConflict()) {
+					tw.enterSubtree();
+					continue;
+				}
+
 				if (f == null) { // working tree file does not exist
-					if (c != null
+					if (entry != null
-							&& (!update || GITLINK == c.getEntryFileMode())) {
+							&& (!update || GITLINK == entry.getFileMode())) {
 						builder.add(entry);
 					}
 					continue;
@@ -196,6 +201,14 @@ public class AddCommand extends GitCommand<DirCache> {
 					continue;
 				}
 
+				if (f.getEntryRawMode() == TYPE_TREE) {
+					// Index entry exists and is symlink, gitlink or file,
+					// otherwise the tree would have been entered above.
+					// Replace the index entry by diving into tree of files.
+					tw.enterSubtree();
+					continue;
+				}
+
 				byte[] path = tw.getRawPath();
 				if (entry == null || entry.getStage() > 0) {
 					entry = new DirCacheEntry(path);

org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheBuildIterator.java

@@ -130,4 +130,9 @@ public class DirCacheBuildIterator extends DirCacheIterator {
 		if (cur < cnt)
 			builder.keep(cur, cnt - cur);
 	}
+
+	@Override
+	protected boolean needsStopWalk() {
+		return ptr < cache.getEntryCount();
+	}
 }

org.eclipse.jgit/src/org/eclipse/jgit/treewalk/AbstractTreeIterator.java

@@ -691,6 +691,14 @@ public abstract class AbstractTreeIterator {
 		// Do nothing by default.  Most iterators do not care.
 	}
 
+	/**
+	 * @return true if the iterator implements {@link #stopWalk()}.
+	 * @since 4.2
+	 */
+	protected boolean needsStopWalk() {
+		return false;
+	}
+
 	/**
 	 * @return the length of the name component of the path for the current entry
 	 */

org.eclipse.jgit/src/org/eclipse/jgit/treewalk/EmptyTreeIterator.java

@@ -142,4 +142,9 @@ public class EmptyTreeIterator extends AbstractTreeIterator {
 		if (parent != null)
 			parent.stopWalk();
 	}
+
+	@Override
+	protected boolean needsStopWalk() {
+		return parent != null && parent.needsStopWalk();
+	}
 }

org.eclipse.jgit/src/org/eclipse/jgit/treewalk/NameConflictTreeWalk.java

@@ -43,6 +43,8 @@
 
 package org.eclipse.jgit.treewalk;
 
+import java.io.IOException;
+
 import org.eclipse.jgit.dircache.DirCacheBuilder;
 import org.eclipse.jgit.errors.CorruptObjectException;
 import org.eclipse.jgit.lib.FileMode;
@@ -338,6 +340,41 @@ public class NameConflictTreeWalk extends TreeWalk {
 			dfConflict = null;
 	}
 
+	void stopWalk() throws IOException {
+		if (!needsStopWalk()) {
+			return;
+		}
+
+		// Name conflicts make aborting early difficult. Multiple paths may
+		// exist between the file and directory versions of a name. To ensure
+		// the directory version is skipped over (as it was previously visited
+		// during the file version step) requires popping up the stack and
+		// finishing out each subtree that the walker dove into. Siblings in
+		// parents do not need to be recursed into, bounding the cost.
+		for (;;) {
+			AbstractTreeIterator t = min();
+			if (t.eof()) {
+				if (depth > 0) {
+					exitSubtree();
+					popEntriesEqual();
+					continue;
+				}
+				return;
+			}
+			currentHead = t;
+			skipEntriesEqual();
+		}
+	}
+
+	private boolean needsStopWalk() {
+		for (AbstractTreeIterator t : trees) {
+			if (t.needsStopWalk()) {
+				return true;
+			}
+		}
+		return false;
+	}
+
 	/**
 	 * True if the current entry is covered by a directory/file conflict.
 	 *

org.eclipse.jgit/src/org/eclipse/jgit/treewalk/TreeWalk.java

@@ -57,6 +57,7 @@ import org.eclipse.jgit.attributes.Attributes;
 import org.eclipse.jgit.attributes.AttributesNode;
 import org.eclipse.jgit.attributes.AttributesNodeProvider;
 import org.eclipse.jgit.attributes.AttributesProvider;
+import org.eclipse.jgit.dircache.DirCacheBuildIterator;
 import org.eclipse.jgit.dircache.DirCacheIterator;
 import org.eclipse.jgit.errors.CorruptObjectException;
 import org.eclipse.jgit.errors.IncorrectObjectTypeException;
@@ -256,7 +257,7 @@ public class TreeWalk implements AutoCloseable, AttributesProvider {
 
 	private boolean postOrderTraversal;
 
-	private int depth;
+	int depth;
 
 	private boolean advance;
 
@@ -665,12 +666,29 @@ public class TreeWalk implements AutoCloseable, AttributesProvider {
 				return true;
 			}
 		} catch (StopWalkException stop) {
-			for (final AbstractTreeIterator t : trees)
+			stopWalk();
-				t.stopWalk();
 			return false;
 		}
 	}
 
+	/**
+	 * Notify iterators the walk is aborting.
+	 * <p>
+	 * Primarily to notify {@link DirCacheBuildIterator} the walk is aborting so
+	 * that it can copy any remaining entries.
+	 *
+	 * @throws IOException
+	 *             if traversal of remaining entries throws an exception during
+	 *             object access. This should never occur as remaining trees
+	 *             should already be in memory, however the methods used to
+	 *             finish traversal are declared to throw IOException.
+	 */
+	void stopWalk() throws IOException {
+		for (AbstractTreeIterator t : trees) {
+			t.stopWalk();
+		}
+	}
+
 	/**
 	 * Obtain the tree iterator for the current entry.
 	 * <p>
@@ -1065,7 +1083,7 @@ public class TreeWalk implements AutoCloseable, AttributesProvider {
 		}
 	}
 
-	private void exitSubtree() {
+	void exitSubtree() {
 		depth--;
 		for (int i = 0; i < trees.length; i++)
 			trees[i] = trees[i].parent;