Merge seems to get confused by (reverted) cherry-picks

Merge seems to get confused by (reverted) cherry-picks

[Björn Steinbrink]

Hi,

"git merge" produces a (IMHO) wrong result, when a commit from the
branch that is to be merged in was cherry-picked into the current branch
and later reverted on the original branch. Basically ignoring the
revert.

Example:
mkdir gmt
cd gmt
git init

/bin/echo -e "1\n2\n3\n4\n5\n6\n" > file
git add file
git commit -m init

/bin/echo -e "1\n2\n3\na\n4\n5\n6\n" > file
git add file
git commit -m add

git revert --no-edit HEAD

git checkout -b test HEAD~2
sleep 1 # Avoid race
git cherry-pick master^
git merge master


That last "git merge" call happily tells:
Already uptodate!
Merge made by recursive.

And "file" still contains the "a" that was added in the second commit.

Seems broken to me, of course I want that revert to "show up" in the
merge result, probably as a conflict for me to resolve.

Björn

Re: Merge seems to get confused by (reverted) cherry-picks

[Björn Steinbrink]

On 2008.09.03 09:20:11 +0200, Björn Steinbrink wrote:
> Hi,
> 
> "git merge" produces a (IMHO) wrong result, when a commit from the
> branch that is to be merged in was cherry-picked into the current branch
> and later reverted on the original branch. Basically ignoring the
> revert.
> 
> Example:
> mkdir gmt
> cd gmt
> git init
> 
> /bin/echo -e "1\n2\n3\n4\n5\n6\n" > file
> git add file
> git commit -m init
> 
> /bin/echo -e "1\n2\n3\na\n4\n5\n6\n" > file
> git add file
> git commit -m add
> 
> git revert --no-edit HEAD
> 
> git checkout -b test HEAD~2
> sleep 1 # Avoid race
> git cherry-pick master^
> git merge master
> 
> 
> That last "git merge" call happily tells:
> Already uptodate!
> Merge made by recursive.
> 
> And "file" still contains the "a" that was added in the second commit.
> 
> Seems broken to me, of course I want that revert to "show up" in the
> merge result, probably as a conflict for me to resolve.

I knew I would forget something... That's with:
$ git --version
git version 1.6.0.1.196.g01914

Björn

Re: Merge seems to get confused by (reverted) cherry-picks

[Junio C Hamano]

Björn Steinbrink <B.Steinbrink@gmx.de> writes:

> "git merge" produces a (IMHO) wrong result, when a commit from the
> branch that is to be merged in was cherry-picked into the current branch
> and later reverted on the original branch. Basically ignoring the
> revert.

There are a few issues around 3-way merge.

One thing is, what happened in between the common ancestor and the final
results on histories before you initiate the merges does not matter.  When
doing a 3-way merge, you look only at three endpoints: your final state,
their final state and the common ancestor between the two.

Your history looks like this:

            123a456
               3-----------?
              /           /
             /           /
            0-----1-----2
         123456       123456

During the development between 0..2, your undecision might have caused the
contents of the file fluctuate back and forth many number of times, but in
the end result, you (the one who went 0..1..2) decided that for your
development, you do not have to change the contents of that path.  The
path might have been a xyzzy.h file, and you once added an extern decl of
a function because you thought a function in xyzzy.c might be needed by
another file frotz.c, but it turned out that the function can stay private
and you removed that extern decl from xyzzy.h and ended up in the same
state.

But the other person who built the history 0..3 decided that having the
change is better than not having it.  Perhaps his code does use the
function from some other place and needs an extern.

You are merging the two histories, which means by definition you trust
your decision and the other guys decision with equal weight.  And here is
another thing.

When you compare the path in question at 0 and 2, you see they are
identical.  And you are interpreting that "I say they MUST STAY THE SAME,
while they say they want to change it some way, that is a conflict".

But in 3-way merge context, you do not interpret the fact that something
is identical between 0..2 as "they MUST STAY THE SAME".  Instead, you read
it as "My history does not care what happens to that path -- if the other
guy wants to change it, I'll happily take it."

    Note.  I am not claiming that the above interpretation will always
    match what you would expect.  I am just explaining how the underlying
    concept of 3-way merge works in general.  If you think about it in a
    realistic context, such as the "extern in xyzzy.h you did not need to
    add but the other guy needed to have", you'll realize that more often
    than not, "I do not care and let the other guy decide" interpretation
    results in a more useful result.

That essentially boils down to three rules:

 (0) If both of you did not change anything, the final result won't have
     any change (obvious);

 (1) If you decided you do not have a need to change a path, but the other
     one saw a need, you take the change;

 (2) If you and the other one both wanted to change a path but in a
     different way, you need to merge at the contents level.

And you are seeing rule (1) in action.

Re: Merge seems to get confused by (reverted) cherry-picks

[Björn Steinbrink]

On 2008.09.03 00:50:43 -0700, Junio C Hamano wrote:
> But in 3-way merge context, you do not interpret the fact that something
> is identical between 0..2 as "they MUST STAY THE SAME".  Instead, you read
> it as "My history does not care what happens to that path -- if the other
> guy wants to change it, I'll happily take it."
> 
>     Note.  I am not claiming that the above interpretation will always
>     match what you would expect.  I am just explaining how the underlying
>     concept of 3-way merge works in general.  If you think about it in a
>     realistic context, such as the "extern in xyzzy.h you did not need to
>     add but the other guy needed to have", you'll realize that more often
>     than not, "I do not care and let the other guy decide" interpretation
>     results in a more useful result.
> 
> That essentially boils down to three rules:
> 
>  (0) If both of you did not change anything, the final result won't have
>      any change (obvious);
> 
>  (1) If you decided you do not have a need to change a path, but the other
>      one saw a need, you take the change;
> 
>  (2) If you and the other one both wanted to change a path but in a
>      different way, you need to merge at the contents level.
> 
> And you are seeing rule (1) in action.

OK, so that basically means "if you cherry-pick, you better make sure
that you don't have to revert or just get your fine-toothed comb ready
when you merge later", right? Any advice on how to deal with such a
situation?

The actual use case I had was somewhat like this:
Some new code got added in commit A on the master branch as it was
intended for the next major release. Then it became obvious that it
needs to go into a minor release, so it got cherry-picked into the
maintainance branch. For some reason that new code then got moved into a
different file. That move happened on the maintainance branch (ie.
immediate bugfix). And then came the regular merge of the maintainance
branch into the master branch, which ended up with the code being
duplicated.

(Well, actually, it's a two-level maintainance branch setup, but you get
the idea...)

Of course, in a perfect world the first commit would never have been
cherry-picked and would've been originally made on the maintainance
branch, and the problem would have never shown up. But that's not the
case and I'm not omniscient, so I would have surely missed the problem,
if it wasn't for that two-level setup, which for this merge meant that
the merge result should have been equal to the branch being merged, so
it was easy to spot.

Björn

Re: Merge seems to get confused by (reverted) cherry-picks

[Linus Torvalds]

On Wed, 3 Sep 2008, Björn Steinbrink wrote:
> 
> OK, so that basically means "if you cherry-pick, you better make sure
> that you don't have to revert or just get your fine-toothed comb ready
> when you merge later", right? Any advice on how to deal with such a
> situation?

Well, it's not actually even really related to cherry-picks in particular, 
although yes, cherry-picks and reverts are perhaps the simplest case to 
explain.

At a more fundamental level, it's about git _not_ caring about any 
individual commit in the history, and only caring about the "big picture". 

In this, git is fairly consistent - the same way that git never cares 
about any individual _file_, but always merges the whole tree, it also 
never really cares about any individual _commit_, but always the whole 
history.

So it really doesn't matter if one commit undid another - the only thing 
that matters as far as git merging is concerned is what the final set of 
changes were from the common base point.

Now, there's nothing to say that git _couldn't_ try to look at individual 
commits when deciding how to merge, but I actually think it's 
fundamentally wrong to do so, for pretty much the same reason I think it's 
fundamentally wrong to try to encode rename information.

The fact is, should it really matter whether something was "reverted", or 
whether multiple gradual changes made ot go back to what it used to be? 
Git says no, an considers the two to be totally equivalent in the end. 
exactly the same way that git doesn't matter whether you first created a 
new file and later deleted a similar old file, or whether you renamed it. 

The only thing that matters is the end result.

So if you have one branch that first does "A", and then does "revert A", 
as far as git is concerned, that branch didn't do anything at all when it 
comes to data.

So when you merge it with another branch that does "A" too, the end result 
is that the merged contents will have A. That's fairly easy to understand 
if you just think of git as caring about the whole end result and just 
doing a three-way merge at the end points (which is what it does), but I 
would also like to explain why I think it's fundamentally the _right_ 
thing to do, not just an "implementation detail because it's simpler".

When one branch does the "revert A" does that really mean that A was bad? 
No. It could mean that A was "unnecessary" or "not quite ready". The 
revert, after all, was literally done in the context of _that_ branch, and 
the reasons may well have been totally private to that branch. Git doesn't 
know, and git shouldn't care - the only thing that should matter is the 
end result.

To really hammer this point in, let's say that "A" was really doing two 
different things - A1 and A2 - to two different files. And let's further 
say that it had been cherry-picked because the one branch needed just a 
part of it - A1. And then later, in a fit of cleanup madness, that branch 
undid A2, because it really didn't need it.

When you merge the two branches, what do you expect? You could argue that 
you would expect A2 to be undone in the original branch too. It was, after 
all, a partial revert. But I think you'll agree that it's not at all 
"obvious" any more.

In fact, I will argue that it would be horribly _wrong_, because the 
branch that undid part of A could have done it two different ways:

 - it could do the "cherry-pick A" as one commit and the "undo A2" as 
   another (as I implied above)

 - but it could equally well have done a "cherry-pick just part of A", and 
   done it as just one commit (perhaps because it noticed that A2 didn't 
   even _compile_ within the context of that branch, and did an '--amend' 
   to fix it up rather than create a new commit.

See? Shouldn't both really act the same? Should it really make a 
difference to what git does if there was a cherry-pick and a partial 
revert, or a partial cherry-pick? Should _how_ you do something matter 
more than the end result? HELL NO!

And is the "partial revert" really any different from the "total revert"?

Now, if you're a math person, think of the "limit behavior" as A2 
approaches all of A. The final end result is that you were to revert _all_ 
of A. Should that limit case be fundamentally different from the case of 
A2 being just a _part_ of A? What should the logic be? What if all of A 
was reverted except for the whitespace cleanups that it did (almost by 
mistake?)

So in the end, the answer is that git doesn't care about individual 
commits, because caring about individual commits is totally crazy. Git 
_remembers_ them, of course, and it can _show_ you them when you merge, 
but the actual end result depends purely on the *state* of the merge (and 
the "big picture" of the history, ie where the branches join etc), not of 
the small details of how you got to that state.

And that is fundamentally the only sane thing to do.

Here's another final thought to leave you with:

 - what if the other branch had decided that instead of reverting it, it 
   could just do a "git rebase -i" _without_ it, because that other branch 
   had never been exposed to anybody else?

See? The "how you got to some state" really must be immaterial in a stable 
merge strategy. I realize that I'm at odds with some SCM people on this, 
but I'm ok with that, because I also realize that all those other SCM 
people are just _stupid_.

			Linus

Re: Merge seems to get confused by (reverted) cherry-picks

[Ittay Dror]

Note: codeville tried to implement a merge algorithm that considers the 
history to decide what the user wants to do: 
http://revctrl.org/PreciseCodevilleMerge. Maybe worth while exploring?

Ittay

Junio C Hamano wrote:
> Björn Steinbrink <B.Steinbrink@gmx.de> writes:
>
>   
>> "git merge" produces a (IMHO) wrong result, when a commit from the
>> branch that is to be merged in was cherry-picked into the current branch
>> and later reverted on the original branch. Basically ignoring the
>> revert.
>>     
>
> There are a few issues around 3-way merge.
>
> One thing is, what happened in between the common ancestor and the final
> results on histories before you initiate the merges does not matter.  When
> doing a 3-way merge, you look only at three endpoints: your final state,
> their final state and the common ancestor between the two.
>
> Your history looks like this:
>
>             123a456
>                3-----------?
>               /           /
>              /           /
>             0-----1-----2
>          123456       123456
>
> During the development between 0..2, your undecision might have caused the
> contents of the file fluctuate back and forth many number of times, but in
> the end result, you (the one who went 0..1..2) decided that for your
> development, you do not have to change the contents of that path.  The
> path might have been a xyzzy.h file, and you once added an extern decl of
> a function because you thought a function in xyzzy.c might be needed by
> another file frotz.c, but it turned out that the function can stay private
> and you removed that extern decl from xyzzy.h and ended up in the same
> state.
>
> But the other person who built the history 0..3 decided that having the
> change is better than not having it.  Perhaps his code does use the
> function from some other place and needs an extern.
>
> You are merging the two histories, which means by definition you trust
> your decision and the other guys decision with equal weight.  And here is
> another thing.
>
> When you compare the path in question at 0 and 2, you see they are
> identical.  And you are interpreting that "I say they MUST STAY THE SAME,
> while they say they want to change it some way, that is a conflict".
>
> But in 3-way merge context, you do not interpret the fact that something
> is identical between 0..2 as "they MUST STAY THE SAME".  Instead, you read
> it as "My history does not care what happens to that path -- if the other
> guy wants to change it, I'll happily take it."
>
>     Note.  I am not claiming that the above interpretation will always
>     match what you would expect.  I am just explaining how the underlying
>     concept of 3-way merge works in general.  If you think about it in a
>     realistic context, such as the "extern in xyzzy.h you did not need to
>     add but the other guy needed to have", you'll realize that more often
>     than not, "I do not care and let the other guy decide" interpretation
>     results in a more useful result.
>
> That essentially boils down to three rules:
>
>  (0) If both of you did not change anything, the final result won't have
>      any change (obvious);
>
>  (1) If you decided you do not have a need to change a path, but the other
>      one saw a need, you take the change;
>
>  (2) If you and the other one both wanted to change a path but in a
>      different way, you need to merge at the contents level.
>
> And you are seeing rule (1) in action.
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>   

-- 
Ittay Dror <ittayd@tikalk.com>
Tikal <http://www.tikalk.com>
Tikal Project <http://tikal.sourceforge.net>


-- 
--
Ittay Dror <ittay.dror@gmail.com>

Re: Merge seems to get confused by (reverted) cherry-picks

[Andreas Ericsson]

Ittay Dror wrote:
> Note: codeville tried to implement a merge algorithm that considers the 
> history to decide what the user wants to do: 
> http://revctrl.org/PreciseCodevilleMerge. Maybe worth while exploring?
> 

I believe that one would still fail for this case, as "ImplicitUndo"
isn't included.

Besides that, I'd rather have something simple that does something
predictable than something complex that pushes the "gets it right"
figure from 98% to 99.2%, while failing in completely annoying and
surprising ways sometimes. Someone or something still has to verify
that the merge produced something sane.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Re: Merge seems to get confused by (reverted) cherry-picks

[Johan Herland]

On Wednesday 03 September 2008, Ittay Dror wrote:
> Note: codeville tried to implement a merge algorithm that considers
> the history to decide what the user wants to do:
> http://revctrl.org/PreciseCodevilleMerge. Maybe worth while
> exploring?

You haven't been here long, have you? ;)

There was an infamous mailing list discussion between Bram Cohen (who 
created PreciseCodevilleMerge), and Linus Torvalds back in 2005, 
discussing merge strategies. It is well recounted here:

http://wincent.com/a/about/wincent/weblog/archives/2007/07/a_look_back_bra.php

Also, nowadays, even Bram himself seems to have conceded 
PreciseCodevilleMerge in favour of traditional 3-way merge, at least if 
you look at item #3 in the following posting from his blog:

http://bramcohen.livejournal.com/52148.html



...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

Re: Merge seems to get confused by (reverted) cherry-picks

[Mike Hommey]

On Wed, Sep 03, 2008 at 01:16:05PM +0200, Johan Herland wrote:
> On Wednesday 03 September 2008, Ittay Dror wrote:
> > Note: codeville tried to implement a merge algorithm that considers
> > the history to decide what the user wants to do:
> > http://revctrl.org/PreciseCodevilleMerge. Maybe worth while
> > exploring?
> 
> You haven't been here long, have you? ;)
> 
> There was an infamous mailing list discussion between Bram Cohen (who 
> created PreciseCodevilleMerge), and Linus Torvalds back in 2005, 
> discussing merge strategies. It is well recounted here:
> 
> http://wincent.com/a/about/wincent/weblog/archives/2007/07/a_look_back_bra.php

I quite agree with the footnote statement, except on one thing: it would
make it easier to have additional tools to help with such merge
resolutions automatically.

While resolving a rename/delete merge conflict is not a big deal,
resolving function moves from one file to another (possibly new) file is
another story.

Mike