Possible --boundary bug

Possible --boundary bug

[Marco Costalba]

Trying to convert qgit to use the new and cool --boundary option I
found this one:

>From git tree

$ git-rev-list --boundary --topo-order --parents 5aa44d5..ab57c8d |
grep eb38cc689e8
-e646de0d14bac20ef6e156c1742b9e62fb0b9020
eb38cc689e84a8fd01c1856e889fe8d3b4f1bfb4
4b953cdc04fec8783e2c654a671005492fda9b01
5ca5396c9ecba947c8faac7673195d309a6ba2ea
eb38cc689e84a8fd01c1856e889fe8d3b4f1bfb4

and also

$ git-rev-list --boundary --topo-order --parents 5aa44d5..ab57c8d |
grep c64965750
8c0db2f5193153ea8a51bb45b0512c5a3889023b
21a02335f821c89a989cf0b533d2ae0adb6da16e
c649657501bada28794a30102d9c13cc28ca0e5e

But perhaps correct output should be:

$ git-rev-list --boundary --topo-order --parents 5aa44d5..ab57c8d |
grep eb38cc689e8
-e646de0d14bac20ef6e156c1742b9e62fb0b9020
eb38cc689e84a8fd01c1856e889fe8d3b4f1bfb4
-4b953cdc04fec8783e2c654a671005492fda9b01
5ca5396c9ecba947c8faac7673195d309a6ba2ea
eb38cc689e84a8fd01c1856e889fe8d3b4f1bfb4

and

$ git-rev-list --boundary --topo-order --parents 5aa44d5..ab57c8d |
grep c64965750
-8c0db2f5193153ea8a51bb45b0512c5a3889023b
21a02335f821c89a989cf0b533d2ae0adb6da16e
c649657501bada28794a30102d9c13cc28ca0e5e

It seems the '-' flag is mistakenly missing because of boundary  revs:

c649657501bada28794a30102d9c13cc28ca0e5e and
eb38cc689e84a8fd01c1856e889fe8d3b4f1bfb4


Marco

Re: Possible --boundary bug

[Marco Costalba]

Sorry, the good description is below, please ignore the wrong previous one.

On 3/30/06, Marco Costalba <mcostalba@gmail.com> wrote:
> Trying to convert qgit to use the new and cool --boundary option I
> found this one:
>
> From git tree
>
> $ git-rev-list --boundary --topo-order --parents 5aa44d5..ab57c8d |
> grep eb38cc689e8
> -e646de0d14bac20ef6e156c1742b9e62fb0b9020
> eb38cc689e84a8fd01c1856e889fe8d3b4f1bfb4
> 4b953cdc04fec8783e2c654a671005492fda9b01
> 5ca5396c9ecba947c8faac7673195d309a6ba2ea
> eb38cc689e84a8fd01c1856e889fe8d3b4f1bfb4
>
> and also
>
> $ git-rev-list --boundary --topo-order --parents 5aa44d5..ab57c8d |
> grep c64965750
> 8c0db2f5193153ea8a51bb45b0512c5a3889023b
> 21a02335f821c89a989cf0b533d2ae0adb6da16e
> c649657501bada28794a30102d9c13cc28ca0e5e
>

It seems the lines:
-c649657501bada28794a30102d9c13cc28ca0e5e  .......

and

-eb38cc689e84a8fd01c1856e889fe8d3b4f1bfb4 ......

are missing though the two revs are boundary revs.


Marco

 P.S: Sorry for lengthy output but  --abbrev option:

 git-rev-list --boundary --topo-order --abbrev=8 --parents  5aa44d5..ab57c8d

 does seems to work only for prettyprinted parent names, I guess this
from patches log messages because is not documented.

Re: Possible --boundary bug

[Junio C Hamano]

"Marco Costalba" <mcostalba@gmail.com> writes:

> Sorry, the good description is below, please ignore the wrong previous one.

I think this patch should fix it.

-- >8 --
rev-list --boundary: fix re-injecting boundary commits.

Marco reported that

	$ git rev-list --boundary --topo-order --parents 5aa44d5..ab57c8d

misses these two boundary commits.

        c649657501bada28794a30102d9c13cc28ca0e5e
        eb38cc689e84a8fd01c1856e889fe8d3b4f1bfb4

Indeed, we can see that gitk shows these two commits at the
bottom, because the --boundary code failed to output them.

The code did not check to avoid pushing the same uninteresting
commit twice to the result list.  I am not sure why this fixes
the reported problem, but this seems to fix it.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
diff --git a/revision.c b/revision.c
index abc8745..c2a95aa 100644
--- a/revision.c
+++ b/revision.c
@@ -420,24 +420,33 @@ static void limit_list(struct rev_info *
 		p = &commit_list_insert(commit, p)->next;
 	}
 	if (revs->boundary) {
-		list = newlist;
-		while (list) {
+		/* mark the ones that are on the result list first */
+		for (list = newlist; list; list = list->next) {
 			struct commit *commit = list->item;
+			commit->object.flags |= TMP_MARK;
+		}
+		for (list = newlist; list; list = list->next) {
+			struct commit *commit = list->item;
 			struct object *obj = &commit->object;
-			struct commit_list *parent = commit->parents;
-			if (obj->flags & (UNINTERESTING|BOUNDARY)) {
-				list = list->next;
-				continue;
-			}
-			while (parent) {
+			struct commit_list *parent;
+			if (obj->flags & UNINTERESTING)
+				continue;
+			for (parent = commit->parents;
+			     parent;
+			     parent = parent->next) {
 				struct commit *pcommit = parent->item;
-				parent = parent->next;
 				if (!(pcommit->object.flags & UNINTERESTING))
 					continue;
 				pcommit->object.flags |= BOUNDARY;
+				if (pcommit->object.flags & TMP_MARK)
+					continue;
+				pcommit->object.flags |= TMP_MARK;
 				p = &commit_list_insert(pcommit, p)->next;
 			}
-			list = list->next;
+		}
+		for (list = newlist; list; list = list->next) {
+			struct commit *commit = list->item;
+			commit->object.flags &= ~TMP_MARK;
 		}
 	}
 	revs->commits = newlist;

Re: Possible --boundary bug

[Marco Costalba]

On 3/31/06, Junio C Hamano <junkio@cox.net> wrote:
> "Marco Costalba" <mcostalba@gmail.com> writes:
>
> > Sorry, the good description is below, please ignore the wrong previous one.
>
> I think this patch should fix it.
>

Yes. It works for me.

Thanks
Marco