Re: [RFC] Second parent for reverts

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Daniel Barkalow <barkalow@xxxxxxxxxxxx> writes:

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

-
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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux