From af54f16635f4d164ca22c0c9b264bc14c2b5ccd0 Mon Sep 17 00:00:00 2001 From: Robin Rosenberg Date: Sat, 15 Dec 2012 00:30:42 +0100 Subject: [PATCH] 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 --- .../jgit/dircache/DirCacheIteratorTest.java | 128 ++++++++++++++++++ .../jgit/dircache/DirCacheIterator.java | 18 ++- .../eclipse/jgit/dircache/DirCacheTree.java | 5 + 3 files changed, 150 insertions(+), 1 deletion(-) 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 752441322..6b3f700d2 100644 --- 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 e685e0cad..706e05748 100644 --- 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 c71c91332..5acde4385 100644 --- 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(); + } }