Git development
 help / color / mirror / Atom feed
* Re: [PATCH][v2] http authentication via prompts (with correct line lengths)
From: Junio C Hamano @ 2009-03-14  6:43 UTC (permalink / raw)
  To: Mike Gaffney; +Cc: Mike Ralphson, git
In-Reply-To: <49BA55E2.1060604@gmail.com>

Mike Gaffney <mr.gaffo@gmail.com> writes:

> I was going to try and clean this up this weekend or early next week. I'm also
> trying to encourage open source submissions at work and was using this
> as an example patch to get people going (we need the fix to use git). So
> I do plan finishing this, just have to do it when I have time.

Thanks.

^ permalink raw reply

* Re: [PATCH] read-tree A B C: do not create a bogus index and do not segfault
From: Junio C Hamano @ 2009-03-14  6:41 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Daniel Barkalow, git, Jiri Olsa, Johannes Schindelin
In-Reply-To: <alpine.LFD.2.00.0903120929250.32478@localhost.localdomain>

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Thu, 12 Mar 2009, Daniel Barkalow wrote:
>
>> I think it might be a good idea to take this as evidence that nobody is 
>> using read-tree with multiple trees without merge, and just disallow it. 
>
> Hmm. It _has_ been used. It's been useful for really odd things 
> historically, namely trying to merge different trees by hand. So while I 
> agree that we could probably remove it, it _is_ a very interesting 
> feature in theory, and since we have the code.. 
>
> So I'd say "add a few tests for the known horrible cases" should be the 
> first approach. If it ever actually breaks again and becomes a big 
> maintenance headache, maybe _then_ remove the feature as not being worth 
> the pain?

I've added trivial tests and will start cooking it by letting it go
through the usual pu/next/master/maint cycle.

I think you are thinking about something like the "gitk" merge (aka "The
coolest merge EVER!" [*1*]), but you can do the same thing more safely
with -m option, giving an empty tree as the common ancestor.  Especially
if you run it in the aggressive mode, "addition" from our tree and their
tree, when it is an overlay, will not overlap, and will all cleanly
resolve to stage #0.

I suspect you can also use a single tree "read-tree", immediately followed
by another "read-tree --prefix=''" to read in two trees into the index and
write the results out.

[Footnote]

*1* http://article.gmane.org/gmane.comp.version-control.git/5126

^ permalink raw reply

* Re: [PATCH][v2] http authentication via prompts (with correct line  lengths)
From: Junio C Hamano @ 2009-03-14  5:55 UTC (permalink / raw)
  To: Mike Ralphson; +Cc: Daniel Stenberg, Mike Gaffney, git
In-Reply-To: <e2b179460903130353p1d3c1cb2n8286c2a284724156@mail.gmail.com>

Mike Ralphson <mike.ralphson@gmail.com> writes:

> 2009/3/13 Daniel Stenberg <daniel@haxx.se>:
>>Driven by use cases such as this, I also recently produced the
>>"symbols-in-versions" document in the libcurl tree which should
>> help apps to know what should works when:
>
>> http://cool.haxx.se/cvs.cgi/curl/docs/libcurl/symbols-in-versions?rev=HEAD&content-type=text/vnd.viewcvs-markup
>
> Very helpful, thanks.

Yeah, I wish we new about it much earlier.  Thanks, Daniel.

> Junio, if I check all the unprotected CURL* options against this list,
> would that give us our absolute minimum supported version? If so,
> would it then be ok to remove any unnecessary ifdefs for lower
> versions if they exist?

Sounds like a good plan.  Please get the ball rolling.

^ permalink raw reply

* Re: [PATCH 2/2] [TopGit] Portability: Don't use alternation ("|") in sed regular expressions
From: Junio C Hamano @ 2009-03-14  5:54 UTC (permalink / raw)
  To: martin f krafft; +Cc: Uwe Kleine-König, Brian Campbell, git, Petr Baudis
In-Reply-To: <20090312152630.GA26379@piper.oerlikon.madduck.net>

martin f krafft <madduck@madduck.net> writes:

> also sprach Uwe Kleine-König <u.kleine-koenig@pengutronix.de> [2009.03.12.1620 +0100]:
>> IMHO we should reuse as much as possible from git.git.  For me even
>> requiring a git.git checkout to use its files would be OK.  I consider
>> that even better then duplicating the relevant files.
>
> Maybe we could even start to think about integrating TopGit back
> into git.git?

Heh, it would need massive style fixes before that happens. I am fairly
picky on shell script styles.

^ permalink raw reply

* Re: [PATCH 1/2] Add a test for checking whether gitattributes is honored by checkout.
From: Junio C Hamano @ 2009-03-14  4:36 UTC (permalink / raw)
  To: Kristian Amlie; +Cc: git
In-Reply-To: <1236950656-1967-2-git-send-email-kristian.amlie@nokia.com>

Please do not waste new tests for these.  I think they can be added to
t0020-crlf.sh or t0021-conversion.sh.

^ permalink raw reply

* Re: [PATCH 2/2] Make Git respect changes to .gitattributes during checkout.
From: Junio C Hamano @ 2009-03-14  4:17 UTC (permalink / raw)
  To: Kristian Amlie; +Cc: git
In-Reply-To: <1236950656-1967-3-git-send-email-kristian.amlie@nokia.com>

Kristian Amlie <kristian.amlie@nokia.com> writes:

> The fix is twofold:
>
> First, we force .gitattributes files to always be the first ones
> checked out. This is the part in check_updates().
>
> Second, we make sure that the checked out attributes get honored by
> popping off elements on the attribute stack, until we reach the level
> where a new .gitattributes was checked out. The next time someone
> calls git_checkattr(), it will reconstruct the attributes from that
> point.

Yikes.  The patch is too ugly beyond words.

In a well structured git program, we always read from the work tree and
the index (to see if there is something changed---you need to be able to
apply convert_to_git when you do this), internally compute what should
happen (e.g. decide that the new result needs to be checked out for a
path), and then write it out (you apply convert_to_working_tree while you
do this).  So how about doing something like the attached patch?

The patch allows the caller to tell the usual "read from the working tree,
if not use the version from the index as the fallback" logic to be swapped
around, and take the index version.  It may or may not pass your new tests
(I haven't even compile tested it), but I think the damage is minimized
compared to your version.

It is great that you are trying to fix this issue for the most obvious
"switching between two branches while not having a local change" case, but
frankly, I do not think this is solvable in more general cases, and that
is why it was kept "known to be broken, not worth fixing" state.

For example, you may notice that, after making a clean checkout, one path
has a wrong attribute assigned to it, and may try to correct it.  But how?

 $ edit .gitattributes ;# mark foo.dat as binary
 $ rm foo.dat
 $ git checkout foo.dat ;# make sure the new settings is correct???

Without this patch, this would have worked as expected, because we always
use the one from the work tree if available.

We have not "git add"ed .gitattributes yet because we would want to make
sure the change is correct before doing so.  The patch takes attributes
for foo.dat from the old .gitattributes that is sitting in the index,
which still has the wrong information, so in that sense, it is a
regression.

To fix that, I _think_ you can further hack read_attr_from_index() to see
if the attribute file is marked with CE_UPDATE, i.e. it is something we
are checking out in this invocation of "check_updates()", and use it only
if it is, or something like that, but there probably are more corner cases
in which whichever one between the work tree and the index we take, it is
a wrong one.

There are other codepaths that call checkout_entry() that needs a
treatment similar to the one done to check_updates() by this patch, and I
suspect there are a few places that we do not even use checkout_entry().
I think you can fairly easily wrap write_out_results() in builtin-apply.c
in a similar way.  I do not know what merge-recursive does offhand, but I
suspect it would be a lot harder to fix.

Anyway, here is the patch.

 attr.c         |   63 ++++++++++++++++++++++++++++++++++++++++++++-----------
 attr.h         |    6 +++++
 unpack-trees.c |    3 ++
 3 files changed, 59 insertions(+), 13 deletions(-)

diff --git a/attr.c b/attr.c
index 17f6a4d..72f6807 100644
--- a/attr.c
+++ b/attr.c
@@ -1,3 +1,4 @@
+#define NO_THE_INDEX_COMPATIBILITY_MACROS
 #include "cache.h"
 #include "attr.h"
 
@@ -318,6 +319,9 @@ static struct attr_stack *read_attr_from_array(const char **list)
 	return res;
 }
 
+static enum git_attr_direction direction;
+static struct index_state *use_index;
+
 static struct attr_stack *read_attr_from_file(const char *path, int macro_ok)
 {
 	FILE *fp = fopen(path, "r");
@@ -340,9 +344,10 @@ static void *read_index_data(const char *path)
 	unsigned long sz;
 	enum object_type type;
 	void *data;
+	struct index_state *istate = use_index ? use_index : &the_index;
 
 	len = strlen(path);
-	pos = cache_name_pos(path, len);
+	pos = index_name_pos(istate, path, len);
 	if (pos < 0) {
 		/*
 		 * We might be in the middle of a merge, in which
@@ -350,15 +355,15 @@ static void *read_index_data(const char *path)
 		 */
 		int i;
 		for (i = -pos - 1;
-		     (pos < 0 && i < active_nr &&
-		      !strcmp(active_cache[i]->name, path));
+		     (pos < 0 && i < istate->cache_nr &&
+		      !strcmp(istate->cache[i]->name, path));
 		     i++)
-			if (ce_stage(active_cache[i]) == 2)
+			if (ce_stage(istate->cache[i]) == 2)
 				pos = i;
 	}
 	if (pos < 0)
 		return NULL;
-	data = read_sha1_file(active_cache[pos]->sha1, &type, &sz);
+	data = read_sha1_file(istate->cache[pos]->sha1, &type, &sz);
 	if (!data || type != OBJ_BLOB) {
 		free(data);
 		return NULL;
@@ -366,18 +371,12 @@ static void *read_index_data(const char *path)
 	return data;
 }
 
-static struct attr_stack *read_attr(const char *path, int macro_ok)
+static struct attr_stack *read_attr_from_index(const char *path, int macro_ok)
 {
 	struct attr_stack *res;
 	char *buf, *sp;
 	int lineno = 0;
 
-	res = read_attr_from_file(path, macro_ok);
-	if (res)
-		return res;
-
-	res = xcalloc(1, sizeof(*res));
-
 	/*
 	 * There is no checked out .gitattributes file there, but
 	 * we might have it in the index.  We allow operation in a
@@ -385,8 +384,9 @@ static struct attr_stack *read_attr(const char *path, int macro_ok)
 	 */
 	buf = read_index_data(path);
 	if (!buf)
-		return res;
+		return NULL;
 
+	res = xcalloc(1, sizeof(*res));
 	for (sp = buf; *sp; ) {
 		char *ep;
 		int more;
@@ -401,6 +401,25 @@ static struct attr_stack *read_attr(const char *path, int macro_ok)
 	return res;
 }
 
+static struct attr_stack *read_attr(const char *path, int macro_ok)
+{
+	struct attr_stack *res;
+
+	if (direction == GIT_ATTR_CHECKOUT) {
+		res = read_attr_from_index(path, macro_ok);
+		if (!res)
+			res = read_attr_from_file(path, macro_ok);
+	}
+	else {
+		res = read_attr_from_file(path, macro_ok);
+		if (!res)
+			res = read_attr_from_index(path, macro_ok);
+	}
+	if (!res)
+		res = xcalloc(1, sizeof(*res));
+	return res;
+}
+
 #if DEBUG_ATTR
 static void debug_info(const char *what, struct attr_stack *elem)
 {
@@ -428,6 +447,15 @@ static void debug_set(const char *what, const char *match, struct git_attr *attr
 #define debug_set(a,b,c,d) do { ; } while (0)
 #endif
 
+static void drop_attr_stack(void)
+{
+	while (attr_stack) {
+		struct attr_stack *elem = attr_stack;
+		attr_stack = elem->prev;
+		free_attr_elem(elem);
+	}
+}
+
 static void bootstrap_attr_stack(void)
 {
 	if (!attr_stack) {
@@ -642,3 +670,12 @@ int git_checkattr(const char *path, int num, struct git_attr_check *check)
 
 	return 0;
 }
+
+void git_attr_set_direction(enum git_attr_direction new, struct index_state *istate)
+{
+	enum git_attr_direction old = direction;
+	direction = new;
+	if (new != old)
+		drop_attr_stack();
+	use_index = istate;
+}
diff --git a/attr.h b/attr.h
index f1c2038..3a2f4ec 100644
--- a/attr.h
+++ b/attr.h
@@ -31,4 +31,10 @@ struct git_attr_check {
 
 int git_checkattr(const char *path, int, struct git_attr_check *);
 
+enum git_attr_direction {
+	GIT_ATTR_CHECKIN,
+	GIT_ATTR_CHECKOUT
+};
+void git_attr_set_direction(enum git_attr_direction, struct index_state *);
+
 #endif /* ATTR_H */
diff --git a/unpack-trees.c b/unpack-trees.c
index e547282..661218c 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -7,6 +7,7 @@
 #include "unpack-trees.h"
 #include "progress.h"
 #include "refs.h"
+#include "attr.h"
 
 /*
  * Error messages expected by scripts out of plumbing commands such as
@@ -105,6 +106,7 @@ static int check_updates(struct unpack_trees_options *o)
 		cnt = 0;
 	}
 
+	git_attr_set_direction(GIT_ATTR_CHECKOUT, &o->result);
 	for (i = 0; i < index->cache_nr; i++) {
 		struct cache_entry *ce = index->cache[i];
 
@@ -130,6 +132,7 @@ static int check_updates(struct unpack_trees_options *o)
 		}
 	}
 	stop_progress(&progress);
+	git_attr_set_direction(GIT_ATTR_CHECKIN, NULL);
 	return errs != 0;
 }
 

^ permalink raw reply related

* [PATCH] config: --replace-all with one argument exits properly with a better message.
From: Carlos Rica @ 2009-03-14  2:42 UTC (permalink / raw)
  To: git, johannes.schindelin, gitster

'config --replace-all ONE_ARG' was being treated as 'config NAME VALUE',
showing the error "key does not contain a section: --replace-all".

Now it exits before with an error message asking for the missing value.
Documentation is updated and a new test is added to ensure that
configuration remains the same when no value is provided.

Signed-off-by: Carlos Rica <jasampler@gmail.com>
---
 Documentation/git-config.txt |    2 +-
 builtin-config.c             |    2 ++
 t/t1300-repo-config.sh       |    9 ++++++++-
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 82ce89e..7131ee3 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 [verse]
 'git config' [<file-option>] [type] [-z|--null] name [value [value_regex]]
 'git config' [<file-option>] [type] --add name value
-'git config' [<file-option>] [type] --replace-all name [value [value_regex]]
+'git config' [<file-option>] [type] --replace-all name value [value_regex]
 'git config' [<file-option>] [type] [-z|--null] --get name [value_regex]
 'git config' [<file-option>] [type] [-z|--null] --get-all name [value_regex]
 'git config' [<file-option>] [type] [-z|--null] --get-regexp name_regex [value_regex]
diff --git a/builtin-config.c b/builtin-config.c
index d52a057..005b6ea 100644
--- a/builtin-config.c
+++ b/builtin-config.c
@@ -386,6 +386,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 			return git_config_set_multivar(argv[2], NULL, NULL, 1);
 		else if (!strcmp(argv[1], "--get"))
 			return get_value(argv[2], NULL);
+		else if (!strcmp(argv[1], "--replace-all"))
+			return error("missing value for --replace-all");
 		else if (!strcmp(argv[1], "--get-all")) {
 			do_all = 1;
 			return get_value(argv[2], NULL);
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 3c06842..9c81e04 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -118,7 +118,14 @@ EOF
 
 test_expect_success 'multiple unset is correct' 'cmp .git/config expect'
 
-mv .git/config2 .git/config
+cp .git/config2 .git/config
+
+test_expect_success '--replace-all missing value' '
+	test_must_fail git config --replace-all beta.haha &&
+	test_cmp .git/config2 .git/config
+'
+
+rm .git/config2
 
 test_expect_success '--replace-all' \
 	'git config --replace-all beta.haha gamma'
-- 
1.5.4.3

^ permalink raw reply related

* [PATCH] git-push.txt: describe how to default to pushing only current branch
From: Chris Johnsen @ 2009-03-14  1:27 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Chris Johnsen
In-Reply-To: <20090313164941.GA16504@sigill.intra.peff.net>

---
Jeff King <peff@peff.net> writes:
>   git config remote.$remote.push HEAD
> 
> It isn't mentioned in the git-push manpage; maybe a documentation
> patch to give an example using HEAD would make sense?

Here is a patch. It also attempts to document bare 'git push'.

In the resulting manpage the inline commands are not very
obvious (the HTML looks OK though). There is some sort of
formatting in there, but it does not seem to display any
differently from the surrounding text when I use man to view it
on my system.  Would it be better to do something like wrap
double quotes around the inline commands to help readers viewing
the manpage?
---
 Documentation/git-push.txt |   26 ++++++++++++++++++++++++--
 1 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 4e7e5a7..fd53c49 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -24,8 +24,8 @@ every time you push into it, by setting up 'hooks' there.  See
 documentation for linkgit:git-receive-pack[1].
 
 
-OPTIONS
--------
+OPTIONS[[OPTIONS]]
+------------------
 <repository>::
 	The "remote" repository that is destination of a push
 	operation.  This parameter can be either a URL
@@ -187,6 +187,28 @@ reason::
 Examples
 --------
 
+git push::
+	Works like `git push <remote>`, where <remote> is the
+	current branch's remote (or `origin`, if no remote is
+	configured for the current branch).
+
+git push origin::
+	Without additional configuration, works like
+	`git push origin :`.
++
+The default behavior of this command when no <refspec> is given can be
+configured by setting the `push` option of the remote.
++
+For example, to default to pushing only the current branch to `origin`
+use `git config remote.origin.push HEAD`.  Any valid <refspec> (like
+the ones in the examples below) can be configured as the default for
+`git push origin`.
+
+git push origin :::
+	Push "matching" branches to `origin`. See
+	<refspec> in the <<OPTIONS,OPTIONS>> section above for a
+	description of "matching" branches.
+
 git push origin master::
 	Find a ref that matches `master` in the source repository
 	(most likely, it would find `refs/heads/master`), and update
-- 
1.6.2

^ permalink raw reply related

* Re: Not pushing all branches?
From: Miles Bader @ 2009-03-14  1:08 UTC (permalink / raw)
  To: git
In-Reply-To: <7v4oxxgpil.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:
>    - The first "-" one; even though it may be useful to be able to say
>      "the remote the current branch is associated with by default", using
>      "-" as a short-hand for that might be harmful to the long term UI
>      health, and further study was requested, which hasn't been responded
>      yet.

I've often wished for such a thing in some contexts, actually...
e.g., "git diff REMOTE_BRANCH" to see what updates are pending if I
merge...  Also, it would be nice to have a more concise way to say
"git merge REMOTE_BRANCH".

I'm not sure "-" seems like the best syntax though... maybe it's a bit
_too_ short.

[Is there a general standard syntax for "keywords" in git, e.g., to
distinguish them from branch/rev names?  I mean, if the standard syntax
were "@foo", then one could imagine "git diff @remote" or something.]

-miles

-- 
Run away!  Run away!

^ permalink raw reply

* [PATCH] Autoconf: Disable inline for compilers that don't support it.
From: Allan Caffee @ 2009-03-14  1:04 UTC (permalink / raw)
  To: git
In-Reply-To: <20090114223832.GC30710@genesis.frugalware.org>

The Autoconf macro AC_C_INLINE will redefine the inline keyword to whatever the
current compiler supports (including possibly nothing).

Signed-off-by: Allan Caffee <allan.caffee@gmail.com>
---
On Wed, 14 Jan 2009, Miklos Vajna wrote:
> On Wed, Jan 14, 2009 at 01:32:56PM -0500, Corey Stup
> <coreystup@gmail.com> wrote:
> > When trying to compile with a C89 compliant compiler, I'm coming
> > across a couple issues:
> > - "inline" use
>
> AFAIK that can be avoided with -Dinline=.

But some compilers support other variations of this like __inline__ or
__inline.  Luckily Autoconf has a builtin method for handling this.

diff --git a/configure.ac b/configure.ac
index 082a03d..69fa25e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -308,6 +308,9 @@ AC_SUBST(OLD_ICONV)
 ## Checks for typedefs, structures, and compiler characteristics.
 AC_MSG_NOTICE([CHECKS for typedefs, structures, and compiler characteristics])
 #
+# Check for compilers ability to inline functions.
+AC_C_INLINE
+#
 # Define NO_D_INO_IN_DIRENT if you don't have d_ino in your struct dirent.
 AC_CHECK_MEMBER(struct dirent.d_ino,
 [NO_D_INO_IN_DIRENT=],
-- 
1.5.4.3

^ permalink raw reply related

* [JGIT PATCH 6/5 v2] Add tests for RevWalk and its supporting code
From: Shawn O. Pearce @ 2009-03-14  0:56 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <20090314001345.GI22920@spearce.org>

These new tests cover some of the common cases we see with using a
RevWalk, and increase our code coverage in this critical area of
the JGit library.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 "Shawn O. Pearce" <spearce@spearce.org> wrote:
 > I have yet to track down the full thing... but I know its busted.
 
 This updated test patch includes a test for the bug I was talking
 about, and fixed with 4/5 v2.

 .../org/spearce/jgit/revwalk/RevFlagSetTest.java   |  131 +++++++++++
 .../org/spearce/jgit/revwalk/RevWalkCullTest.java  |   96 ++++++++
 .../spearce/jgit/revwalk/RevWalkFilterTest.java    |  233 ++++++++++++++++++++
 .../spearce/jgit/revwalk/RevWalkMergeBaseTest.java |  117 ++++++++++
 .../org/spearce/jgit/revwalk/RevWalkSortTest.java  |  164 ++++++++++++++
 .../org/spearce/jgit/revwalk/RevWalkTestCase.java  |  102 +++++++++
 6 files changed, 843 insertions(+), 0 deletions(-)
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevFlagSetTest.java
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkCullTest.java
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkFilterTest.java
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkMergeBaseTest.java
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkSortTest.java
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkTestCase.java

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevFlagSetTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevFlagSetTest.java
new file mode 100644
index 0000000..76f3cbb
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevFlagSetTest.java
@@ -0,0 +1,131 @@
+/*
+ * Copyright (C) 2008, Google Inc.
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.revwalk;
+
+import java.util.Arrays;
+import java.util.Iterator;
+
+public class RevFlagSetTest extends RevWalkTestCase {
+	public void testEmpty() {
+		final RevFlagSet set = new RevFlagSet();
+		assertEquals(0, set.mask);
+		assertEquals(0, set.size());
+		assertNotNull(set.iterator());
+		assertFalse(set.iterator().hasNext());
+	}
+
+	public void testAddOne() {
+		final String flagName = "flag";
+		final RevFlag flag = rw.newFlag(flagName);
+		assertTrue(0 != flag.mask);
+		assertSame(flagName, flag.name);
+
+		final RevFlagSet set = new RevFlagSet();
+		assertTrue(set.add(flag));
+		assertFalse(set.add(flag));
+		assertEquals(flag.mask, set.mask);
+		assertEquals(1, set.size());
+		final Iterator<RevFlag> i = set.iterator();
+		assertTrue(i.hasNext());
+		assertSame(flag, i.next());
+		assertFalse(i.hasNext());
+	}
+
+	public void testAddTwo() {
+		final RevFlag flag1 = rw.newFlag("flag_1");
+		final RevFlag flag2 = rw.newFlag("flag_2");
+		assertTrue((flag1.mask & flag2.mask) == 0);
+
+		final RevFlagSet set = new RevFlagSet();
+		assertTrue(set.add(flag1));
+		assertTrue(set.add(flag2));
+		assertEquals(flag1.mask | flag2.mask, set.mask);
+		assertEquals(2, set.size());
+	}
+
+	public void testContainsAll() {
+		final RevFlag flag1 = rw.newFlag("flag_1");
+		final RevFlag flag2 = rw.newFlag("flag_2");
+		final RevFlagSet set1 = new RevFlagSet();
+		assertTrue(set1.add(flag1));
+		assertTrue(set1.add(flag2));
+
+		assertTrue(set1.containsAll(set1));
+		assertTrue(set1.containsAll(Arrays
+				.asList(new RevFlag[] { flag1, flag2 })));
+
+		final RevFlagSet set2 = new RevFlagSet();
+		set2.add(rw.newFlag("flag_3"));
+		assertFalse(set1.containsAll(set2));
+	}
+
+	public void testEquals() {
+		final RevFlag flag1 = rw.newFlag("flag_1");
+		final RevFlag flag2 = rw.newFlag("flag_2");
+		final RevFlagSet set = new RevFlagSet();
+		assertTrue(set.add(flag1));
+		assertTrue(set.add(flag2));
+
+		assertTrue(new RevFlagSet(set).equals(set));
+		assertTrue(new RevFlagSet(Arrays.asList(new RevFlag[] { flag1, flag2 }))
+				.equals(set));
+	}
+
+	public void testRemove() {
+		final RevFlag flag1 = rw.newFlag("flag_1");
+		final RevFlag flag2 = rw.newFlag("flag_2");
+		final RevFlagSet set = new RevFlagSet();
+		assertTrue(set.add(flag1));
+		assertTrue(set.add(flag2));
+
+		assertTrue(set.remove(flag1));
+		assertFalse(set.remove(flag1));
+		assertEquals(flag2.mask, set.mask);
+		assertFalse(set.contains(flag1));
+	}
+
+	public void testContains() {
+		final RevFlag flag1 = rw.newFlag("flag_1");
+		final RevFlag flag2 = rw.newFlag("flag_2");
+		final RevFlagSet set = new RevFlagSet();
+		set.add(flag1);
+		assertTrue(set.contains(flag1));
+		assertFalse(set.contains(flag2));
+		assertFalse(set.contains("bob"));
+	}
+}
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkCullTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkCullTest.java
new file mode 100644
index 0000000..93bd645
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkCullTest.java
@@ -0,0 +1,96 @@
+/*
+ * Copyright (C) 2008, Google Inc.
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.revwalk;
+
+import org.spearce.jgit.lib.ObjectId;
+
+public class RevWalkCullTest extends RevWalkTestCase {
+	public void testProperlyCullAllAncestors1() throws Exception {
+		// Credit goes to Junio C Hamano <gitster@pobox.com> for this
+		// test case in git-core (t/t6009-rev-list-parent.sh)
+		//
+		// We induce a clock skew so two is dated before one.
+		//
+		final ObjectId a = commit();
+		final ObjectId b = commit(-2400, a);
+		final ObjectId c = commit(b);
+		final ObjectId d = commit(c);
+
+		markStart(a);
+		markUninteresting(d);
+		assertNull(rw.next());
+	}
+
+	public void testProperlyCullAllAncestors2() throws Exception {
+		// Despite clock skew on c1 being very old it should not
+		// produce, neither should a or b, or any part of that chain.
+		//
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c1 = commit(-5, b);
+		final ObjectId c2 = commit(10, b);
+		final ObjectId d = commit(c1, c2);
+
+		markStart(d);
+		markUninteresting(c1);
+		assertCommit(d, rw.next());
+		assertCommit(c2, rw.next());
+		assertNull(rw.next());
+	}
+
+	public void testProperlyCullAllAncestors_LongHistory() throws Exception {
+		final ObjectId a = commit();
+		ObjectId b = commit(a);
+		for (int i = 0; i < 24; i++) {
+			b = commit(b);
+			if ((i & 2) == 0)
+				markUninteresting(b);
+		}
+		final ObjectId c = commit(b);
+
+		markStart(c);
+		markUninteresting(b);
+		assertCommit(c, rw.next());
+		assertNull(rw.next());
+
+		// We should have aborted before we got back so far that "a"
+		// would be parsed. Thus, its parents shouldn't be allocated.
+		//
+		assertNull(rw.lookupCommit(a).parents);
+	}
+}
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkFilterTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkFilterTest.java
new file mode 100644
index 0000000..cf2975d
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkFilterTest.java
@@ -0,0 +1,233 @@
+/*
+ * Copyright (C) 2008, Google Inc.
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.revwalk;
+
+import java.io.IOException;
+
+import org.spearce.jgit.errors.IncorrectObjectTypeException;
+import org.spearce.jgit.errors.MissingObjectException;
+import org.spearce.jgit.errors.StopWalkException;
+import org.spearce.jgit.lib.ObjectId;
+import org.spearce.jgit.revwalk.filter.AndRevFilter;
+import org.spearce.jgit.revwalk.filter.NotRevFilter;
+import org.spearce.jgit.revwalk.filter.OrRevFilter;
+import org.spearce.jgit.revwalk.filter.RevFilter;
+
+public class RevWalkFilterTest extends RevWalkTestCase {
+	private static final MyAll MY_ALL = new MyAll();
+
+	public void testFilter_ALL() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c = commit(b);
+
+		rw.setRevFilter(RevFilter.ALL);
+		markStart(c);
+		assertCommit(c, rw.next());
+		assertCommit(b, rw.next());
+		assertCommit(a, rw.next());
+		assertNull(rw.next());
+	}
+
+	public void testFilter_Negate_ALL() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c = commit(b);
+
+		rw.setRevFilter(RevFilter.ALL.negate());
+		markStart(c);
+		assertNull(rw.next());
+	}
+
+	public void testFilter_NOT_ALL() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c = commit(b);
+
+		rw.setRevFilter(NotRevFilter.create(RevFilter.ALL));
+		markStart(c);
+		assertNull(rw.next());
+	}
+
+	public void testFilter_NONE() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c = commit(b);
+
+		rw.setRevFilter(RevFilter.NONE);
+		markStart(c);
+		assertNull(rw.next());
+	}
+
+	public void testFilter_NOT_NONE() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c = commit(b);
+
+		rw.setRevFilter(NotRevFilter.create(RevFilter.NONE));
+		markStart(c);
+		assertCommit(c, rw.next());
+		assertCommit(b, rw.next());
+		assertCommit(a, rw.next());
+		assertNull(rw.next());
+	}
+
+	public void testFilter_ALL_And_NONE() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c = commit(b);
+
+		rw.setRevFilter(AndRevFilter.create(RevFilter.ALL, RevFilter.NONE));
+		markStart(c);
+		assertNull(rw.next());
+	}
+
+	public void testFilter_NONE_And_ALL() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c = commit(b);
+
+		rw.setRevFilter(AndRevFilter.create(RevFilter.NONE, RevFilter.ALL));
+		markStart(c);
+		assertNull(rw.next());
+	}
+
+	public void testFilter_ALL_Or_NONE() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c = commit(b);
+
+		rw.setRevFilter(OrRevFilter.create(RevFilter.ALL, RevFilter.NONE));
+		markStart(c);
+		assertCommit(c, rw.next());
+		assertCommit(b, rw.next());
+		assertCommit(a, rw.next());
+		assertNull(rw.next());
+	}
+
+	public void testFilter_NONE_Or_ALL() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c = commit(b);
+
+		rw.setRevFilter(OrRevFilter.create(RevFilter.NONE, RevFilter.ALL));
+		markStart(c);
+		assertCommit(c, rw.next());
+		assertCommit(b, rw.next());
+		assertCommit(a, rw.next());
+		assertNull(rw.next());
+	}
+
+	public void testFilter_MY_ALL_And_NONE() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c = commit(b);
+
+		rw.setRevFilter(AndRevFilter.create(MY_ALL, RevFilter.NONE));
+		markStart(c);
+		assertNull(rw.next());
+	}
+
+	public void testFilter_NONE_And_MY_ALL() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c = commit(b);
+
+		rw.setRevFilter(AndRevFilter.create(RevFilter.NONE, MY_ALL));
+		markStart(c);
+		assertNull(rw.next());
+	}
+
+	public void testFilter_MY_ALL_Or_NONE() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c = commit(b);
+
+		rw.setRevFilter(OrRevFilter.create(MY_ALL, RevFilter.NONE));
+		markStart(c);
+		assertCommit(c, rw.next());
+		assertCommit(b, rw.next());
+		assertCommit(a, rw.next());
+		assertNull(rw.next());
+	}
+
+	public void testFilter_NONE_Or_MY_ALL() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c = commit(b);
+
+		rw.setRevFilter(OrRevFilter.create(RevFilter.NONE, MY_ALL));
+		markStart(c);
+		assertCommit(c, rw.next());
+		assertCommit(b, rw.next());
+		assertCommit(a, rw.next());
+		assertNull(rw.next());
+	}
+
+	public void testFilter_NO_MERGES() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c1 = commit(b);
+		final ObjectId c2 = commit(b);
+		final ObjectId d = commit(c1, c2);
+		final ObjectId e = commit(d);
+
+		rw.setRevFilter(RevFilter.NO_MERGES);
+		markStart(e);
+		assertCommit(e, rw.next());
+		assertCommit(c2, rw.next());
+		assertCommit(c1, rw.next());
+		assertCommit(b, rw.next());
+		assertCommit(a, rw.next());
+		assertNull(rw.next());
+	}
+
+	private static class MyAll extends RevFilter {
+		@Override
+		public RevFilter clone() {
+			return this;
+		}
+
+		@Override
+		public boolean include(RevWalk walker, RevCommit cmit)
+				throws StopWalkException, MissingObjectException,
+				IncorrectObjectTypeException, IOException {
+			return true;
+		}
+	}
+}
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkMergeBaseTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkMergeBaseTest.java
new file mode 100644
index 0000000..b05e774
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkMergeBaseTest.java
@@ -0,0 +1,117 @@
+/*
+ * Copyright (C) 2008, Google Inc.
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.revwalk;
+
+import org.spearce.jgit.lib.ObjectId;
+import org.spearce.jgit.revwalk.filter.RevFilter;
+
+public class RevWalkMergeBaseTest extends RevWalkTestCase {
+	public void testNone() throws Exception {
+		final ObjectId c1 = commit(commit(commit()));
+		final ObjectId c2 = commit(commit(commit()));
+
+		rw.setRevFilter(RevFilter.MERGE_BASE);
+		markStart(c1);
+		markStart(c2);
+		assertNull(rw.next());
+	}
+
+	public void testSimple() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c1 = commit(commit(commit(commit(commit(b)))));
+		final ObjectId c2 = commit(commit(commit(commit(commit(b)))));
+
+		rw.setRevFilter(RevFilter.MERGE_BASE);
+		markStart(c1);
+		markStart(c2);
+		assertCommit(b, rw.next());
+		assertNull(rw.next());
+	}
+
+	public void testMultipleHeads_SameBase1() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c1 = commit(commit(commit(commit(commit(b)))));
+		final ObjectId c2 = commit(commit(commit(commit(commit(b)))));
+		final ObjectId c3 = commit(commit(commit(b)));
+
+		rw.setRevFilter(RevFilter.MERGE_BASE);
+		markStart(c1);
+		markStart(c2);
+		markStart(c3);
+		assertCommit(b, rw.next());
+		assertNull(rw.next());
+	}
+
+	public void testMultipleHeads_SameBase2() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c = commit(b);
+		final ObjectId d1 = commit(commit(commit(commit(commit(b)))));
+		final ObjectId d2 = commit(commit(commit(commit(commit(c)))));
+		final ObjectId d3 = commit(commit(commit(c)));
+
+		rw.setRevFilter(RevFilter.MERGE_BASE);
+		markStart(d1);
+		markStart(d2);
+		markStart(d3);
+		assertCommit(b, rw.next());
+		assertNull(rw.next());
+	}
+
+	public void testCrissCross() throws Exception {
+		// See http://marc.info/?l=git&m=111463358500362&w=2 for a nice
+		// description of what this test is creating. We don't have a
+		// clean merge base for d,e as they each merged the parents b,c
+		// in different orders.
+		//
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c = commit(a);
+		final ObjectId d = commit(b, c);
+		final ObjectId e = commit(c, b);
+
+		rw.setRevFilter(RevFilter.MERGE_BASE);
+		markStart(d);
+		markStart(e);
+		assertCommit(c, rw.next());
+		assertCommit(b, rw.next());
+		assertNull(rw.next());
+	}
+}
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkSortTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkSortTest.java
new file mode 100644
index 0000000..6f2eedc
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkSortTest.java
@@ -0,0 +1,164 @@
+/*
+ * Copyright (C) 2008, Google Inc.
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.revwalk;
+
+import org.spearce.jgit.lib.ObjectId;
+
+public class RevWalkSortTest extends RevWalkTestCase {
+	public void testSort_Default() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(1, a);
+		final ObjectId c = commit(1, b);
+		final ObjectId d = commit(1, c);
+
+		markStart(d);
+		assertCommit(d, rw.next());
+		assertCommit(c, rw.next());
+		assertCommit(b, rw.next());
+		assertCommit(a, rw.next());
+		assertNull(rw.next());
+	}
+
+	public void testSort_COMMIT_TIME_DESC() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c = commit(b);
+		final ObjectId d = commit(c);
+
+		rw.sort(RevSort.COMMIT_TIME_DESC);
+		markStart(d);
+		assertCommit(d, rw.next());
+		assertCommit(c, rw.next());
+		assertCommit(b, rw.next());
+		assertCommit(a, rw.next());
+		assertNull(rw.next());
+	}
+
+	public void testSort_REVERSE() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c = commit(b);
+		final ObjectId d = commit(c);
+
+		rw.sort(RevSort.REVERSE);
+		markStart(d);
+		assertCommit(a, rw.next());
+		assertCommit(b, rw.next());
+		assertCommit(c, rw.next());
+		assertCommit(d, rw.next());
+		assertNull(rw.next());
+	}
+
+	public void testSort_COMMIT_TIME_DESC_OutOfOrder1() throws Exception {
+		// Despite being out of order time-wise, a strand-of-pearls must
+		// still maintain topological order.
+		//
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c = commit(-5, b);
+		final ObjectId d = commit(10, c);
+		assertTrue(parse(a).getCommitTime() < parse(d).getCommitTime());
+		assertTrue(parse(c).getCommitTime() < parse(b).getCommitTime());
+
+		rw.sort(RevSort.COMMIT_TIME_DESC);
+		markStart(d);
+		assertCommit(d, rw.next());
+		assertCommit(c, rw.next());
+		assertCommit(b, rw.next());
+		assertCommit(a, rw.next());
+		assertNull(rw.next());
+	}
+
+	public void testSort_COMMIT_TIME_DESC_OutOfOrder2() throws Exception {
+		// c1 is back dated before its parent.
+		//
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c1 = commit(-5, b);
+		final ObjectId c2 = commit(10, b);
+		final ObjectId d = commit(c1, c2);
+
+		rw.sort(RevSort.COMMIT_TIME_DESC);
+		markStart(d);
+		assertCommit(d, rw.next());
+		assertCommit(c2, rw.next());
+		assertCommit(b, rw.next());
+		assertCommit(a, rw.next());
+		assertCommit(c1, rw.next());
+		assertNull(rw.next());
+	}
+
+	public void testSort_TOPO() throws Exception {
+		// c1 is back dated before its parent.
+		//
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c1 = commit(-5, b);
+		final ObjectId c2 = commit(10, b);
+		final ObjectId d = commit(c1, c2);
+
+		rw.sort(RevSort.TOPO);
+		markStart(d);
+		assertCommit(d, rw.next());
+		assertCommit(c2, rw.next());
+		assertCommit(c1, rw.next());
+		assertCommit(b, rw.next());
+		assertCommit(a, rw.next());
+		assertNull(rw.next());
+	}
+
+	public void testSort_TOPO_REVERSE() throws Exception {
+		// c1 is back dated before its parent.
+		//
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c1 = commit(-5, b);
+		final ObjectId c2 = commit(10, b);
+		final ObjectId d = commit(c1, c2);
+
+		rw.sort(RevSort.TOPO);
+		rw.sort(RevSort.REVERSE, true);
+		markStart(d);
+		assertCommit(a, rw.next());
+		assertCommit(b, rw.next());
+		assertCommit(c1, rw.next());
+		assertCommit(c2, rw.next());
+		assertCommit(d, rw.next());
+		assertNull(rw.next());
+	}
+}
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkTestCase.java b/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkTestCase.java
new file mode 100644
index 0000000..bd696dd
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkTestCase.java
@@ -0,0 +1,102 @@
+/*
+ * Copyright (C) 2008, Google Inc.
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.revwalk;
+
+import java.util.Date;
+
+import org.spearce.jgit.lib.Commit;
+import org.spearce.jgit.lib.ObjectId;
+import org.spearce.jgit.lib.ObjectWriter;
+import org.spearce.jgit.lib.PersonIdent;
+import org.spearce.jgit.lib.RepositoryTestCase;
+import org.spearce.jgit.lib.Tree;
+
+/** Support for tests of the {@link RevWalk} class. */
+public abstract class RevWalkTestCase extends RepositoryTestCase {
+	protected ObjectWriter ow;
+
+	protected ObjectId emptyTree;
+
+	protected long nowTick;
+
+	protected RevWalk rw;
+
+	public void setUp() throws Exception {
+		super.setUp();
+		ow = new ObjectWriter(db);
+		emptyTree = ow.writeTree(new Tree(db));
+		nowTick = 1236977987000L;
+		rw = new RevWalk(db);
+	}
+
+	protected void tick(final int secDelta) {
+		nowTick += secDelta * 1000L;
+	}
+
+	protected ObjectId commit(final ObjectId... parents) throws Exception {
+		return commit(1, parents);
+	}
+
+	protected ObjectId commit(final int secDelta, final ObjectId... parents)
+			throws Exception {
+		tick(secDelta);
+		final Commit c = new Commit(db);
+		c.setTreeId(emptyTree);
+		c.setParentIds(parents);
+		c.setAuthor(new PersonIdent(jauthor, new Date(nowTick)));
+		c.setCommitter(new PersonIdent(jcommitter, new Date(nowTick)));
+		c.setMessage("");
+		return ow.writeCommit(c);
+	}
+
+	protected RevCommit parse(final ObjectId commitId) throws Exception {
+		return rw.parseCommit(commitId);
+	}
+
+	protected void markStart(final ObjectId commitId) throws Exception {
+		rw.markStart(parse(commitId));
+	}
+
+	protected void markUninteresting(final ObjectId commitId) throws Exception {
+		rw.markUninteresting(parse(commitId));
+	}
+
+	protected void assertCommit(final ObjectId commitId, final RevCommit commit) {
+		assertEquals(commitId.name(), commit != null ? commit.name() : null);
+	}
+}
-- 
1.6.2.288.gc3f22

^ permalink raw reply related

* [JGIT PATCH 4/5 v2] Fix RevWalk with Linus Torvald's occasional bad commit date hack
From: Shawn O. Pearce @ 2009-03-14  0:54 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <1236910062-18476-5-git-send-email-spearce@spearce.org>

In git-core commit 7d004199d134c9d465e013064f72dbc04507f6c0 Linus
describes a hack he created to improve the handling of cases where
commit dates are out of order, such as a child commit having a date
older than its parent.  These cases can show up when there is bad
imported data, or significant clock skew between distributed peers.

The original git-core bug report identified a failure in:

  git rev-list A..B C

where commits contained in both A and B were reported, due to out
of order commit dates.

We keep running until the most recent commit in the pending queue
is more recent than the last commit sent to the caller.  If the
pending queue suddenly goes "backwards" due to one of our parent's
having a more recent commit date, this new check ensures we will
keep processing the graph until we see a more consistent cut.

We process an extra OVER_SCAN commits after we decide that all
remaining commits are UNINTERESTING.  Processing these extra
commits ensures flags are carried back further in the graph,
increasing the chances that we correctly mark relevant nodes.

As a result of this hack, we may produce a commit to our caller,
but then later mark it UNINTERESTING if we discover date skew.
To handle such cases, callers could buffer produced commits and
filter out those that are UNINTERESTING, but this somewhat costly
and may not always be necessary.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---

 This version inverts the "n.commitTime <= last.commitTime",
 I confused myself and wrote it backwards.

 The next patch I'm going to send with the tests has a test for
 this condition, to verify its correct.

 .../org/spearce/jgit/revwalk/PendingGenerator.java |   51 ++++++++++++++++++--
 1 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/PendingGenerator.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/PendingGenerator.java
index 144e909..4e24431 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/PendingGenerator.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/PendingGenerator.java
@@ -42,6 +42,7 @@
 import org.spearce.jgit.errors.IncorrectObjectTypeException;
 import org.spearce.jgit.errors.MissingObjectException;
 import org.spearce.jgit.errors.StopWalkException;
+import org.spearce.jgit.lib.ObjectId;
 import org.spearce.jgit.revwalk.filter.RevFilter;
 
 /**
@@ -60,6 +61,24 @@
 
 	private static final int UNINTERESTING = RevWalk.UNINTERESTING;
 
+	/**
+	 * Number of additional commits to scan after we think we are done.
+	 * <p>
+	 * This small buffer of commits is scanned to ensure we didn't miss anything
+	 * as a result of clock skew when the commits were made. We need to set our
+	 * constant to 1 additional commit due to the use of a pre-increment
+	 * operator when accessing the value.
+	 */
+	private static final int OVER_SCAN = 5 + 1;
+
+	/** A commit near the end of time, to initialize {@link #last} with. */
+	private static final RevCommit INIT_LAST;
+
+	static {
+		INIT_LAST = new RevCommit(ObjectId.zeroId());
+		INIT_LAST.commitTime = Integer.MAX_VALUE;
+	}
+
 	private final RevWalk walker;
 
 	private final DateRevQueue pending;
@@ -68,6 +87,17 @@
 
 	private final int output;
 
+	/** Last commit produced to the caller from {@link #next()}. */
+	private RevCommit last = INIT_LAST;
+
+	/**
+	 * Number of commits we have remaining in our over-scan allotment.
+	 * <p>
+	 * Only relevant if there are {@link #UNINTERESTING} commits in the
+	 * {@link #pending} queue.
+	 */
+	private int overScan = OVER_SCAN;
+
 	boolean canDispose;
 
 	PendingGenerator(final RevWalk w, final DateRevQueue p,
@@ -112,14 +142,27 @@ RevCommit next() throws MissingObjectException,
 				walker.carryFlagsImpl(c);
 
 				if ((c.flags & UNINTERESTING) != 0) {
-					if (pending.everbodyHasFlag(UNINTERESTING))
-						throw StopWalkException.INSTANCE;
-					c.dispose();
+					if (pending.everbodyHasFlag(UNINTERESTING)) {
+						final RevCommit n = pending.peek();
+						if (n != null && n.commitTime >= last.commitTime) {
+							// This is too close to call. The next commit we
+							// would pop is dated after the last one produced.
+							// We have to keep going to ensure that we carry
+							// flags as much as necessary.
+							//
+							overScan = OVER_SCAN;
+						} else if (--overScan == 0)
+							throw StopWalkException.INSTANCE;
+					} else {
+						overScan = OVER_SCAN;
+					}
+					if (canDispose)
+						c.dispose();
 					continue;
 				}
 
 				if (produce)
-					return c;
+					return last = c;
 				else if (canDispose)
 					c.dispose();
 			}
-- 
1.6.2.288.gc3f22

^ permalink raw reply related

* Re: [JGIT PATCH 0/6] Add tests for RevWalk and its supporting code
From: Shawn O. Pearce @ 2009-03-14  0:13 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <20090313223933.GH22920@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> wrote:
> These new tests cover some of the common cases we see with using a
> RevWalk, and increase our code coverage in this critical area of
> the JGit library.
> 
> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
> ---
>  Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote:
>  > fredag 13 mars 2009 03:07:37 skrev "Shawn O. Pearce" <spearce@spearce.org>:
>  > > Today I uncovered some ugly cases with "jgit rev-list B ^A", where
>  > > some commits reachable from A were still being output, even though
>  > > we asked that they be excluded.
>  > 
>  > How about a test suite to prove this is better than before?
> 
>  Its better.  :-)

Its actually not better.  Although the tests I'm replying to are
right, there's another bug introduced by this series that they
don't detect, but that shows up on the Linux kernel repository.

I have yet to track down the full thing... but I know its busted.
 
-- 
Shawn.

^ permalink raw reply

* [PATCH] git-gui: don't hide the Browse button when resizing the repo chooser
From: Markus Heidelberg @ 2009-03-13 23:42 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git

Rather shrink the input field for "Create New Repository" and "Open
Existing Repository" as it's already done for "Clone Existing
Repository".

Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
---
 git-gui/lib/choose_repository.tcl |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/git-gui/lib/choose_repository.tcl b/git-gui/lib/choose_repository.tcl
index f9ff62a..09277e9 100644
--- a/git-gui/lib/choose_repository.tcl
+++ b/git-gui/lib/choose_repository.tcl
@@ -398,6 +398,8 @@ method _do_new {} {
 	grid $w_body.where.l $w_body.where.t $w_body.where.b -sticky ew
 	pack $w_body.where -fill x
 
+	grid columnconfigure $w_body.where 1 -weight 1
+
 	trace add variable @local_path write [cb _write_local_path]
 	bind $w_body.h <Destroy> [list trace remove variable @local_path write [cb _write_local_path]]
 	update
@@ -998,6 +1000,8 @@ method _do_open {} {
 	grid $w_body.where.l $w_body.where.t $w_body.where.b -sticky ew
 	pack $w_body.where -fill x
 
+	grid columnconfigure $w_body.where 1 -weight 1
+
 	trace add variable @local_path write [cb _write_local_path]
 	bind $w_body.h <Destroy> [list trace remove variable @local_path write [cb _write_local_path]]
 	update
-- 
1.6.2.106.gc2bb8

^ permalink raw reply related

* [JGIT PATCH 0/6] Add tests for RevWalk and its supporting code
From: Shawn O. Pearce @ 2009-03-13 22:39 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <200903132100.26527.robin.rosenberg.lists@dewire.com>

These new tests cover some of the common cases we see with using a
RevWalk, and increase our code coverage in this critical area of
the JGit library.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote:
 > fredag 13 mars 2009 03:07:37 skrev "Shawn O. Pearce" <spearce@spearce.org>:
 > > Today I uncovered some ugly cases with "jgit rev-list B ^A", where
 > > some commits reachable from A were still being output, even though
 > > we asked that they be excluded.
 > 
 > How about a test suite to prove this is better than before?

 Its better.  :-)

 These tests don't pass before the series.  They pass after.
 
 .../org/spearce/jgit/revwalk/RevFlagSetTest.java   |  131 +++++++++++
 .../org/spearce/jgit/revwalk/RevWalkCullTest.java  |   75 +++++++
 .../spearce/jgit/revwalk/RevWalkFilterTest.java    |  233 ++++++++++++++++++++
 .../spearce/jgit/revwalk/RevWalkMergeBaseTest.java |  117 ++++++++++
 .../org/spearce/jgit/revwalk/RevWalkSortTest.java  |  164 ++++++++++++++
 .../org/spearce/jgit/revwalk/RevWalkTestCase.java  |  102 +++++++++
 6 files changed, 822 insertions(+), 0 deletions(-)
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevFlagSetTest.java
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkCullTest.java
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkFilterTest.java
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkMergeBaseTest.java
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkSortTest.java
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkTestCase.java

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevFlagSetTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevFlagSetTest.java
new file mode 100644
index 0000000..76f3cbb
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevFlagSetTest.java
@@ -0,0 +1,131 @@
+/*
+ * Copyright (C) 2008, Google Inc.
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.revwalk;
+
+import java.util.Arrays;
+import java.util.Iterator;
+
+public class RevFlagSetTest extends RevWalkTestCase {
+	public void testEmpty() {
+		final RevFlagSet set = new RevFlagSet();
+		assertEquals(0, set.mask);
+		assertEquals(0, set.size());
+		assertNotNull(set.iterator());
+		assertFalse(set.iterator().hasNext());
+	}
+
+	public void testAddOne() {
+		final String flagName = "flag";
+		final RevFlag flag = rw.newFlag(flagName);
+		assertTrue(0 != flag.mask);
+		assertSame(flagName, flag.name);
+
+		final RevFlagSet set = new RevFlagSet();
+		assertTrue(set.add(flag));
+		assertFalse(set.add(flag));
+		assertEquals(flag.mask, set.mask);
+		assertEquals(1, set.size());
+		final Iterator<RevFlag> i = set.iterator();
+		assertTrue(i.hasNext());
+		assertSame(flag, i.next());
+		assertFalse(i.hasNext());
+	}
+
+	public void testAddTwo() {
+		final RevFlag flag1 = rw.newFlag("flag_1");
+		final RevFlag flag2 = rw.newFlag("flag_2");
+		assertTrue((flag1.mask & flag2.mask) == 0);
+
+		final RevFlagSet set = new RevFlagSet();
+		assertTrue(set.add(flag1));
+		assertTrue(set.add(flag2));
+		assertEquals(flag1.mask | flag2.mask, set.mask);
+		assertEquals(2, set.size());
+	}
+
+	public void testContainsAll() {
+		final RevFlag flag1 = rw.newFlag("flag_1");
+		final RevFlag flag2 = rw.newFlag("flag_2");
+		final RevFlagSet set1 = new RevFlagSet();
+		assertTrue(set1.add(flag1));
+		assertTrue(set1.add(flag2));
+
+		assertTrue(set1.containsAll(set1));
+		assertTrue(set1.containsAll(Arrays
+				.asList(new RevFlag[] { flag1, flag2 })));
+
+		final RevFlagSet set2 = new RevFlagSet();
+		set2.add(rw.newFlag("flag_3"));
+		assertFalse(set1.containsAll(set2));
+	}
+
+	public void testEquals() {
+		final RevFlag flag1 = rw.newFlag("flag_1");
+		final RevFlag flag2 = rw.newFlag("flag_2");
+		final RevFlagSet set = new RevFlagSet();
+		assertTrue(set.add(flag1));
+		assertTrue(set.add(flag2));
+
+		assertTrue(new RevFlagSet(set).equals(set));
+		assertTrue(new RevFlagSet(Arrays.asList(new RevFlag[] { flag1, flag2 }))
+				.equals(set));
+	}
+
+	public void testRemove() {
+		final RevFlag flag1 = rw.newFlag("flag_1");
+		final RevFlag flag2 = rw.newFlag("flag_2");
+		final RevFlagSet set = new RevFlagSet();
+		assertTrue(set.add(flag1));
+		assertTrue(set.add(flag2));
+
+		assertTrue(set.remove(flag1));
+		assertFalse(set.remove(flag1));
+		assertEquals(flag2.mask, set.mask);
+		assertFalse(set.contains(flag1));
+	}
+
+	public void testContains() {
+		final RevFlag flag1 = rw.newFlag("flag_1");
+		final RevFlag flag2 = rw.newFlag("flag_2");
+		final RevFlagSet set = new RevFlagSet();
+		set.add(flag1);
+		assertTrue(set.contains(flag1));
+		assertFalse(set.contains(flag2));
+		assertFalse(set.contains("bob"));
+	}
+}
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkCullTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkCullTest.java
new file mode 100644
index 0000000..ad78b16
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkCullTest.java
@@ -0,0 +1,75 @@
+/*
+ * Copyright (C) 2008, Google Inc.
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.revwalk;
+
+import org.spearce.jgit.lib.ObjectId;
+
+public class RevWalkCullTest extends RevWalkTestCase {
+	public void testProperlyCullAllAncestors1() throws Exception {
+		// Credit goes to Junio C Hamano <gitster@pobox.com> for this
+		// test case in git-core (t/t6009-rev-list-parent.sh)
+		//
+		// We induce a clock skew so two is dated before one.
+		//
+		final ObjectId a = commit();
+		final ObjectId b = commit(-2400, a);
+		final ObjectId c = commit(b);
+		final ObjectId d = commit(c);
+
+		markStart(a);
+		markUninteresting(d);
+		assertNull(rw.next());
+	}
+
+	public void testProperlyCullAllAncestors2() throws Exception {
+		// Despite clock skew on c1 being very old it should not
+		// produce, neither should a or b, or any part of that chain.
+		//
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c1 = commit(-5, b);
+		final ObjectId c2 = commit(10, b);
+		final ObjectId d = commit(c1, c2);
+
+		markStart(d);
+		markUninteresting(c1);
+		assertCommit(d, rw.next());
+		assertCommit(c2, rw.next());
+		assertNull(rw.next());
+	}
+}
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkFilterTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkFilterTest.java
new file mode 100644
index 0000000..cf2975d
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkFilterTest.java
@@ -0,0 +1,233 @@
+/*
+ * Copyright (C) 2008, Google Inc.
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.revwalk;
+
+import java.io.IOException;
+
+import org.spearce.jgit.errors.IncorrectObjectTypeException;
+import org.spearce.jgit.errors.MissingObjectException;
+import org.spearce.jgit.errors.StopWalkException;
+import org.spearce.jgit.lib.ObjectId;
+import org.spearce.jgit.revwalk.filter.AndRevFilter;
+import org.spearce.jgit.revwalk.filter.NotRevFilter;
+import org.spearce.jgit.revwalk.filter.OrRevFilter;
+import org.spearce.jgit.revwalk.filter.RevFilter;
+
+public class RevWalkFilterTest extends RevWalkTestCase {
+	private static final MyAll MY_ALL = new MyAll();
+
+	public void testFilter_ALL() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c = commit(b);
+
+		rw.setRevFilter(RevFilter.ALL);
+		markStart(c);
+		assertCommit(c, rw.next());
+		assertCommit(b, rw.next());
+		assertCommit(a, rw.next());
+		assertNull(rw.next());
+	}
+
+	public void testFilter_Negate_ALL() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c = commit(b);
+
+		rw.setRevFilter(RevFilter.ALL.negate());
+		markStart(c);
+		assertNull(rw.next());
+	}
+
+	public void testFilter_NOT_ALL() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c = commit(b);
+
+		rw.setRevFilter(NotRevFilter.create(RevFilter.ALL));
+		markStart(c);
+		assertNull(rw.next());
+	}
+
+	public void testFilter_NONE() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c = commit(b);
+
+		rw.setRevFilter(RevFilter.NONE);
+		markStart(c);
+		assertNull(rw.next());
+	}
+
+	public void testFilter_NOT_NONE() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c = commit(b);
+
+		rw.setRevFilter(NotRevFilter.create(RevFilter.NONE));
+		markStart(c);
+		assertCommit(c, rw.next());
+		assertCommit(b, rw.next());
+		assertCommit(a, rw.next());
+		assertNull(rw.next());
+	}
+
+	public void testFilter_ALL_And_NONE() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c = commit(b);
+
+		rw.setRevFilter(AndRevFilter.create(RevFilter.ALL, RevFilter.NONE));
+		markStart(c);
+		assertNull(rw.next());
+	}
+
+	public void testFilter_NONE_And_ALL() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c = commit(b);
+
+		rw.setRevFilter(AndRevFilter.create(RevFilter.NONE, RevFilter.ALL));
+		markStart(c);
+		assertNull(rw.next());
+	}
+
+	public void testFilter_ALL_Or_NONE() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c = commit(b);
+
+		rw.setRevFilter(OrRevFilter.create(RevFilter.ALL, RevFilter.NONE));
+		markStart(c);
+		assertCommit(c, rw.next());
+		assertCommit(b, rw.next());
+		assertCommit(a, rw.next());
+		assertNull(rw.next());
+	}
+
+	public void testFilter_NONE_Or_ALL() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c = commit(b);
+
+		rw.setRevFilter(OrRevFilter.create(RevFilter.NONE, RevFilter.ALL));
+		markStart(c);
+		assertCommit(c, rw.next());
+		assertCommit(b, rw.next());
+		assertCommit(a, rw.next());
+		assertNull(rw.next());
+	}
+
+	public void testFilter_MY_ALL_And_NONE() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c = commit(b);
+
+		rw.setRevFilter(AndRevFilter.create(MY_ALL, RevFilter.NONE));
+		markStart(c);
+		assertNull(rw.next());
+	}
+
+	public void testFilter_NONE_And_MY_ALL() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c = commit(b);
+
+		rw.setRevFilter(AndRevFilter.create(RevFilter.NONE, MY_ALL));
+		markStart(c);
+		assertNull(rw.next());
+	}
+
+	public void testFilter_MY_ALL_Or_NONE() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c = commit(b);
+
+		rw.setRevFilter(OrRevFilter.create(MY_ALL, RevFilter.NONE));
+		markStart(c);
+		assertCommit(c, rw.next());
+		assertCommit(b, rw.next());
+		assertCommit(a, rw.next());
+		assertNull(rw.next());
+	}
+
+	public void testFilter_NONE_Or_MY_ALL() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c = commit(b);
+
+		rw.setRevFilter(OrRevFilter.create(RevFilter.NONE, MY_ALL));
+		markStart(c);
+		assertCommit(c, rw.next());
+		assertCommit(b, rw.next());
+		assertCommit(a, rw.next());
+		assertNull(rw.next());
+	}
+
+	public void testFilter_NO_MERGES() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c1 = commit(b);
+		final ObjectId c2 = commit(b);
+		final ObjectId d = commit(c1, c2);
+		final ObjectId e = commit(d);
+
+		rw.setRevFilter(RevFilter.NO_MERGES);
+		markStart(e);
+		assertCommit(e, rw.next());
+		assertCommit(c2, rw.next());
+		assertCommit(c1, rw.next());
+		assertCommit(b, rw.next());
+		assertCommit(a, rw.next());
+		assertNull(rw.next());
+	}
+
+	private static class MyAll extends RevFilter {
+		@Override
+		public RevFilter clone() {
+			return this;
+		}
+
+		@Override
+		public boolean include(RevWalk walker, RevCommit cmit)
+				throws StopWalkException, MissingObjectException,
+				IncorrectObjectTypeException, IOException {
+			return true;
+		}
+	}
+}
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkMergeBaseTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkMergeBaseTest.java
new file mode 100644
index 0000000..b05e774
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkMergeBaseTest.java
@@ -0,0 +1,117 @@
+/*
+ * Copyright (C) 2008, Google Inc.
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.revwalk;
+
+import org.spearce.jgit.lib.ObjectId;
+import org.spearce.jgit.revwalk.filter.RevFilter;
+
+public class RevWalkMergeBaseTest extends RevWalkTestCase {
+	public void testNone() throws Exception {
+		final ObjectId c1 = commit(commit(commit()));
+		final ObjectId c2 = commit(commit(commit()));
+
+		rw.setRevFilter(RevFilter.MERGE_BASE);
+		markStart(c1);
+		markStart(c2);
+		assertNull(rw.next());
+	}
+
+	public void testSimple() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c1 = commit(commit(commit(commit(commit(b)))));
+		final ObjectId c2 = commit(commit(commit(commit(commit(b)))));
+
+		rw.setRevFilter(RevFilter.MERGE_BASE);
+		markStart(c1);
+		markStart(c2);
+		assertCommit(b, rw.next());
+		assertNull(rw.next());
+	}
+
+	public void testMultipleHeads_SameBase1() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c1 = commit(commit(commit(commit(commit(b)))));
+		final ObjectId c2 = commit(commit(commit(commit(commit(b)))));
+		final ObjectId c3 = commit(commit(commit(b)));
+
+		rw.setRevFilter(RevFilter.MERGE_BASE);
+		markStart(c1);
+		markStart(c2);
+		markStart(c3);
+		assertCommit(b, rw.next());
+		assertNull(rw.next());
+	}
+
+	public void testMultipleHeads_SameBase2() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c = commit(b);
+		final ObjectId d1 = commit(commit(commit(commit(commit(b)))));
+		final ObjectId d2 = commit(commit(commit(commit(commit(c)))));
+		final ObjectId d3 = commit(commit(commit(c)));
+
+		rw.setRevFilter(RevFilter.MERGE_BASE);
+		markStart(d1);
+		markStart(d2);
+		markStart(d3);
+		assertCommit(b, rw.next());
+		assertNull(rw.next());
+	}
+
+	public void testCrissCross() throws Exception {
+		// See http://marc.info/?l=git&m=111463358500362&w=2 for a nice
+		// description of what this test is creating. We don't have a
+		// clean merge base for d,e as they each merged the parents b,c
+		// in different orders.
+		//
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c = commit(a);
+		final ObjectId d = commit(b, c);
+		final ObjectId e = commit(c, b);
+
+		rw.setRevFilter(RevFilter.MERGE_BASE);
+		markStart(d);
+		markStart(e);
+		assertCommit(c, rw.next());
+		assertCommit(b, rw.next());
+		assertNull(rw.next());
+	}
+}
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkSortTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkSortTest.java
new file mode 100644
index 0000000..6f2eedc
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkSortTest.java
@@ -0,0 +1,164 @@
+/*
+ * Copyright (C) 2008, Google Inc.
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.revwalk;
+
+import org.spearce.jgit.lib.ObjectId;
+
+public class RevWalkSortTest extends RevWalkTestCase {
+	public void testSort_Default() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(1, a);
+		final ObjectId c = commit(1, b);
+		final ObjectId d = commit(1, c);
+
+		markStart(d);
+		assertCommit(d, rw.next());
+		assertCommit(c, rw.next());
+		assertCommit(b, rw.next());
+		assertCommit(a, rw.next());
+		assertNull(rw.next());
+	}
+
+	public void testSort_COMMIT_TIME_DESC() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c = commit(b);
+		final ObjectId d = commit(c);
+
+		rw.sort(RevSort.COMMIT_TIME_DESC);
+		markStart(d);
+		assertCommit(d, rw.next());
+		assertCommit(c, rw.next());
+		assertCommit(b, rw.next());
+		assertCommit(a, rw.next());
+		assertNull(rw.next());
+	}
+
+	public void testSort_REVERSE() throws Exception {
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c = commit(b);
+		final ObjectId d = commit(c);
+
+		rw.sort(RevSort.REVERSE);
+		markStart(d);
+		assertCommit(a, rw.next());
+		assertCommit(b, rw.next());
+		assertCommit(c, rw.next());
+		assertCommit(d, rw.next());
+		assertNull(rw.next());
+	}
+
+	public void testSort_COMMIT_TIME_DESC_OutOfOrder1() throws Exception {
+		// Despite being out of order time-wise, a strand-of-pearls must
+		// still maintain topological order.
+		//
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c = commit(-5, b);
+		final ObjectId d = commit(10, c);
+		assertTrue(parse(a).getCommitTime() < parse(d).getCommitTime());
+		assertTrue(parse(c).getCommitTime() < parse(b).getCommitTime());
+
+		rw.sort(RevSort.COMMIT_TIME_DESC);
+		markStart(d);
+		assertCommit(d, rw.next());
+		assertCommit(c, rw.next());
+		assertCommit(b, rw.next());
+		assertCommit(a, rw.next());
+		assertNull(rw.next());
+	}
+
+	public void testSort_COMMIT_TIME_DESC_OutOfOrder2() throws Exception {
+		// c1 is back dated before its parent.
+		//
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c1 = commit(-5, b);
+		final ObjectId c2 = commit(10, b);
+		final ObjectId d = commit(c1, c2);
+
+		rw.sort(RevSort.COMMIT_TIME_DESC);
+		markStart(d);
+		assertCommit(d, rw.next());
+		assertCommit(c2, rw.next());
+		assertCommit(b, rw.next());
+		assertCommit(a, rw.next());
+		assertCommit(c1, rw.next());
+		assertNull(rw.next());
+	}
+
+	public void testSort_TOPO() throws Exception {
+		// c1 is back dated before its parent.
+		//
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c1 = commit(-5, b);
+		final ObjectId c2 = commit(10, b);
+		final ObjectId d = commit(c1, c2);
+
+		rw.sort(RevSort.TOPO);
+		markStart(d);
+		assertCommit(d, rw.next());
+		assertCommit(c2, rw.next());
+		assertCommit(c1, rw.next());
+		assertCommit(b, rw.next());
+		assertCommit(a, rw.next());
+		assertNull(rw.next());
+	}
+
+	public void testSort_TOPO_REVERSE() throws Exception {
+		// c1 is back dated before its parent.
+		//
+		final ObjectId a = commit();
+		final ObjectId b = commit(a);
+		final ObjectId c1 = commit(-5, b);
+		final ObjectId c2 = commit(10, b);
+		final ObjectId d = commit(c1, c2);
+
+		rw.sort(RevSort.TOPO);
+		rw.sort(RevSort.REVERSE, true);
+		markStart(d);
+		assertCommit(a, rw.next());
+		assertCommit(b, rw.next());
+		assertCommit(c1, rw.next());
+		assertCommit(c2, rw.next());
+		assertCommit(d, rw.next());
+		assertNull(rw.next());
+	}
+}
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkTestCase.java b/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkTestCase.java
new file mode 100644
index 0000000..bd696dd
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkTestCase.java
@@ -0,0 +1,102 @@
+/*
+ * Copyright (C) 2008, Google Inc.
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.revwalk;
+
+import java.util.Date;
+
+import org.spearce.jgit.lib.Commit;
+import org.spearce.jgit.lib.ObjectId;
+import org.spearce.jgit.lib.ObjectWriter;
+import org.spearce.jgit.lib.PersonIdent;
+import org.spearce.jgit.lib.RepositoryTestCase;
+import org.spearce.jgit.lib.Tree;
+
+/** Support for tests of the {@link RevWalk} class. */
+public abstract class RevWalkTestCase extends RepositoryTestCase {
+	protected ObjectWriter ow;
+
+	protected ObjectId emptyTree;
+
+	protected long nowTick;
+
+	protected RevWalk rw;
+
+	public void setUp() throws Exception {
+		super.setUp();
+		ow = new ObjectWriter(db);
+		emptyTree = ow.writeTree(new Tree(db));
+		nowTick = 1236977987000L;
+		rw = new RevWalk(db);
+	}
+
+	protected void tick(final int secDelta) {
+		nowTick += secDelta * 1000L;
+	}
+
+	protected ObjectId commit(final ObjectId... parents) throws Exception {
+		return commit(1, parents);
+	}
+
+	protected ObjectId commit(final int secDelta, final ObjectId... parents)
+			throws Exception {
+		tick(secDelta);
+		final Commit c = new Commit(db);
+		c.setTreeId(emptyTree);
+		c.setParentIds(parents);
+		c.setAuthor(new PersonIdent(jauthor, new Date(nowTick)));
+		c.setCommitter(new PersonIdent(jcommitter, new Date(nowTick)));
+		c.setMessage("");
+		return ow.writeCommit(c);
+	}
+
+	protected RevCommit parse(final ObjectId commitId) throws Exception {
+		return rw.parseCommit(commitId);
+	}
+
+	protected void markStart(final ObjectId commitId) throws Exception {
+		rw.markStart(parse(commitId));
+	}
+
+	protected void markUninteresting(final ObjectId commitId) throws Exception {
+		rw.markUninteresting(parse(commitId));
+	}
+
+	protected void assertCommit(final ObjectId commitId, final RevCommit commit) {
+		assertEquals(commitId.name(), commit != null ? commit.name() : null);
+	}
+}
-- 
1.6.2.288.gc3f22

^ permalink raw reply related

* Re: [PATCH JGIT] Add "compare with Git Index" action.
From: Robin Rosenberg @ 2009-03-13 22:26 UTC (permalink / raw)
  To: Yann Simon; +Cc: Shawn O. Pearce, git
In-Reply-To: <551f769b0903130328g49ce9971t53e1571d1b7de06c@mail.gmail.com>

fredag 13 mars 2009 11:28:48 skrev Yann Simon <yann.simon.fr@gmail.com>:
> 2009/3/11 Robin Rosenberg <robin.rosenberg.lists@dewire.com>:
> > Saving works, but the diff regions aren't updated on save.
> 
> Sorry, I do not understand. Can you explain me more?

The compare ui marks the hunks when you bring it up. I think it should update on
save to the same view you would see when it is first brought up.

> > When there is a diff
> > this would give us the partial staging similar to git gui, if only (not your fault) the
> > commit dialog would allow us to make a distinction between changes in the
> > workdir and the index.
> 
> Yes, I know that this patch is not very usefull for the moment.
> I was more looking for a review as an inclusion.
> And you found one bug. Thank you for that!

Well it is, sort of though it's not *the* solution.

-- robin

^ permalink raw reply

* Re: Google Summer of Code 2009: GIT
From: saurabh gupta @ 2009-03-13 20:18 UTC (permalink / raw)
  To: Rogan Dawes; +Cc: david, Johannes Schindelin, git
In-Reply-To: <49BA2A4E.3030506@dawes.za.net>

On Fri, Mar 13, 2009 at 3:11 PM, Rogan Dawes <lists@dawes.za.net> wrote:
> saurabh gupta wrote:
>>> exactly. and how you mark the conflict to have it be valid XML is
>>> going to depend on details of the type of file. there are probably
>>> a few basic methods that will work the vast majority of the time,
>>> but with some details needing to be configurable.
>>>
>>> for example, if the XML document is a ODF document, it may be
>>> possible to add 'revision' tags around the conflict that are
>>> already understood by the editor.
>>
>> Exactly. This includes the work to modify the xml tags and add
>> contents to represent marker in the best way.
>
> On the XML topic, one last thing to keep in mind is the DTD/XSD which
> governs the file.

This is another point of thinking. A merge helper changing an xml file
may need to modify the schema file also accordingly. Or, by proper
implementation, the need of changing the schema file can be escaped.
:-|


-- 
Saurabh Gupta
Senior,
NSIT,New Delhi, India

^ permalink raw reply

* Re: Transparently encrypt repository contents with GPG
From: Junio C Hamano @ 2009-03-13 20:23 UTC (permalink / raw)
  To: Michael J Gruber
  Cc: Sverre Rabbelier, Thomas Rast, Michael J Gruber,
	Matthias Nothhaft, git
In-Reply-To: <49BA6606.1070403@fastmail.fm>

Michael J Gruber <michaeljgruber+gmane@fastmail.fm> writes:

> In .gitattributes (or.git/info/a..) use
>
> * filter=gpg diff=gpg
>
> In your config:
>
> [filter "gpg"]
>         smudge = gpg -d -q --batch --no-tty
>         clean = gpg -ea -q --batch --no-tty -r C920A124
> [diff "gpg"]
>         textconv = decrypt
>
> This gives you textual diffs even in log! You want use gpg-agent here.

Don't do this.

Think why the smudge/clean pair exists.

The version controlled data, the contents, may not be suitable for
consumption in the work tree in its verbatim form.  For example, a cross
platform project would want to consistently use LF line termination inside
a repository, but on a platform whose tools expect CRLF line endings, the
contents cannot be used verbatim.  We "smudge" the contents running
unix2dos when checking things out on such platforms, and "clean" the
platform specific CRLF line endings by running dos2unix when checking
things in.  By doing so, you can see what really got changed between
versions without getting distracted, and more importantly, "you" in this
sentence is not limited to the human end users alone.

git internally runs diff and xdelta to see what was changed, so that:

 * it can reduce storage requirement when it runs pack-objects;

 * it can check what path in the preimage was similar to what other path
   in the postimage, to deduce a rename;

 * it can check what blocks of lines in the postimage came from what other
   blocks of lines in the preimage, to pass blames across file boundaries.

If your "clean" encrypts and "smudge" decrypts, it means you are refusing
all the benifit git offers.  You are making a pair of similar "smudged"
contents totally dissimilar in their "clean" counterparts.  That is simply
backwards.

As the sole raison d'etre of diff.textconv is to allow potentially lossy
conversion (e.g. msword-to-text) applied to the preimage and postimage
pair of contents (that are supposed to be "clean") before giving a textual
diff to human consumption, the above config may appear to work, but if you
really want an encrypted repository, you should be using an encrypting
filesystem.  That would give an added benefit that the work tree
associated with your repository would also be encrypted.

^ permalink raw reply

* Re: Not pushing all branches?
From: Junio C Hamano @ 2009-03-13 20:02 UTC (permalink / raw)
  To: John Tapsell; +Cc: Peter Krefting, git
In-Reply-To: <43d8ce650903130125m6335d189obbcdb86ec9036083@mail.gmail.com>

John Tapsell <johnflux@gmail.com> writes:

> 2009/3/13 Peter Krefting <peter@softwolves.pp.se>:
>> Hi!
>>
>> Doing "git push remote" pushes all my local branches by default. Is there a
>> way to set it to *not* do that, and (for this particular remote repository)
>> just push the current branch?
>
>> Or failing that, not allow me to run "git
>> push" without specifying a branch?
>
> I've been pushing for this behaviour, and there was a patch a few days
> ago to do this.  I'm not sure if it is/will be committed.

The current status of the series is roughly as follows:

 * Finn Arne sent out a 6 patch series that consists of:

   0e118fe remote: Make "-" an alias for the current remote
   5a18380 New config option push.default
   0b9dcb9 git push: New options --matching and --current
   bf8552b git push: Display warning on unconfigured default push
   cf9d5ab git push: Document that "nothing" is the future push default
   3c2bcc2 git push: Change default for "git push" to nothing.

 * The main topic of the series are patches 2, 4, 5, 6:

   - Introduce a new configuration push.default;

   - Issue a warning when push.default is not set and 'git push' is run
     without saying what refspecs to push, and tell the users that the
     warning can be squelched by setting the configuration (set it to
     'matching' to keep the traditional, 'nothing' to get what Peter
     wants);

   - Reword the warning that the default will change to 'nothing';

   - Switch the default to 'nothing'.

   Which is a reasonable transition plan _if_ we were to change the
   default (except that I think the last one should still keep giving
   warning for people who learned git from the current documentation and
   the start using it after the default is changed).

   If you are changing the default, you have to make people who like the
   current "matching" behaviour suffer no matter how you go about it.  The
   above "start warning early, give chance to people to say 'I want to
   keep my default' before the default changes, and then finally change
   the default" would ease the pain of transition for them.  And the
   configuration option will help people who want new default to set it
   right away.

 * The series is queued in 'pu' for now, as it has a few issues (see mail
   archive for discussions):

   - The first "-" one; even though it may be useful to be able to say
     "the remote the current branch is associated with by default", using
     "-" as a short-hand for that might be harmful to the long term UI
     health, and further study was requested, which hasn't been responded
     yet.

   - The third "--matching/--current" one; --matching is unnecessary as we
     already have ":", --current turns out to be different from HEAD and
     is misnamed.  There also was somebody with an opinion that --current
     adds unnecessary complexity only to encourage a wrong workflow.

   In any case, these two do not have anything to do with the issue that
   "'matching refs' behaviour of a lazy 'git push' that does not say what
   refspecs to push is not always a useful default", and should be done as
   separate patches.  They should come after the dust settles after either
   applying the first two of the main part of the series or deciding to
   drop the main part of the series.

   Also the last one needs to adjust the tests because majority of them
   rely on the current 'matching' behaviour.  As the series lacks it,
   merging it all to 'pu' would make the result not pass the test suite,
   and I excluded the last few patches from 'pu' for now.

   The size of such a patch would be a rough indication how much pain you
   are proposing to incur on existing users.

 * Finn Arne sent the first one of a "replacement" patch series, which I
   looked at, but haven't had time to actually replace the ones that are
   queued in 'pu' (and I haven't seen the second and subsequent ones yet,
   so there is no rush on my end to do so at this moment).

^ permalink raw reply

* Re: [JGIT PATCH 0/5] RevWalk fixes for UNINTERESTING
From: Robin Rosenberg @ 2009-03-13 20:00 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <1236910062-18476-1-git-send-email-spearce@spearce.org>

fredag 13 mars 2009 03:07:37 skrev "Shawn O. Pearce" <spearce@spearce.org>:
> Today I uncovered some ugly cases with "jgit rev-list B ^A", where
> some commits reachable from A were still being output, even though
> we asked that they be excluded.

How about a test suite to prove this is better than before?

-- robin

^ permalink raw reply

* Re: [JGIT PATCH 2/3] Fix ObjectWalk to handle single-entry subtrees correctly
From: Shawn O. Pearce @ 2009-03-13 18:37 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <1236967912-15088-2-git-send-email-spearce@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> wrote:
> diff --git a/org.spearce.jgit/src/org/spearce/jgit/treewalk/CanonicalTreeParser.java b/org.spearce.jgit/src/org/spearce/jgit/treewalk/CanonicalTreeParser.java
> index 8028b14..5c331ca 100644
> --- a/org.spearce.jgit/src/org/spearce/jgit/treewalk/CanonicalTreeParser.java
> +++ b/org.spearce.jgit/src/org/spearce/jgit/treewalk/CanonicalTreeParser.java
> @@ -145,8 +145,19 @@ public CanonicalTreeParser resetRoot(final Repository repo,
>  
>  	/** @return this iterator, or its parent, if the tree is at eof. */
>  	public CanonicalTreeParser next() {
> -		next(1);
> -		return eof() && parent != null ? (CanonicalTreeParser) parent : this;
> +		CanonicalTreeParser p = this;
> +		for (;;) {
> +			p.next(1);
> +			if (p.eof() && parent != null) {
> +				// Parent was left pointing at the entry for us; advance
> +				// the parent to the next entry, possibly unwinding many
> +				// levels up the tree.
> +				//
> +				p = (CanonicalTreeParser) p.parent;
> +				continue;
> +			}
> +			return p;
> +		}

The parent != null test is wrong, we need "p.parent" here:

diff --git a/org.spearce.jgit/src/org/spearce/jgit/treewalk/CanonicalTreeParser.java b/org.spearce.jgit/src/org/spearce/jgit/treewalk/CanonicalTreeParser.java
index 5c331ca..7f89cff 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/treewalk/CanonicalTreeParser.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/treewalk/CanonicalTreeParser.java
@@ -148,7 +148,7 @@ public CanonicalTreeParser next() {
 		CanonicalTreeParser p = this;
 		for (;;) {
 			p.next(1);
-			if (p.eof() && parent != null) {
+			if (p.eof() && p.parent != null) {
 				// Parent was left pointing at the entry for us; advance
 				// the parent to the next entry, possibly unwinding many
 				// levels up the tree.
-- 
Shawn.

^ permalink raw reply related

* [JGIT PATCH 3/3] Use a common skipObject method to avoid UNINTERESTING items
From: Shawn O. Pearce @ 2009-03-13 18:11 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <1236967912-15088-2-git-send-email-spearce@spearce.org>

All cases are using the same logic to decide that we should skip
this current object and not return it to the caller.  A common
implementation makes the code easier to follow, especially as it
reduces the ugly line wrap involved in the loop body.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../src/org/spearce/jgit/revwalk/ObjectWalk.java   |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/ObjectWalk.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/ObjectWalk.java
index a481639..b92629e 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/ObjectWalk.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/ObjectWalk.java
@@ -255,8 +255,7 @@ public RevObject nextObject() throws MissingObjectException,
 				if ((o.flags & SEEN) != 0)
 					break;
 				o.flags |= SEEN;
-				if ((o.flags & UNINTERESTING) != 0
-						&& !hasRevSort(RevSort.BOUNDARY))
+				if (skipObject(o))
 					break;
 				fromTreeWalk = true;
 				return o;
@@ -267,8 +266,7 @@ public RevObject nextObject() throws MissingObjectException,
 				if ((o.flags & SEEN) != 0)
 					break;
 				o.flags |= SEEN;
-				if ((o.flags & UNINTERESTING) != 0
-						&& !hasRevSort(RevSort.BOUNDARY))
+				if (skipObject(o))
 					break;
 				nextSubtree = o;
 				fromTreeWalk = true;
@@ -294,7 +292,7 @@ public RevObject nextObject() throws MissingObjectException,
 			if ((o.flags & SEEN) != 0)
 				continue;
 			o.flags |= SEEN;
-			if ((o.flags & UNINTERESTING) != 0 && !hasRevSort(RevSort.BOUNDARY))
+			if (skipObject(o))
 				continue;
 			if (o instanceof RevTree) {
 				currentTree = (RevTree) o;
@@ -304,6 +302,10 @@ public RevObject nextObject() throws MissingObjectException,
 		}
 	}
 
+	private final boolean skipObject(final RevObject o) {
+		return (o.flags & UNINTERESTING) != 0 && !hasRevSort(RevSort.BOUNDARY);
+	}
+
 	/**
 	 * Verify all interesting objects are available, and reachable.
 	 * <p>
-- 
1.6.2.288.gc3f22

^ permalink raw reply related

* [JGIT PATCH 2/3] Fix ObjectWalk to handle single-entry subtrees correctly
From: Shawn O. Pearce @ 2009-03-13 18:11 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <1236967912-15088-1-git-send-email-spearce@spearce.org>

"jgit push" has been pushing more objects than necessary
for a while now, due to ObjectWalk failing to mark a subtree
uninteresting if it contains exactly one subtree child, such as
"org.spearce.egit.core/src" which has only one child, "org".

The bug was caused by the child iterator being advanced past the
first item before we even evaluated it, as the for loop always
invoked treeWalk.next() at the end of each iteration.  However,
a new subtree iterator is positioned on the first item and must
not call next() until after we have handled that first entry.

While debugging this I also noticed strange behavior from the
ObjectWalk evaulating a subtree again, after we had exited it.
This was because the parent iterator was never advanced past the
child when we entered into it.  We now advance the parent whenever
we leave the child.

Note that its important we do the parent advance *after* we exit the
child, as they share the same path buffer.  Advancing the parent
before we enter into the child would cause the child to corrupt
the parent's next item path.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../src/org/spearce/jgit/revwalk/ObjectWalk.java   |   54 +++++++++++---------
 .../spearce/jgit/treewalk/CanonicalTreeParser.java |   40 +++++++++++++-
 2 files changed, 66 insertions(+), 28 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/ObjectWalk.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/ObjectWalk.java
index 0d67a38..a481639 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/ObjectWalk.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/ObjectWalk.java
@@ -82,7 +82,7 @@
 
 	private boolean fromTreeWalk;
 
-	private boolean enterSubtree;
+	private RevTree nextSubtree;
 
 	/**
 	 * Create a new revision and object walker for a given repository.
@@ -239,50 +239,52 @@ public RevObject nextObject() throws MissingObjectException,
 			IncorrectObjectTypeException, IOException {
 		fromTreeWalk = false;
 
-		if (enterSubtree) {
-			treeWalk = treeWalk.createSubtreeIterator(db, idBuffer, curs);
-			enterSubtree = false;
+		if (nextSubtree != null) {
+			treeWalk = treeWalk.createSubtreeIterator0(db, nextSubtree, curs);
+			nextSubtree = null;
 		}
 
-		for (; !treeWalk.eof(); treeWalk = treeWalk.next()) {
+		while (!treeWalk.eof()) {
 			final FileMode mode = treeWalk.getEntryFileMode();
 			final int sType = mode.getObjectType();
 
 			switch (sType) {
 			case Constants.OBJ_BLOB: {
 				treeWalk.getEntryObjectId(idBuffer);
-				final RevObject o = lookupAny(idBuffer, sType);
+				final RevBlob o = lookupBlob(idBuffer);
 				if ((o.flags & SEEN) != 0)
-					continue;
+					break;
 				o.flags |= SEEN;
 				if ((o.flags & UNINTERESTING) != 0
 						&& !hasRevSort(RevSort.BOUNDARY))
-					continue;
+					break;
 				fromTreeWalk = true;
 				return o;
 			}
 			case Constants.OBJ_TREE: {
 				treeWalk.getEntryObjectId(idBuffer);
-				final RevObject o = lookupAny(idBuffer, sType);
+				final RevTree o = lookupTree(idBuffer);
 				if ((o.flags & SEEN) != 0)
-					continue;
+					break;
 				o.flags |= SEEN;
 				if ((o.flags & UNINTERESTING) != 0
 						&& !hasRevSort(RevSort.BOUNDARY))
-					continue;
-				enterSubtree = true;
+					break;
+				nextSubtree = o;
 				fromTreeWalk = true;
 				return o;
 			}
 			default:
 				if (FileMode.GITLINK.equals(mode))
-					continue;
+					break;
 				treeWalk.getEntryObjectId(idBuffer);
 				throw new CorruptObjectException("Invalid mode " + mode
 						+ " for " + idBuffer.name() + " "
 						+ treeWalk.getEntryPathString() + " in " + currentTree
 						+ ".");
 			}
+
+			treeWalk = treeWalk.next();
 		}
 
 		for (;;) {
@@ -361,7 +363,7 @@ public String getPathString() {
 	public void dispose() {
 		super.dispose();
 		pendingObjects = new BlockObjQueue();
-		enterSubtree = false;
+		nextSubtree = null;
 		currentTree = null;
 	}
 
@@ -369,7 +371,7 @@ public void dispose() {
 	protected void reset(final int retainFlags) {
 		super.reset(retainFlags);
 		pendingObjects = new BlockObjQueue();
-		enterSubtree = false;
+		nextSubtree = null;
 	}
 
 	private void addObject(final RevObject o) {
@@ -387,34 +389,36 @@ private void markTreeUninteresting(final RevTree tree)
 		tree.flags |= UNINTERESTING;
 
 		treeWalk = treeWalk.resetRoot(db, tree, curs);
-		for (;!treeWalk.eof(); treeWalk = treeWalk.next()) {
+		while (!treeWalk.eof()) {
 			final FileMode mode = treeWalk.getEntryFileMode();
 			final int sType = mode.getObjectType();
 
 			switch (sType) {
 			case Constants.OBJ_BLOB: {
 				treeWalk.getEntryObjectId(idBuffer);
-				lookupAny(idBuffer, sType).flags |= UNINTERESTING;
-				continue;
+				lookupBlob(idBuffer).flags |= UNINTERESTING;
+				break;
 			}
 			case Constants.OBJ_TREE: {
 				treeWalk.getEntryObjectId(idBuffer);
-				final RevObject subtree = lookupAny(idBuffer, sType);
-				if ((subtree.flags & UNINTERESTING) == 0) {
-					subtree.flags |= UNINTERESTING;
-					treeWalk = treeWalk.createSubtreeIterator(db, idBuffer,
-							curs);
+				final RevTree t = lookupTree(idBuffer);
+				if ((t.flags & UNINTERESTING) == 0) {
+					t.flags |= UNINTERESTING;
+					treeWalk = treeWalk.createSubtreeIterator0(db, t, curs);
+					continue;
 				}
-				continue;
+				break;
 			}
 			default:
 				if (FileMode.GITLINK.equals(mode))
-					continue;
+					break;
 				treeWalk.getEntryObjectId(idBuffer);
 				throw new CorruptObjectException("Invalid mode " + mode
 						+ " for " + idBuffer.name() + " "
 						+ treeWalk.getEntryPathString() + " in " + tree + ".");
 			}
+
+			treeWalk = treeWalk.next();
 		}
 	}
 }
diff --git a/org.spearce.jgit/src/org/spearce/jgit/treewalk/CanonicalTreeParser.java b/org.spearce.jgit/src/org/spearce/jgit/treewalk/CanonicalTreeParser.java
index 8028b14..5c331ca 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/treewalk/CanonicalTreeParser.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/treewalk/CanonicalTreeParser.java
@@ -145,8 +145,19 @@ public CanonicalTreeParser resetRoot(final Repository repo,
 
 	/** @return this iterator, or its parent, if the tree is at eof. */
 	public CanonicalTreeParser next() {
-		next(1);
-		return eof() && parent != null ? (CanonicalTreeParser) parent : this;
+		CanonicalTreeParser p = this;
+		for (;;) {
+			p.next(1);
+			if (p.eof() && parent != null) {
+				// Parent was left pointing at the entry for us; advance
+				// the parent to the next entry, possibly unwinding many
+				// levels up the tree.
+				//
+				p = (CanonicalTreeParser) p.parent;
+				continue;
+			}
+			return p;
+		}
 	}
 
 	/**
@@ -192,8 +203,31 @@ public CanonicalTreeParser createSubtreeIterator(final Repository repo,
 			final ObjectId me = idBuffer.toObjectId();
 			throw new IncorrectObjectTypeException(me, Constants.TYPE_TREE);
 		}
+		return createSubtreeIterator0(repo, idBuffer, curs);
+	}
+
+	/**
+	 * Back door to quickly create a subtree iterator for any subtree.
+	 * <p>
+	 * Don't use this unless you are ObjectWalk. The method is meant to be
+	 * called only once the current entry has been identified as a tree and its
+	 * identity has been converted into an ObjectId.
+	 *
+	 * @param repo
+	 *            repository to load the tree data from.
+	 * @param id
+	 *            ObjectId of the tree to open.
+	 * @param curs
+	 *            window cursor to use during repository access.
+	 * @return a new parser that walks over the current subtree.
+	 * @throws IOException
+	 *             a loose object or pack file could not be read.
+	 */
+	public final CanonicalTreeParser createSubtreeIterator0(
+			final Repository repo, final AnyObjectId id, final WindowCursor curs)
+			throws IOException {
 		final CanonicalTreeParser p = new CanonicalTreeParser(this);
-		p.reset(repo, idBuffer, curs);
+		p.reset(repo, id, curs);
 		return p;
 	}
 
-- 
1.6.2.288.gc3f22

^ permalink raw reply related

* [JGIT PATCH 1/3] Add lookupBlob to RevWalk
From: Shawn O. Pearce @ 2009-03-13 18:11 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git

Its useful if you want to get a handle on a blob object.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../src/org/spearce/jgit/revwalk/RevWalk.java      |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java
index 316f722..e47f9d9 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java
@@ -510,6 +510,25 @@ public void setTreeFilter(final TreeFilter newFilter) {
 	}
 
 	/**
+	 * Locate a reference to a blob without loading it.
+	 * <p>
+	 * The blob may or may not exist in the repository. It is impossible to tell
+	 * from this method's return value.
+	 * 
+	 * @param id
+	 *            name of the blob object.
+	 * @return reference to the blob object. Never null.
+	 */
+	public RevBlob lookupBlob(final AnyObjectId id) {
+		RevBlob c = (RevBlob) objects.get(id);
+		if (c == null) {
+			c = new RevBlob(id);
+			objects.add(c);
+		}
+		return c;
+	}
+
+	/**
 	 * Locate a reference to a tree without loading it.
 	 * <p>
 	 * The tree may or may not exist in the repository. It is impossible to tell
-- 
1.6.2.288.gc3f22

^ permalink raw reply related

* Re: [PATCH] test-lib: write test results to test-results/<basename>-<pid>
From: SZEDER Gábor @ 2009-03-13 17:44 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, gitster, Sverre Rabbelier
In-Reply-To: <20090313172002.GA16232@neumann>

On Fri, Mar 13, 2009 at 06:20:02PM +0100, SZEDER Gábor wrote:
> Hi,
> 
> 
> On Fri, Mar 13, 2009 at 05:36:13PM +0100, Johannes Schindelin wrote:
> > On Fri, 13 Mar 2009, Johannes Schindelin wrote:
> > 
> > > The earlier code meant to attempt to strip everything except the test
> > > number, but only stripped the part starting with the last dash.
> > > 
> > > However, there is no reason why we should not use the whole basename.
> 
> I agree.
> 
> > > 
> > > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > > ---
> > > 
> > > 	Even if this is not strictly necessary after Hannes' test cleanup, 
> > > 	it would still be nice.
> >
> > Just to clarify: it fixes the issue that these two tests share the same 
> > file in test-results/: t5521-pull-options.sh  t5521-pull-symlink.sh
> > 
> > As a consequence, one's results overwrite the other one's.
> 
> The pid of the test process makes the name of the test result file
> unique for each test, even in the mentioned case, e.g. it would be
> something like t5521-pull-12345 and t5521-pull-23456.

Correction:  those files are not always unique, because although
unlikely, it's possible that these two tests get the same pid.

But with Hannes' patch this issue goes away, and the rest of my
previous mail still holds.


Best,
Gábor

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox