Le lundi 18 mai 2009, Junio C Hamano a écrit : > Christian Couder <chriscool@xxxxxxxxxxxxx> 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. I agree that unparsing all the remaining parents recursively may not be enough in some cases, but I don't know much about these cases. Is it only when revs->prune is set? I think it should be ok when checking that all good revs are ancestor of the bad rev, but I may be missing something. It should be ok too when using "git replace" and/or grafts as the parent list of a commit should be changed before its parents are changed. I also found the clear_commit_marks function that could be used when checking ancestors so I will resend a patch series using it and without patch 2/3. But I can continue to work on unparsing commits to improve the "git replace" series. I need to find some test cases though, and I cannot think of any interesting one right now. By the way, when you say that you suspect an attempt to replace an object that is directly listed on the command line would not work very well with the current replace series, is it related to the unparsing commit problem? Thanks, Christian. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html