Re: Strange effect merging empty file
- From
Jeff King <peff@peff.net>
- Date
- Mar 22, 2012, 18:25 UTC
- Message-ID
- <20120322182533.GA20360@sigill.intra.peff.net>
- In-Reply-To
- <20120322175952.GA13069@sigill.intra.peff.net>
On Thu, Mar 22, 2012 at 01:59:53PM -0400, Jeff King wrote:
Show 10 quoted lines
> > 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.
We can get around it by just checking for the empty-blob sha1. The patch below should do the right thing, and passes the whole test suite.
---
diff --git a/cache.h b/cache.h index e5e1aa4..61671b6 100644 --- a/cache.h +++ b/cache.h @@ -708,6 +708,8 @@ static inline void hashclr(unsigned char *hash) #define EMPTY_TREE_SHA1_BIN \ ((const unsigned char *) EMPTY_TREE_SHA1_BIN_LITERAL) +int is_empty_blob_sha1(const unsigned char *sha1); + int git_mkstemp(char *path, size_t n, const char *template); int git_mkstemps(char *path, size_t n, const char *template, int suffix_len); 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