Re: [RFC] Second parent for reverts

From: Junio C Hamano <junkio@cox.net>
Date: May 10, 2007, 01:43 UTC
Message-ID: <7vzm4do3m2.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <7v7irhslx1.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> writes:

Show 22 quoted lines

>> But a' doesn't actually take anything from b, since it's reverting all of 
>> b (unless it's only reverting part of b), and, if b isn't there, it 
>> doesn't need a commit message, either, so it's not different from a. So 
>> the flow should be:
>>
>> a -> b -> c -> d -> e
>>   \              /
>>    --------------
>>
>> And this means blame work correctly: lines that b changed will be blamed 
>> on a (or an ancestor), because e will match a there and be different from 
>> d. So I think git-revert should simply add in the reverted patch's parent. 
>> Does this analysis make sense to other people?
>
> The revert operation at the tree level (not commit level) treats
> AS IF b is a common ancestor between a and d and computes a
> merge between a and d using that fake common ancestor to reach
> at e.  So it is understandable that you are confused that the
> result somehow has something to do with a merge between a and d.
>
> But other than that, the "analysis" does not make any sense to
> me.

Side note.

In the same spirit as gitk and history browsers pay attention to the in-body SHA-1 of reverted commits, you could make git-blame pay attention to the revert message. I think the rough outline would go like this.

 (1) you run pass_blame_to_parent() on 'e' as usual and give as
     much blame as you can to 'd'; the remainder are attributed
     to 'e' but it is actually a revert of what 'b' did.

 (2) you notice that 'e' is a revert of 'b';

 (3) 'b' always has only one parent, as we do not revert a
     merge; so you can find 'a' easily.

 (4) instead of "take the responsibility for the remaining
     entries" as usual in assign_blame() while drilling down
     'e', you find the matching blob using find_origin and
     find_rename between 'a' and 'e'.  And pass blames that are
     attributed to 'e' down to 'a'.

 (5) then you keep going, digging 'd' and 'a'.