{"thread":{"id":"30014","subject":"Strange effect merging empty file","startedAt":"2012-03-21T10:28:12Z","lastAt":"2012-03-23T04:56:14Z","messageCount":25,"participants":["Ralf Nyren","Zbigniew Jędrzejewski-Szmek","Junio C Hamano","Randal L. Schwartz","Jeff King","Jonathan Nieder"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"187380","messageId":"4F69AD3C.4070203@ericsson.com","threadId":"30014","inReplyTo":null,"subject":"Strange effect merging empty file","fromName":"Ralf Nyren","fromEmail":"ralf.nyren@ericsson.com","sentAt":"2012-03-21T10:28:12Z","receivedAt":"2012-03-21T10:28:12Z","isPatch":false,"sender":{"key":"ralf.nyren@ericsson.com","avatar":null},"body":"Hi,\n\nI found a \"strange effect\" when merging from a branch containing a \nchange of a previously empty file. The change is added to another empty \nfile in the current branch by the merge.\n\nI guess git sees it as a renamed file which is logical from a \ncontent-perspective.\n\nNot sure what to do with this, I would not say it is a bug really...\n\nReproduce as follows (should be cut-n-paste friendly):\n\n# Start with a new repository\ngit init\necho 'Readme file' > README\ngit add README\ngit commit -m 'Initial commit'\n\n# Setup the branch to be merged\ngit checkout -b import\ntouch empty.txt\necho hello world > hello.txt\ngit add empty.txt hello.txt\ngit commit -m 'Import 1.0'\ngit tag IMPORT_1.0\necho This file is no longer empty > empty.txt\ngit commit -m 'Import 1.1' empty.txt\ngit tag IMPORT_1.1\n\n# Setup master branch\ngit checkout master\nmkdir static\ntouch static/.gitignore\ngit add static/.gitignore\ngit commit -m 'Static web content'\n\n# Merge import 1.0 and remove the empty file\ngit merge IMPORT_1.0\ngit rm empty.txt\ngit commit -m 'Remove empty file' empty.txt\n\n# Merge import 1.1 and watch empty.txt contents show up in .gitignore\ngit merge IMPORT_1.1\ncat static/.gitignore\n\nregards, Ralf\n\n"},{"id":"187383","messageId":"4F69B375.5050205@in.waw.pl","threadId":"30014","inReplyTo":"4F69AD3C.4070203@ericsson.com","subject":"Re: Strange effect merging empty file","fromName":"Zbigniew Jędrzejewski-Szmek","fromEmail":"zbyszek@in.waw.pl","sentAt":"2012-03-21T10:54:45Z","receivedAt":"2012-03-21T10:54:45Z","isPatch":false,"sender":{"key":"zbyszek@in.waw.pl","avatar":"https://avatars.githubusercontent.com/u/349618?v=4"},"body":"On 03/21/2012 11:28 AM, Ralf Nyren wrote:\n> Hi,\n>\n> I found a \"strange effect\" when merging from a branch containing a\n> change of a previously empty file. The change is added to another empty\n> file in the current branch by the merge.\n>\n> I guess git sees it as a renamed file which is logical from a\n> content-perspective.\n>\n> Not sure what to do with this, I would not say it is a bug really...\nIt does seem like a bug. An unrelated file is clobbered.\n\n> Reproduce as follows (should be cut-n-paste friendly):\nHere's a slightly simplified version that is actually cut-n-paste \nfriendly after recent changes to always edit merge comments...\n\n# Start with a new repository\ngit init\necho Readme file >README\ngit add README\ngit commit -m 'Initial commit'\n\n# Setup the branch to be merged\ngit checkout -b import\ntouch empty.txt\necho hello world >hello.txt\ngit add empty.txt hello.txt\ngit commit -m 'Import 1.0'\necho This file is no longer empty >empty.txt\ngit commit -m 'Import 1.1' empty.txt\n\n# Setup master branch\ngit checkout master\ntouch .gitignore\ngit add .gitignore\ngit commit -m 'Static web content'\n\n# Merge import 1.0 and remove the empty file\ngit merge --no-edit import^\ngit rm empty.txt\ngit commit -m 'Remove empty file'\n\n# Merge import 1.1 and watch empty.txt contents show up in .gitignore\ngit merge --no-edit import\ncat .gitignore\n\n\nRegards,\nZbyszek\n"},{"id":"187406","messageId":"7v62dx3mhe.fsf@alter.siamese.dyndns.org","threadId":"30014","inReplyTo":"4F69B375.5050205@in.waw.pl","subject":"Re: Strange effect merging empty file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-21T17:14:21Z","receivedAt":"2012-03-21T17:14:21Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Zbigniew Jędrzejewski-Szmek writes:\n\n> # Merge import 1.0 and remove the empty file\n> git merge --no-edit import^\n> git rm empty.txt\n> git commit -m 'Remove empty file'\n>\n> # Merge import 1.1 and watch empty.txt contents show up in .gitignore\n> git merge --no-edit import\n> cat .gitignore\n\nGiven what merge-recursive does, I am not very surprised, even though I\nagree that it feels like a very surprising end user experience ;-)\n"},{"id":"187458","messageId":"86iphwomnq.fsf@red.stonehenge.com","threadId":"30014","inReplyTo":"4F69B375.5050205@in.waw.pl","subject":"Re: Strange effect merging empty file","fromName":"Randal L. Schwartz","fromEmail":"merlyn@stonehenge.com","sentAt":"2012-03-22T12:17:13Z","receivedAt":"2012-03-22T12:17:13Z","isPatch":false,"sender":{"key":"merlyn@stonehenge.com","avatar":"https://gravatar.com/avatar/dc528d210743ff0333e6213f9ee7b33b23f1b7bc1f3c5a8c2d819074ecd7ab19?d=mp&s=160"},"body":">>>>> \"Zbigniew\" == Zbigniew Jędrzejewski-Szmek writes:\n\nZbigniew> touch .gitignore\n\nIf you're doing this to make sure git makes an empty directory, always\nput the directory name in there as a comment... that makes it (a) not an\nempty file (so merge doesn't get confused) and (b) unique, so when you\nchange it, merge won't try to change other similar empty files.\n\n-- \nRandal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095\n \nSmalltalk/Perl/Unix consulting, Technical writing, Comedy, etc. etc.\nSee http://methodsandmessages.posterous.com/ for Smalltalk discussion\n"},{"id":"187459","messageId":"4F6B1D8C.9040500@ericsson.com","threadId":"30014","inReplyTo":"86iphwomnq.fsf@red.stonehenge.com","subject":"Re: Strange effect merging empty file","fromName":"Ralf Nyren","fromEmail":"ralf.nyren@ericsson.com","sentAt":"2012-03-22T12:39:40Z","receivedAt":"2012-03-22T12:39:40Z","isPatch":false,"sender":{"key":"ralf.nyren@ericsson.com","avatar":null},"body":"On 22/03/12 13:17, Randal L. Schwartz wrote:\n>>>>>> \"Zbigniew\" == Zbigniew Jędrzejewski-Szmek writes:\n>\n> Zbigniew> touch .gitignore\n>\n> If you're doing this to make sure git makes an empty directory, always\n> put the directory name in there as a comment... that makes it (a) not an\n> empty file (so merge doesn't get confused) and (b) unique, so when you\n> change it, merge won't try to change other similar empty files.\n\nGood point.\n\nRenaming directories makes the comment out-of-date but since uniqueness \nis what matters it should not be a problem.\n\nregards, Ralf\n\n\n"},{"id":"187461","messageId":"4F6B1F48.3040007@in.waw.pl","threadId":"30014","inReplyTo":"86iphwomnq.fsf@red.stonehenge.com","subject":"Re: Strange effect merging empty file","fromName":"Zbigniew Jędrzejewski-Szmek","fromEmail":"zbyszek@in.waw.pl","sentAt":"2012-03-22T12:47:04Z","receivedAt":"2012-03-22T12:47:04Z","isPatch":false,"sender":{"key":"zbyszek@in.waw.pl","avatar":"https://avatars.githubusercontent.com/u/349618?v=4"},"body":"On 03/22/2012 01:17 PM, Randal L. Schwartz wrote:\n>>>>>> \"Zbigniew\" == Zbigniew Jędrzejewski-Szmek writes:\n>\n> Zbigniew> touch .gitignore\n>\n> If you're doing this to make sure git makes an empty directory, always\n> put the directory name in there as a comment... that makes it (a) not an\n> empty file (so merge doesn't get confused) and (b) unique, so when you\n> change it, merge won't try to change other similar empty files.\n\nYes, this will indeed fix this particular problem. But in general, empty \nfiles can be used for various reasons, and it can be a pretty nasty \nsurprise if they sprout random content as a result of a merge.\n\nMaybe merge-recursive could special-case empty files?\n\nZbyszek\n"},{"id":"187469","messageId":"20120322140140.GA8803@sigill.intra.peff.net","threadId":"30014","inReplyTo":"4F6B1F48.3040007@in.waw.pl","subject":"Re: Strange effect merging empty file","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-03-22T14:01:40Z","receivedAt":"2012-03-22T14:01:40Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 22, 2012 at 01:47:04PM +0100, Zbigniew Jędrzejewski-Szmek wrote:\n\n> Yes, this will indeed fix this particular problem. But in general,\n> empty files can be used for various reasons, and it can be a pretty\n> nasty surprise if they sprout random content as a result of a merge.\n> \n> Maybe merge-recursive could special-case empty files?\n\nI was thinking the same thing. And it seems this came up once before,\nand the list seemed to favor special-casing merge-recursive (but not\ndiffcore):\n\n http://thread.gmane.org/gmane.comp.version-control.git/116917/focus=117082\n\n-Peff\n"},{"id":"187479","messageId":"7vty1gy3eh.fsf@alter.siamese.dyndns.org","threadId":"30014","inReplyTo":"20120322140140.GA8803@sigill.intra.peff.net","subject":"Re: Strange effect merging empty file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-22T17:03:02Z","receivedAt":"2012-03-22T17:03:02Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I was thinking the same thing. And it seems this came up once before,\n> and the list seemed to favor special-casing merge-recursive (but not\n> diffcore):\n>\n> http://thread.gmane.org/gmane.comp.version-control.git/116917/focus=117082\n\nYeah, thanks for digging up the old thread. I was looking at the patch to\nmerge-recursive from Dscho on that thread and I think it identified the\nplace that needs patching correctly. I was on a tablet, without the access\nto the surrounding code outside the patch context, so I do not know if the\nlogic to detect the pure-rename of an empty file in the patch was correct,\nor the patch still applies to the current codebase, though.\n"},{"id":"187484","messageId":"20120322175952.GA13069@sigill.intra.peff.net","threadId":"30014","inReplyTo":"7vty1gy3eh.fsf@alter.siamese.dyndns.org","subject":"Re: Strange effect merging empty file","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-03-22T17:59:53Z","receivedAt":"2012-03-22T17:59:53Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 22, 2012 at 10:03:02AM -0700, Junio C Hamano wrote:\n\n> > I was thinking the same thing. And it seems this came up once before,\n> > and the list seemed to favor special-casing merge-recursive (but not\n> > diffcore):\n> >\n> > http://thread.gmane.org/gmane.comp.version-control.git/116917/focus=117082\n> \n> Yeah, thanks for digging up the old thread. I was looking at the patch to\n> merge-recursive from Dscho on that thread and I think it identified the\n> place that needs patching correctly. I was on a tablet, without the access\n> to the surrounding code outside the patch context, so I do not know if the\n> logic to detect the pure-rename of an empty file in the patch was correct,\n> or the patch still applies to the current codebase, though.\n\nIt's easy to apply the patch manually, and I have written a test.\nHowever, it seems to cause lots of other parts of t6022 to fail. I'll\ntry to dig up the cause.\n\n-Peff\n"},{"id":"187485","messageId":"20120322182533.GA20360@sigill.intra.peff.net","threadId":"30014","inReplyTo":"20120322175952.GA13069@sigill.intra.peff.net","subject":"Re: Strange effect merging empty file","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-03-22T18:25:33Z","receivedAt":"2012-03-22T18:25:33Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 22, 2012 at 01:59:53PM -0400, Jeff King wrote:\n\n> > Yeah, thanks for digging up the old thread. I was looking at the patch to\n> > merge-recursive from Dscho on that thread and I think it identified the\n> > place that needs patching correctly. I was on a tablet, without the access\n> > to the surrounding code outside the patch context, so I do not know if the\n> > logic to detect the pure-rename of an empty file in the patch was correct,\n> > or the patch still applies to the current codebase, though.\n> \n> It's easy to apply the patch manually, and I have written a test.\n> However, it seems to cause lots of other parts of t6022 to fail. I'll\n> try to dig up the cause.\n\nFound it. The diff code is very smart about doing as little work as\npossible. For a raw diff (i.e., not patch), we can often get away with\nnot loading the blob at all, and therefore have no idea what the size\nis. The inexact rename code may load it, of course, but any file which\nis an exact rename will have a \"0\" size, also.\n\nWe can get around it by just checking for the empty-blob sha1. The patch\nbelow should do the right thing, and passes the whole test suite.\n\n---\ndiff --git a/cache.h b/cache.h\nindex e5e1aa4..61671b6 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -708,6 +708,8 @@ static inline void hashclr(unsigned char *hash)\n #define EMPTY_TREE_SHA1_BIN \\\n \t ((const unsigned char *) EMPTY_TREE_SHA1_BIN_LITERAL)\n \n+int is_empty_blob_sha1(const unsigned char *sha1);\n+\n int git_mkstemp(char *path, size_t n, const char *template);\n \n int git_mkstemps(char *path, size_t n, const char *template, int suffix_len);\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 6479a60..ed4ff16 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -502,7 +502,7 @@ static struct string_list *get_renames(struct merge_options *o,\n \t\tstruct string_list_item *item;\n \t\tstruct rename *re;\n \t\tstruct diff_filepair *pair = diff_queued_diff.queue[i];\n-\t\tif (pair->status != 'R') {\n+\t\tif (pair->status != 'R' || is_empty_blob_sha1(pair->one->sha1)) {\n \t\t\tdiff_free_filepair(pair);\n \t\t\tcontinue;\n \t\t}\ndiff --git a/read-cache.c b/read-cache.c\nindex 274e54b..dfabad0 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -157,7 +157,7 @@ static int ce_modified_check_fs(struct cache_entry *ce, struct stat *st)\n \treturn 0;\n }\n \n-static int is_empty_blob_sha1(const unsigned char *sha1)\n+int is_empty_blob_sha1(const unsigned char *sha1)\n {\n \tstatic const unsigned char empty_blob_sha1[20] = {\n \t\t0xe6,0x9d,0xe2,0x9b,0xb2,0xd1,0xd6,0x43,0x4b,0x8b,\ndiff --git a/t/t6022-merge-rename.sh b/t/t6022-merge-rename.sh\nindex 9d8584e..1104249 100755\n--- a/t/t6022-merge-rename.sh\n+++ b/t/t6022-merge-rename.sh\n@@ -884,4 +884,20 @@ test_expect_success 'no spurious \"refusing to lose untracked\" message' '\n \t! grep \"refusing to lose untracked file\" errors.txt\n '\n \n+test_expect_success 'do not follow renames for empty files' '\n+\tgit checkout -f -b empty-base &&\n+\t>empty1 &&\n+\tgit add empty1 &&\n+\tgit commit -m base &&\n+\techo content >empty1 &&\n+\tgit add empty1 &&\n+\tgit commit -m fill &&\n+\tgit checkout -b empty-topic HEAD^ &&\n+\tgit mv empty1 empty2 &&\n+\tgit commit -m rename &&\n+\ttest_must_fail git merge empty-base &&\n+\t>expect &&\n+\ttest_cmp expect empty2\n+'\n+\n test_done\n"},{"id":"187493","messageId":"20120322185246.GA27037@sigill.intra.peff.net","threadId":"30014","inReplyTo":"20120322182533.GA20360@sigill.intra.peff.net","subject":"Re: Strange effect merging empty file","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-03-22T18:52:46Z","receivedAt":"2012-03-22T18:52:46Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 22, 2012 at 02:25:33PM -0400, Jeff King wrote:\n\n> We can get around it by just checking for the empty-blob sha1. The patch\n> below should do the right thing, and passes the whole test suite.\n\nHere it is broken up properly into commits:\n\n [1/3]: drop casts from users EMPTY_TREE_SHA1_BIN\n [2/3]: make is_empty_blob_sha1 available everywhere\n [3/3]: merge-recursive: don't detect renames from empty files\n\nThe first one is just a cleanup I noticed while adding\nEMPTY_BLOB_SHA1_BIN, and is not strictly related. The second one is the\nrefactoring bits, and the third one is the real change.\n\n-Peff\n"},{"id":"187494","messageId":"7v62dwxybd.fsf@alter.siamese.dyndns.org","threadId":"30014","inReplyTo":"20120322182533.GA20360@sigill.intra.peff.net","subject":"Re: Strange effect merging empty file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-22T18:52:54Z","receivedAt":"2012-03-22T18:52:54Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Thu, Mar 22, 2012 at 01:59:53PM -0400, Jeff King wrote:\n>\n>> > Yeah, thanks for digging up the old thread. I was looking at the patch to\n>> > merge-recursive from Dscho on that thread and I think it identified the\n>> > place that needs patching correctly. I was on a tablet, without the access\n>> > to the surrounding code outside the patch context, so I do not know if the\n>> > logic to detect the pure-rename of an empty file in the patch was correct,\n>> > or the patch still applies to the current codebase, though.\n>> \n>> It's easy to apply the patch manually, and I have written a test.\n>> However, it seems to cause lots of other parts of t6022 to fail. I'll\n>> try to dig up the cause.\n>\n> Found it. The diff code is very smart about doing as little work as\n> possible. For a raw diff (i.e., not patch), we can often get away with\n> not loading the blob at all, and therefore have no idea what the size\n> is. The inexact rename code may load it, of course, but any file which\n> is an exact rename will have a \"0\" size, also.\n\nThanks.\n\nThe \"I do not know if the logic is correct\" reservation pays off ;-)\n\nI still wonder why checking only the preimage side is sufficient, though.\nShouldn't we check both sides?\n\n> diff --git a/merge-recursive.c b/merge-recursive.c\n> index 6479a60..ed4ff16 100644\n> --- a/merge-recursive.c\n> +++ b/merge-recursive.c\n> @@ -502,7 +502,7 @@ static struct string_list *get_renames(struct merge_options *o,\n> \t\tstruct string_list_item *item;\n> \t\tstruct rename *re;\n> \t\tstruct diff_filepair *pair = diff_queued_diff.queue[i];\n> -\t\tif (pair->status != 'R') {\n> +\t\tif (pair->status != 'R' || is_empty_blob_sha1(pair->one->sha1)) {\n> \t\t\tdiff_free_filepair(pair);\n> \t\t\tcontinue;\n> \t\t}\n> diff --git a/read-cache.c b/read-cache.c\n> index 274e54b..dfabad0 100644\n> --- a/read-cache.c\n> +++ b/read-cache.c\n> @@ -157,7 +157,7 @@ static int ce_modified_check_fs(struct cache_entry *ce, struct stat *st)\n> \treturn 0;\n> }\n> \n> -static int is_empty_blob_sha1(const unsigned char *sha1)\n> +int is_empty_blob_sha1(const unsigned char *sha1)\n> {\n> \tstatic const unsigned char empty_blob_sha1[20] = {\n> \t\t0xe6,0x9d,0xe2,0x9b,0xb2,0xd1,0xd6,0x43,0x4b,0x8b,\n> diff --git a/t/t6022-merge-rename.sh b/t/t6022-merge-rename.sh\n> index 9d8584e..1104249 100755\n> --- a/t/t6022-merge-rename.sh\n> +++ b/t/t6022-merge-rename.sh\n> @@ -884,4 +884,20 @@ test_expect_success 'no spurious \"refusing to lose untracked\" message' '\n> \t! grep \"refusing to lose untracked file\" errors.txt\n> '\n> \n> +test_expect_success 'do not follow renames for empty files' '\n> +\tgit checkout -f -b empty-base &&\n> +\t>empty1 &&\n> +\tgit add empty1 &&\n> +\tgit commit -m base &&\n> +\techo content >empty1 &&\n> +\tgit add empty1 &&\n> +\tgit commit -m fill &&\n> +\tgit checkout -b empty-topic HEAD^ &&\n> +\tgit mv empty1 empty2 &&\n> +\tgit commit -m rename &&\n> +\ttest_must_fail git merge empty-base &&\n> +\t>expect &&\n> +\ttest_cmp expect empty2\n> +'\n> +\n> test_done\n"},{"id":"187495","messageId":"20120322185324.GA32727@sigill.intra.peff.net","threadId":"30014","inReplyTo":"20120322185246.GA27037@sigill.intra.peff.net","subject":"[PATCH 1/3] drop casts from users EMPTY_TREE_SHA1_BIN","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-03-22T18:53:24Z","receivedAt":"2012-03-22T18:53:24Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This macro already evaluates to the correct type, as it\ncasts the string literal to \"unsigned char *\" itself\n(and callers who want the literal can use the _LITERAL\nform).\n\nSigned-off-by: Jeff King \n---\n builtin/diff.c | 2 +-\n merge-recursive.c | 2 +-\n sequencer.c | 2 +-\n 3 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/diff.c b/builtin/diff.c\nindex 424c815..9069dc4 100644\n--- a/builtin/diff.c\n+++ b/builtin/diff.c\n@@ -327,7 +327,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)\n \t\t\t\tadd_head_to_pending(&rev);\n \t\t\t\tif (!rev.pending.nr) {\n \t\t\t\t\tstruct tree *tree;\n-\t\t\t\t\ttree = lookup_tree((const unsigned char*)EMPTY_TREE_SHA1_BIN);\n+\t\t\t\t\ttree = lookup_tree(EMPTY_TREE_SHA1_BIN);\n \t\t\t\t\tadd_pending_object(&rev, &tree->object, \"HEAD\");\n \t\t\t\t}\n \t\t\t\tbreak;\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 6479a60..318d32e 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1914,7 +1914,7 @@ int merge_recursive(struct merge_options *o,\n \t\t/* if there is no common ancestor, use an empty tree */\n \t\tstruct tree *tree;\n \n-\t\ttree = lookup_tree((const unsigned char *)EMPTY_TREE_SHA1_BIN);\n+\t\ttree = lookup_tree(EMPTY_TREE_SHA1_BIN);\n \t\tmerged_common_ancestors = make_virtual_commit(tree, \"ancestor\");\n \t}\n \ndiff --git a/sequencer.c b/sequencer.c\nindex a37846a..4307364 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -164,7 +164,7 @@ static void write_message(struct strbuf *msgbuf, const char *filename)\n \n static struct tree *empty_tree(void)\n {\n-\treturn lookup_tree((const unsigned char *)EMPTY_TREE_SHA1_BIN);\n+\treturn lookup_tree(EMPTY_TREE_SHA1_BIN);\n }\n \n static int error_dirty_index(struct replay_opts *opts)\n-- \n1.7.10.rc0.9.gdcbe9\n"},{"id":"187496","messageId":"20120322185339.GB32727@sigill.intra.peff.net","threadId":"30014","inReplyTo":"20120322185246.GA27037@sigill.intra.peff.net","subject":"[PATCH 2/3] make is_empty_blob_sha1 available everywhere","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-03-22T18:53:39Z","receivedAt":"2012-03-22T18:53:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The read-cache implementation defines this static function,\nbut it is a generally useful concept in git. Let's give\nthe empty blob the same treatment as the empty tree,\nproviding both hex and binary forms of the sha1.\n\nSigned-off-by: Jeff King \n---\n cache.h | 13 +++++++++++++\n read-cache.c | 10 ----------\n 2 files changed, 13 insertions(+), 10 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex e5e1aa4..5280574 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -708,6 +708,19 @@ static inline void hashclr(unsigned char *hash)\n #define EMPTY_TREE_SHA1_BIN \\\n \t ((const unsigned char *) EMPTY_TREE_SHA1_BIN_LITERAL)\n \n+#define EMPTY_BLOB_SHA1_HEX \\\n+\t\"e69de29bb2d1d6434b8b29ae775ad8c2e48c5391\"\n+#define EMPTY_BLOB_SHA1_BIN_LITERAL \\\n+\t\"\\xe6\\x9d\\xe2\\x9b\\xb2\\xd1\\xd6\\x43\\x4b\\x8b\" \\\n+\t\"\\x29\\xae\\x77\\x5a\\xd8\\xc2\\xe4\\x8c\\x53\\x91\"\n+#define EMPTY_BLOB_SHA1_BIN \\\n+\t((const unsigned char *) EMPTY_BLOB_SHA1_BIN_LITERAL)\n+\n+static inline int is_empty_blob_sha1(const unsigned char *sha1)\n+{\n+\treturn !hashcmp(sha1, EMPTY_BLOB_SHA1_BIN);\n+}\n+\n int git_mkstemp(char *path, size_t n, const char *template);\n \n int git_mkstemps(char *path, size_t n, const char *template, int suffix_len);\ndiff --git a/read-cache.c b/read-cache.c\nindex 274e54b..6c8f395 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -157,16 +157,6 @@ static int ce_modified_check_fs(struct cache_entry *ce, struct stat *st)\n \treturn 0;\n }\n \n-static int is_empty_blob_sha1(const unsigned char *sha1)\n-{\n-\tstatic const unsigned char empty_blob_sha1[20] = {\n-\t\t0xe6,0x9d,0xe2,0x9b,0xb2,0xd1,0xd6,0x43,0x4b,0x8b,\n-\t\t0x29,0xae,0x77,0x5a,0xd8,0xc2,0xe4,0x8c,0x53,0x91\n-\t};\n-\n-\treturn !hashcmp(sha1, empty_blob_sha1);\n-}\n-\n static int ce_match_stat_basic(struct cache_entry *ce, struct stat *st)\n {\n \tunsigned int changed = 0;\n-- \n1.7.10.rc0.9.gdcbe9\n"},{"id":"187497","messageId":"20120322185349.GC32727@sigill.intra.peff.net","threadId":"30014","inReplyTo":"20120322185246.GA27037@sigill.intra.peff.net","subject":"[PATCH 3/3] merge-recursive: don't detect renames from empty files","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-03-22T18:53:49Z","receivedAt":"2012-03-22T18:53:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Merge-recursive detects renames so that if one side modifies\n\"foo\" and the other side moves it to \"bar\", the modification\nis applied to \"bar\". However, our rename detection is based\non content analysis, it can be wrong (i.e., two files were\nnot intended as a rename, but just happen to have the same\nor similar content).\n\nThis is quite rare if the files actually contain content,\nsince two unrelated files are unlikely to have exactly the\nsame content. However, empty files present a problem, in\nthat there is nothing to analyze. An uninteresting\nplaceholder file with zero bytes may or may not be related\nto a placeholder file with another name.\n\nThe result is that adding content to an empty file may cause\nconfusion if the other side of a merge removed it; your\ncontent may end up in another random placeholder file that\nwas added.\n\nLet's err on the side of caution and not consider empty\nfiles as renames. This will cause a modify/delete conflict\non the merge, which will let the user sort it out\nthemselves.\n\nWe could do the same thing for general diff rename\ndetection. However, the stakes are much less high there, as\nwe are explicitly reporting the rename to the user. It's\nonly the automatic nature of merge-recursive that makes the\nresult confusing. So there's not as much need for caution\nwhen just showing a diff.\n\nSigned-off-by: Jeff King \n---\n merge-recursive.c | 2 +-\n t/t6022-merge-rename.sh | 16 ++++++++++++++++\n 2 files changed, 17 insertions(+), 1 deletion(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 318d32e..0255f50 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -502,7 +502,7 @@ static struct string_list *get_renames(struct merge_options *o,\n \t\tstruct string_list_item *item;\n \t\tstruct rename *re;\n \t\tstruct diff_filepair *pair = diff_queued_diff.queue[i];\n-\t\tif (pair->status != 'R') {\n+\t\tif (pair->status != 'R' || is_empty_blob_sha1(pair->one->sha1)) {\n \t\t\tdiff_free_filepair(pair);\n \t\t\tcontinue;\n \t\t}\ndiff --git a/t/t6022-merge-rename.sh b/t/t6022-merge-rename.sh\nindex 9d8584e..1104249 100755\n--- a/t/t6022-merge-rename.sh\n+++ b/t/t6022-merge-rename.sh\n@@ -884,4 +884,20 @@ test_expect_success 'no spurious \"refusing to lose untracked\" message' '\n \t! grep \"refusing to lose untracked file\" errors.txt\n '\n \n+test_expect_success 'do not follow renames for empty files' '\n+\tgit checkout -f -b empty-base &&\n+\t>empty1 &&\n+\tgit add empty1 &&\n+\tgit commit -m base &&\n+\techo content >empty1 &&\n+\tgit add empty1 &&\n+\tgit commit -m fill &&\n+\tgit checkout -b empty-topic HEAD^ &&\n+\tgit mv empty1 empty2 &&\n+\tgit commit -m rename &&\n+\ttest_must_fail git merge empty-base &&\n+\t>expect &&\n+\ttest_cmp expect empty2\n+'\n+\n test_done\n-- \n1.7.10.rc0.9.gdcbe9\n"},{"id":"187500","messageId":"20120322190303.GA32756@sigill.intra.peff.net","threadId":"30014","inReplyTo":"7v62dwxybd.fsf@alter.siamese.dyndns.org","subject":"Re: Strange effect merging empty file","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-03-22T19:03:03Z","receivedAt":"2012-03-22T19:03:03Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 22, 2012 at 11:52:54AM -0700, Junio C Hamano wrote:\n\n> I still wonder why checking only the preimage side is sufficient, though.\n> Shouldn't we check both sides?\n> [...]\n> > +\t\tif (pair->status != 'R' || is_empty_blob_sha1(pair->one->sha1)) {\n\nIf the source is an empty file, then the other side must be an empty\nfile, too, no? Otherwise it would not be a rename. It could not be\nexact, for obvious reasons. And it could not be inexact, because there\nis no content to analyze on the source side.\n\nDitto for the destination side. How could something be renamed to an\nempty file, but not be empty on the source side?\n\nThat is a slight layering violation, in that we are making assumptions\nabout how the diffcore-rename subsystem works. In theory diffcore-rename\ncould learn to read rename annotations from the commit message or\nsomething (bleh!). But then, we'd probably want to update\nmerge-recursive in that instance to accept those \"yes, it's definitely a\nrename\" markers, even for an empty file.\n\nI think it would be slightly cleaner to tell diffcore-rename \"be\nconservative, and don't use empty files as sources\" via an option. It's\nnot as trivial as this one-liner, but it shouldn't be much more than the\nsmall patch you posted in the earlier thread.\n\n-Peff\n"},{"id":"187502","messageId":"7vwr6cwiux.fsf@alter.siamese.dyndns.org","threadId":"30014","inReplyTo":"20120322190303.GA32756@sigill.intra.peff.net","subject":"Re: Strange effect merging empty file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-22T19:12:06Z","receivedAt":"2012-03-22T19:12:06Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> That is a slight layering violation, in that we are making assumptions\n> about how the diffcore-rename subsystem works.\n\nI do not think I have to say any more than that. The special case we want\nto have is for the \"empty to empty\" case and nothing else, and I do not\nwant to see people having to remember to look at the merge-recursive code\nif/when rename detection starts to treat \"empty to small\" as \"rename with\nminor modification.\"\n"},{"id":"187504","messageId":"20120322191851.GA23293@burratino","threadId":"30014","inReplyTo":"20120322185349.GC32727@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] merge-recursive: don't detect renames from empty files","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-03-22T19:18:51Z","receivedAt":"2012-03-22T19:18:51Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n\n> We could do the same thing for general diff rename\n> detection. However, the stakes are much less high there, as\n> we are explicitly reporting the rename to the user. It's\n> only the automatic nature of merge-recursive that makes the\n> result confusing. So there's not as much need for caution\n> when just showing a diff.\n\nThe stakes may be different, but doesn't the same justification apply\nanyway? If \"git diff -M\" chooses a random pairing to describe a\nrenaming of multiple empty files, that seems just as confusing as\nmerge-recursive making the same mistake.\n\nIf adding this check in diffcore is more complicated, doing it in\nmerge-recursive for now seems fine and prudent, but if we are doing it\nat the merge-recursive level just to be conservative then that seems\nlike the wrong layer.\n\nThanks for a clean and pleasant patch.\nJonathan\n"},{"id":"187532","messageId":"20120322215355.GA750@sigill.intra.peff.net","threadId":"30014","inReplyTo":"20120322191851.GA23293@burratino","subject":"Re: [PATCH 3/3] merge-recursive: don't detect renames from empty files","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-03-22T21:53:55Z","receivedAt":"2012-03-22T21:53:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 22, 2012 at 02:18:51PM -0500, Jonathan Nieder wrote:\n\n> > We could do the same thing for general diff rename\n> > detection. However, the stakes are much less high there, as\n> > we are explicitly reporting the rename to the user. It's\n> > only the automatic nature of merge-recursive that makes the\n> > result confusing. So there's not as much need for caution\n> > when just showing a diff.\n> \n> The stakes may be different, but doesn't the same justification apply\n> anyway? If \"git diff -M\" chooses a random pairing to describe a\n> renaming of multiple empty files, that seems just as confusing as\n> merge-recursive making the same mistake.\n\nMaybe \"stakes\" was not the best word. My thinking was something like the\nfollowing. Matching empty files is a heuristic. So sometimes it will be\nright, and sometimes it will be wrong. We want to make sure that the\nconfusion caused by being wrong is less than the goodness caused by\nbeing right. When we find renames for a merge, the badness in being\nwrong is quite high. And the lack of goodness in failing to be right is\nnot all that high; you'll get a conflict which will bring the issue to\nthe user's attention, and they can fall back to using \"git diff\" to\ninvestigate the situation.\n\nWhereas with a regular diff, the badness of being wrong is not very\nhigh. The user sees the diff and says \"Really? Stupid git, that wasn't a\nrename\". And the lack of goodness in failing to be right is somewhat\nworse, because there is no way to fall back and ask git \"what renames\nwould you have found if you relaxed the heuristics a bit more?\"\n\nOf course, one could make that fallback an option. And given that we\nare, by definition, talking about trivial empty files, it's not like the\nrename detection somehow makes the diff a whole lot nicer. It just says\n\"rename X to Y\" instead of \"deleted Y, added X\". The real value in the\nrename detection is seeing the interdiff between X and Y, but it would\nalways be empty in this case anyway.\n\nSo I could go either way.\n\n> If adding this check in diffcore is more complicated, doing it in\n> merge-recursive for now seems fine and prudent, but if we are doing it\n> at the merge-recursive level just to be conservative then that seems\n> like the wrong layer.\n\nIt's not really more complicated, and based on Junio's response, I think\nwe want to do it there anyway. Doing it unconditionally for diff and\nmerge actually would make the code even simpler, then.\n\n-Peff\n"},{"id":"187534","messageId":"20120322224651.GA14874@sigill.intra.peff.net","threadId":"30014","inReplyTo":"7vwr6cwiux.fsf@alter.siamese.dyndns.org","subject":"[PATCH 0/2] merging renames of empty files","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-03-22T22:46:51Z","receivedAt":"2012-03-22T22:46:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 22, 2012 at 12:12:06PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > That is a slight layering violation, in that we are making assumptions\n> > about how the diffcore-rename subsystem works.\n> \n> I do not think I have to say any more than that. The special case we want\n> to have is for the \"empty to empty\" case and nothing else, and I do not\n> want to see people having to remember to look at the merge-recursive code\n> if/when rename detection starts to treat \"empty to small\" as \"rename with\n> minor modification.\"\n\nHere's a 2-patch series to replace the old 3/3 (they go on top of the\nfirst two cleanups from the previous iteration).\n\n [1/2]: teach diffcore-rename to optionally ignore empty content\n [2/2]: merge-recursive: don't detect renames of empty files\n\nThinking on this more, it is actually a more generic problem than just\nempty files. It is really a problem of having generic placeholder files\nwith the same content. So a fully general solution would be something\nlike a gitattribute for \"don't do renames on this\". However, in\npractice, these placeholder files are empty (since any non-empty file is\nlikely to actually have different content). So I think just dropping the\nempty files as rename candidates is a pretty good heuristic, and it's\nnice and simple.\n\nAfter responding to Jonathan, I'm on the fence about whether diff should\nfollow the same heuristic. I left the diff behavior unchanged, but a 3/2\nthat turns it off by default would be a trivial one-liner.\n\n-Peff\n"},{"id":"187536","messageId":"20120322225213.GA14902@sigill.intra.peff.net","threadId":"30014","inReplyTo":"20120322224651.GA14874@sigill.intra.peff.net","subject":"[PATCH 1/2] teach diffcore-rename to optionally ignore empty content","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-03-22T22:52:13Z","receivedAt":"2012-03-22T22:52:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Our rename detection is a heuristic, matching pairs of\nremoved and added files with similar or identical content.\nIt's unlikely to be wrong when there is actual content to\ncompare, and we already take care not to do inexact rename\ndetection when there is not enough content to produce good\nresults.\n\nHowever, we always do exact rename detection, even when the\nblob is tiny or empty. It's easy to get false positives with\nan empty blob, simply because it is an obvious content to\nuse as a boilerplate (e.g., when telling git that an empty\ndirectory is worth tracking via an empty .gitignore).\n\nThis patch lets callers specify whether or not they are\ninterested in using empty files as rename sources and\ndestinations. The default is \"yes\", keeping the original\nbehavior. It works by detecting the empty-blob sha1 for\nrename sources and destinations.\n\nOne more flexible alternative would be to allow the caller\nto specify a minimum size for a blob to be \"interesting\" for\nrename detection. But that would catch small boilerplate\nfiles, not large ones (e.g., if you had the GPL COPYING file\nin many directories).\n\nA better alternative would be to allow a \"-rename\"\ngitattribute to allow boilerplate files to be marked as\nsuch. I'll leave the complexity of that solution until such\ntime as somebody actually wants it. The complaints we've\nseen so far revolve around empty files, so let's start with\nthe simple thing.\n\nSigned-off-by: Jeff King \n---\nFrom the previous discussion, we know we could get away with just\ndropping empty files from the rename_src list. However, doing it for\nboth the src and dst lists is a little more obvious and robust. And\nsince some of the rename detection is O(src*dst), keeping the lists\nas small as possible is a good thing.\n\nI added command-line triggers mostly for testing and debugging, and\ndidn't bother to advertise them in the documentation. Obviously if we\ndecide that diff should just have this behavior, this patch can be\neven smaller.\n\n diff.c | 5 +++++\n diff.h | 2 +-\n diffcore-rename.c | 6 ++++++\n 3 files changed, 12 insertions(+), 1 deletion(-)\n\ndiff --git a/diff.c b/diff.c\nindex 377ec1e..0b70aad 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3136,6 +3136,7 @@ void diff_setup(struct diff_options *options)\n \toptions->rename_limit = -1;\n \toptions->dirstat_permille = diff_dirstat_permille_default;\n \toptions->context = 3;\n+\tDIFF_OPT_SET(options, RENAME_EMPTY);\n \n \toptions->change = diff_change;\n \toptions->add_remove = diff_addremove;\n@@ -3506,6 +3507,10 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \t}\n \telse if (!strcmp(arg, \"--no-renames\"))\n \t\toptions->detect_rename = 0;\n+\telse if (!strcmp(arg, \"--rename-empty\"))\n+\t\tDIFF_OPT_SET(options, RENAME_EMPTY);\n+\telse if (!strcmp(arg, \"--no-rename-empty\"))\n+\t\tDIFF_OPT_CLR(options, RENAME_EMPTY);\n \telse if (!strcmp(arg, \"--relative\"))\n \t\tDIFF_OPT_SET(options, RELATIVE_NAME);\n \telse if (!prefixcmp(arg, \"--relative=\")) {\ndiff --git a/diff.h b/diff.h\nindex cb68743..dd48eca 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -60,7 +60,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)\n #define DIFF_OPT_SILENT_ON_REMOVE (1 << 5)\n #define DIFF_OPT_FIND_COPIES_HARDER (1 << 6)\n #define DIFF_OPT_FOLLOW_RENAMES (1 << 7)\n-/* (1 << 8) unused */\n+#define DIFF_OPT_RENAME_EMPTY (1 << 8)\n /* (1 << 9) unused */\n #define DIFF_OPT_HAS_CHANGES (1 << 10)\n #define DIFF_OPT_QUICK (1 << 11)\ndiff --git a/diffcore-rename.c b/diffcore-rename.c\nindex f639601..216a7a4 100644\n--- a/diffcore-rename.c\n+++ b/diffcore-rename.c\n@@ -512,9 +512,15 @@ void diffcore_rename(struct diff_options *options)\n \t\t\telse if (options->single_follow &&\n \t\t\t\t strcmp(options->single_follow, p->two->path))\n \t\t\t\tcontinue; /* not interested */\n+\t\t\telse if (!DIFF_OPT_TST(options, RENAME_EMPTY) &&\n+\t\t\t\t is_empty_blob_sha1(p->two->sha1))\n+\t\t\t\tcontinue;\n \t\t\telse\n \t\t\t\tlocate_rename_dst(p->two, 1);\n \t\t}\n+\t\telse if (!DIFF_OPT_TST(options, RENAME_EMPTY) &&\n+\t\t\t is_empty_blob_sha1(p->one->sha1))\n+\t\t\tcontinue;\n \t\telse if (!DIFF_PAIR_UNMERGED(p) && !DIFF_FILE_VALID(p->two)) {\n \t\t\t/*\n \t\t\t * If the source is a broken \"delete\", and\n-- \n1.7.10.rc0.9.gdcbe9\n"},{"id":"187537","messageId":"20120322225223.GB14902@sigill.intra.peff.net","threadId":"30014","inReplyTo":"20120322224651.GA14874@sigill.intra.peff.net","subject":"[PATCH 2/2] merge-recursive: don't detect renames of empty files","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-03-22T22:52:24Z","receivedAt":"2012-03-22T22:52:24Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Merge-recursive detects renames so that if one side modifies\n\"foo\" and the other side moves it to \"bar\", the modification\nis applied to \"bar\". However, our rename detection is based\non content analysis, it can be wrong (i.e., two files were\nnot intended as a rename, but just happen to have the same\nor similar content).\n\nThis is quite rare if the files actually contain content,\nsince two unrelated files are unlikely to have exactly the\nsame content. However, empty files present a problem, in\nthat there is nothing to analyze. An uninteresting\nplaceholder file with zero bytes may or may not be related\nto a placeholder file with another name.\n\nThe result is that adding content to an empty file may cause\nconfusion if the other side of a merge removed it; your\ncontent may end up in another random placeholder file that\nwas added.\n\nLet's err on the side of caution and not consider empty\nfiles as renames. This will cause a modify/delete conflict\non the merge, which will let the user sort it out\nthemselves.\n\nSigned-off-by: Jeff King \n---\n merge-recursive.c | 1 +\n t/t6022-merge-rename.sh | 16 ++++++++++++++++\n 2 files changed, 17 insertions(+)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 318d32e..0fb1743 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -485,6 +485,7 @@ static struct string_list *get_renames(struct merge_options *o,\n \trenames = xcalloc(1, sizeof(struct string_list));\n \tdiff_setup(&opts);\n \tDIFF_OPT_SET(&opts, RECURSIVE);\n+\tDIFF_OPT_CLR(&opts, RENAME_EMPTY);\n \topts.detect_rename = DIFF_DETECT_RENAME;\n \topts.rename_limit = o->merge_rename_limit >= 0 ? o->merge_rename_limit :\n \t\t\t o->diff_rename_limit >= 0 ? o->diff_rename_limit :\ndiff --git a/t/t6022-merge-rename.sh b/t/t6022-merge-rename.sh\nindex 9d8584e..1104249 100755\n--- a/t/t6022-merge-rename.sh\n+++ b/t/t6022-merge-rename.sh\n@@ -884,4 +884,20 @@ test_expect_success 'no spurious \"refusing to lose untracked\" message' '\n \t! grep \"refusing to lose untracked file\" errors.txt\n '\n \n+test_expect_success 'do not follow renames for empty files' '\n+\tgit checkout -f -b empty-base &&\n+\t>empty1 &&\n+\tgit add empty1 &&\n+\tgit commit -m base &&\n+\techo content >empty1 &&\n+\tgit add empty1 &&\n+\tgit commit -m fill &&\n+\tgit checkout -b empty-topic HEAD^ &&\n+\tgit mv empty1 empty2 &&\n+\tgit commit -m rename &&\n+\ttest_must_fail git merge empty-base &&\n+\t>expect &&\n+\ttest_cmp expect empty2\n+'\n+\n test_done\n-- \n1.7.10.rc0.9.gdcbe9\n"},{"id":"187544","messageId":"7vpqc4us0u.fsf@alter.siamese.dyndns.org","threadId":"30014","inReplyTo":"20120322224651.GA14874@sigill.intra.peff.net","subject":"Re: [PATCH 0/2] merging renames of empty files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-22T23:37:05Z","receivedAt":"2012-03-22T23:37:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Here's a 2-patch series to replace the old 3/3 (they go on top of the\n> first two cleanups from the previous iteration).\n\nHrm. As this is probably useful for older maintenance track, I would have\npreferred not to see the first one that touches sequencer.c that did not\nexist in the 1.7.7.x maintenance track, as the change is purely \"cosmetic\"\nand does not have anything to do with fixing the over-agressive merge.\n\n> [1/2]: teach diffcore-rename to optionally ignore empty content\n> [2/2]: merge-recursive: don't detect renames of empty files\n>\n> Thinking on this more, it is actually a more generic problem than just\n> empty files. It is really a problem of having generic placeholder files\n> with the same content. So a fully general solution would be something\n> like a gitattribute for \"don't do renames on this\". However, in\n> practice, these placeholder files are empty (since any non-empty file is\n> likely to actually have different content). So I think just dropping the\n> empty files as rename candidates is a pretty good heuristic, and it's\n> nice and simple.\n\nI thought that our recommendation for keeping an otherwise empty\ndirectories around was to have .gitignore file with two entries in it,\nnamely:\n\n\t.*\n *\n\nSo these files will be everywhere and without being empty, no?\n\nBut I tend to prefer the simplicity of limiting this to empty files\nanyway.\n\n> After responding to Jonathan, I'm on the fence about whether diff should\n> follow the same heuristic. I left the diff behavior unchanged, but a 3/2\n> that turns it off by default would be a trivial one-liner.\n\nI am also torn, but somewhat in favor of avoiding random permutations of\nempty files.\n\nI can think of only one situation somebody _could_ argue that detecting\nempty-to-empty rename is halfway sensible: when there is only one empty\nfile that was removed, and one new empty file that was added. Even in\nsuch a case, the user could have done \"rm this; >that\", and depending on\nthe nature of these two files, \"mv this that\" may not have made _any_\nsense even if they are both empty, and that is why I said \"halfway\"\nsensible.\n"},{"id":"187550","messageId":"20120323002300.GA15940@sigill.intra.peff.net","threadId":"30014","inReplyTo":"7vpqc4us0u.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 0/2] merging renames of empty files","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-03-23T00:23:00Z","receivedAt":"2012-03-23T00:23:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 22, 2012 at 04:37:05PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > Here's a 2-patch series to replace the old 3/3 (they go on top of the\n> > first two cleanups from the previous iteration).\n> \n> Hrm. As this is probably useful for older maintenance track, I would have\n> preferred not to see the first one that touches sequencer.c that did not\n> exist in the 1.7.7.x maintenance track, as the change is purely \"cosmetic\"\n> and does not have anything to do with fixing the over-agressive merge.\n\nI considered that, but assumed this was not a maint fix, but rather a\nnew \"feature\" to improve the heuristics. That being said, the first\npatch is entirely unrelated. The others don't depend on it textually or\nsemantically, and it does not need to be part of the series (I just\nhappened to notice it while in the area).\n\n> I thought that our recommendation for keeping an otherwise empty\n> directories around was to have .gitignore file with two entries in it,\n> namely:\n> \n> \t.*\n> *\n> \n> So these files will be everywhere and without being empty, no?\n\nI dunno. I have never recommended that, but maybe it is something people\ndo.\n\n> But I tend to prefer the simplicity of limiting this to empty files\n> anyway.\n\nYeah. We might want to do a gitattributes thing on top, but I'd much\nrather have this in the meantime, as it seems like it handles the bulk\nof the complaints we have actually seen on the list.\n\n-Peff\n"},{"id":"187556","messageId":"7vehsjvrtd.fsf@alter.siamese.dyndns.org","threadId":"30014","inReplyTo":"20120323002300.GA15940@sigill.intra.peff.net","subject":"Re: [PATCH 0/2] merging renames of empty files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-23T04:56:14Z","receivedAt":"2012-03-23T04:56:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Thu, Mar 22, 2012 at 04:37:05PM -0700, Junio C Hamano wrote:\n>\n>> Hrm. As this is probably useful for older maintenance track, I would have\n>> preferred not to see the first one that touches sequencer.c that did not\n>> exist in the 1.7.7.x maintenance track, as the change is purely \"cosmetic\"\n>> and does not have anything to do with fixing the over-agressive merge.\n>\n> I considered that, but assumed this was not a maint fix, but rather a\n> new \"feature\" to improve the heuristics.\n\nOk. Perhaps I over-reacted to the issue, which sounded like a grave\nmismerge that can go unnoticed.\n"}]}