Re: [PATCH] graph: avoid infinite loop in graph_show_commit()

2012-09-25 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 Has either of you tried the patch with the problematic case the
 other patch tries to solve?  Michal's old patch does smell like it
 is going in the better direction in that it stops looping when we
 know we would only be showing the padding, which is a sign that we
 are done with showing the commit.

I think this should suffice.  I do not know if Michal's patch is the
right fix, though.  It appears to me that --graph assumes one
commit is shown only once, but diff-tree -m and friends want to
show a merge commit twice and is fundamentally incompatible with the
assumption.  We might be off either fixing that in the graph code
(not with a band-aid like patches from you two to make it punt), or
forbidding the combination altogether.


 t/t4202-log.sh | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 71be59d..14f73e3 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -264,6 +264,16 @@ test_expect_success 'log --graph with merge' '
test_cmp expect actual
 '
 
+test_expect_success 'log --raw --graph -m with merge' '
+   git log --raw --graph --oneline -m master | head -n 500 actual 
+   grep initial actual
+'
+
+test_expect_success 'diff-tree --graph' '
+   git diff-tree --graph master^ | head -n 500 actual 
+   grep one actual
+'
+
 cat  expect \EOF
 *   commit master
 |\  Merge: A B
-- 
1.7.12.1.451.gb433296

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] graph: avoid infinite loop in graph_show_commit()

2012-09-23 Thread Nguyen Thai Ngoc Duy
On Sun, Sep 23, 2012 at 6:55 PM, Michal Kiedrowicz
michal.kiedrow...@gmail.com wrote:
 Nguyễn Thái Ngọc Duy pclouds at gmail.com writes:


 The loop can be triggered with git diff-tree --graph commit where
 the commit is a non-merge. It goes like this


 Isn't this the same issue as in
 http://article.gmane.org/gmane.comp.version-control.git/123979
 ? (with slightly different fix)

I don't know. I'm not familiar enough with graph.c to tell. Maybe Adam
can have a look?

The patch that is cut out is
http://article.gmane.org/gmane.comp.version-control.git/206205
-- 
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html