threads / bug / 3761

Possible --boundary bug

Subject: Possible --boundary bug

## tl;dr

4 messages between Mar 30, 2006 and Mar 31, 2006.

replies: 3people: 2as markdown or json

Marco Costalba· Mar 30, 2006, 18:34 UTC · lore

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

Marco Costalba· Mar 30, 2006, 20:55 UTC · re: Marco Costalba · lore

Re: Possible --boundary bug

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

On 3/30/06, Marco Costalba <mcostalba@gmail.com> wrote:

Show 21 quoted lines

> 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.

Junio C Hamano· Mar 31, 2006, 07:58 UTC · re: Marco Costalba · lore

Re: Possible --boundary bug

"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;

Marco Costalba· Mar 31, 2006, 16:39 UTC · re: Junio C Hamano · lore

Re: Possible --boundary bug

On 3/31/06, Junio C Hamano <junkio@cox.net> wrote:

Show 6 quoted lines

> "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