On Fri, Jul 07, 2017 at 10:10:54AM -0700, Junio C Hamano wrote:
> > diff --git a/builtin/log.c b/builtin/log.c
> > index 8ca1de9894..9c8bb3b5c3 100644
> > --- a/builtin/log.c
> > +++ b/builtin/log.c
> > @@ -374,9 +374,9 @@ static int cmd_log_walk(struct rev_info *rev)
> > if (!rev->reflog_info) {
> > /* we allow cycles in reflog ancestry */
> > free_commit_buffer(commit);
> > + free_commit_list(commit->parents);
> > + commit->parents = NULL;
>
> After step 6/7, we no longer "allow cycles in reflog ancestry", as
> there will be no reflog ancestry to speak of ;-), so it would be
> nice to remove the comment above in that step. But alternatively,
> we can rephrase the comment here, to say something like "the same
> commit can be shown multiple times while showing entries from the
> reflog" instead.
I actually think the comment is a bit obtuse in the first place. The
real issue is that we show commits multiple times. That's caused by
cycles, yes, but also by us clearing the SEEN flag. ;)
Maybe this on top?
-- >8 --
Subject: [PATCH] log: clarify comment about reflog cycles
When we're walking reflogs, we leave the commit buffer and
parents in place. A comment explains that this is due to
"cycles". But the interesting thing is the unsaid
implication: that the cycles (plus our clearing of the SEEN
flag) will cause us to show commits multiple times. Let's
spell it out.
Signed-off-by: Jeff King <peff@xxxxxxxx>
---
builtin/log.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/builtin/log.c b/builtin/log.c
index 9c8bb3b5c..630d6cff2 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -372,7 +372,10 @@ static int cmd_log_walk(struct rev_info *rev)
*/
rev->max_count++;
if (!rev->reflog_info) {
- /* we allow cycles in reflog ancestry */
+ /*
+ * We may show a given commit multiple times when
+ * walking the reflogs.
+ */
free_commit_buffer(commit);
free_commit_list(commit->parents);
commit->parents = NULL;
--
2.13.2.1066.gabaed60bd
