In case revs->limited == 1, the revision walker really reads
everything into revs->commits. The behaviour introduced in
c4025103fa does not behave correctly in that case.
It used to say: everything which is _still_ in the pipeline
must be a boundary commit.
So, in the case that revs->limited == 1, filter out all commits
which are not dangling, in effect marking only the dangling
ones as boundary commits.
This bug was noticed by Santi Béjar.
Signed-off-by: Johannes Schindelin <Johannes.Schindelin@xxxxxx>
---
On Mon, 5 Mar 2007, Junio C Hamano wrote:
> "Santi Béjar" <sbejar@xxxxxxxxx> writes:
>
> > the --topo-order does not play well with --boundary and
> > --max-count.
> >
> > $ git-rev-list --boundary --max-count=50 5ced0 | wc -l
> > 56
> > $ git-rev-list --topo-order --boundary --max-count=50 5ced0 \
> > | wc -l
> > 8846
> >
> > (5ced0 is git.git's master). I think it should be 56 for both.
> > It presents this behaviour since c4025103fa, when was added
> > --boundary support for git-rev-list --max-count and --max-age.
>
> I think the code that does --boundary when the list is limited
> with --max-count is not quite right, even without topo-order.
Right.
> It needs to find out commits (be they marked as UNINTERESTING or
> not) still in the revs->commits that are _not_ reachable by any
> other commits in the list, or something like that.
>
> I suspect that would unfortunately be very expensive. Dscho,
> have better ideas?
Unfortunately not. Fortunately, it is nowhere near as expensive as
I originally thought. It just has to iterate through revs->commits
three times, which is way cheaper than sorting the commits in the
first place.
Junio, if you apply this, could you make extra sure that I did not
fsck up Santi's family name (I am running on a peculiar mixture of
UTF-8 and ISO-8859-1 terminals...).
revision.c | 31 +++++++++++++++++++++++++++----
1 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/revision.c b/revision.c
index f5b8ae4..8d47fac 100644
--- a/revision.c
+++ b/revision.c
@@ -1294,6 +1294,26 @@ static struct commit *get_revision_1(struct rev_info *revs)
return NULL;
}
+static struct commit_list *get_dangling_commits(struct commit_list *list)
+{
+ struct commit_list *result = NULL, *iter, *parent;
+
+ for (iter = list; iter; iter = iter->next)
+ for (parent = iter->item->parents;
+ parent;
+ parent = parent->next)
+ parent->item->object.flags |= TMP_MARK;
+ for (iter = list; iter; iter = iter->next)
+ if (!(iter->item->object.flags & TMP_MARK))
+ commit_list_insert(iter->item, &result);
+ for (iter = list; iter; iter = iter->next)
+ for (parent = iter->item->parents;
+ parent;
+ parent = parent->next)
+ parent->item->object.flags &= ~TMP_MARK;
+ return result;
+}
+
struct commit *get_revision(struct rev_info *revs)
{
struct commit *c = NULL;
@@ -1345,12 +1365,15 @@ struct commit *get_revision(struct rev_info *revs)
break;
case 0:
if (revs->boundary) {
- struct commit_list *list = revs->commits;
- while (list) {
+ struct commit_list *list;
+ if (revs->limited) {
+ list = get_dangling_commits(revs->commits);
+ free_commit_list(revs->commits);
+ revs->commits = list;
+ }
+ for (list = revs->commits; list; list = list->next)
list->item->object.flags |=
BOUNDARY_SHOW | BOUNDARY;
- list = list->next;
- }
/* all remaining commits are boundary commits */
revs->max_count = -1;
revs->limited = 1;
--
1.5.0.3.2518.g2f72-dirty