On Tue, Jul 28, 2020 at 09:00:51AM -0400, Derrick Stolee wrote: > On 7/28/2020 5:13 AM, Abhishek Kumar via GitGitGadget wrote: > > From: Abhishek Kumar <abhishekkumar8222@xxxxxxxxx> > > > > In indegree_walk_step(), we add unvisited parents to the indegree queue. > > However, parents are not guaranteed to be parsed. As the indegree queue > > sorts by generation number, let's parse parents before inserting them to > > ensure the correct priority order. > > You mentioned this in your blog post. I'm sorry that such a small > issue caused you pain. Perhaps you could summarize a little bit of > how that investigation led you to find this issue? Indeed ;-). I feel like forgetting to call 'parse_commit_gently()' is a rite of passage for this part of the code in some sense. > Question: is this something that is only necessary when we change > the generation number, or is it something that is only _exposed_ > by the test suite when we change the generation number? It seems that > it is likely to be an existing bug, but it might be hard to expose > in a test case. I tend to agree that this bug probably existed before Abhishek's changes, but that it's probably more trouble than it's worth to tickle with a test case. So, I'd be fine with this fix as it is (provided that the style nit is addressed below, too). > > Signed-off-by: Abhishek Kumar <abhishekkumar8222@xxxxxxxxx> > > --- > > revision.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/revision.c b/revision.c > > index 6aa7f4f567..23287d26c3 100644 > > --- a/revision.c > > +++ b/revision.c > > @@ -3343,6 +3343,9 @@ static void indegree_walk_step(struct rev_info *revs) > > struct commit *parent = p->item; > > int *pi = indegree_slab_at(&info->indegree, parent); > > > > + if (parse_commit_gently(parent, 1) < 0) > > + return ; > > Drop the extra space. > > Thanks, > -Stolee Thanks, Taylor
