Odd merge behaviour involving reverts

I am encountering an odd problem with how merging branches are working. I expect I am doing something wrong, I just need to know what.

Here is my situation...

I have a master branch. We have a branch off of that that some developers are doing work on. They claim it is ready. We merge it into the master branch. It breaks something so we revert the merge. They make changes to the code. they get it to a point where they say it is ok and we merge again.

When examined, we find that code changes made before the revert are not in the master branch, but code changes after are in the master branch.

We now have code that does not contain all of the changes in the branch.

Is there a different way to undo merges into a branch that does not block further merges from that branch?

What am i doing wrong here?

On Thu, 18 Dec 2008, Alan wrote:

Reverting a merge is your problem.

You can do it, but you seem to have done it without understanding what it causes.

A revert of a merge becomes a regular commit that just undoes everything that the merge did in your branch. When you then do the next merge, you'll do that merge with that in mind, so now git will essentially consider the previous merge to be the base line, but your revert undid everything that that one brought in, so the new merge will really only contain the new stuff from the branch you are merging.

So if a merge causes problems, you generally should either undo it _entirely_ (ie do a 'git reset --hard ORIG_HEAD'), not revert it.

Of course, if you had already made the merged state public, or done development on top of it, you can't really do that. In which case a revert works, but if you want it back, you should revert the revert, not merge the branch again - because what you merged last time you threw away, and won't be applied again.

		Linus

On Thu, 2008-12-18 at 15:58 -0800, Linus Torvalds wrote:

Obviously.

So I will have to look for previous reverts (or "pre-verts") before merging again.

On this code base, there were 30+ branches merged before the powers that be decided to have me pull that one. It was in the middle of the pile.

I think I know how to fix it. I am just concerned about having it occur again if someone else makes the same mistake I did.

Thanks.

On Thu, 18 Dec 2008, Alan wrote:

I suspect we should warn about reverting merges. I'm surprised we don't already. Reverting a merge isn't "wrong", but it's a whole lot more subtle than reverting a regular commit.

Reverting a regular commit just effectively undoes what that commit did, and is fairly straightforward. But reverting a merge commit also undoes the _data_ that the commit changed, but it does absolutely nothing to the effects on _history_ that the merge had.

So the merge will still exist, and it will still be seen as joining the two branches together, and future merges will see that merge as the last shared state - and the revert that reverted the merge brought in will not affect that at all.

So a "revert" undoes the data changes, but it's very much _not_ an "undo" in the sense that it doesn't undo the effects of a commit on the repository history.

So if you think of "revert" as "undo", then you're going to always miss this part of reverts. Yes, it undoes the data, but no, it doesn't undo history.

			Linus

On Thu, 2008-12-18 at 16:21 -0800, Linus Torvalds wrote:

So what is the recommended way to undo mistaken merges caught after the fact that will not fubar later merges?

On Thu, 18 Dec 2008, Alan wrote:

Oh, I suspect reverting is the right thing, it's just that then you need to remember to revert the revert if you intend to merge that branch again later.

So reverting a merge isn't _wrong_ per se. It's just that you need to be aware of the consequences, and if it becomes a common situation, you have a problem in your usage patterns.

Btw, even with non-merge commits, "revert" can have interesting effects exactly because it doesn't change history. For example, let's say that you had a history like this, with two branches:

	--> a --> b --> c --> d
	      |
	      +-> e --> !a

where the second branch reverts 'a', but the first one does not. What happens when you merge the two branches? Is the revert sticky? In this case, yes, a merge will cause the revert to stick. But what happens if you had

	--> a --> b --> c --> d
	      |
	      +-> e --> f --> !e

and 'b' and 'e' were the same patch (just applied in two different branches), and '!e' reverts that patch in the second branch. What happens to 'b' when you merge? Would you expect for 'b' to go away, since the revert undid the same data in the second branch?

In that second case, the revert of 'e' will basically make git act as if 'e' didn't happen at all in the second branch, and so when you merge them, 'b' _will_ exist in the end result, so now the revert didn't "take".

All of this is very self-consistent (it's a very direct result of how merges work and how revert works), but I'm just bringing these things up as examples of how 'revert' is not a totally trivial matter. You'll always find cases where you might have wished that it had acted differently when you merge things.

			Linus

Quoting Linus Torvalds <torvalds@linux-foundation.org>:

If I understand Alan's case correctly, I think he does not want to "undo" the revert but wants to merge an updated version of the branch, as if no mistaken merge nor its revert happened in the past.

If you revert the revert on the branch before merging, doesn't it mean that you will be merging what the older version of the branch did (that is in the revert of the revert as a single huge patch) and what the updated version of the branch wants to do?  Wouldn't that lead to a mess with huge conflicts?

On Fri, 19 Dec 2008, Nanako Shiraishi wrote:

Actually, the reverse is likely true. If the branch you are merging is actually doing something branch-specific - ie it's a "topic branch", then it's likely that the new stuff that is on that branch depends on the previous stuff on the branch.

And thats' the thing that got reverted - so with just a revert, it's quite likely that you'll get conflicts. But if you revert the revert, now the new stuff you're merging actually makes more sense, and is less likely to conflict.

Another way of looking at it is that a merge is something that can be done both ways: think of the _other_ branch merging yours. The original revert ends up being a big change-patch that undoes everything that other branch did, so now if that other branch were to merge the main branch, you'd be merging a lot of changes. But reverting the revert will undo all those changes, so again, it's more likely that the merge will succeed.

So revertign a revert is usually going to make subsequent merges easier rather than the reverse.

The _big_ problem with reverting a whole merge is that it effectively becomes one commit that does a big change. That's how _normal_ merges tend to look like in CVS or SVN (ie the "merge" is really just another commit that brings in a lot of changes), and it's a total and utter f*cking disaster!

But don't get me wrong - it's not something you can't work with. I'm just trying to say that it's absolutely not a "good model". It's workable, but it has all these painful issues.

For example, think about what reverting a merge (and then reverting the revert) does to bisectability. Ignore the fact that the revert of a revert is undoing it - just think of it as a "single commit that does a lot". Because that is what it does.

When you have a problem you are chasing down, and you hit a "revert this merge", what you're hitting is essentially a single commit that contains all the changes (but obviously in reverse) of all the commits that got merged. So it's debugging hell, because now you don't have lots of small changes that you can try to pinpoint which _part_ of it changes.

But does it all work? Sure it does. You can revert a merge, and from a purely technical angle, git did it very naturally and had no real troubles. It just considered it a change from "state before merge" to "state after merge", and that was it. Nothing complicated, nothing odd, nothing really dangerous. Git will do it without even thinking about it.

So from a technical angle, there's nothing wrong with reverting a merge, but from a workflow angle it's something that you generally should try to avoid.

If at all possible, for example, if you find a problem that got merged into the main tree, rather than revert the merge, try _really_ hard to bisect the problem down into the branch you merged, and just fix it, or try to revert the individual commit that caused it.

Yes, it's more complex, and no, it's not always going to work (sometimes the answer is: "oops, I really shouldn't have merged it, because it wasn't ready yet, and I really need to undo _all_ of the merge"). So then you really should revert the merge, but when you want to re-do the merge, you now need to do it by reverting the revert.

		Linus

On Thu, Dec 18, 2008 at 11:01 PM, Linus Torvalds <torvalds@linux-foundation.org> wrote:

Instead of reverting the revert, why not rebase the branch which was merged to the point after the revert was done?

j.

On Thu, 18 Dec 2008, Jay Soffian wrote:

Sure, you can do that too. However, that has its own set of problems, ie all the "ok, who else was working based on this branch" thing.

But no, there are no single right answers. Rebasing may well be a reasonable approach.

		Linus

On Thu, 18 Dec 2008, Linus Torvalds wrote:

Another option is to do the big revert on the master branch, and on the side branch merge the parent of the revert (normally), and then merge the revert but take the side branch tree.

That last merge puts the revert in the branch's history, but rejects its effects. Now merging the branch again will find the revert to be the common ancestor, and see the full change of the side branch as the contribution of that side. Also, blame will come down the side branch, ignore the merge (on the side branch, the merge contributed no content changes), and go into the original side branch commits.

You can also think of it as the side branch saying, "I noticed that you reverted my changes, but I'm keeping them anyway". When the final merge comes, git will see that the revert isn't new information to the side branch, and nullify its effect. (Of course, the side branch needs to merge the parent of the revert so that it isn't rejecting the other changes made on the main branch before the revert)

	-Daniel
*This .sig left intentionally blank*

Nanako Shiraishi <nanako3@lavabit.com> writes:

The history immediately after the "revert of the merge" would look like this:

 ---o---o---o---M---x---x---W---x
               /     
       ---A---B

where A and B are on the side development that was not so good, M is the merge that brings those premature changes into the mainline, x are unrelated changes already made on the mainline and W is the "revert of the merge M" (doesn't W look M upside down?). IOW, "diff W^..W" is similar to "diff -R M^..M".

I think you misunderstood what "merging an updated version of the branch" meant by Alan's description to mean that the side branch developers discarded their faulty A and B, and redone the changes, which would have resulted in something like:

 ---o---o---o---M---x---x---W---x---x
                                 \
                                  A'--B'--C'

If that were the situation, suggestion by Linus to revert the revert and then merge would result in something like this:

 ---o---o---o---M---x---x---W---x---x---Y---*
                                 \         /
                                  A'--B'--C'

where Y is the revert of W, A' and B'are rerolled A and B, and there may also be a further fix-up C' on the side branch. "diff Y^..Y" is similar to "diff -R W^..W" (which in turn means it is similar to "diff M^..M"), and "diff A'^..C'" by definition would be similar but different from that, because it is a rerolled series of the earlier change. There would be a lot of overlap as you feared. In such a case, not having Y (revert of the revert) would result in a much more trivial merge:

 ---o---o---o---M---x---x---W---x---x-------*
                                 \         /
                                  A'--B'--C'

because problematic large commits M and W are already outside of the scope of this final merge.

But I think what Alan's developers did is different. They did this instead:

 ---o---o---o---M---x---x---W---x
               /
       ---A---B-------------------C---D

where C and D are to fix what was broken in A and B. In such a situation, what Linus suggests makes perfect sense. You first revert the revert, which would result in this:

 ---o---o---o---M---x---x---W---x---Y
               /
       ---A---B-------------------C---D

where Y is the revert of W, which would (ignoring possible conflicts between what W and W..Y changed) be equivalent to not having W nor Y at all in the history:

 ---o---o---o---M---x---x-------x----
               /
       ---A---B-------------------C---D

and merging the side branch again will not have conflict arising from an earlier revert and revert of revert.

 ---o---o---o---M---x---x-------x-------*
               /                       /
       ---A---B-------------------C---D

Of course the changes made in C and D still can conflict with what was done by any of the x, but that is just a normal merge conflict.

To recap, these are two very different scenarios, and wants two very different resolution strategies:

 - If the faulty side branch whose effects were discarded by an earlier
   revert of a merge was rebuilt from scratch (i.e. rebasing and fixing,
   as you seem to have interpreted), then re-merging the result without
   doing anything else fancy would be the right thing to do.

 - If the faulty side branch was fixed by adding corrections on top, then
   doing a revert of the previous revert would be the right thing to do.

I hope this clears up confusion and fear.

Quoting Junio C Hamano <gitster@pobox.com>:

You are correct that I misunderstood what Alan meant by corrected branch.

I think your explanation will help people if we make it part of the documentation.  Especially because two different cases need two different recovery methods, and people need to learn which is which.

Thank you for your detailed response.

Nanako Shiraishi <nanako3@lavabit.com> writes:

Sure. It needs copyediting to make it readable standalone by not mentioning "your misunderstanding", inlining "earlier Linus's suggestion", etc., though.

Patches welcome ;-)

Quoting Junio C Hamano <gitster@pobox.com>:

Okay, I'll send one later.

Thanks.

Nanako Shiraishi <nanako3@lavabit.com> writes:

Thanks.

On Thursday 2008 December 18 17:25:34 Alan wrote:

Don't do tests on the master branch.  Or at least, do as little as possible.  Also, try not to revert merges.  Depending on your workflow and team synamics it might be easier to rewrite history than wrangle reverts of a merge commit.