Re: [PATCH] graph API: fix bug in graph_is_interesting()

From: Adam Simpkins <simpkins@facebook.com>
Date: Aug 19, 2009, 02:29 UTC
Message-ID: <20090819022918.GO8147@facebook.com>
In-Reply-To: <7vk5103chi.fsf@alter.siamese.dyndns.org>

On Tue, Aug 18, 2009 at 04:53:45PM -0700, Junio C Hamano wrote:

Show 14 quoted lines

> Adam Simpkins <simpkins@facebook.com> writes:
> 
> > -	return (commit->object.flags & (UNINTERESTING | TREESAME)) ? 0 : 1;
> > +	return simplify_commit(graph->revs, commit) == commit_show;
> 
> If you do this after revision.c finished the traversal (e.g. "limited"
> case), I think it should be Ok.
> 
> But calling simplify_commit() while the traversal is still in progress is
> asking for trouble.  I do not recall the details anymore but when I tried
> to make the "simplify-merges" algorithm incremental, I had seen funny
> breakage caused by calling simplify_commit() twice on the same commit.
> 
> I suspect that this change will break the primary traversal.

The --graph option always enables revs->topo_order, which in turn enables revs->limited, so this should always be after limit_list() has already been called.

However, it looks like the call to rewrite_parents() is the only modifying operation in simplify_commit(), and all the rest can easily be split out into a separate helper function. I'll submit another version of the patch that makes the non-modifying simplify_commit() behavior available as a separate function.

-- 
Adam Simpkins
simpkins@facebook.com