git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@osdl.org>
To: Paul Mackerras <paulus@samba.org>
Cc: Junio C Hamano <junkio@cox.net>,
	Git Mailing List <git@vger.kernel.org>,
	Marco Costalba <mcostalba@yahoo.it>,
	Aneesh Kumar <aneesh.kumar@gmail.com>
Subject: Re: The merge from hell...
Date: Sat, 4 Feb 2006 12:59:59 -0800 (PST)	[thread overview]
Message-ID: <Pine.LNX.4.64.0602041239471.3854@g5.osdl.org> (raw)
In-Reply-To: <Pine.LNX.4.64.0602041125440.3854@g5.osdl.org>



On Sat, 4 Feb 2006, Linus Torvalds wrote:
> 
> Doing a 
> 
> 	git-rev-list --parents HEAD |
> 		egrep '^.{90}' |
> 		cut -d' ' -f1 | 
> 		git-diff-tree --pretty --cc --stdin
> 		| less -S
> 
> on the kernel is actually interesting. It's interesting because it shows 
> that out of 1391 merges, in the kernel, only _19_ actually had these close 
> calls. Some - but certainly not all - of them actually did need manual 
> fixup.

There are some doubly interesting lessons when I looked closer.

In particular, some merges that needed manual fixups do _not_ show up. I 
found that surprising, at first. I expected that if I had to fix something 
manually, it would obviously show up in the "--cc" output.

Not so.  In fact, the one I looked closer at didn't show up even in the 
"long" version, aka "-c".

The reason? A lot of the manual fixups end up selecting one version or the 
other - the clash is because two people fixes the same bug slightly 
differently, and the manual merge will end up just selecting one of them. 
So then even "-c" won't show it, because it will notice that the whole 
file was actually the same as one of the branches merged.

That may be a bit non-intuitive (maybe it shouldn't be, and it was just me 
who didn't think about it the right way when I was initially surprised), 
but it was definitely the right thing (both from a merge standpoint _and_ 
from a "what happened" standpoint) in the cases I looked at. The merge may 
have been manual, but the end _result_ was trivial, and thus isn't shown.

So even after looking at it more, and searching for "interesting" cases 
from the other side, I really like the current git-diff-tree --cc output. 
It sometimes shows you things you wouldn't expect, and it sometimes 
doesn't show you things you'd expect to show up, but in both cases it 
shows/avoids the _right_ things.

However, the point that a "diff" itself isn't totally unambigious is well 
taken. You're right that the very first hunk of the 12-way merge is really 
not interesting.

However, looking at the other cases, it seems to not really be a huge 
problem - that seems to be the only case in the whole kernel - and the 
git-diff-tree algorithm may show an unnecessary hunk once in a blue moon, 
but that's better than having the heuristics fail the other way around (ie 
not showing a hunk).

That's what the gitk problem was, btw (showing too little, not too much). 
Current gitk fails on this trivial case:

	mkdir test-merge
	cd test-merge/
	git-init-db 

	#
	# Initial silly contents
	#
	echo "Hello" > hello
	echo "Hi" > hi
	git add hello hi
	git commit -m "Initial"

	#
	# Create another branch
	#
	git branch other

	#
	# Edit the contents on the master branch,
	# commit it.
	#
	echo "Hello there" > hello
	git commit -m "first change" hello

	#
	# Edit/commit the other differently
	#
	git checkout other
	echo "Hello differently" > hello
	git commit -m "second change" hello

	#
	# Try to merge - this will fail
	#
	git checkout master
	git merge "Clashing merge" HEAD other

	#
	# Do an evil merge conflict that also edits a 
	# nonconflicting file
	#
	echo "Hello third version" > hello
	echo "Hidden hi change" > hi 
	git commit -m "Evil merge conflict" hello hi

At this point, "git-diff-tree --cc HEAD" shows exactly the right thing, 
but "gitk" doesn't show anything at all for that merge (it shows the 
"hello" file in the file pane, but no diff at all, and certainly not the 
"hi" file which it _should_ show).

		Linus

  reply	other threads:[~2006-02-04 21:00 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-02-02  6:28 The merge from hell Linus Torvalds
2006-02-02  7:05 ` Junio C Hamano
2006-02-02  7:40   ` [PATCH] combine-diff: reuse diff from the same blob Junio C Hamano
2006-02-02  7:51   ` The merge from hell Linus Torvalds
2006-02-02  7:55     ` Linus Torvalds
2006-02-02  8:08       ` Linus Torvalds
2006-02-02  8:18         ` [PATCH] combine-diff: update --cc "uninteresting hunks" logic Junio C Hamano
2006-02-02  9:34           ` [PATCH] combine-diff: add safety check to --cc Junio C Hamano
2006-02-02 23:03             ` Linus Torvalds
2006-02-03  0:02               ` Junio C Hamano
2006-02-03  1:05                 ` Linus Torvalds
2006-02-03  5:49                 ` [Attn - repository browser authors] diff-tree combined format Junio C Hamano
2006-02-03 12:17                   ` Marco Costalba
2006-02-03 19:55                     ` Junio C Hamano
2006-02-03 21:35                     ` Junio C Hamano
2006-02-04 12:03                       ` Marco Costalba
2006-02-04 11:23                   ` Paul Mackerras
2006-02-03  5:28               ` [PATCH] combine-diff: add safety check to --cc Junio C Hamano
2006-02-04  5:38               ` Paul Mackerras
2006-02-04  6:12                 ` Junio C Hamano
2006-02-04 10:46   ` The merge from hell Paul Mackerras
2006-02-04 12:22     ` Junio C Hamano
2006-02-04 19:42     ` Linus Torvalds
2006-02-04 20:59       ` Linus Torvalds [this message]
2006-02-02  7:25 ` Marco Costalba
2006-02-02  8:02   ` Linus Torvalds
2006-02-02  8:07     ` Aneesh Kumar
2006-02-02  8:27       ` Junio C Hamano
2006-02-02  8:44       ` Linus Torvalds
2006-02-02 10:41         ` Junio C Hamano
2006-02-05 19:42           ` Linus Torvalds
2006-02-05 19:49             ` Add a "git show" command to show a commit Linus Torvalds
2006-02-05 19:58               ` Fix git-rev-parse over-eager errors Linus Torvalds
2006-02-05 20:11                 ` Junio C Hamano
2006-02-05 22:03                   ` Linus Torvalds
2006-02-06  6:20                     ` Junio C Hamano
2006-02-05 22:45               ` Add a "git show" command to show a commit Junio C Hamano
2006-02-05 22:55                 ` Linus Torvalds
2006-02-05 22:59                   ` Linus Torvalds
2006-02-06  0:25                     ` Junio C Hamano
  -- strict thread matches above, loose matches on Subject: below --
2006-02-03  4:20 The merge from hell Brown, Len
2006-02-03  5:45 ` Linus Torvalds
2006-02-03  6:28   ` Junio C Hamano
2006-02-03 16:21     ` Dave Jones
2006-02-03  6:04 Brown, Len
2006-02-03  6:16 ` Linus Torvalds
2006-02-03  6:33   ` Junio C Hamano
2006-02-03  6:41 linux
2006-02-03 18:34 Brown, Len
2006-02-04  2:35 ` Junio C Hamano
2006-02-04  2:47   ` Linus Torvalds

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Pine.LNX.4.64.0602041239471.3854@g5.osdl.org \
    --to=torvalds@osdl.org \
    --cc=aneesh.kumar@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=junkio@cox.net \
    --cc=mcostalba@yahoo.it \
    --cc=paulus@samba.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).