Re: [RFC] Second parent for reverts

From: Junio C Hamano <junkio@cox.net>
Date: May 9, 2007, 21:54 UTC
Message-ID: <7v7irhslx1.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <Pine.LNX.4.64.0705091406350.18541@iabervon.org>

Daniel Barkalow <barkalow@iabervon.org> writes:

Show 31 quoted lines

> The discussion about having a header to specify, for a revert commit, what 
> it reverts made me realize that this header *would* be useful, but that we 
> don't need a *new* header for it. I think that the right method is to add 
> the parent of the reverted commit as a second parent for the revert.
>
> If you have:
>
> a -> b -> c -> d
>
> And you want to revert b, the most exact flow would be:
>
> a -> b -> c -> d -> e
>        \         /
>         -> a' ---
>
> I.e., you exactly remove the effects of b to generate a commit that has 
> the same tree as a, and then you merge.
>
> 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.

When you have a merge commit somebody else made, you should be able to reproduce it yourself, with the help from the intent of the merge explained in the commit log of the merge commit (usually it says "I am merging this development line with that one"). With a->b->c->d history, what is the sane merge result between a and d? It won't be e. The only clue that you did a revert of b is contained in the message "Revert b", because b is not a common ancestor between a and d.

An interesting tangent is that you can revert both b and c with a single tree-level operation by pretending as if c is a common ancestor between a and d and run tree-level 3-way merge to apply difference between c and d on top of a to come up with e. It would not make sense to record neither one of a b c as the fake second parent.

I would suggest to leave a revert as a revert. It is not a merge.

In general, you should think of an act of making a commit (not limited to a merge but a single parent) to mean that you are making this statement:

	I have considered the development history that leads to
	the parent commits this commit records, and the tree
	recorded with this commit suits the purpose of my branch
	better than all of them.

Obviously, if you considered the history that leads to 'd', you have considered the hsitory that leads to 'a' as well, so recording both 'a' and 'd' as parents does not make much sense. You could have recorded 'e' as an Octopus of 'a', 'b', ... 'd', and the statement does not change.