Re: [PATCH v2 3/7] log: do not free parents when walking reflog
- From
Jeff King <peff@peff.net>
- Date
- Jul 9, 2017, 10:13 UTC
- Message-ID
- <20170709101351.qcwgtzly72wwvwmq@sigill.intra.peff.net>
- In-Reply-To
- <xmqqwp7kb2ap.fsf@gitster.mtv.corp.google.com>
On Fri, Jul 07, 2017 at 10:10:54AM -0700, Junio C Hamano wrote:
Show 17 quoted lines
> > 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@peff.net> --- 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