Re: [PATCH 4/2] Fix parent rewriting in --early-output

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 13, 2007, 08:24 UTC
Message-ID: <7vabpiv9go.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <20071113080125.GB14735@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

Show 13 quoted lines

> Junio C Hamano <gitster@pobox.com> wrote:
>
>> I have to wonder what would happen if a much higher level caller
>> caused the objects to get parsed before coming into the revision
>> walking machinery, e.g. after the command line processing for
>> A...B walked the ancestry chain until their common ancestors are
>> found.  So these commits between A and B are parsed, but the
>> revision limiting machinery hasn't done its operation to set
>> TREECHANGE and/or UNINTERESTING in add_parents_to_list() on
>> these commits yet.
>
> That's one of the problems with the way the revision walking
> machinery is built.  Its fast, but it can really only be used once.

Yes but not quite. As long as you do not use overlapping set of flag bits without cleaning, you are almost Ok.

The reason I say "almost" is that I think the true problem with the first patch by Linus was to load "parsed" bit any semantics other than "we have read and parsed the data so do not bother rereading it". If it used another mechanism (e.g. another flag bit that means "we have done TREECHANGE and stuff"), returning rewrite_one_ok to punt when seeing an unprocessed node would have been safe, as it would not have munged the parent list. The replacement "TREESAME" patch would work much better without using such an extra bit, because it allows us to directly check "if we already decided this does not change from the parent", and the lack of the bit covers both "we haven't processed it" and "we processed but we do not want to prune" cases, and not pruning is safe and easily and correctly re-processible in the post clean-up phase of the early_output series.

But in general, you're right. An operation that changes the ancestry shape is irreversible, and you would need to cause reparsing of the commit objects after you are done with revision traversal, and at that point, discarding and re-reading everything from scratch, although is heavy-handed, is one plausible approach to tackle the problem.