DirCacheIterator: Fix reset() and back()

reset() was broken and probably only worked when the position was
at the beginning. More serious was that back() sometimes descended
into the tree rather than skipping backward at the same level. Sometimes
this would result in false conflicts, but one could suspect silent
errors too. back() is called by the NamingConflictTreeWalk when looking
for directory/file conflicts.

Also added toString to DirCacheTree to simplify debugging.

Bug: 396127
Change-Id: Iaa1b4e20e623d84c2e5ac26748f42e991080dbcd

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/dircache/DirCacheIteratorTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/dircache/DirCacheIteratorTest.java
index 752441322a09093acaf970924d02994ec888c523..6b3f700d203ad6b4d5d1fd8e1935c5c8566cb836 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/dircache/DirCacheIteratorTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/dircache/DirCacheIteratorTest.java
@@ -55,6 +55,7 @@ import java.util.Collections;
 import org.eclipse.jgit.junit.JGitTestUtil;
 import org.eclipse.jgit.lib.FileMode;
 import org.eclipse.jgit.lib.RepositoryTestCase;
+import org.eclipse.jgit.treewalk.AbstractTreeIterator;
 import org.eclipse.jgit.treewalk.TreeWalk;
 import org.eclipse.jgit.treewalk.filter.PathFilterGroup;
 import org.eclipse.jgit.util.FS;
@@ -252,6 +253,133 @@ public class DirCacheIteratorTest extends RepositoryTestCase {
                assertEquals(paths.length, pathIdx);
        }
 
+       @Test
+       public void testReset() throws Exception {
+               final DirCache dc = DirCache.newInCore();
+
+               final FileMode mode = FileMode.REGULAR_FILE;
+               final String[] paths = { "a.", "a/b", "a/c/e", "a/c/f", "a/d", "a0b" };
+               final DirCacheEntry[] ents = new DirCacheEntry[paths.length];
+               for (int i = 0; i < paths.length; i++) {
+                       ents[i] = new DirCacheEntry(paths[i]);
+                       ents[i].setFileMode(mode);
+               }
+
+               final DirCacheBuilder b = dc.builder();
+               for (int i = 0; i < ents.length; i++)
+                       b.add(ents[i]);
+               b.finish();
+
+               DirCacheIterator dci = new DirCacheIterator(dc);
+               assertFalse(dci.eof());
+               assertEquals("a.", dci.getEntryPathString());
+               dci.next(1);
+               assertFalse(dci.eof());
+               assertEquals("a", dci.getEntryPathString());
+               dci.next(1);
+               assertFalse(dci.eof());
+               assertEquals("a0b", dci.getEntryPathString());
+               dci.next(1);
+               assertTrue(dci.eof());
+
+               // same entries the second time
+               dci.reset();
+               assertFalse(dci.eof());
+               assertEquals("a.", dci.getEntryPathString());
+               dci.next(1);
+               assertFalse(dci.eof());
+               assertEquals("a", dci.getEntryPathString());
+               dci.next(1);
+               assertFalse(dci.eof());
+               assertEquals("a0b", dci.getEntryPathString());
+               dci.next(1);
+               assertTrue(dci.eof());
+
+               // Step backwards
+               dci.back(1);
+               assertFalse(dci.eof());
+               assertEquals("a0b", dci.getEntryPathString());
+               dci.back(1);
+               assertFalse(dci.eof());
+               assertEquals("a", dci.getEntryPathString());
+               dci.back(1);
+               assertFalse(dci.eof());
+               assertEquals("a.", dci.getEntryPathString());
+               assertTrue(dci.first());
+
+               // forward
+               assertFalse(dci.eof());
+               assertEquals("a.", dci.getEntryPathString());
+               dci.next(1);
+               assertFalse(dci.eof());
+               assertEquals("a", dci.getEntryPathString());
+               dci.next(1);
+               assertFalse(dci.eof());
+               assertEquals("a0b", dci.getEntryPathString());
+               dci.next(1);
+               assertTrue(dci.eof());
+
+               // backwqrd halways, and forward again
+               dci.back(1);
+               assertFalse(dci.eof());
+               assertEquals("a0b", dci.getEntryPathString());
+               dci.back(1);
+               assertFalse(dci.eof());
+               assertEquals("a", dci.getEntryPathString());
+
+               dci.next(1);
+               assertFalse(dci.eof());
+               assertEquals("a0b", dci.getEntryPathString());
+               dci.next(1);
+               assertTrue(dci.eof());
+
+               dci.reset(); // a.
+               dci.next(1); // a
+               AbstractTreeIterator sti = dci.createSubtreeIterator(null);
+               assertEquals("a/b", sti.getEntryPathString());
+               sti.next(1);
+               assertEquals("a/c", sti.getEntryPathString());
+               sti.next(1);
+               assertEquals("a/d", sti.getEntryPathString());
+               sti.back(2);
+               assertEquals("a/b", sti.getEntryPathString());
+
+       }
+
+       @Test
+       public void testBackBug396127() throws Exception {
+               final DirCache dc = DirCache.newInCore();
+
+               final FileMode mode = FileMode.REGULAR_FILE;
+               final String[] paths = { "git-gui/po/fr.po",
+                               "git_remote_helpers/git/repo.py" };
+               final DirCacheEntry[] ents = new DirCacheEntry[paths.length];
+               for (int i = 0; i < paths.length; i++) {
+                       ents[i] = new DirCacheEntry(paths[i]);
+                       ents[i].setFileMode(mode);
+               }
+
+               final DirCacheBuilder b = dc.builder();
+               for (int i = 0; i < ents.length; i++)
+                       b.add(ents[i]);
+               b.finish();
+
+               DirCacheIterator dci = new DirCacheIterator(dc);
+               assertFalse(dci.eof());
+               assertEquals("git-gui", dci.getEntryPathString());
+               dci.next(1);
+               assertFalse(dci.eof());
+               assertEquals("git_remote_helpers", dci.getEntryPathString());
+               dci.back(1);
+               assertFalse(dci.eof());
+               assertEquals("git-gui", dci.getEntryPathString());
+               dci.next(1);
+               assertEquals("git_remote_helpers", dci.getEntryPathString());
+               dci.next(1);
+               assertTrue(dci.eof());
+
+       }
+
        @Test
        public void testTwoLevelSubtree_FilterPath() throws Exception {
                final DirCache dc = DirCache.newInCore();

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheIterator.java b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheIterator.java
index e685e0cad8e1877867a998e37e63ca5ab0ba069d..706e0574801313bb509a73caae3f4157d0fc43b2 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheIterator.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheIterator.java
@@ -169,6 +169,9 @@ public class DirCacheIterator extends AbstractTreeIterator {
        public void reset() {
                if (!first()) {
                        ptr = treeStart;
+                       nextSubtreePos = 0;
+                       currentEntry = null;
+                       currentSubtree = null;
                        if (!eof())
                                parseEntry();
                }
@@ -203,16 +206,29 @@ public class DirCacheIterator extends AbstractTreeIterator {
                        if (currentSubtree != null)
                                nextSubtreePos--;
                        ptr--;
-                       parseEntry();
+                       parseEntry(false);
                        if (currentSubtree != null)
                                ptr -= currentSubtree.getEntrySpan() - 1;
                }
        }
 
        private void parseEntry() {
+               parseEntry(true);
+       }
+
+       private void parseEntry(boolean forward) {
                currentEntry = cache.getEntry(ptr);
                final byte[] cep = currentEntry.path;
 
+               if (!forward) {
+                       if (nextSubtreePos > 0) {
+                               final DirCacheTree p = tree.getChild(nextSubtreePos - 1);
+                               if (p.contains(cep, pathOffset, cep.length)) {
+                                       nextSubtreePos--;
+                                       currentSubtree = p;
+                               }
+                       }
+               }
                if (nextSubtreePos != tree.getChildCount()) {
                        final DirCacheTree s = tree.getChild(nextSubtreePos);
                        if (s.contains(cep, pathOffset, cep.length)) {

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheTree.java b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheTree.java
index c71c91332f819609366b6947ca1f61a8508df6e1..5acde4385ebf5d24b39cbe8a649c5053b60b52ec 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheTree.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheTree.java
@@ -552,4 +552,9 @@ public class DirCacheTree {
                                return aPos;
                return -1;
        }
+
+       @Override
+       public String toString() {
+               return getNameString();
+       }
 }