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

[Junio C Hamano <gitster@pobox.com>]

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

> 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.