[PATCH] rev-list --max-age, --max-count: support --boundary

[PATCH] rev-list --max-age, --max-count: support --boundary

[Johannes Schindelin]

Now, when saying --max-age=<timestamp>, or --max-count=<n>, together
with --boundary, rev-list prints the boundary commits, i.e. the
commits which are _just_ not shown without --boundary, i.e. their
children are, but they aren't.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---

	Of course, this is meant to speed up an otherwise expensive
	operation in git-bundle.

	However, this touches a very sensitive point in core git:
	the revision walking machinery. It passes all tests, but
	some eyeballing is much appreciated.

	Ah, and somebody better at writing short concise descriptions
	then me should look into documenting "edge" objects and
	"boundary" commits to the glossary (possibly after thinking
	if "edge" and "boundary" do mean the same after all).

	'nough for one night.

 revision.c |   24 ++++++++++++++++++++----
 1 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/revision.c b/revision.c
index 5b1794b..87aea9c 100644
--- a/revision.c
+++ b/revision.c
@@ -1233,9 +1233,15 @@ static struct commit *get_revision_1(struct rev_info *revs)
 		 */
 		if (!revs->limited) {
 			if (revs->max_age != -1 &&
-			    (commit->date < revs->max_age))
-				continue;
-			add_parents_to_list(revs, commit, &revs->commits);
+			    (commit->date < revs->max_age)) {
+				if (revs->boundary)
+					commit->object.flags |=
+						BOUNDARY_SHOW | BOUNDARY;
+				else
+					continue;
+			} else
+				add_parents_to_list(revs, commit,
+						&revs->commits);
 		}
 		if (commit->object.flags & SHOWN)
 			continue;
@@ -1336,7 +1342,17 @@ struct commit *get_revision(struct rev_info *revs)
 	case -1:
 		break;
 	case 0:
-		return NULL;
+		if (revs->boundary) {
+			struct commit_list *list = revs->commits;
+			while (list) {
+				list->item->object.flags |=
+					BOUNDARY_SHOW | BOUNDARY;
+				list = list->next;
+			}
+			revs->max_count = -1;
+			revs->limited = 1;
+		} else
+			return NULL;
 	default:
 		revs->max_count--;
 	}

Re: [PATCH] rev-list --max-age, --max-count: support --boundary

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Now, when saying --max-age=<timestamp>, or --max-count=<n>, together
> with --boundary, rev-list prints the boundary commits, i.e. the
> commits which are _just_ not shown without --boundary, i.e. their
> children are, but they aren't.
>
> Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> ---
>
> 	Of course, this is meant to speed up an otherwise expensive
> 	operation in git-bundle.
>
> 	However, this touches a very sensitive point in core git:
> 	the revision walking machinery. It passes all tests, but
> 	some eyeballing is much appreciated.

I think this looks fine.

Initially I thought setting BOUNDARY_SHOW unconditionally would
be wrong (the "limited" case takes pains to avoid showing
redundant boundary commits), and that was exactly why I said it
would be a bit more involved change, but I do not think of a
topology that needs redundancy removal outside of "limited" case.

> @@ -1336,7 +1342,17 @@ struct commit *get_revision(struct rev_info *revs)
>  	case -1:
>  		break;
>  	case 0:
> -		return NULL;
> +		if (revs->boundary) {
> +			struct commit_list *list = revs->commits;
> +			while (list) {
> +				list->item->object.flags |=
> +					BOUNDARY_SHOW | BOUNDARY;
> +				list = list->next;
> +			}
> +			revs->max_count = -1;
> +			revs->limited = 1;
> +		} else
> +			return NULL;

The resetting of max_count and limited probably needs some
commenting.

Re: [PATCH] rev-list --max-age, --max-count: support --boundary

[Johannes Schindelin]

Hi,

On Mon, 19 Feb 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > @@ -1336,7 +1342,17 @@ struct commit *get_revision(struct rev_info *revs)
> >  	case -1:
> >  		break;
> >  	case 0:
> > -		return NULL;
> > +		if (revs->boundary) {
> > +			struct commit_list *list = revs->commits;
> > +			while (list) {
> > +				list->item->object.flags |=
> > +					BOUNDARY_SHOW | BOUNDARY;
> > +				list = list->next;
> > +			}
> > +			revs->max_count = -1;
> > +			revs->limited = 1;
> > +		} else
> > +			return NULL;
> 
> The resetting of max_count and limited probably needs some
> commenting.

Right. Could you --amend with this, if you find it sufficient?

---

 revision.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/revision.c b/revision.c
index 87aea9c..be57191 100644
--- a/revision.c
+++ b/revision.c
@@ -1349,6 +1349,7 @@ struct commit *get_revision(struct rev_info *revs)
 					BOUNDARY_SHOW | BOUNDARY;
 				list = list->next;
 			}
+			/* all remaining commits are boundary commits */
 			revs->max_count = -1;
 			revs->limited = 1;
 		} else