Re: [PATCH 2/3] commit: add function to unparse a commit and its parents

From: Junio C Hamano <gitster@pobox.com>
Date: May 18, 2009, 06:27 UTC
Message-ID: <7v3ab3exht.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20090517153647.6403.44036.chriscool@tuxfamily.org>

Christian Couder <chriscool@tuxfamily.org> writes:

> Its parents are recursively unparsed too, because they might have
> been changed. But its tree is not unparsed as it should not have
> been modifed.

It is a bug in any codepath if it used commit->tree without first checking if commit->parsed is true to begin with, so you could NULLify commit->tree but that shouldn't make any difference. I agree leaving the tree object as-is would make sense.

I am not convinced that unparsing all the _remaining_ parents recursively like your patch does is enough. Isn't there a codepath that

 - parses a commit A to find list of true parents X, Y, Z;

 - iterates over that list and descend into one of these parents X, doing
   nasty things such as pruning its parents list after parsing it; and

 - decides to prune that parent X from the parent list, making the parent
   list of A into Y and Z?

After such a sequence, calling your unparse_commit(A) will unparse A and remaining parents of Y and Z, but will still leave X in the dirty state.