Re: Strange effect merging empty file
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:53:21
Jeff King [off-list ref] writes:
On Thu, Mar 22, 2012 at 01:59:53PM -0400, Jeff King wrote:quoted
quoted
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?
quoted hunk
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