Jeff King <peff@xxxxxxxx> writes:
> 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.
Thanks.
The "I do not know if the logic is correct" reservation pays off ;-)
I still wonder why checking only the preimage side is sufficient, though.
Shouldn't we check both sides?
> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html