threads / discuss / 1730

bogus merges

Subject: bogus merges

## tl;dr

13 messages between Sep 5, 2005 and Sep 12, 2005.

replies: 12people: 7as markdown or json

Wayne Scott· Sep 5, 2005, 14:38 UTC · lore
A recent commit in linux-2.6 looks like this:
commit b129a8ccd53f74c43e4c83c8e0031a4990040830
Merge: 6b39374a27eb4be7e9d82145ae270ba02ea90dc8
194d0710e1a7fe92dcf860ddd31fded8c3103b7a
Author: Russell King <rmk@dyn-67.arm.linux.org.uk>
Date:   Wed Aug 31 10:12:14 2005 +0100
    [SERIAL] Clean up and fix tty transmission start/stoping
    
The problem is that two parents of this merge are separate.  One is an
ancestor of the other.  This means the merge is really pointless and
probably accidental.

Really 'git commit' should detect problems like this automatically and prevent them from getting in the tree.

-Wayne
Linus Torvalds· Sep 5, 2005, 16:01 UTC · re: Wayne Scott · lore

Re: bogus merges

On Mon, 5 Sep 2005, Wayne Scott wrote:
>
> A recent commit in linux-2.6 looks like this:

It hopefully shouldn't happen any more with the improved and fixed git-merge-base.

> Author: Russell King <rmk@dyn-67.arm.linux.org.uk>
> Date:   Wed Aug 31 10:12:14 2005 +0100

I suspect rmk is using cogito-0.13, and that it still has the older git-merge-base that can get confused by the date-ordering problem. And when git-merge-base gives the wrong result (not either of the commit objects it was given as an argument), then "git resolve" will do the wrong thing.

I just checked, and the _current_ git-merge-base definitely gives the right result:

	linux$ git-merge-base -a \
		6b39374a27eb4be7e9d82145ae270ba02ea90dc8 \
		194d0710e1a7fe92dcf860ddd31fded8c3103b7a
results in
	194d0710e1a7fe92dcf860ddd31fded8c3103b7a

ie it correctly notices that the second commit is the parent of the first one.

> Really 'git commit' should detect problems like this automatically and
> prevent them from getting in the tree.

Well, that would depend on having the fixed git-merge-base in the first place, which in turn would mean that such a commit wouldn't happen at all, so it's kind of circular. It's not worth fixing anywhere else, since once you fix it in git-merge-base, it just becomes a non-issue.

Hopefully we'll have a new cogito release soon, and this particular bug will be a thing of the past.

		Linus
Wayne Scott· Sep 6, 2005, 13:08 UTC · re: Linus Torvalds · lore

Re: bogus merges

On 9/5/05, Linus Torvalds <torvalds@osdl.org> wrote:
Show 7 quoted lines
> > Really 'git commit' should detect problems like this automatically and
> > prevent them from getting in the tree.
> 
> Well, that would depend on having the fixed git-merge-base in the first
> place, which in turn would mean that such a commit wouldn't happen at all,
> so it's kind of circular. It's not worth fixing anywhere else, since once
> you fix it in git-merge-base, it just becomes a non-issue.

This just error checking to prevent future bugs from getting committed to the tree. These kinds of things are very hard to repair after the fact.

-Wayne
Russell King· Sep 11, 2005, 11:06 UTC · re: Wayne Scott · lore

Re: bogus merges

On Tue, Sep 06, 2005 at 08:08:57AM -0500, Wayne Scott wrote:
Show 12 quoted lines
> On 9/5/05, Linus Torvalds <torvalds@osdl.org> wrote:
> > > Really 'git commit' should detect problems like this automatically and
> > > prevent them from getting in the tree.
> > 
> > Well, that would depend on having the fixed git-merge-base in the first
> > place, which in turn would mean that such a commit wouldn't happen at all,
> > so it's kind of circular. It's not worth fixing anywhere else, since once
> > you fix it in git-merge-base, it just becomes a non-issue.
> 
> This just error checking to prevent future bugs from getting committed
> to the tree.  These kinds of things are very hard to repair after the
> fact.
They don't get repaired.

The real problem is that there needs to be a way for cogito to remain more up to date with git, so that when problems are fixed in git they can propagate into cogito faster.

I'm not sure separating cogito from git is a good solution though - I've no real view on how stable the interface between those two bits are, but I'd hate for a git upgrade to break cogito in some subtle way.

-- 
Russell King
Daniel Barkalow· Sep 6, 2005, 18:28 UTC · re: Linus Torvalds · lore

Re: bogus merges

On Mon, 5 Sep 2005, Linus Torvalds wrote:
Show 6 quoted lines
> On Mon, 5 Sep 2005, Wayne Scott wrote:
> >
> > A recent commit in linux-2.6 looks like this:
> 
> It hopefully shouldn't happen any more with the improved and fixed
> git-merge-base.

Couldn't it also happen if there's stale data in MERGE_HEAD when you commit a normal patch? The description doesn't look like a merge at all, but rather like a normal patch that inappropriately picked up an extra head. I'd guess he tried to merge something, got a conflict, decided that he didn't really want to do that anyway, switched to a different branch, applied a patch, and committed without noticing the note that he seemed to be committing a merge.

Probably the right thing is actually to clean up more when switching tasks, but it would probably also be worth checking that merges make sense as well.

	-Daniel
*This .sig left intentionally blank*
Russell King· Sep 11, 2005, 11:01 UTC · re: Daniel Barkalow · lore

Re: bogus merges

On Tue, Sep 06, 2005 at 02:28:39PM -0400, Daniel Barkalow wrote:
Show 10 quoted lines
> On Mon, 5 Sep 2005, Linus Torvalds wrote:
> > On Mon, 5 Sep 2005, Wayne Scott wrote:
> > >
> > > A recent commit in linux-2.6 looks like this:
> > 
> > It hopefully shouldn't happen any more with the improved and fixed
> > git-merge-base.
> 
> Couldn't it also happen if there's stale data in MERGE_HEAD when you 
> commit a normal patch?
I don't think Cogito has the idea of a MERGE_HEAD file.
Show 6 quoted lines
> The description doesn't look like a merge at all, 
> but rather like a normal patch that inappropriately picked up an extra 
> head. I'd guess he tried to merge something, got a conflict, decided that 
> he didn't really want to do that anyway, switched to a different branch, 
> applied a patch, and committed without noticing the note that he seemed to 
> be committing a merge.

"HE'd" hadn't (please don't use the third person when you're talking about someone who is being copied on the thread, thanks.) Look closer at the two heads (I've cut out the author info):

commit b129a8ccd53f74c43e4c83c8e0031a4990040830
tree 4c40afd836be87166d6d014380262f1baa19694f
parent 6b39374a27eb4be7e9d82145ae270ba02ea90dc8
parent 194d0710e1a7fe92dcf860ddd31fded8c3103b7a
committer Russell King <rmk+kernel@arm.linux.org.uk> Wed, 31 Aug 2005 10:12:14 +0100
    [SERIAL] Clean up and fix tty transmission start/stoping
which was apparantly (but not really) a merge between:
commit 6b39374a27eb4be7e9d82145ae270ba02ea90dc8
tree 09933113cf28f253db1dd539463bdab741d67139
parent 62c592edead3c3a045662595f7ade3c12f133373
parent ed735ccbefaf7e5e3ef61418f7e209b8c59308a7
committer Linus Torvalds <torvalds@g5.osdl.org> Tue, 30 Aug 2005 11:16:30 -0700
    Merge refs/heads/upstream from master.kernel.org:/pub/scm/linux/kernel/git/jgarzik/netdev-2.6.git
commit 194d0710e1a7fe92dcf860ddd31fded8c3103b7a
tree da03b56fa4dee221c53af5770492d391f0d09459
parent a68d2ebc1581a3aec57bd032651e013fa609f530
parent 9bbd03758945858c9303f3258b418b94c4ffd735
committer Linus Torvalds <torvalds@g5.osdl.org> Wed, 03 Aug 2005 13:09:43 -0700
    Merge master.kernel.org:/home/rmk/linux-2.6-arm

However, the serial tree (which commit b129a8ccd53f74c43e4c83c8e0031a4990040830 was created in) was last pulled in commit 975f957dc408925805dd8f5aa4217b7eeea2d005. Following the commit trail, you'll end up at commit 6b39374a27eb4be7e9d82145ae270ba02ea90dc8 above.

commit 975f957dc408925805dd8f5aa4217b7eeea2d005
tree 2198bb72323a016d93c7440c9240bac94a5c0016
parent 2321fbd2b87539edc1fbfc2e186528a1ef93835f
parent 661299d9d0437a0ff72240f3d60016ac3a361a6e
committer Linus Torvalds <torvalds@g5.osdl.org> Mon, 29 Aug 2005 10:34:59 -0700
    Merge HEAD from master.kernel.org:/home/rmk/linux-2.6-serial.git

which was the result of the following merge between the head of the serial tree and Linus' tree:

commit 661299d9d0437a0ff72240f3d60016ac3a361a6e
tree 765512576314fc3612b503f182b9ae4e60fcf849
parent 05caac585f8abd6c0113856bc8858e3ef214d8a6
parent 41c018b7ecb60b1c2c4d5dee0cd37d32a94c45af
committer Russell King <rmk+kernel@arm.linux.org.uk> Thu, 28 Jul 2005 09:30:20 +0100
    Merge with Linus' 2.6 tree
This was at the head of the serial tree at the time:
commit 05caac585f8abd6c0113856bc8858e3ef214d8a6
tree ac9f8f2cc032281af09200da514257d120510906
parent 241fc4367b3ca5d407b043599ed980304a70b91f
committer Russell King <rmk+kernel@arm.linux.org.uk> Wed, 27 Jul 2005 11:41:18 +0100
    [SERIAL] Convert parport_serial to use new 8250_pci interfaces

So, the question is, if the serial git tree was all happy in the merge on 29th August (975f957dc408925805dd8f5aa4217b7eeea2d005) why did the next commit to that tree (being b129a8ccd53f74c43e4c83c8e0031a4990040830) go wrong when the previous merge would have been a standard fast-forward operation.

There is _no_ reason why the serial tree would have been wound back to 3rd August.

> Probably the right thing is actually to clean up more when switching 
> tasks, but it would probably also be worth checking that merges make sense 
> as well.

You might be right except for one small detail. I don't "switch tasks" with a single git tree. I have a set of individual git trees, one per "task". They always remain in a clean state due to the way I work:

	- update tree to Linus if Linus tree is a superset of the tree
	- apply changes in patch form to tree and commit each in turn
	- send linus a request to merge

However, occasionally, I update the tree when Linus' tree is not a superset, as was the case in 661299d9d0437a0ff72240f3d60016ac3a361a6e above.

-- 
Russell King
Daniel Barkalow· Sep 11, 2005, 18:08 UTC · re: Russell King · lore

Re: bogus merges

On Sun, 11 Sep 2005, Russell King wrote:
Show 57 quoted lines
> Look closer at the two heads (I've cut out the author info):
> 
> commit b129a8ccd53f74c43e4c83c8e0031a4990040830
> tree 4c40afd836be87166d6d014380262f1baa19694f
> parent 6b39374a27eb4be7e9d82145ae270ba02ea90dc8
> parent 194d0710e1a7fe92dcf860ddd31fded8c3103b7a
> committer Russell King <rmk+kernel@arm.linux.org.uk> Wed, 31 Aug 2005 10:12:14 +0100
>     [SERIAL] Clean up and fix tty transmission start/stoping
> 
> which was apparantly (but not really) a merge between:
> 
> commit 6b39374a27eb4be7e9d82145ae270ba02ea90dc8
> tree 09933113cf28f253db1dd539463bdab741d67139
> parent 62c592edead3c3a045662595f7ade3c12f133373
> parent ed735ccbefaf7e5e3ef61418f7e209b8c59308a7
> committer Linus Torvalds <torvalds@g5.osdl.org> Tue, 30 Aug 2005 11:16:30 -0700
>     Merge refs/heads/upstream from master.kernel.org:/pub/scm/linux/kernel/git/jgarzik/netdev-2.6.git
> 
> commit 194d0710e1a7fe92dcf860ddd31fded8c3103b7a
> tree da03b56fa4dee221c53af5770492d391f0d09459
> parent a68d2ebc1581a3aec57bd032651e013fa609f530
> parent 9bbd03758945858c9303f3258b418b94c4ffd735
> committer Linus Torvalds <torvalds@g5.osdl.org> Wed, 03 Aug 2005 13:09:43 -0700
>     Merge master.kernel.org:/home/rmk/linux-2.6-arm
> 
> However, the serial tree (which commit b129a8ccd53f74c43e4c83c8e0031a4990040830
> was created in) was last pulled in commit 975f957dc408925805dd8f5aa4217b7eeea2d005.
> Following the commit trail, you'll end up at commit
> 6b39374a27eb4be7e9d82145ae270ba02ea90dc8 above.
> 
> commit 975f957dc408925805dd8f5aa4217b7eeea2d005
> tree 2198bb72323a016d93c7440c9240bac94a5c0016
> parent 2321fbd2b87539edc1fbfc2e186528a1ef93835f
> parent 661299d9d0437a0ff72240f3d60016ac3a361a6e
> committer Linus Torvalds <torvalds@g5.osdl.org> Mon, 29 Aug 2005 10:34:59 -0700
>     Merge HEAD from master.kernel.org:/home/rmk/linux-2.6-serial.git
> 
> which was the result of the following merge between the head of the
> serial tree and Linus' tree:
> 
> commit 661299d9d0437a0ff72240f3d60016ac3a361a6e
> tree 765512576314fc3612b503f182b9ae4e60fcf849
> parent 05caac585f8abd6c0113856bc8858e3ef214d8a6
> parent 41c018b7ecb60b1c2c4d5dee0cd37d32a94c45af
> committer Russell King <rmk+kernel@arm.linux.org.uk> Thu, 28 Jul 2005 09:30:20 +0100
>     Merge with Linus' 2.6 tree
> 
> This was at the head of the serial tree at the time:
> 
> commit 05caac585f8abd6c0113856bc8858e3ef214d8a6
> tree ac9f8f2cc032281af09200da514257d120510906
> parent 241fc4367b3ca5d407b043599ed980304a70b91f
> committer Russell King <rmk+kernel@arm.linux.org.uk> Wed, 27 Jul 2005 11:41:18 +0100
>     [SERIAL] Convert parport_serial to use new 8250_pci interfaces
> 
> You might be right except for one small detail.  I don't "switch tasks"
> with a single git tree.

I think I was right, but "switching tasks" has to be interpretted as switching from a merge you don't care to complete to doing further development on that tree, and the "switch" is done by a fast-forward (to the result of someone else completing the merge).

Show 10 quoted lines
>  I have a set of individual git trees, one per
> "task".  They always remain in a clean state due to the way I work:
> 
> 	- update tree to Linus if Linus tree is a superset of the tree
> 	- apply changes in patch form to tree and commit each in turn
> 	- send linus a request to merge
> 
> However, occasionally, I update the tree when Linus' tree is not a
> superset, as was the case in 661299d9d0437a0ff72240f3d60016ac3a361a6e
> above.
Ah, okay. So I think the chain of events is:
 28 Jul: You merge with Linus, when he hasn't pulled your tree. This goes 
         fine.
 03 Aug: You pull from Linus again, but decide that you don't actually 
         care about tracking mainline in this tree so tightly. This leaves
         194d... in .git/merging, because Cogito doesn't realize you're 
         not actually planning to complete the merge.
 29 Aug: Linus pulls from you. This goes fine.
 30 Aug: You update the tree again from Linus. This is a fast-forward and 
         goes fine, and doesn't touch .git/merging because it completes
         the process without requiring input or a commit.
 31 Aug: You make a different change and commit. This is the first time 
         you committed there since 03 Aug, and it thinks that you're still 
         merging your current head (now from 30 Aug) with 194d..., which 
         it marked down explicitly to track this when you started the 
         merge on the 3rd, and you don't get sufficient information to 
         realize that it's messed up.

I think that the fundamental bug is that tree_timewrap() doesn't clear .git/merging, which means that it can still think you're doing a merge when you've started doing something else (such as doing further development after updating to someone else's merge of your tree).

But I still think that it would be worth checking for the parents in merge commits making sense, because the fact that stale data is likely to be from abandoned operations on ancestors.

	-Daniel
*This .sig left intentionally blank*
Petr Baudis· Sep 12, 2005, 00:58 UTC · re: Daniel Barkalow · lore

Re: bogus merges

Dear diary, on Sun, Sep 11, 2005 at 08:08:14PM CEST, I got a letter where Daniel Barkalow <barkalow@iabervon.org> told me that...

> I think that the fundamental bug is that tree_timewrap() doesn't clear 
> .git/merging, which means that it can still think you're doing a merge 
> when you've started doing something else (such as doing further 
> development after updating to someone else's merge of your tree).
Oh, nice catch. Thanks, fixed.
-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
If you want the holes in your knowledge showing up try teaching
someone.  -- Alan Cox
Russell King· Sep 11, 2005, 10:21 UTC · re: Linus Torvalds · lore

Re: bogus merges

On Mon, Sep 05, 2005 at 09:01:32AM -0700, Linus Torvalds wrote:
Show 10 quoted lines
> On Mon, 5 Sep 2005, Wayne Scott wrote:
> > A recent commit in linux-2.6 looks like this:
> 
> It hopefully shouldn't happen any more with the improved and fixed
> git-merge-base.
> 
> > Author: Russell King <rmk@dyn-67.arm.linux.org.uk>
> > Date:   Wed Aug 31 10:12:14 2005 +0100
> 
> I suspect rmk is using cogito-0.13

Correct, and rmk will probably be extremely nervous about upgrading when 0.14 appears.

-- 
Russell King
Martin Langhoff· Sep 11, 2005, 11:48 UTC · re: Russell King · lore

Re: bogus merges

On 9/11/05, Russell King <rmk@arm.linux.org.uk> wrote:
Show 5 quoted lines
> On Mon, Sep 05, 2005 at 09:01:32AM -0700, Linus Torvalds wrote:
> > I suspect rmk is using cogito-0.13
> 
> Correct, and rmk will probably be extremely nervous about upgrading when
> 0.14 appears.

Well, *actually* cogito-0.13 didn't include git-core, so we have to look for the reasons elsewhere. Could be the leftover MERGE_HEAD Daniel mentions.

Russel, can you confirm what git-core version you are/were running?
cheers,.
martin
A Large Angry SCM· Sep 11, 2005, 15:11 UTC · re: Martin Langhoff · lore

Re: bogus merges

Martin Langhoff wrote:
Show 11 quoted lines
> On 9/11/05, Russell King <rmk@arm.linux.org.uk> wrote:
>>On Mon, Sep 05, 2005 at 09:01:32AM -0700, Linus Torvalds wrote:
>>>I suspect rmk is using cogito-0.13
>>Correct, and rmk will probably be extremely nervous about upgrading when
>>0.14 appears.
> 
> Well, *actually* cogito-0.13 didn't include git-core, so we have to
> look for the reasons elsewhere. Could be the leftover MERGE_HEAD
> Daniel mentions.
> 
> Russel, can you confirm what git-core version you are/were running?
Russel,

How are you updating your tree to Linus'? If you are rsyncing from kernel.org, you're probably getting a MERGE_HEAD with the rsync. A while ago I got annoyed enough add the equivalent of:

	rm -f ${LOCAL_REPOS}/.git/MERGE_HEAD
to my (very stupid) git rsync script.
Russell King· Sep 11, 2005, 18:36 UTC · re: A Large Angry SCM · lore

Re: bogus merges

On Sun, Sep 11, 2005 at 11:11:49AM -0400, A Large Angry SCM wrote:
Show 22 quoted lines
> Martin Langhoff wrote:
> > On 9/11/05, Russell King <rmk@arm.linux.org.uk> wrote:
> >>On Mon, Sep 05, 2005 at 09:01:32AM -0700, Linus Torvalds wrote:
> >>>I suspect rmk is using cogito-0.13
> >>Correct, and rmk will probably be extremely nervous about upgrading when
> >>0.14 appears.
> > 
> > Well, *actually* cogito-0.13 didn't include git-core, so we have to
> > look for the reasons elsewhere. Could be the leftover MERGE_HEAD
> > Daniel mentions.
> > 
> > Russel, can you confirm what git-core version you are/were running?
> 
> Russel,
> 
> How are you updating your tree to Linus'? If you are rsyncing from 
> kernel.org, you're probably getting a MERGE_HEAD with the rsync. A while 
> ago I got annoyed enough add the equivalent of:
> 
> 	rm -f ${LOCAL_REPOS}/.git/MERGE_HEAD
> 
> to my (very stupid) git rsync script.
I think you can forget MERGE_HEAD.  Why?  I use cogito for pulling.

cg-pull gets the head, and then rsyncs the objects found in the object directory. It doesn't touch MERGE_HEAD. None of the cogito scripts that I have know anything about MERGE_HEAD itself.

(Plus, it's actually a two stage pull - I have a cron-based cg-pull into a master repository on one box, which I then cg-pull to the development box which is only powered when I'm working.)

-- 
Russell King
Russell King· Sep 11, 2005, 18:00 UTC · re: Martin Langhoff · lore

Re: bogus merges

On Sun, Sep 11, 2005 at 11:48:10PM +1200, Martin Langhoff wrote:
Show 12 quoted lines
> On 9/11/05, Russell King <rmk@arm.linux.org.uk> wrote:
> > On Mon, Sep 05, 2005 at 09:01:32AM -0700, Linus Torvalds wrote:
> > > I suspect rmk is using cogito-0.13
> > 
> > Correct, and rmk will probably be extremely nervous about upgrading when
> > 0.14 appears.
> 
> Well, *actually* cogito-0.13 didn't include git-core, so we have to
> look for the reasons elsewhere. Could be the leftover MERGE_HEAD
> Daniel mentions.
> 
> Russel, can you confirm what git-core version you are/were running?

Nope, because now that I look I'm actually using cogito 0.12.1 not 0.13 as I previously mentioned. Oops.

-- 
Russell King

← back to recent threads