From 614a477b079412d05762808b7ab0d014cf05e80f Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Mon, 5 May 2014 12:42:23 -0700 Subject: [PATCH 1/3] RevWalkTextBuiltin: Add -n to limit number of commits returned Implementing the -<#> flag from C git is less trivial. Change-Id: Idb55a303304a6d4055aaf37d0b4dcf92c684e25f --- .../src/org/eclipse/jgit/pgm/RevWalkTextBuiltin.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/RevWalkTextBuiltin.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/RevWalkTextBuiltin.java index e23fb356a..94156aa7e 100644 --- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/RevWalkTextBuiltin.java +++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/RevWalkTextBuiltin.java @@ -146,6 +146,9 @@ abstract class RevWalkTextBuiltin extends TextBuiltin { revLimiter.add(MessageRevFilter.create(msg)); } + @Option(name = "--max-count", aliases = "-n", metaVar = "metaVar_n") + private int maxCount = -1; + @Override protected void run() throws Exception { walk = createWalk(); @@ -218,7 +221,8 @@ abstract class RevWalkTextBuiltin extends TextBuiltin { protected int walkLoop() throws Exception { int n = 0; for (final RevCommit c : walk) { - n++; + if (++n > maxCount && maxCount >= 0) + break; show(c); } if (walk instanceof ObjectWalk) { From dbf922ce91c2bcde8b4604cc2a4ed8ecf8de54f4 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Mon, 5 May 2014 15:15:13 -0700 Subject: [PATCH 2/3] RevWalk: Allow disabling parent rewriting Previously, setting any TreeFilter on a RevWalk triggered parent rewriting, which in the current StartGenerator implementation ends up buffering the entire commit history in memory. Aside from causing poor performance on large histories, this does not match the default behavior of `git rev-list`, which does not rewrite parent SHAs unless asked to via --parents/--children. Add a new method setRewriteParents() to RevWalk to disable this behavior. Continue rewriting parents by default to maintain backwards compatibility. Change-Id: I1f38e05526071c75ca58095e312663de5e6f334d --- .../jgit/revwalk/RevWalkPathFilter1Test.java | 94 +++++++++++++++++++ .../src/org/eclipse/jgit/revwalk/RevWalk.java | 29 +++++- .../eclipse/jgit/revwalk/StartGenerator.java | 4 +- 3 files changed, 124 insertions(+), 3 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkPathFilter1Test.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkPathFilter1Test.java index d2083e6b5..6ec529c31 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkPathFilter1Test.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkPathFilter1Test.java @@ -128,6 +128,25 @@ public class RevWalkPathFilter1Test extends RevWalkTestCase { assertNull(rw.next()); } + @Test + public void testStringOfPearls_FilePath1_NoParentRewriting() + throws Exception { + final RevCommit a = commit(tree(file("d/f", blob("a")))); + final RevCommit b = commit(tree(file("d/f", blob("a"))), a); + final RevCommit c = commit(tree(file("d/f", blob("b"))), b); + filter("d/f"); + markStart(c); + rw.setRewriteParents(false); + + assertCommit(c, rw.next()); + assertEquals(1, c.getParentCount()); + assertCommit(b, c.getParent(0)); + + assertCommit(a, rw.next()); // b was skipped + assertEquals(0, a.getParentCount()); + assertNull(rw.next()); + } + @Test public void testStringOfPearls_FilePath2() throws Exception { final RevCommit a = commit(tree(file("d/f", blob("a")))); @@ -147,6 +166,28 @@ public class RevWalkPathFilter1Test extends RevWalkTestCase { assertNull(rw.next()); } + @Test + public void testStringOfPearls_FilePath2_NoParentRewriting() + throws Exception { + final RevCommit a = commit(tree(file("d/f", blob("a")))); + final RevCommit b = commit(tree(file("d/f", blob("a"))), a); + final RevCommit c = commit(tree(file("d/f", blob("b"))), b); + final RevCommit d = commit(tree(file("d/f", blob("b"))), c); + filter("d/f"); + markStart(d); + rw.setRewriteParents(false); + + // d was skipped + assertCommit(c, rw.next()); + assertEquals(1, c.getParentCount()); + assertCommit(b, c.getParent(0)); + + // b was skipped + assertCommit(a, rw.next()); + assertEquals(0, a.getParentCount()); + assertNull(rw.next()); + } + @Test public void testStringOfPearls_DirPath2() throws Exception { final RevCommit a = commit(tree(file("d/f", blob("a")))); @@ -166,6 +207,28 @@ public class RevWalkPathFilter1Test extends RevWalkTestCase { assertNull(rw.next()); } + @Test + public void testStringOfPearls_DirPath2_NoParentRewriting() + throws Exception { + final RevCommit a = commit(tree(file("d/f", blob("a")))); + final RevCommit b = commit(tree(file("d/f", blob("a"))), a); + final RevCommit c = commit(tree(file("d/f", blob("b"))), b); + final RevCommit d = commit(tree(file("d/f", blob("b"))), c); + filter("d"); + markStart(d); + rw.setRewriteParents(false); + + // d was skipped + assertCommit(c, rw.next()); + assertEquals(1, c.getParentCount()); + assertCommit(b, c.getParent(0)); + + // b was skipped + assertCommit(a, rw.next()); + assertEquals(0, a.getParentCount()); + assertNull(rw.next()); + } + @Test public void testStringOfPearls_FilePath3() throws Exception { final RevCommit a = commit(tree(file("d/f", blob("a")))); @@ -192,4 +255,35 @@ public class RevWalkPathFilter1Test extends RevWalkTestCase { assertEquals(0, a.getParentCount()); assertNull(rw.next()); } + + @Test + public void testStringOfPearls_FilePath3_NoParentRewriting() + throws Exception { + final RevCommit a = commit(tree(file("d/f", blob("a")))); + final RevCommit b = commit(tree(file("d/f", blob("a"))), a); + final RevCommit c = commit(tree(file("d/f", blob("b"))), b); + final RevCommit d = commit(tree(file("d/f", blob("b"))), c); + final RevCommit e = commit(tree(file("d/f", blob("b"))), d); + final RevCommit f = commit(tree(file("d/f", blob("b"))), e); + final RevCommit g = commit(tree(file("d/f", blob("b"))), f); + final RevCommit h = commit(tree(file("d/f", blob("b"))), g); + final RevCommit i = commit(tree(file("d/f", blob("c"))), h); + filter("d/f"); + markStart(i); + rw.setRewriteParents(false); + + assertCommit(i, rw.next()); + assertEquals(1, i.getParentCount()); + assertCommit(h, i.getParent(0)); + + // h..d was skipped + assertCommit(c, rw.next()); + assertEquals(1, c.getParentCount()); + assertCommit(b, c.getParent(0)); + + // b was skipped + assertCommit(a, rw.next()); + assertEquals(0, a.getParentCount()); + assertNull(rw.next()); + } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevWalk.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevWalk.java index b3c4cced7..79cc42d17 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevWalk.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevWalk.java @@ -190,6 +190,8 @@ public class RevWalk implements Iterable { private boolean retainBody; + private boolean rewriteParents = true; + boolean shallowCommitsInitialized; /** @@ -533,8 +535,9 @@ public class RevWalk implements Iterable { * will not be simplified. *

* If non-null and not {@link TreeFilter#ALL} then the tree filter will be - * installed and commits will have their ancestry simplified to hide commits - * that do not contain tree entries matched by the filter. + * installed. Commits will have their ancestry simplified to hide commits that + * do not contain tree entries matched by the filter, unless + * {@code setRewriteParents(false)} is called. *

* Usually callers should be inserting a filter graph including * {@link TreeFilter#ANY_DIFF} along with one or more @@ -550,6 +553,28 @@ public class RevWalk implements Iterable { treeFilter = newFilter != null ? newFilter : TreeFilter.ALL; } + /** + * Set whether to rewrite parent pointers when filtering by modified paths. + *

+ * By default, when {@link #setTreeFilter(TreeFilter)} is called with non- + * null and non-{@link TreeFilter#ALL} filter, commits will have their + * ancestry simplified and parents rewritten to hide commits that do not match + * the filter. + *

+ * This behavior can be bypassed by passing false to this method. + * + * @param rewrite + * whether to rewrite parents; defaults to true. + * @since 3.4 + */ + public void setRewriteParents(boolean rewrite) { + rewriteParents = rewrite; + } + + boolean getRewriteParents() { + return rewriteParents; + } + /** * Should the body of a commit or tag be retained after parsing its headers? *

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/StartGenerator.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/StartGenerator.java index 5b264fcf3..9c4e53c97 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/StartGenerator.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/StartGenerator.java @@ -128,7 +128,9 @@ class StartGenerator extends Generator { pending = new DateRevQueue(q); if (tf != TreeFilter.ALL) { rf = AndRevFilter.create(new RewriteTreeFilter(w, tf), rf); - pendingOutputType |= HAS_REWRITE | NEEDS_REWRITE; + pendingOutputType |= HAS_REWRITE; + if (w.getRewriteParents()) + pendingOutputType |= NEEDS_REWRITE; } walker.queue = q; From 99008648d176c04842f8ccb99177579d0cbe89a0 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Mon, 5 May 2014 15:21:16 -0700 Subject: [PATCH 3/3] Do not rewrite parents in RevWalkTextBuiltins Default behavior of C git is to skip parent rewriting unless particular history simplification or --parents flags are passed. Since JGit has no such flags, JGit should not rewrite parents. Change-Id: I9ba0e70fe6d5f49f975b71eea46f93198900f37d --- .../src/org/eclipse/jgit/pgm/RevWalkTextBuiltin.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/RevWalkTextBuiltin.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/RevWalkTextBuiltin.java index 94156aa7e..d6063c31b 100644 --- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/RevWalkTextBuiltin.java +++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/RevWalkTextBuiltin.java @@ -211,11 +211,15 @@ abstract class RevWalkTextBuiltin extends TextBuiltin { } protected RevWalk createWalk() { + RevWalk result; if (objects) - return new ObjectWalk(db); - if (argWalk != null) - return argWalk; - return argWalk = new RevWalk(db); + result = new ObjectWalk(db); + else if (argWalk != null) + result = argWalk; + else + result = argWalk = new RevWalk(db); + result.setRewriteParents(false); + return result; } protected int walkLoop() throws Exception {