Re: [PATCH] git-blame shouldn't crash if run in an unmerged tree

["Björn Steinbrink" <B.Steinbrink@gmx.de>]

On 2007.10.18 02:34:07 -0400, Shawn O. Pearce wrote:
> I'm applying this patch to my maint tree tonight as it does resolve
> the issue for now.  What surprised me was the file that we were
> crashing out on wasn't even the file we wanted to get the blame
> data for.  :-\

The first merge moved some code from file1 (which doesn't exist in the
branch anymore) into file2, so I guess the code move detection comes
into play here.

Actually, in the original case that crashed here, I was curious about
some lines in file2 which looked like they had been automatically merged
from file1, so I tried to use git blame with file2 to see if that really
happened (I didn't expect git to be even able to follow code moves while
merging). Unfortunately, I didn't get such a test case yet, which might
indicate that I've only imagined that merge, and thinking about it, I
think that file2 wasn't marked as modified in "git status". Hm, I'll try
to find that merge conflict again and try that again.

Björn

> 
> --8>--
> From: Linus Torvalds <torvalds@linux-foundation.org>
> Subject: [PATCH] git-blame shouldn't crash if run in an unmerged tree
> 
> If we are in the middle of resolving a merge conflict there may be
> one or more files whose entries in the index represent an unmerged
> state (index entries in the higher-order stages).
> 
> Attempting to run git-blame on any file in such a working directory
> resulted in "fatal: internal error: ce_mode is 0" as we use the magic
> marker for an unmerged entry is 0 (set up by things like diff-lib.c's
> do_diff_cache() and builtin-read-tree.c's read_tree_unmerged())
> and the ce_match_stat_basic() function gets upset about this.
> 
> I'm not entirely sure that the whole "ce_mode = 0" case is a good
> idea to begin with, and maybe the right thing to do is to remove
> that horrid freakish special case, but removing the internal error
> seems to be the simplest fix for now.
> 
>                 Linus
> 
> [sp: Thanks to Björn Steinbrink for the test case]
> 
> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
> ---
>  read-cache.c     |    2 +
>  t/t8004-blame.sh |   73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 75 insertions(+), 0 deletions(-)
>  create mode 100755 t/t8004-blame.sh
> 
> diff --git a/read-cache.c b/read-cache.c
> index 536f4d0..928e8fa 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -149,6 +149,8 @@ static int ce_match_stat_basic(struct cache_entry *ce, struct stat *st)
>  		else if (ce_compare_gitlink(ce))
>  			changed |= DATA_CHANGED;
>  		return changed;
> +	case 0: /* Special case: unmerged file in index */
> +		return MODE_CHANGED | DATA_CHANGED | TYPE_CHANGED;
>  	default:
>  		die("internal error: ce_mode is %o", ntohl(ce->ce_mode));
>  	}
> diff --git a/t/t8004-blame.sh b/t/t8004-blame.sh
> new file mode 100755
> index 0000000..ba19ac1
> --- /dev/null
> +++ b/t/t8004-blame.sh
> @@ -0,0 +1,73 @@
> +#!/bin/sh
> +
> +# Based on a test case submitted by Björn Steinbrink.
> +
> +test_description='git blame on conflicted files'
> +. ./test-lib.sh
> +
> +test_expect_success 'setup first case' '
> +	# Create the old file
> +	echo "Old line" > file1 &&
> +	git add file1 &&
> +	git commit --author "Old Line <ol@localhost>" -m file1.a &&
> +
> +	# Branch
> +	git checkout -b foo &&
> +
> +	# Do an ugly move and change
> +	git rm file1 &&
> +	echo "New line ..."  > file2 &&
> +	echo "... and more" >> file2 &&
> +	git add file2 &&
> +	git commit --author "U Gly <ug@localhost>" -m ugly &&
> +
> +	# Back to master and change something
> +	git checkout master &&
> +	echo "
> +
> +bla" >> file1 &&
> +	git commit --author "Old Line <ol@localhost>" -a -m file1.b &&
> +
> +	# Back to foo and merge master
> +	git checkout foo &&
> +	if git merge master; then
> +		echo needed conflict here
> +		exit 1
> +	else
> +		echo merge failed - resolving automatically
> +	fi &&
> +	echo "New line ...
> +... and more
> +
> +bla
> +Even more" > file2 &&
> +	git rm file1 &&
> +	git commit --author "M Result <mr@localhost>" -a -m merged &&
> +
> +	# Back to master and change file1 again
> +	git checkout master &&
> +	sed s/bla/foo/ <file1 >X &&
> +	rm file1 &&
> +	mv X file1 &&
> +	git commit --author "No Bla <nb@localhost>" -a -m replace &&
> +
> +	# Try to merge into foo again
> +	git checkout foo &&
> +	if git merge master; then
> +		echo needed conflict here
> +		exit 1
> +	else
> +		echo merge failed - test is setup
> +	fi
> +'
> +
> +test_expect_success \
> +	'blame runs on unconflicted file while other file has conflicts' '
> +	git blame file2
> +'
> +
> +test_expect_success 'blame runs on conflicted file in stages 1,3' '
> +	git blame file1
> +'
> +
> +test_done