* [EGIT] Sort order from hell fixes, take 2
@ 2008-02-23 23:50 Robin Rosenberg
2008-02-23 23:50 ` [EGIT PATCH 01/10] Tighten IndexDiffTest to make it test better what it claims to test Robin Rosenberg
2008-03-03 14:17 ` [EGIT] Sort order from hell fixes, take 2 David Watson
0 siblings, 2 replies; 12+ messages in thread
From: Robin Rosenberg @ 2008-02-23 23:50 UTC (permalink / raw)
To: git; +Cc: David Watson
Hi fans,
My previous attempt to fix this failed, so here is another round, including
some new infrastructure like a TreeIterator to support this fix and whatever
will need it.
Feed free to scrutize and invent whatever evil test case might be missing.
The reason I noticed the problem was introduced in c20142, where the unit
tests for org.spearce.jgit was moved to the new project org.spearce.jgit.test .
-- robin
^ permalink raw reply [flat|nested] 12+ messages in thread
* [EGIT PATCH 01/10] Tighten IndexDiffTest to make it test better what it claims to test
2008-02-23 23:50 [EGIT] Sort order from hell fixes, take 2 Robin Rosenberg
@ 2008-02-23 23:50 ` Robin Rosenberg
2008-02-23 23:50 ` [EGIT PATCH 02/10] Extend IndexDiffTest with more tests Robin Rosenberg
2008-03-03 14:17 ` [EGIT] Sort order from hell fixes, take 2 David Watson
1 sibling, 1 reply; 12+ messages in thread
From: Robin Rosenberg @ 2008-02-23 23:50 UTC (permalink / raw)
To: git; +Cc: David Watson, Robin Rosenberg
I.e. test more of the expected state
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
.../tst/org/spearce/jgit/lib/IndexDiffTest.java | 22 ++++++++++++++++++++
1 files changed, 22 insertions(+), 0 deletions(-)
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/IndexDiffTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/IndexDiffTest.java
index ba5d8d7..629c06c 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/IndexDiffTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/IndexDiffTest.java
@@ -31,8 +31,12 @@ public class IndexDiffTest extends RepositoryTestCase {
index.add(trash, new File(trash, "dir/subfile"));
IndexDiff diff = new IndexDiff(tree, index);
diff.diff();
+ assertEquals(2, diff.getAdded().size());
assertTrue(diff.getAdded().contains("file1"));
assertTrue(diff.getAdded().contains("dir/subfile"));
+ assertEquals(0, diff.getChanged().size());
+ assertEquals(0, diff.getModified().size());
+ assertEquals(0, diff.getRemoved().size());
}
public void testRemoved() throws IOException {
@@ -44,11 +48,20 @@ public class IndexDiffTest extends RepositoryTestCase {
tree.addFile("file2");
tree.addFile("dir/file3");
assertEquals(2, tree.memberCount());
+ tree.findBlobMember("file2").setId(new ObjectId("30d67d4672d5c05833b7192cc77a79eaafb5c7ad"));
+ Tree tree2 = (Tree) tree.findTreeMember("dir");
+ tree2.findBlobMember("file3").setId(new ObjectId("873fb8d667d05436d728c52b1d7a09528e6eb59b"));
+ tree2.setId(new ObjectWriter(db).writeTree(tree2));
+ tree.setId(new ObjectWriter(db).writeTree(tree));
IndexDiff diff = new IndexDiff(tree, index);
diff.diff();
+ assertEquals(2, diff.getRemoved().size());
assertTrue(diff.getRemoved().contains("file2"));
assertTrue(diff.getRemoved().contains("dir/file3"));
+ assertEquals(0, diff.getChanged().size());
+ assertEquals(0, diff.getModified().size());
+ assertEquals(0, diff.getAdded().size());
}
public void testModified() throws IOException {
@@ -65,10 +78,19 @@ public class IndexDiffTest extends RepositoryTestCase {
tree.addFile("dir/file3").setId(new ObjectId("0123456789012345678901234567890123456789"));
assertEquals(2, tree.memberCount());
+ Tree tree2 = (Tree) tree.findTreeMember("dir");
+ tree2.setId(new ObjectWriter(db).writeTree(tree2));
+ tree.setId(new ObjectWriter(db).writeTree(tree));
IndexDiff diff = new IndexDiff(tree, index);
diff.diff();
+ assertEquals(2, diff.getChanged().size());
assertTrue(diff.getChanged().contains("file2"));
assertTrue(diff.getChanged().contains("dir/file3"));
+ assertEquals(1, diff.getModified().size());
assertTrue(diff.getModified().contains("dir/file3"));
+ assertEquals(0, diff.getAdded().size());
+ assertEquals(0, diff.getRemoved().size());
+ assertEquals(0, diff.getMissing().size());
}
+
}
--
1.5.4.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [EGIT PATCH 02/10] Extend IndexDiffTest with more tests
2008-02-23 23:50 ` [EGIT PATCH 01/10] Tighten IndexDiffTest to make it test better what it claims to test Robin Rosenberg
@ 2008-02-23 23:50 ` Robin Rosenberg
2008-02-23 23:50 ` [EGIT PATCH 03/10] WorkdirCheckout: more test for names that are close Robin Rosenberg
0 siblings, 1 reply; 12+ messages in thread
From: Robin Rosenberg @ 2008-02-23 23:50 UTC (permalink / raw)
To: git; +Cc: David Watson, Robin Rosenberg
This adds tests for some nasty cases. These can be summarized
by declaring that this is the correct order:
a.b
a/b
a=b
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
.../tst/org/spearce/jgit/lib/IndexDiffTest.java | 67 ++++++++++++++++++++
1 files changed, 67 insertions(+), 0 deletions(-)
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/IndexDiffTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/IndexDiffTest.java
index 629c06c..4692fa2 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/IndexDiffTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/IndexDiffTest.java
@@ -93,4 +93,71 @@ public class IndexDiffTest extends RepositoryTestCase {
assertEquals(0, diff.getMissing().size());
}
+ public void testUnchangedSimple() throws IOException {
+ GitIndex index = new GitIndex(db);
+
+ index.add(trash, writeTrashFile("a.b", "a.b"));
+ index.add(trash, writeTrashFile("a.c", "a.c"));
+ index.add(trash, writeTrashFile("a=c", "a=c"));
+ index.add(trash, writeTrashFile("a=d", "a=d"));
+
+ Tree tree = new Tree(db);
+ // got the hash id'd from the data using echo -n a.b|git hash-object -t blob --stdin
+ tree.addFile("a.b").setId(new ObjectId("f6f28df96c2b40c951164286e08be7c38ec74851"));
+ tree.addFile("a.c").setId(new ObjectId("6bc0e647512d2a0bef4f26111e484dc87df7f5ca"));
+ tree.addFile("a=c").setId(new ObjectId("06022365ddbd7fb126761319633bf73517770714"));
+ tree.addFile("a=d").setId(new ObjectId("fa6414df3da87840700e9eeb7fc261dd77ccd5c2"));
+
+ tree.setId(new ObjectWriter(db).writeTree(tree));
+
+ IndexDiff diff = new IndexDiff(tree, index);
+ diff.diff();
+ assertEquals(0, diff.getChanged().size());
+ assertEquals(0, diff.getAdded().size());
+ assertEquals(0, diff.getRemoved().size());
+ assertEquals(0, diff.getMissing().size());
+ assertEquals(0, diff.getModified().size());
+ }
+
+ /**
+ * This test has both files and directories that involve
+ * the tricky ordering used by Git.
+ *
+ * @throws IOException
+ */
+ public void testUnchangedComplex() throws IOException {
+ GitIndex index = new GitIndex(db);
+
+ index.add(trash, writeTrashFile("a.b", "a.b"));
+ index.add(trash, writeTrashFile("a.c", "a.c"));
+ index.add(trash, writeTrashFile("a/b.b/b", "a/b.b/b"));
+ index.add(trash, writeTrashFile("a/b", "a/b"));
+ index.add(trash, writeTrashFile("a/c", "a/c"));
+ index.add(trash, writeTrashFile("a=c", "a=c"));
+ index.add(trash, writeTrashFile("a=d", "a=d"));
+
+ Tree tree = new Tree(db);
+ // got the hash id'd from the data using echo -n a.b|git hash-object -t blob --stdin
+ tree.addFile("a.b").setId(new ObjectId("f6f28df96c2b40c951164286e08be7c38ec74851"));
+ tree.addFile("a.c").setId(new ObjectId("6bc0e647512d2a0bef4f26111e484dc87df7f5ca"));
+ tree.addFile("a/b.b/b").setId(new ObjectId("8d840bd4e2f3a48ff417c8e927d94996849933fd"));
+ tree.addFile("a/b").setId(new ObjectId("db89c972fc57862eae378f45b74aca228037d415"));
+ tree.addFile("a/c").setId(new ObjectId("52ad142a008aeb39694bafff8e8f1be75ed7f007"));
+ tree.addFile("a=c").setId(new ObjectId("06022365ddbd7fb126761319633bf73517770714"));
+ tree.addFile("a=d").setId(new ObjectId("fa6414df3da87840700e9eeb7fc261dd77ccd5c2"));
+
+ Tree tree3 = (Tree) tree.findTreeMember("a/b.b");
+ tree3.setId(new ObjectWriter(db).writeTree(tree3));
+ Tree tree2 = (Tree) tree.findTreeMember("a");
+ tree2.setId(new ObjectWriter(db).writeTree(tree2));
+ tree.setId(new ObjectWriter(db).writeTree(tree));
+
+ IndexDiff diff = new IndexDiff(tree, index);
+ diff.diff();
+ assertEquals(0, diff.getChanged().size());
+ assertEquals(0, diff.getAdded().size());
+ assertEquals(0, diff.getRemoved().size());
+ assertEquals(0, diff.getMissing().size());
+ assertEquals(0, diff.getModified().size());
+ }
}
--
1.5.4.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [EGIT PATCH 03/10] WorkdirCheckout: more test for names that are close
2008-02-23 23:50 ` [EGIT PATCH 02/10] Extend IndexDiffTest with more tests Robin Rosenberg
@ 2008-02-23 23:50 ` Robin Rosenberg
2008-02-23 23:50 ` [EGIT PATCH 04/10] Split a big test in ReadTreeTest into smaller tests Robin Rosenberg
0 siblings, 1 reply; 12+ messages in thread
From: Robin Rosenberg @ 2008-02-23 23:50 UTC (permalink / raw)
To: git; +Cc: David Watson, Robin Rosenberg
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
.../tst/org/spearce/jgit/lib/ReadTreeTest.java | 16 +++++++++++++++-
1 files changed, 15 insertions(+), 1 deletions(-)
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ReadTreeTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ReadTreeTest.java
index 7ac48c9..6faedc7 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ReadTreeTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ReadTreeTest.java
@@ -477,7 +477,21 @@ public class ReadTreeTest extends RepositoryTestCase {
assertNoConflicts();
}
-
+
+ public void testCloseNameConflictsX0() throws IOException {
+ setupCase(mkmap("a/a", "a/a-c"), mkmap("a/a","a/a", "b.b/b.b","b.b/b.bs"), mkmap("a/a", "a/a-c") );
+ checkout();
+ go();
+ assertNoConflicts();
+ }
+
+ public void testCloseNameConflicts1() throws IOException {
+ setupCase(mkmap("a/a", "a/a-c"), mkmap("a/a","a/a", "a.a/a.a","a.a/a.a"), mkmap("a/a", "a/a-c") );
+ checkout();
+ go();
+ assertNoConflicts();
+ }
+
private void checkout() throws IOException {
readTree = new WorkDirCheckout(db, trash, head, index, merge);
readTree.checkout();
--
1.5.4.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [EGIT PATCH 04/10] Split a big test in ReadTreeTest into smaller tests
2008-02-23 23:50 ` [EGIT PATCH 03/10] WorkdirCheckout: more test for names that are close Robin Rosenberg
@ 2008-02-23 23:50 ` Robin Rosenberg
2008-02-23 23:50 ` [EGIT PATCH 05/10] Fix git sort order compare bug Robin Rosenberg
0 siblings, 1 reply; 12+ messages in thread
From: Robin Rosenberg @ 2008-02-23 23:50 UTC (permalink / raw)
To: git; +Cc: David Watson, Robin Rosenberg
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
.../tst/org/spearce/jgit/lib/ReadTreeTest.java | 84 +++++++++++++------
1 files changed, 57 insertions(+), 27 deletions(-)
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ReadTreeTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ReadTreeTest.java
index 6faedc7..70c1396 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ReadTreeTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ReadTreeTest.java
@@ -273,50 +273,55 @@ public class ReadTreeTest extends RepositoryTestCase {
*19 D 0 F Update
*/
- public void testDirectoryFileConflicts() throws Exception {
+ public void testDirectoryFileConflicts_1() throws Exception {
// 1
doit(mk("DF/DF"), mk("DF"), mk("DF/DF"));
assertNoConflicts();
assertUpdated("DF");
assertRemoved("DF/DF");
-
+ }
+
+ public void testDirectoryFileConflicts_2() throws Exception {
// 2
setupCase(mk("DF/DF"), mk("DF"), mk("DF/DF"));
writeTrashFile("DF/DF", "different");
go();
assertConflict("DF/DF");
+ }
+
+ public void testDirectoryFileConflicts_3() throws Exception {
// 3 - the first to break!
- tearDown();
- setUp();
doit(mk("DF/DF"), mk("DF/DF"), mk("DF"));
assertUpdated("DF/DF");
assertRemoved("DF");
-
+ }
+
+ public void testDirectoryFileConflicts_4() throws Exception {
// 4 (basically same as 3, just with H and M different)
- tearDown();
- setUp();
doit(mk("DF/DF"), mkmap("DF/DF", "foo"), mk("DF"));
assertUpdated("DF/DF");
assertRemoved("DF");
+ }
+
+ public void testDirectoryFileConflicts_5() throws Exception {
// 5
- tearDown();
- setUp();
doit(mk("DF/DF"), mk("DF"), mk("DF"));
assertRemoved("DF/DF");
+ }
+
+ public void testDirectoryFileConflicts_6() throws Exception {
// 6
- tearDown();
- setUp();
setupCase(mk("DF/DF"), mk("DF"), mk("DF"));
writeTrashFile("DF", "different");
go();
assertRemoved("DF/DF");
-
+ }
+
+ public void testDirectoryFileConflicts_7() throws Exception {
// 7
- tearDown();
- setUp();
doit(mk("DF"), mk("DF"), mk("DF/DF"));
assertUpdated("DF");
assertRemoved("DF/DF");
@@ -334,29 +339,40 @@ public class ReadTreeTest extends RepositoryTestCase {
assertConflict("DF/DF/DF/DF/DF");
assertUpdated("DF/DF");
+ }
+
+ // 8 ?
+
+ public void testDirectoryFileConflicts_9() throws Exception {
// 9
- tearDown();
- setUp();
doit(mk("DF"), mkmap("DF", "QP"), mk("DF/DF"));
assertRemoved("DF/DF");
assertUpdated("DF");
-
+ }
+
+ public void testDirectoryFileConflicts_10() throws Exception {
// 10
cleanUpDF();
doit(mk("DF"), mk("DF/DF"), mk("DF/DF"));
assertNoConflicts();
+ }
+
+ public void testDirectoryFileConflicts_11() throws Exception {
// 11
- cleanUpDF();
doit(mk("DF"), mk("DF/DF"), mkmap("DF/DF", "asdf"));
assertConflict("DF/DF");
-
+ }
+
+ public void testDirectoryFileConflicts_12() throws Exception {
// 12
cleanUpDF();
doit(mk("DF"), mk("DF/DF"), mk("DF"));
assertRemoved("DF");
assertUpdated("DF/DF");
-
+ }
+
+ public void testDirectoryFileConflicts_13() throws Exception {
// 13
cleanUpDF();
setupCase(mk("DF"), mk("DF/DF"), mk("DF"));
@@ -364,29 +380,39 @@ public class ReadTreeTest extends RepositoryTestCase {
go();
assertConflict("DF");
assertUpdated("DF/DF");
-
+ }
+
+ public void testDirectoryFileConflicts_14() throws Exception {
// 14
cleanUpDF();
doit(mk("DF"), mk("DF/DF"), mkmap("DF", "Foo"));
assertConflict("DF");
assertUpdated("DF/DF");
-
+ }
+
+ public void testDirectoryFileConflicts_15() throws Exception {
// 15
doit(mkmap(), mk("DF/DF"), mk("DF"));
assertRemoved("DF");
assertUpdated("DF/DF");
-
+ }
+
+ public void testDirectoryFileConflicts_15b() throws Exception {
// 15, take 2, just to check multi-leveled
doit(mkmap(), mk("DF/DF/DF/DF"), mk("DF"));
assertRemoved("DF");
assertUpdated("DF/DF/DF/DF");
-
+ }
+
+ public void testDirectoryFileConflicts_16() throws Exception {
// 16
cleanUpDF();
doit(mkmap(), mk("DF"), mk("DF/DF/DF"));
assertRemoved("DF/DF/DF");
assertUpdated("DF");
-
+ }
+
+ public void testDirectoryFileConflicts_17() throws Exception {
// 17
cleanUpDF();
setupCase(mkmap(), mk("DF"), mk("DF/DF/DF"));
@@ -394,13 +420,17 @@ public class ReadTreeTest extends RepositoryTestCase {
go();
assertConflict("DF/DF/DF");
assertUpdated("DF");
-
+ }
+
+ public void testDirectoryFileConflicts_18() throws Exception {
// 18
cleanUpDF();
doit(mk("DF/DF"), mk("DF/DF/DF/DF"), null);
assertRemoved("DF/DF");
assertUpdated("DF/DF/DF/DF");
-
+ }
+
+ public void testDirectoryFileConflicts_19() throws Exception {
// 19
cleanUpDF();
doit(mk("DF/DF/DF/DF"), mk("DF/DF/DF"), null);
--
1.5.4.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [EGIT PATCH 05/10] Fix git sort order compare bug
2008-02-23 23:50 ` [EGIT PATCH 04/10] Split a big test in ReadTreeTest into smaller tests Robin Rosenberg
@ 2008-02-23 23:50 ` Robin Rosenberg
2008-02-23 23:50 ` [EGIT PATCH 06/10] Use the proper comparison algorithm Robin Rosenberg
0 siblings, 1 reply; 12+ messages in thread
From: Robin Rosenberg @ 2008-02-23 23:50 UTC (permalink / raw)
To: git; +Cc: David Watson, Robin Rosenberg
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
.../tst/org/spearce/jgit/lib/T0002_Tree.java | 44 ++++++++++++++++++++
.../src/org/spearce/jgit/lib/Tree.java | 10 ++++-
2 files changed, 52 insertions(+), 2 deletions(-)
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0002_Tree.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0002_Tree.java
index 24b368f..7c7f6c0 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0002_Tree.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0002_Tree.java
@@ -17,6 +17,7 @@
package org.spearce.jgit.lib;
import java.io.IOException;
+import java.io.UnsupportedEncodingException;
import java.util.ArrayList;
import java.util.List;
@@ -24,6 +25,49 @@ public class T0002_Tree extends RepositoryTestCase {
private static final ObjectId SOME_FAKE_ID = new ObjectId(
"0123456789abcdef0123456789abcdef01234567");
+ private int compareNamesUsingSpecialCompare(String a,String b) throws UnsupportedEncodingException {
+ char lasta = '\0';
+ byte[] abytes;
+ if (a.length() > 0 && a.charAt(a.length()-1) == '/') {
+ lasta = '/';
+ a = a.substring(0, a.length() - 1);
+ }
+ abytes = a.getBytes("ISO-8859-1");
+ char lastb = '\0';
+ byte[] bbytes;
+ if (b.length() > 0 && b.charAt(b.length()-1) == '/') {
+ lastb = '/';
+ b = b.substring(0, b.length() - 1);
+ }
+ bbytes = b.getBytes("ISO-8859-1");
+ return Tree.compareNames(abytes, bbytes, lasta, lastb);
+ }
+
+ public void test000_sort_01() throws UnsupportedEncodingException {
+ assertEquals(0, compareNamesUsingSpecialCompare("a","a"));
+ }
+ public void test000_sort_02() throws UnsupportedEncodingException {
+ assertEquals(-1, compareNamesUsingSpecialCompare("a","b"));
+ assertEquals(1, compareNamesUsingSpecialCompare("b","a"));
+ }
+ public void test000_sort_03() throws UnsupportedEncodingException {
+ assertEquals(1, compareNamesUsingSpecialCompare("a:","a"));
+ assertEquals(1, compareNamesUsingSpecialCompare("a/","a"));
+ assertEquals(-1, compareNamesUsingSpecialCompare("a","a/"));
+ assertEquals(-1, compareNamesUsingSpecialCompare("a","a:"));
+ assertEquals(1, compareNamesUsingSpecialCompare("a:","a/"));
+ assertEquals(-1, compareNamesUsingSpecialCompare("a/","a:"));
+ }
+ public void test000_sort_04() throws UnsupportedEncodingException {
+ assertEquals(-1, compareNamesUsingSpecialCompare("a.a","a/a"));
+ assertEquals(1, compareNamesUsingSpecialCompare("a/a","a.a"));
+ }
+ public void test000_sort_05() throws UnsupportedEncodingException {
+ assertEquals(-1, compareNamesUsingSpecialCompare("a.","a/"));
+ assertEquals(1, compareNamesUsingSpecialCompare("a/","a."));
+
+ }
+
public void test001_createEmpty() throws IOException {
final Tree t = new Tree(db);
assertTrue("isLoaded", t.isLoaded());
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Tree.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Tree.java
index ab83917..5d8e0e0 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Tree.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Tree.java
@@ -65,7 +65,10 @@ public class Tree extends TreeEntry implements Treeish {
else if (aj > lastb)
return 1;
else
- return 0;
+ if (j == a.length - 1)
+ return 0;
+ else
+ return -1;
}
if (k < nameEnd) {
int bk = nameUTF8[k] & 0xff;
@@ -74,7 +77,10 @@ public class Tree extends TreeEntry implements Treeish {
else if (lasta > bk)
return 1;
else
- return 0;
+ if (k == nameEnd - 1)
+ return 0;
+ else
+ return 1;
}
if (lasta < lastb)
return -1;
--
1.5.4.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [EGIT PATCH 06/10] Use the proper comparison algorithm
2008-02-23 23:50 ` [EGIT PATCH 05/10] Fix git sort order compare bug Robin Rosenberg
@ 2008-02-23 23:50 ` Robin Rosenberg
2008-02-23 23:50 ` [EGIT PATCH 07/10] GitIndex: Get access to raw name and file mode Robin Rosenberg
0 siblings, 1 reply; 12+ messages in thread
From: Robin Rosenberg @ 2008-02-23 23:50 UTC (permalink / raw)
To: git; +Cc: David Watson, Robin Rosenberg
We must walk in Git sort order.
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
.../src/org/spearce/jgit/lib/IndexTreeWalker.java | 31 +------------------
1 files changed, 2 insertions(+), 29 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/IndexTreeWalker.java b/org.spearce.jgit/src/org/spearce/jgit/lib/IndexTreeWalker.java
index 93d5bb2..c17cea1 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/IndexTreeWalker.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/IndexTreeWalker.java
@@ -288,38 +288,11 @@ public class IndexTreeWalker {
}
static int compare(TreeEntry t, Entry i) {
- if (t.getFullName().equals(i.getName())) {
- if (t instanceof Tree)
- return 1;
- return 0;
- }
- return t.getFullName().compareTo(i.getName());
+ return Tree.compareNames(t.getFullNameUTF8(), i.getNameUTF8(), TreeEntry.lastChar(t), TreeEntry.lastChar(i));
}
static int compare(TreeEntry t1, TreeEntry t2) {
- if (t1.getName().equals(t2.getName())) {
- if (t1 instanceof Tree && t2 instanceof Tree)
- return 0;
- if (t1 instanceof Tree)
- return 1;
- if (t2 instanceof Tree)
- return -1;
- return 0;
- }
- return t1.getName().compareTo(t2.getName());
+ return Tree.compareNames(t1.getNameUTF8(), t2.getNameUTF8(), TreeEntry.lastChar(t1), TreeEntry.lastChar(t2));
}
- static int compare(byte[] name1, byte[] name2) {
- for (int i = 0; i < name1.length && i < name2.length; i++) {
- if (name1[i] < name2[i])
- return -1;
- if (name1[i] > name2[i])
- return 1;
- }
- if (name1.length < name2.length)
- return -1;
- if (name2.length < name1.length)
- return 1;
- return 0;
- }
}
--
1.5.4.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [EGIT PATCH 07/10] GitIndex: Get access to raw name and file mode
2008-02-23 23:50 ` [EGIT PATCH 06/10] Use the proper comparison algorithm Robin Rosenberg
@ 2008-02-23 23:50 ` Robin Rosenberg
2008-02-23 23:50 ` [EGIT PATCH 08/10] TreeEntry: Accessors for full raw name and mode bits Robin Rosenberg
0 siblings, 1 reply; 12+ messages in thread
From: Robin Rosenberg @ 2008-02-23 23:50 UTC (permalink / raw)
To: git; +Cc: David Watson, Robin Rosenberg
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
.../src/org/spearce/jgit/lib/GitIndex.java | 15 +++++++++++++++
1 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/GitIndex.java b/org.spearce.jgit/src/org/spearce/jgit/lib/GitIndex.java
index 69ed270..d73866a 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/GitIndex.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/GitIndex.java
@@ -598,6 +598,13 @@ public class GitIndex {
}
/**
+ * @return path name for this entry as byte array, hopefully UTF-8 encoded
+ */
+ public byte[] getNameUTF8() {
+ return name;
+ }
+
+ /**
* @return SHA-1 of the entry managed by this index
*/
public ObjectId getObjectId() {
@@ -655,6 +662,14 @@ public class GitIndex {
else
flags &= ~0x4000;
}
+
+ /**
+ * Return raw file mode bits. See {@link FileMode}
+ * @return file mode bits
+ */
+ public int getModeBits() {
+ return mode;
+ }
}
static class Header {
--
1.5.4.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [EGIT PATCH 08/10] TreeEntry: Accessors for full raw name and mode bits
2008-02-23 23:50 ` [EGIT PATCH 07/10] GitIndex: Get access to raw name and file mode Robin Rosenberg
@ 2008-02-23 23:50 ` Robin Rosenberg
2008-02-23 23:50 ` [EGIT PATCH 09/10] Implement a Tree iterator Robin Rosenberg
0 siblings, 1 reply; 12+ messages in thread
From: Robin Rosenberg @ 2008-02-23 23:50 UTC (permalink / raw)
To: git; +Cc: David Watson, Robin Rosenberg
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
.../src/org/spearce/jgit/lib/TreeEntry.java | 23 ++++++++++++++++++++
1 files changed, 23 insertions(+), 0 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/TreeEntry.java b/org.spearce.jgit/src/org/spearce/jgit/lib/TreeEntry.java
index 8d46230..e73f5d5 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/TreeEntry.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/TreeEntry.java
@@ -19,6 +19,8 @@ package org.spearce.jgit.lib;
import java.io.IOException;
import java.io.UnsupportedEncodingException;
+import org.spearce.jgit.lib.GitIndex.Entry;
+
/**
* This class represents an entry in a tree, like a blob or another tree.
*/
@@ -189,6 +191,14 @@ public abstract class TreeEntry implements Comparable {
return r.toString();
}
+ /**
+ * @return repository relative name of the entry
+ * FIXME better encoding
+ */
+ public byte[] getFullNameUTF8() {
+ return getFullName().getBytes();
+ }
+
public int compareTo(final Object o) {
if (this == o)
return 0;
@@ -211,6 +221,19 @@ public abstract class TreeEntry implements Comparable {
}
/**
+ * Helper for accessing tree/blob/index methods.
+ *
+ * @param i
+ * @return '/' for Tre entries and NUL for non-treeish objects
+ */
+ final public static int lastChar(Entry i) {
+ // FIXME, gitlink etc. Currently Trees cannot appear in the
+ // index so '\0' is always returned, except maybe for submodules
+ // which we do not support yet.
+ return FileMode.TREE.equals(i.getModeBits()) ? '/' : '\0';
+ }
+
+ /**
* See @{link {@link #accept(TreeVisitor, int)}.
*
* @param tv
--
1.5.4.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [EGIT PATCH 09/10] Implement a Tree iterator
2008-02-23 23:50 ` [EGIT PATCH 08/10] TreeEntry: Accessors for full raw name and mode bits Robin Rosenberg
@ 2008-02-23 23:50 ` Robin Rosenberg
2008-02-23 23:50 ` [EGIT PATCH 10/10] Rewritten IndexTreeWalker Robin Rosenberg
0 siblings, 1 reply; 12+ messages in thread
From: Robin Rosenberg @ 2008-02-23 23:50 UTC (permalink / raw)
To: git; +Cc: David Watson, Robin Rosenberg
This is an implementation of the Iterator interace. It can
be used for iterating over all entries in a tree in Git
order. Since it is an iterator it can be used to walk more
than one tree in parallel.
Although only post order is needed, preorder and leaves-only
mode is also implemented here.
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
.../spearce/jgit/lib/TreeIteratorLeafOnlyTest.java | 85 +++++++++
.../jgit/lib/TreeIteratorPostOrderTest.java | 103 +++++++++++
.../spearce/jgit/lib/TreeIteratorPreOrderTest.java | 103 +++++++++++
.../src/org/spearce/jgit/lib/TreeIterator.java | 182 ++++++++++++++++++++
4 files changed, 473 insertions(+), 0 deletions(-)
create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/lib/TreeIteratorLeafOnlyTest.java
create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/lib/TreeIteratorPostOrderTest.java
create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/lib/TreeIteratorPreOrderTest.java
create mode 100644 org.spearce.jgit/src/org/spearce/jgit/lib/TreeIterator.java
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/TreeIteratorLeafOnlyTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/TreeIteratorLeafOnlyTest.java
new file mode 100644
index 0000000..7d65982
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/TreeIteratorLeafOnlyTest.java
@@ -0,0 +1,85 @@
+package org.spearce.jgit.lib;
+
+import java.io.IOException;
+
+public class TreeIteratorLeafOnlyTest extends RepositoryTestCase {
+
+ /** Empty tree */
+ public void testEmpty() {
+ Tree tree = new Tree(db);
+ TreeIterator i = makeIterator(tree);
+ assertFalse(i.hasNext());
+ }
+
+ /**
+ * one file
+ *
+ * @throws IOException
+ */
+ public void testSimpleF1() throws IOException {
+ Tree tree = new Tree(db);
+ tree.addFile("x");
+ TreeIterator i = makeIterator(tree);
+ assertTrue(i.hasNext());
+ assertEquals("x", i.next().getName());
+ }
+
+ /**
+ * two files
+ *
+ * @throws IOException
+ */
+ public void testSimpleF2() throws IOException {
+ Tree tree = new Tree(db);
+ tree.addFile("a");
+ tree.addFile("x");
+ TreeIterator i = makeIterator(tree);
+ assertTrue(i.hasNext());
+ assertEquals("a", i.next().getName());
+ assertEquals("x", i.next().getName());
+ }
+
+ /**
+ * Empty tree
+ *
+ * @throws IOException
+ */
+ public void testSimpleT() throws IOException {
+ Tree tree = new Tree(db);
+ tree.addTree("a");
+ TreeIterator i = makeIterator(tree);
+ assertFalse(i.hasNext());
+ }
+
+ public void testTricky() throws IOException {
+ Tree tree = new Tree(db);
+ tree.addFile("a.b");
+ tree.addFile("a.c");
+ tree.addFile("a/b.b/b");
+ tree.addFile("a/b");
+ tree.addFile("a/c");
+ tree.addFile("a=c");
+ tree.addFile("a=d");
+
+ TreeIterator i = makeIterator(tree);
+ assertTrue(i.hasNext());
+ assertEquals("a.b", i.next().getFullName());
+ assertTrue(i.hasNext());
+ assertEquals("a.c", i.next().getFullName());
+ assertTrue(i.hasNext());
+ assertEquals("a/b", i.next().getFullName());
+ assertTrue(i.hasNext());
+ assertEquals("a/b.b/b", i.next().getFullName());
+ assertTrue(i.hasNext());
+ assertEquals("a/c", i.next().getFullName());
+ assertTrue(i.hasNext());
+ assertEquals("a=c", i.next().getFullName());
+ assertTrue(i.hasNext());
+ assertEquals("a=d", i.next().getFullName());
+ assertFalse(i.hasNext());
+ }
+
+ private TreeIterator makeIterator(Tree tree) {
+ return new TreeIterator(tree);
+ }
+}
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/TreeIteratorPostOrderTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/TreeIteratorPostOrderTest.java
new file mode 100644
index 0000000..a6def1d
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/TreeIteratorPostOrderTest.java
@@ -0,0 +1,103 @@
+package org.spearce.jgit.lib;
+
+import java.io.IOException;
+
+public class TreeIteratorPostOrderTest extends RepositoryTestCase {
+
+ /** Empty tree */
+ public void testEmpty() {
+ Tree tree = new Tree(db);
+ TreeIterator i = makeIterator(tree);
+ assertTrue(i.hasNext());
+ assertEquals("", i.next().getFullName());
+ assertFalse(i.hasNext());
+ }
+
+ /**
+ * one file
+ *
+ * @throws IOException
+ */
+ public void testSimpleF1() throws IOException {
+ Tree tree = new Tree(db);
+ tree.addFile("x");
+ TreeIterator i = makeIterator(tree);
+ assertTrue(i.hasNext());
+ assertEquals("x", i.next().getName());
+ assertTrue(i.hasNext());
+ assertEquals("", i.next().getFullName());
+ assertFalse(i.hasNext());
+ }
+
+ /**
+ * two files
+ *
+ * @throws IOException
+ */
+ public void testSimpleF2() throws IOException {
+ Tree tree = new Tree(db);
+ tree.addFile("a");
+ tree.addFile("x");
+ TreeIterator i = makeIterator(tree);
+ assertTrue(i.hasNext());
+ assertEquals("a", i.next().getName());
+ assertEquals("x", i.next().getName());
+ assertTrue(i.hasNext());
+ assertEquals("", i.next().getFullName());
+ assertFalse(i.hasNext());
+ }
+
+ /**
+ * Empty tree
+ *
+ * @throws IOException
+ */
+ public void testSimpleT() throws IOException {
+ Tree tree = new Tree(db);
+ tree.addTree("a");
+ TreeIterator i = makeIterator(tree);
+ assertTrue(i.hasNext());
+ assertEquals("a", i.next().getFullName());
+ assertTrue(i.hasNext());
+ assertEquals("", i.next().getFullName());
+ assertFalse(i.hasNext());
+ }
+
+ public void testTricky() throws IOException {
+ Tree tree = new Tree(db);
+ tree.addFile("a.b");
+ tree.addFile("a.c");
+ tree.addFile("a/b.b/b");
+ tree.addFile("a/b");
+ tree.addFile("a/c");
+ tree.addFile("a=c");
+ tree.addFile("a=d");
+
+ TreeIterator i = makeIterator(tree);
+ assertTrue(i.hasNext());
+ assertEquals("a.b", i.next().getFullName());
+ assertTrue(i.hasNext());
+ assertEquals("a.c", i.next().getFullName());
+ assertTrue(i.hasNext());
+ assertEquals("a/b", i.next().getFullName());
+ assertTrue(i.hasNext());
+ assertEquals("a/b.b/b", i.next().getFullName());
+ assertTrue(i.hasNext());
+ assertEquals("a/b.b", i.next().getFullName());
+ assertTrue(i.hasNext());
+ assertEquals("a/c", i.next().getFullName());
+ assertTrue(i.hasNext());
+ assertEquals("a", i.next().getFullName());
+ assertTrue(i.hasNext());
+ assertEquals("a=c", i.next().getFullName());
+ assertTrue(i.hasNext());
+ assertEquals("a=d", i.next().getFullName());
+ assertTrue(i.hasNext());
+ assertEquals("", i.next().getFullName());
+ assertFalse(i.hasNext());
+ }
+
+ private TreeIterator makeIterator(Tree tree) {
+ return new TreeIterator(tree, TreeIterator.Order.POSTORDER);
+ }
+}
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/TreeIteratorPreOrderTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/TreeIteratorPreOrderTest.java
new file mode 100644
index 0000000..e7e9bf4
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/TreeIteratorPreOrderTest.java
@@ -0,0 +1,103 @@
+package org.spearce.jgit.lib;
+
+import java.io.IOException;
+
+public class TreeIteratorPreOrderTest extends RepositoryTestCase {
+
+ /** Empty tree */
+ public void testEmpty() {
+ Tree tree = new Tree(db);
+ TreeIterator i = makeIterator(tree);
+ assertTrue(i.hasNext());
+ assertEquals("", i.next().getFullName());
+ assertFalse(i.hasNext());
+ }
+
+ /**
+ * one file
+ *
+ * @throws IOException
+ */
+ public void testSimpleF1() throws IOException {
+ Tree tree = new Tree(db);
+ tree.addFile("x");
+ TreeIterator i = makeIterator(tree);
+ assertTrue(i.hasNext());
+ assertEquals("", i.next().getFullName());
+ assertTrue(i.hasNext());
+ assertEquals("x", i.next().getName());
+ assertFalse(i.hasNext());
+ }
+
+ /**
+ * two files
+ *
+ * @throws IOException
+ */
+ public void testSimpleF2() throws IOException {
+ Tree tree = new Tree(db);
+ tree.addFile("a");
+ tree.addFile("x");
+ TreeIterator i = makeIterator(tree);
+ assertTrue(i.hasNext());
+ assertEquals("", i.next().getFullName());
+ assertTrue(i.hasNext());
+ assertEquals("a", i.next().getName());
+ assertEquals("x", i.next().getName());
+ assertFalse(i.hasNext());
+ }
+
+ /**
+ * Empty tree
+ *
+ * @throws IOException
+ */
+ public void testSimpleT() throws IOException {
+ Tree tree = new Tree(db);
+ tree.addTree("a");
+ TreeIterator i = makeIterator(tree);
+ assertTrue(i.hasNext());
+ assertEquals("", i.next().getFullName());
+ assertTrue(i.hasNext());
+ assertEquals("a", i.next().getFullName());
+ assertFalse(i.hasNext());
+ }
+
+ public void testTricky() throws IOException {
+ Tree tree = new Tree(db);
+ tree.addFile("a.b");
+ tree.addFile("a.c");
+ tree.addFile("a/b.b/b");
+ tree.addFile("a/b");
+ tree.addFile("a/c");
+ tree.addFile("a=c");
+ tree.addFile("a=d");
+
+ TreeIterator i = makeIterator(tree);
+ assertTrue(i.hasNext());
+ assertEquals("", i.next().getFullName());
+ assertTrue(i.hasNext());
+ assertEquals("a.b", i.next().getFullName());
+ assertTrue(i.hasNext());
+ assertEquals("a.c", i.next().getFullName());
+ assertTrue(i.hasNext());
+ assertEquals("a", i.next().getFullName());
+ assertTrue(i.hasNext());
+ assertEquals("a/b", i.next().getFullName());
+ assertTrue(i.hasNext());
+ assertEquals("a/b.b", i.next().getFullName());
+ assertTrue(i.hasNext());
+ assertEquals("a/b.b/b", i.next().getFullName());
+ assertTrue(i.hasNext());
+ assertEquals("a/c", i.next().getFullName());
+ assertTrue(i.hasNext());
+ assertEquals("a=c", i.next().getFullName());
+ assertTrue(i.hasNext());
+ assertEquals("a=d", i.next().getFullName());
+ assertFalse(i.hasNext());
+ }
+
+ private TreeIterator makeIterator(Tree tree) {
+ return new TreeIterator(tree, TreeIterator.Order.PREORDER);
+ }
+}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/TreeIterator.java b/org.spearce.jgit/src/org/spearce/jgit/lib/TreeIterator.java
new file mode 100644
index 0000000..e45d1fc
--- /dev/null
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/TreeIterator.java
@@ -0,0 +1,182 @@
+/*
+ * Copyright (C) 2008 Robin Rosenberg
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License, version 2.1, as published by the Free Software Foundation.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
+ */
+
+package org.spearce.jgit.lib;
+
+import java.io.IOException;
+import java.util.Iterator;
+
+/**
+ * A tree iterator iterates over a tree and all its members recursing into
+ * subtrees according to order.
+ *
+ * Default is to only visit leafs. An {@link Order} value can be supplied to
+ * make the iteration include Tree nodes as well either before or after the
+ * child nodes have been visited.
+ */
+public class TreeIterator implements Iterator<TreeEntry> {
+
+ private Tree tree;
+
+ private int index;
+
+ private TreeIterator sub;
+
+ private Order order;
+
+ private boolean visitTreeNodes;
+
+ private boolean hasVisitedTree;
+
+ /**
+ * Traversal order
+ */
+ public enum Order {
+ /**
+ * Visit node first, then leaves
+ */
+ PREORDER,
+
+ /**
+ * Visit leaves first, then node
+ */
+ POSTORDER
+ };
+
+ /**
+ * Construct a {@link TreeIterator} for visiting all non-tree nodes.
+ *
+ * @param start
+ */
+ public TreeIterator(Tree start) {
+ this(start, Order.PREORDER, false);
+ }
+
+ /**
+ * Construct a {@link TreeIterator} visiting all nodes in a tree in a given
+ * order.
+ *
+ * @param start Root node
+ * @param order {@link Order}
+ */
+ public TreeIterator(Tree start, Order order) {
+ this(start, order, true);
+ }
+
+ /**
+ * Construct a {@link TreeIterator}
+ *
+ * @param start First node to visit
+ * @param order Visitation {@link Order}
+ * @param visitTreeNode True to include tree node
+ */
+ private TreeIterator(Tree start, Order order, boolean visitTreeNode) {
+ this.tree = start;
+ this.visitTreeNodes = visitTreeNode;
+ this.index = -1;
+ this.order = order;
+ if (!visitTreeNodes)
+ this.hasVisitedTree = true;
+ try {
+ step();
+ } catch (IOException e) {
+ throw new Error(e);
+ }
+ }
+
+ public TreeEntry next() {
+ try {
+ TreeEntry ret = nextTreeEntry();
+ step();
+ return ret;
+ } catch (IOException e) {
+ throw new Error(e);
+ }
+ }
+
+ private TreeEntry nextTreeEntry() throws IOException {
+ TreeEntry ret;
+ if (sub != null)
+ ret = sub.nextTreeEntry();
+ else {
+ if (index < 0 && order == Order.PREORDER) {
+ return tree;
+ }
+ if (order == Order.POSTORDER && index == tree.memberCount()) {
+ return tree;
+ }
+ ret = tree.members()[index];
+ }
+ return ret;
+ }
+
+ public boolean hasNext() {
+ try {
+ return hasNextTreeEntry();
+ } catch (IOException e) {
+ throw new Error(e);
+ }
+ }
+
+ private boolean hasNextTreeEntry() throws IOException {
+ if (tree == null)
+ return false;
+ return sub != null
+ || index < tree.memberCount()
+ || order == Order.POSTORDER && index == tree.memberCount();
+ }
+
+ private boolean step() throws IOException {
+ if (tree == null)
+ return false;
+
+ if (sub != null) {
+ if (sub.step())
+ return true;
+ sub = null;
+ }
+
+ if (index < 0 && !hasVisitedTree && order == Order.PREORDER) {
+ hasVisitedTree = true;
+ return true;
+ }
+
+ while (++index < tree.memberCount()) {
+ TreeEntry e = tree.members()[index];
+ if (e instanceof Tree) {
+ sub = new TreeIterator((Tree) e, order, visitTreeNodes);
+ if (sub.hasNextTreeEntry())
+ return true;
+ sub = null;
+ continue;
+ }
+ return true;
+ }
+
+ if (index == tree.memberCount() && !hasVisitedTree
+ && order == Order.POSTORDER) {
+ hasVisitedTree = true;
+ return true;
+ }
+ return false;
+ }
+
+ public void remove() {
+ throw new IllegalStateException(
+ "TreeIterator does not suppport remove()");
+ }
+}
--
1.5.4.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [EGIT PATCH 10/10] Rewritten IndexTreeWalker
2008-02-23 23:50 ` [EGIT PATCH 09/10] Implement a Tree iterator Robin Rosenberg
@ 2008-02-23 23:50 ` Robin Rosenberg
0 siblings, 0 replies; 12+ messages in thread
From: Robin Rosenberg @ 2008-02-23 23:50 UTC (permalink / raw)
To: git; +Cc: David Watson, Robin Rosenberg
This one works better with git order and hopefull faster
though that is secondary. The IndexTreeWalker used the TreeIterator
for simplicity.
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
.../spearce/jgit/lib/AbstractIndexTreeVisitor.java | 2 +-
.../src/org/spearce/jgit/lib/IndexDiff.java | 3 +-
.../src/org/spearce/jgit/lib/IndexTreeVisitor.java | 3 +-
.../src/org/spearce/jgit/lib/IndexTreeWalker.java | 276 +++++++-------------
.../src/org/spearce/jgit/lib/WorkDirCheckout.java | 5 +-
5 files changed, 105 insertions(+), 184 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/AbstractIndexTreeVisitor.java b/org.spearce.jgit/src/org/spearce/jgit/lib/AbstractIndexTreeVisitor.java
index 6f5ede4..c8962f0 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/AbstractIndexTreeVisitor.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/AbstractIndexTreeVisitor.java
@@ -29,7 +29,7 @@ import org.spearce.jgit.lib.GitIndex.Entry;
*
*/
public class AbstractIndexTreeVisitor implements IndexTreeVisitor {
- public void finishVisitTree(Tree tree, Tree auxTree, int i, String curDir)
+ public void finishVisitTree(Tree tree, Tree auxTree, String curDir)
throws IOException {
// Empty
}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/IndexDiff.java b/org.spearce.jgit/src/org/spearce/jgit/lib/IndexDiff.java
index 5ed22d3..45fc084 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/IndexDiff.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/IndexDiff.java
@@ -69,7 +69,8 @@ public class IndexDiff {
added.add(indexEntry.getName());
anyChanges = true;
} else if (indexEntry == null) {
- removed.add(treeEntry.getFullName());
+ if (!(treeEntry instanceof Tree))
+ removed.add(treeEntry.getFullName());
anyChanges = true;
} else {
if (!treeEntry.getId().equals(indexEntry.getObjectId())) {
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/IndexTreeVisitor.java b/org.spearce.jgit/src/org/spearce/jgit/lib/IndexTreeVisitor.java
index 0510711..26f3a33 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/IndexTreeVisitor.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/IndexTreeVisitor.java
@@ -58,11 +58,10 @@ public interface IndexTreeVisitor {
*
* @param tree
* @param auxTree
- * @param i
* @param curDir
* @throws IOException
*/
- public void finishVisitTree(Tree tree, Tree auxTree, int i, String curDir) throws IOException;
+ public void finishVisitTree(Tree tree, Tree auxTree, String curDir) throws IOException;
/**
* Invoked after handling all child nodes of a tree, during two way merge.
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/IndexTreeWalker.java b/org.spearce.jgit/src/org/spearce/jgit/lib/IndexTreeWalker.java
index c17cea1..8323310 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/IndexTreeWalker.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/IndexTreeWalker.java
@@ -82,189 +82,92 @@ public class IndexTreeWalker {
* @throws IOException
*/
public void walk() throws IOException {
- walk(mainTree, newTree, "/");
+ walk(mainTree, newTree, "");
}
private void walk(Tree tree, Tree auxTree, String curDir) throws IOException {
- TreeEntry[] treeMembers = tree == null ? new TreeEntry[0] : tree.members();
- TreeEntry[] auxTreeMembers = auxTree == null ? new TreeEntry[0] : auxTree.members();
- int treeCounter = 0;
- int auxTreeCounter = 0;
-
+ TreeIterator mi = new TreeIterator(tree, TreeIterator.Order.POSTORDER);
+ TreeIterator ai = new TreeIterator(auxTree, TreeIterator.Order.POSTORDER);
+ TreeEntry m = mi.hasNext() ? mi.next() : null;
+ TreeEntry a = ai.hasNext() ? ai.next() : null;
int curIndexPos = indexCounter;
-
- while (treeCounter < treeMembers.length ||
- indexCounter < indexMembers.length ||
- auxTreeCounter < auxTreeMembers.length) {
- TreeEntry h = treeCounter < treeMembers.length ? treeMembers[treeCounter] : null;
- TreeEntry m = auxTreeCounter < auxTreeMembers.length ? auxTreeMembers[auxTreeCounter] : null;
- Entry i = indexCounter < indexMembers.length ? indexMembers[indexCounter] : null;
-
- String indexName = i == null ? null : i.getName();
- String treeName = h == null ? null : h.getFullName();
- String auxTreeName = m == null ? null : m.getFullName();
+ Entry i = indexCounter < indexMembers.length ? indexMembers[indexCounter++] : null;
+ while (m != null || a != null || i != null) {
+ int cmpma = compare(m, a);
+ int cmpmi = compare(m, i);
+ int cmpai = compare(a, i);
- if (treeName != null && indexName != null && auxTreeName != null) {
- if (eq(h, i) && eq(m, i)) {
- // H == I == M
- visitor.visitEntry(h, m, i, new File(root, treeName));
-
- treeCounter++;
- indexCounter++;
- auxTreeCounter++;
- } else if (eq(h, i) && lt(h, m)) {
- // H == I, H < M
- visitor.visitEntry(h, null, i, new File(root, treeName));
-
- treeCounter++;
- indexCounter++;
- } else if (eq(h, m) && lt(h, i)) {
- // H == M, H < I
- if (h instanceof Tree) {
- walk((Tree)h, (Tree)m, "/" + treeName);
- } else {
- visitor.visitEntry(h, m, null, new File(root, treeName));
- }
-
- treeCounter++;
- auxTreeCounter++;
-
-
- } else if (eq(m, i) && lt(i, h)) {
- // I == M, I < H
- visitor.visitEntry(null, m, i, new File(root, indexName));
-
- indexCounter++;
- auxTreeCounter++;
- } else if (lt(h, i) && lt(h, m)) {
- // H < I, H < M
- if (h instanceof Tree) {
- walk((Tree) h, null, "/" + treeName);
- } else {
- visitor.visitEntry(h, null, null, new File(root, treeName));
- }
-
- treeCounter++;
- } else if (lt(m, i) && lt(m, h)) {
- // M < I, M < H
- if (m instanceof Tree) {
- walk(null, (Tree) m, "/" + auxTreeName);
- } else {
- visitor.visitEntry(null, m, null, new File(root, auxTreeName));
- }
-
- auxTreeCounter++;
- } else { // index entry is first
- // I < H, I < M
- visitor.visitEntry(null, null, i, new File(root, indexName));
-
- indexCounter++;
- }
- } else if (treeName != null && indexName != null) {
- if (eq(h, i)) {
- if (threeTrees)
- visitor.visitEntry(h, null, i, new File(root, indexName));
- else visitor.visitEntry(h, i, new File(root, indexName));
-
- treeCounter++;
- indexCounter++;
- } else if (lt(h, i)) {
- if (h instanceof Tree)
- walk((Tree) h, null, "/" + treeName);
- else {
- if (threeTrees) {
- visitor.visitEntry(h, null, null, new File(root, treeName));
- } else visitor.visitEntry(h, null, new File(root, treeName));
- }
-
- treeCounter++;
- } else { // lt(i, h)
- if (threeTrees) {
- visitor.visitEntry(null, null, i, new File(root, indexName));
- } else visitor.visitEntry(null, i, new File(root, indexName));
-
- indexCounter++;
- }
- } else if (treeName != null && auxTreeName != null) {
- if (eq(h, m)) {
- if (h instanceof Tree) {
- walk((Tree) h, (Tree) m, "/" + treeName);
- } else {
- visitor.visitEntry(h, m, null, new File(root, treeName));
- }
-
- treeCounter++;
- auxTreeCounter++;
- } else if (lt(h, m)) {
- if (h instanceof Tree) {
- walk((Tree) h, null, "/" + treeName);
- } else {
- visitor.visitEntry(h, null, null, new File(root, treeName));
- }
-
- treeCounter++;
- } else { // lt(m, h)
- if (m instanceof Tree) {
- walk(null, (Tree) m, "/" + auxTreeName);
- } else {
- visitor.visitEntry(null, m, null, new File("/" + auxTreeName));
- }
-
- auxTreeCounter++;
- }
-
- } else if (indexName != null && auxTreeName != null) {
- if (eq(m, i)) {
- visitor.visitEntry(null, m, i, new File(root, indexName));
-
- auxTreeCounter++;
- indexCounter++;
- } else if (lt(m, i)) {
- if (m instanceof Tree)
- walk(null, (Tree) m, "/" + auxTreeName);
- else {
- visitor.visitEntry(null, m, null, new File(root, auxTreeName));
- }
-
- auxTreeCounter++;
- } else { // lt(i, m)
- visitor.visitEntry(null, null, i, new File(root, indexName));
-
- indexCounter++;
- }
- } else if (treeName != null) {
- if (h instanceof Tree) {
- walk((Tree) h, null, "/" + treeName);
- } else if (threeTrees)
- visitor.visitEntry(h, null, null, new File(root, treeName));
- else visitor.visitEntry(h, null, new File(root, treeName));
-
- treeCounter++;
- } else if (auxTreeName != null) {
- if (m instanceof Tree) {
- walk(null, (Tree)m, "/" + auxTreeName);
- } else visitor.visitEntry(null, m, null, new File(root, auxTreeName));
-
- auxTreeCounter++;
- } else if (indexName != null) {
- // need to check if we're done with this tree
- String curDirNoSlash = curDir.substring(1);
- if (!indexName.startsWith(curDirNoSlash + "/") && curDirNoSlash.length() != 0)
- break;
-
- if (threeTrees)
- visitor.visitEntry(null, null, i, new File(root, indexName));
- else visitor.visitEntry(null, i, new File(root, indexName));
- indexCounter++;
- }
+ TreeEntry pm = cmpma <= 0 && cmpmi <= 0 ? m : null;
+ TreeEntry pa = cmpma >= 0 && cmpai <= 0 ? a : null;
+ Entry pi = cmpmi >= 0 && cmpai >= 0 ? i : null;
+
+ if (pi != null)
+ visitEntry(pm, pa, pi, root);
+ else
+ finishVisitTree(pm, pa, curIndexPos, root);
+
+ if (pm != null) m = mi.hasNext() ? mi.next() : null;
+ if (pa != null) a = ai.hasNext() ? ai.next() : null;
+ if (pi != null) i = indexCounter < indexMembers.length ? indexMembers[indexCounter++] : null;
}
-
- if (threeTrees) {
- visitor.finishVisitTree(tree, auxTree, indexCounter - curIndexPos,
- curDir.substring(1));
- } else {
- visitor.finishVisitTree(tree, indexCounter - curIndexPos, curDir);
+ }
+
+ private void visitEntry(TreeEntry t1, TreeEntry t2,
+ Entry i, File root) throws IOException {
+
+ assert t1 != null || t2 != null || i != null : "Needs at least one entry";
+ assert root != null : "Needs workdir";
+
+ if (t1 != null && t1.getParent() == null)
+ t1 = null;
+ if (t2 != null && t2.getParent() == null)
+ t2 = null;
+
+ File f = null;
+ if (i != null)
+ f = new File(root, i.getName());
+ else if (t1 != null)
+ f = new File(root, t1.getFullName());
+ else if (t2 != null)
+ f = new File(root, t2.getFullName());
+
+ if (t1 != null || t2 != null || i != null)
+ if (threeTrees)
+ visitor.visitEntry(t1, t2, i, f);
+ else
+ visitor.visitEntry(t1, i, f);
+ }
+
+ private void finishVisitTree(TreeEntry t1, TreeEntry t2, int curIndexPos, File root)
+ throws IOException {
+
+ assert t1 != null || t2 != null : "Needs at least one entry";
+ assert root != null : "Needs workdir";
+
+ if (t1 != null && t1.getParent() == null)
+ t1 = null;
+ if (t2 != null && t2.getParent() == null)
+ t2 = null;
+
+ File f = null;
+ String c= null;
+ if (t1 != null) {
+ c = t1.getFullName();
+ f = new File(root, c);
+ } else if (t2 != null) {
+ c = t2.getFullName();
+ f = new File(root, c);
}
+ if (t1 instanceof Tree || t2 instanceof Tree)
+ if (threeTrees)
+ visitor.finishVisitTree((Tree)t1, (Tree)t2, c);
+ else
+ visitor.finishVisitTree((Tree)t1, indexCounter - curIndexPos, c);
+ else if (t1 != null || t2 != null)
+ if (threeTrees)
+ visitor.visitEntry(t1, t2, null, f);
+ else
+ visitor.visitEntry(t1, null, f);
}
static boolean lt(TreeEntry h, Entry i) {
@@ -288,11 +191,30 @@ public class IndexTreeWalker {
}
static int compare(TreeEntry t, Entry i) {
+ if (t == null && i == null)
+ return 0;
+ if (t == null)
+ return 1;
+ if (i == null)
+ return -1;
return Tree.compareNames(t.getFullNameUTF8(), i.getNameUTF8(), TreeEntry.lastChar(t), TreeEntry.lastChar(i));
}
static int compare(TreeEntry t1, TreeEntry t2) {
- return Tree.compareNames(t1.getNameUTF8(), t2.getNameUTF8(), TreeEntry.lastChar(t1), TreeEntry.lastChar(t2));
+ if (t1 != null && t1.getParent() == null && t2 != null && t2.getParent() == null)
+ return 0;
+ if (t1 != null && t1.getParent() == null)
+ return -1;
+ if (t2 != null && t2.getParent() == null)
+ return 1;
+
+ if (t1 == null && t2 == null)
+ return 0;
+ if (t1 == null)
+ return 1;
+ if (t2 == null)
+ return -1;
+ return Tree.compareNames(t1.getFullNameUTF8(), t2.getFullNameUTF8(), TreeEntry.lastChar(t1), TreeEntry.lastChar(t2));
}
}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/WorkDirCheckout.java b/org.spearce.jgit/src/org/spearce/jgit/lib/WorkDirCheckout.java
index 04785e4..40ad332 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/WorkDirCheckout.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/WorkDirCheckout.java
@@ -243,11 +243,10 @@ public class WorkDirCheckout {
}
@Override
- public void finishVisitTree(Tree tree, Tree auxTree, int i,
- String curDir) throws IOException {
+ public void finishVisitTree(Tree tree, Tree auxTree, String curDir) throws IOException {
if (curDir.length() == 0) return;
- if (auxTree != null && i == 0) {
+ if (auxTree != null) {
if (index.getEntry(curDir) != null)
removed.add(curDir);
}
--
1.5.4.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [EGIT] Sort order from hell fixes, take 2
2008-02-23 23:50 [EGIT] Sort order from hell fixes, take 2 Robin Rosenberg
2008-02-23 23:50 ` [EGIT PATCH 01/10] Tighten IndexDiffTest to make it test better what it claims to test Robin Rosenberg
@ 2008-03-03 14:17 ` David Watson
1 sibling, 0 replies; 12+ messages in thread
From: David Watson @ 2008-03-03 14:17 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
I've looked over this a bit, and it looks like a good set of changes.
On Sun, Feb 24, 2008 at 12:50:33AM +0100, Robin Rosenberg wrote:
> Hi fans,
>
> My previous attempt to fix this failed, so here is another round, including
> some new infrastructure like a TreeIterator to support this fix and whatever
> will need it.
>
> Feed free to scrutize and invent whatever evil test case might be missing.
>
> The reason I noticed the problem was introduced in c20142, where the unit
> tests for org.spearce.jgit was moved to the new project org.spearce.jgit.test .
>
> -- robin
>
>
--
Dave Watson
Software Engineer
MIMvista Corp
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2008-03-03 14:17 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-02-23 23:50 [EGIT] Sort order from hell fixes, take 2 Robin Rosenberg
2008-02-23 23:50 ` [EGIT PATCH 01/10] Tighten IndexDiffTest to make it test better what it claims to test Robin Rosenberg
2008-02-23 23:50 ` [EGIT PATCH 02/10] Extend IndexDiffTest with more tests Robin Rosenberg
2008-02-23 23:50 ` [EGIT PATCH 03/10] WorkdirCheckout: more test for names that are close Robin Rosenberg
2008-02-23 23:50 ` [EGIT PATCH 04/10] Split a big test in ReadTreeTest into smaller tests Robin Rosenberg
2008-02-23 23:50 ` [EGIT PATCH 05/10] Fix git sort order compare bug Robin Rosenberg
2008-02-23 23:50 ` [EGIT PATCH 06/10] Use the proper comparison algorithm Robin Rosenberg
2008-02-23 23:50 ` [EGIT PATCH 07/10] GitIndex: Get access to raw name and file mode Robin Rosenberg
2008-02-23 23:50 ` [EGIT PATCH 08/10] TreeEntry: Accessors for full raw name and mode bits Robin Rosenberg
2008-02-23 23:50 ` [EGIT PATCH 09/10] Implement a Tree iterator Robin Rosenberg
2008-02-23 23:50 ` [EGIT PATCH 10/10] Rewritten IndexTreeWalker Robin Rosenberg
2008-03-03 14:17 ` [EGIT] Sort order from hell fixes, take 2 David Watson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).