Re: Strange effect merging empty file

> On Thu, Mar 22, 2012 at 01:59:53PM -0400, Jeff King wrote:
>
>> > Yeah, thanks for digging up the old thread. I was looking at the patch to
>> > merge-recursive from Dscho on that thread and I think it identified the
>> > place that needs patching correctly. I was on a tablet, without the access
>> > to the surrounding code outside the patch context, so I do not know if the
>> > logic to detect the pure-rename of an empty file in the patch was correct,
>> > or the patch still applies to the current codebase, though.
>> 
>> It's easy to apply the patch manually, and I have written a test.
>> However, it seems to cause lots of other parts of t6022 to fail. I'll
>> try to dig up the cause.
>
> Found it. The diff code is very smart about doing as little work as
> possible. For a raw diff (i.e., not patch), we can often get away with
> not loading the blob at all, and therefore have no idea what the size
> is. The inexact rename code may load it, of course, but any file which
> is an exact rename will have a "0" size, also.

> diff --git a/merge-recursive.c b/merge-recursive.c
> index 6479a60..ed4ff16 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -502,7 +502,7 @@ static struct string_list *get_renames(struct merge_options *o,
>  		struct string_list_item *item;
>  		struct rename *re;
>  		struct diff_filepair *pair = diff_queued_diff.queue[i];
> -		if (pair->status != 'R') {
> +		if (pair->status != 'R' || is_empty_blob_sha1(pair->one->sha1)) {
>  			diff_free_filepair(pair);
>  			continue;
>  		}
> diff --git a/read-cache.c b/read-cache.c
> index 274e54b..dfabad0 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -157,7 +157,7 @@ static int ce_modified_check_fs(struct cache_entry *ce, struct stat *st)
>  	return 0;
>  }
>  
> -static int is_empty_blob_sha1(const unsigned char *sha1)
> +int is_empty_blob_sha1(const unsigned char *sha1)
>  {
>  	static const unsigned char empty_blob_sha1[20] = {
>  		0xe6,0x9d,0xe2,0x9b,0xb2,0xd1,0xd6,0x43,0x4b,0x8b,
> diff --git a/t/t6022-merge-rename.sh b/t/t6022-merge-rename.sh
> index 9d8584e..1104249 100755
> --- a/t/t6022-merge-rename.sh
> +++ b/t/t6022-merge-rename.sh
> @@ -884,4 +884,20 @@ test_expect_success 'no spurious "refusing to lose untracked" message' '
>  	! grep "refusing to lose untracked file" errors.txt
>  '
>  
> +test_expect_success 'do not follow renames for empty files' '
> +	git checkout -f -b empty-base &&
> +	>empty1 &&
> +	git add empty1 &&
> +	git commit -m base &&
> +	echo content >empty1 &&
> +	git add empty1 &&
> +	git commit -m fill &&
> +	git checkout -b empty-topic HEAD^ &&
> +	git mv empty1 empty2 &&
> +	git commit -m rename &&
> +	test_must_fail git merge empty-base &&
> +	>expect &&
> +	test_cmp expect empty2
> +'
> +
>  test_done