Odd merge behaviour involving reverts

Odd merge behaviour involving reverts

[Alan]

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?

Re: Odd merge behaviour involving reverts

[Linus Torvalds]

On Thu, 18 Dec 2008, Alan wrote:
> 
> What am i doing wrong here?

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

Re: Odd merge behaviour involving reverts

[Alan]

On Thu, 2008-12-18 at 15:58 -0800, Linus Torvalds wrote:
> 
> On Thu, 18 Dec 2008, Alan wrote:
> > 
> > What am i doing wrong here?
> 
> Reverting a merge is your problem.
> 
> You can do it, but you seem to have done it without understanding what it 
> causes.

Obviously.

> 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 I will have to look for previous reverts (or "pre-verts") before
merging again.

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

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.

Re: Odd merge behaviour involving reverts

[Linus Torvalds]

On Thu, 18 Dec 2008, Alan wrote:
> 
> 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.

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

Re: Odd merge behaviour involving reverts

[Alan]

On Thu, 2008-12-18 at 16:21 -0800, Linus Torvalds wrote:
> 
> On Thu, 18 Dec 2008, Alan wrote:
> > 
> > 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.
> 
> 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.

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

Re: Odd merge behaviour involving reverts

[Linus Torvalds]

On Thu, 18 Dec 2008, Alan wrote:
> 
> So what is the recommended way to undo mistaken merges caught after the
> fact that will not fubar later merges?

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

Re: Odd merge behaviour involving reverts

[Boyd Stephen Smith Jr.]

[-- Attachment #1: Type: text/plain, Size: 944 bytes --]

On Thursday 2008 December 18 17:25:34 Alan wrote:
> 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.
>
> What am i doing wrong here?

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.
-- 
Boyd Stephen Smith Jr.                     ,= ,-_-. =. 
bss@iguanasuicide.net                     ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy           `-'(. .)`-' 
http://iguanasuicide.net/                      \_/     

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: Odd merge behaviour involving reverts

[Nanako Shiraishi]

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

> On Thu, 18 Dec 2008, Alan wrote:
>> 
>> What am i doing wrong here?
>
> 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.

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?

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

Re: Odd merge behaviour involving reverts

[Linus Torvalds]

On Fri, 19 Dec 2008, Nanako Shiraishi wrote:
> 
> 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?

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

Re: Odd merge behaviour involving reverts

[Jay Soffian]

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

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

j.

Re: Odd merge behaviour involving reverts

[Linus Torvalds]

On Thu, 18 Dec 2008, Jay Soffian wrote:
> 
> Instead of reverting the revert, why not rebase the branch which was
> merged to the point after the revert was done?

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

Re: Odd merge behaviour involving reverts

[Daniel Barkalow]

On Thu, 18 Dec 2008, Linus Torvalds wrote:

> On Fri, 19 Dec 2008, Nanako Shiraishi wrote:
> > 
> > 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?
> 
> 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!

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*

Re: Odd merge behaviour involving reverts

[Junio C Hamano]

Nanako Shiraishi <nanako3@lavabit.com> writes:

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

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.

Re: Odd merge behaviour involving reverts

[Nanako Shiraishi]

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

> I hope this clears up confusion and fear.

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
http://ivory.ap.teacup.com/nanako3/

Re: Odd merge behaviour involving reverts

[Junio C Hamano]

Nanako Shiraishi <nanako3@lavabit.com> writes:

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

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

Patches welcome ;-)

Re: Odd merge behaviour involving reverts

[Nanako Shiraishi]

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

> Nanako Shiraishi <nanako3@lavabit.com> writes:
>
>> 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.
>
> Sure.  It needs copyediting to make it readable standalone by not
> mentioning "your misunderstanding", inlining "earlier Linus's suggestion",
> etc., though.
>
> Patches welcome ;-)

Okay, I'll send one later.

Thanks.
-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

Re: Odd merge behaviour involving reverts

[Junio C Hamano]

Nanako Shiraishi <nanako3@lavabit.com> writes:

> Quoting Junio C Hamano <gitster@pobox.com>:
>
>> Nanako Shiraishi <nanako3@lavabit.com> writes:
>>
>>> 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.
>>
>> Sure.  It needs copyediting to make it readable standalone by not
>> mentioning "your misunderstanding", inlining "earlier Linus's suggestion",
>> etc., though.
>>
>> Patches welcome ;-)
>
> Okay, I'll send one later.

Thanks.