{"thread":{"id":"18957","subject":"correct git merge behavior or corner case?","startedAt":"2009-04-19T22:40:51Z","lastAt":"2009-04-21T19:11:26Z","messageCount":17,"participants":["Tuncer Ayaz","Shawn O. Pearce","Johannes Schindelin","Anders Melchiorsen","Jeff King","Junio C Hamano","Sverre Rabbelier","Michał Kiedrowicz"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"111665","messageId":"4ac8254d0904191540j68246cd8qa36a034209d4c800@mail.gmail.com","threadId":"18957","inReplyTo":null,"subject":"correct git merge behavior or corner case?","fromName":"Tuncer Ayaz","fromEmail":"tuncer.ayaz@gmail.com","sentAt":"2009-04-19T22:40:51Z","receivedAt":"2009-04-19T22:40:51Z","isPatch":false,"sender":{"key":"tuncer.ayaz@gmail.com","avatar":null},"body":"I have stumbled upon the following blog post via one of\nthe news aggegrators and wondered whether the behavior\nis correct and expected or he's expecting something wrong\nor doing something wrong.\n\nI cannot see a wrong usage pattern from what he has written.\n\nhttp://blog.teksol.info/2009/04/15/beware-of-gits-content-tracking.html\n\nas the author hasn't posted here after a couple of days\nI decided to take his question here for at least understanding\nwhat behavior he is experiencing.\n"},{"id":"111678","messageId":"20090420031916.GV23604@spearce.org","threadId":"18957","inReplyTo":"4ac8254d0904191540j68246cd8qa36a034209d4c800@mail.gmail.com","subject":"Re: correct git merge behavior or corner case?","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-04-20T03:19:16Z","receivedAt":"2009-04-20T03:19:16Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Tuncer Ayaz wrote:\n> I have stumbled upon the following blog post via one of\n> the news aggegrators and wondered whether the behavior\n> is correct and expected or he's expecting something wrong\n> or doing something wrong.\n> \n> I cannot see a wrong usage pattern from what he has written.\n> \n> http://blog.teksol.info/2009/04/15/beware-of-gits-content-tracking.html\n> \n> as the author hasn't posted here after a couple of days\n> I decided to take his question here for at least understanding\n> what behavior he is experiencing.\n\nWell, the author of the blog post gave us *no* information about\nwhat he did, or what he expected. He's just showing us a diff,\nwhich we're not even sure how he created, and then complaining\nabout Git not doing something he wanted.\n\n-- \nShawn.\n"},{"id":"111702","messageId":"alpine.DEB.1.00.0904201148150.6955@intel-tinevez-2-302","threadId":"18957","inReplyTo":"4ac8254d0904191540j68246cd8qa36a034209d4c800@mail.gmail.com","subject":"Re: correct git merge behavior or corner case?","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-04-20T09:49:57Z","receivedAt":"2009-04-20T09:49:57Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 20 Apr 2009, Tuncer Ayaz wrote:\n\n> I have stumbled upon the following blog post via one of the news \n> aggegrators and wondered whether the behavior is correct and expected or \n> he's expecting something wrong or doing something wrong.\n> \n> I cannot see a wrong usage pattern from what he has written.\n> \n> http://blog.teksol.info/2009/04/15/beware-of-gits-content-tracking.html\n> \n> as the author hasn't posted here after a couple of days I decided to \n> take his question here for at least understanding what behavior he is \n> experiencing.\n\nFirst, a blog posting is a lousy place to post complaints. That just \ndecreased my confidence level in the competence of Francois.\n\nSecond, he does not state what he expects to see instead. And for the \nlove of God, I cannot see what should be wrong in the output he gets. \nAnother few notches down.\n\nCiao,\nDscho\n"},{"id":"111708","messageId":"41354.bFoQE3daRhY=.1240222235.squirrel@webmail.hotelhot.dk","threadId":"18957","inReplyTo":"alpine.DEB.1.00.0904201148150.6955@intel-tinevez-2-302","subject":"Re: correct git merge behavior or corner case?","fromName":"Anders Melchiorsen","fromEmail":"mail@cup.kalibalik.dk","sentAt":"2009-04-20T10:10:35Z","receivedAt":"2009-04-20T10:10:35Z","isPatch":false,"sender":{"key":"mail@cup.kalibalik.dk","avatar":null},"body":"Johannes Schindelin wrote:\n\n> Second, he does not state what he expects to see instead. And for the\n> love of God, I cannot see what should be wrong in the output he gets.\n> Another few notches down.\n\nI think that I managed to recreate what he is describing.\n\n\n#!/bin/bash\n\ncd $(mktemp -d repo.XXXXX)\n\ngit init\n\ntouch date\ngit add date\ngit commit -memptydate\n\ngit branch parallel\n\ntouch LICENSE\ngit add LICENSE\ngit commit -mLICENSE\n\ngit checkout parallel\ndate >date\ngit add date\ngit commit -mdate\n\ngit checkout master\ngit rm date\ngit commit -mnodate\n\ngit merge parallel\n\ncat LICENSE\n"},{"id":"111826","messageId":"20090421024433.GC14479@coredump.intra.peff.net","threadId":"18957","inReplyTo":"41354.bFoQE3daRhY=.1240222235.squirrel@webmail.hotelhot.dk","subject":"Re: correct git merge behavior or corner case?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-04-21T02:44:33Z","receivedAt":"2009-04-21T02:44:33Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 20, 2009 at 12:10:35PM +0200, Anders Melchiorsen wrote:\n\n> I think that I managed to recreate what he is describing.\n> \n> \n> #!/bin/bash\n> \n> cd $(mktemp -d repo.XXXXX)\n> \n> git init\n> \n> touch date\n> git add date\n> git commit -memptydate\n> \n> git branch parallel\n> \n> touch LICENSE\n> git add LICENSE\n> git commit -mLICENSE\n> \n> git checkout parallel\n> date >date\n> git add date\n> git commit -mdate\n> \n> git checkout master\n> git rm date\n> git commit -mnodate\n> \n> git merge parallel\n> \n> cat LICENSE\n\nSo basically one branch removes a file and adds an identical file under\na different name, while the other branch modifies the original file. Git\ndetects it as a rename, and applies the change from the second branch to\nthe newly added file instead of generating a conflict.\n\nThis is _exactly_ what git's rename detection is designed to do. Yes, it\nseems horribly confusing in this toy example, but that is because it is\na toy example: both 'date' and 'LICENSE' are empty files. But with real\nfiles, if a source file has actual content but is deleted, there is a\nnew filename with the identical or near-identical content, and the patch\napplies to the new content without conflicts, then applying it there is\nprobably exactly what you want.\n\nThe only complaint I have in that example is that there is nothing\nindicating to the user that the patch was applied to a renamed version.\nThe output I get is:\n\n $ git merge parallel\n Merge made by recursive.\n LICENSE | 1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\nPerhaps a note indicating that it applied changes for \"date\" to\n\"LICENSE\" would be helpful.\n\n-Peff\n"},{"id":"111827","messageId":"20090421025107.GD14479@coredump.intra.peff.net","threadId":"18957","inReplyTo":"20090421024433.GC14479@coredump.intra.peff.net","subject":"Re: correct git merge behavior or corner case?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-04-21T02:51:07Z","receivedAt":"2009-04-21T02:51:07Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 20, 2009 at 10:44:33PM -0400, Jeff King wrote:\n\n> This is _exactly_ what git's rename detection is designed to do. Yes, it\n> seems horribly confusing in this toy example, but that is because it is\n> a toy example: both 'date' and 'LICENSE' are empty files. But with real\n> files, if a source file has actual content but is deleted, there is a\n> new filename with the identical or near-identical content, and the patch\n> applies to the new content without conflicts, then applying it there is\n> probably exactly what you want.\n\nLooking back over the blog post, it seems that the original question was\nnot about a toy example, but what looks like some boilerplate that\ninvolved empty files.\n\nMaybe git should refuse to detect exact renames between empty files.\nThat is easy enough to special-case, and would help people who have\nthese sorts of boilerplate hierarchies. It would mean that we fail to\nautomatically resolve something like:\n\n $ touch foo && git add foo && git commit -m boilerplate\n $ git branch other\n $ echo content >foo && git commit -m 'fill in boilerplate'\n $ git checkout other\n $ git mv foo bar && git commit -m reorganize\n $ git merge master\n\nBut the failure case is actually quite reasonable. We just mark it as a\nconflict, which is of course trivial for the user to resolve because the\nancestor, by definition, had nothing in it.\n\n-Peff\n"},{"id":"111830","messageId":"7vskk2bt3x.fsf@gitster.siamese.dyndns.org","threadId":"18957","inReplyTo":"20090421024433.GC14479@coredump.intra.peff.net","subject":"Re: correct git merge behavior or corner case?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-04-21T03:09:22Z","receivedAt":"2009-04-21T03:09:22Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> So basically one branch removes a file and adds an identical file under\n> a different name, while the other branch modifies the original file. Git\n> detects it as a rename, and applies the change from the second branch to\n> the newly added file instead of generating a conflict.\n>\n> This is _exactly_ what git's rename detection is designed to do. Yes, it\n> seems horribly confusing in this toy example, but that is because it is\n> a toy example: both 'date' and 'LICENSE' are empty files. But with real\n> files, if a source file has actual content but is deleted, there is a\n> new filename with the identical or near-identical content, and the patch\n> applies to the new content without conflicts, then applying it there is\n> probably exactly what you want.\n\nI had to briefly wonder what the fallout would be if we begin special-\ncasing empty blobs excluded even from exact renames. We effectively do\nnot consider fuzzy renames for blobs smaller than certain threshold, and\nsane projects would not have an empty file tracked anyway, so...\n\nA much lessor impact change would be to keep the diffcore-rename as-is, so\nthat it does detect exact renames between a pair of empty files, but\nspecial case it in merge-recursive. I think I like the latter approach\nbetter.\n\nIn any case, here is what the damage would look like...\n\n diffcore-rename.c | 13 +++++++++----\n 1 files changed, 9 insertions(+), 4 deletions(-)\n\ndiff --git a/diffcore-rename.c b/diffcore-rename.c\nindex 0b0d6b8..dc1f159 100644\n--- a/diffcore-rename.c\n+++ b/diffcore-rename.c\n@@ -59,11 +59,14 @@ static struct diff_rename_src {\n } *rename_src;\n static int rename_src_nr, rename_src_alloc;\n \n-static struct diff_rename_src *register_rename_src(struct diff_filespec *one,\n-\t\t\t\t\t\t unsigned short score)\n+static void register_rename_src(struct diff_filespec *one,\n+\t\t\t\tunsigned short score)\n {\n \tint first, last;\n \n+\tif (is_empty_blob_sha1(one->sha1))\n+\t\treturn;\n+\n \tfirst = 0;\n \tlast = rename_src_nr;\n \twhile (last > first) {\n@@ -71,7 +74,7 @@ static struct diff_rename_src *register_rename_src(struct diff_filespec *one,\n \t\tstruct diff_rename_src *src = &(rename_src[next]);\n \t\tint cmp = strcmp(one->path, src->one->path);\n \t\tif (!cmp)\n-\t\t\treturn src;\n+\t\t\treturn;\n \t\tif (cmp < 0) {\n \t\t\tlast = next;\n \t\t\tcontinue;\n@@ -91,7 +94,7 @@ static struct diff_rename_src *register_rename_src(struct diff_filespec *one,\n \t\t\t(rename_src_nr - first - 1) * sizeof(*rename_src));\n \trename_src[first].one = one;\n \trename_src[first].score = score;\n-\treturn &(rename_src[first]);\n+\treturn;\n }\n \n static int basename_same(struct diff_filespec *src, struct diff_filespec *dst)\n@@ -436,6 +439,8 @@ 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 (is_empty_blob_sha1(p->two->sha1))\n+\t\t\t\tcontinue; /* not interested */\n \t\t\telse\n \t\t\t\tlocate_rename_dst(p->two, 1);\n \t\t}\n"},{"id":"111853","messageId":"fabb9a1e0904210148w4c6b869l396122baef1c0ee3@mail.gmail.com","threadId":"18957","inReplyTo":"7vskk2bt3x.fsf@gitster.siamese.dyndns.org","subject":"Re: correct git merge behavior or corner case?","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-04-21T08:48:19Z","receivedAt":"2009-04-21T08:48:19Z","isPatch":false,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Tue, Apr 21, 2009 at 05:09, Junio C Hamano wrote:\n> and sane projects would not have an empty file tracked anyway, so...\n\nExcept python projects that are full of empty __init__.py files.... no?\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"111855","messageId":"alpine.DEB.1.00.0904211055160.10279@pacific.mpi-cbg.de","threadId":"18957","inReplyTo":"fabb9a1e0904210148w4c6b869l396122baef1c0ee3@mail.gmail.com","subject":"Re: correct git merge behavior or corner case?","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-04-21T08:56:27Z","receivedAt":"2009-04-21T08:56:27Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 21 Apr 2009, Sverre Rabbelier wrote:\n\n> On Tue, Apr 21, 2009 at 05:09, Junio C Hamano wrote:\n> > and sane projects would not have an empty file tracked anyway, so...\n> \n> Except python projects that are full of empty __init__.py files.... no?\n\nBut they would be a good example why we do _not_ want rename detection \nthere.\n\nI actually agree with Junio, though, that we want this special handling of \nempty files only in merge-recursive.\n\nCiao,\nDscho\n"},{"id":"111857","messageId":"fabb9a1e0904210200j5f82e9ci440b99ab18016938@mail.gmail.com","threadId":"18957","inReplyTo":"alpine.DEB.1.00.0904211055160.10279@pacific.mpi-cbg.de","subject":"Re: correct git merge behavior or corner case?","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-04-21T09:00:30Z","receivedAt":"2009-04-21T09:00:30Z","isPatch":false,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Tue, Apr 21, 2009 at 10:56, Johannes Schindelin\n wrote:\n> But they would be a good example why we do _not_ want rename detection\n> there.\n\nYes, I would agree that they are indeed a good example. The other day\nI moved a directory in a python project (containing several\nsub-directories and as such several empty __init__.py files), and it\nrenamed them rather wrongly :P.\n\nThe diff looked something like:\nRenamed root/old/a/__init__.py to root/new/b/__init__.py\nRenamed root/old/b/__init__.py to root/new/c/__init__.py\nRenamed root/old/c/__init__.py to root/new/a/__init__.py\n\nWhile of course, a better result would have been:\nRenamed root/old/a/__init__.py to root/new/a/__init__.py\nRenamed root/old/b/__init__.py to root/new/b/__init__.py\nRenamed root/old/c/__init__.py to root/new/c/__init__.py\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"111856","messageId":"alpine.DEB.1.00.0904211100350.10279@pacific.mpi-cbg.de","threadId":"18957","inReplyTo":"alpine.DEB.1.00.0904211055160.10279@pacific.mpi-cbg.de","subject":"Re: correct git merge behavior or corner case?","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-04-21T09:01:44Z","receivedAt":"2009-04-21T09:01:44Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 21 Apr 2009, Johannes Schindelin wrote:\n\n> I actually agree with Junio, though, that we want this special handling \n> of empty files only in merge-recursive.\n\nAnd this _might_ be enough (not even compile-tested due to lack of time; \nthe OP did not provide the test as a proper patch):\n\n---\n\n merge-recursive.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 774bacd..b7ea3cd 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -343,7 +343,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' || !re->pair->one->size) {\n \t\t\tdiff_free_filepair(pair);\n \t\t\tcontinue;\n \t\t}\n"},{"id":"111858","messageId":"alpine.DEB.1.00.0904211103380.10279@pacific.mpi-cbg.de","threadId":"18957","inReplyTo":"fabb9a1e0904210200j5f82e9ci440b99ab18016938@mail.gmail.com","subject":"Re: correct git merge behavior or corner case?","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-04-21T09:04:21Z","receivedAt":"2009-04-21T09:04:21Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 21 Apr 2009, Sverre Rabbelier wrote:\n\n> On Tue, Apr 21, 2009 at 10:56, Johannes Schindelin\n> wrote:\n> > But they would be a good example why we do _not_ want rename detection\n> > there.\n> \n> Yes, I would agree that they are indeed a good example. The other day\n> I moved a directory in a python project (containing several\n> sub-directories and as such several empty __init__.py files), and it\n> renamed them rather wrongly :P.\n> \n> The diff looked something like:\n> Renamed root/old/a/__init__.py to root/new/b/__init__.py\n> Renamed root/old/b/__init__.py to root/new/c/__init__.py\n> Renamed root/old/c/__init__.py to root/new/a/__init__.py\n> \n> While of course, a better result would have been:\n> Renamed root/old/a/__init__.py to root/new/a/__init__.py\n> Renamed root/old/b/__init__.py to root/new/b/__init__.py\n> Renamed root/old/c/__init__.py to root/new/c/__init__.py\n\nHeh, at some point I had a patch in my tree (dunno if it still there) to \nuse Levenshtein for rename detection, that should have helped...\n\nCiao,\nDscho\n"},{"id":"111887","messageId":"20090421192700.181f8503@gmail.com","threadId":"18957","inReplyTo":"alpine.DEB.1.00.0904211100350.10279@pacific.mpi-cbg.de","subject":"Re: correct git merge behavior or corner case?","fromName":"Michał Kiedrowicz","fromEmail":"michal.kiedrowicz@gmail.com","sentAt":"2009-04-21T17:27:00Z","receivedAt":"2009-04-21T17:27:00Z","isPatch":false,"sender":{"key":"michal.kiedrowicz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/14072847?v=4"},"body":"Johannes Schindelin wrote:\n\n> Hi,\n> \n> On Tue, 21 Apr 2009, Johannes Schindelin wrote:\n> \n> > I actually agree with Junio, though, that we want this special\n> > handling of empty files only in merge-recursive.\n> \n> And this _might_ be enough (not even compile-tested due to lack of\n> time; the OP did not provide the test as a proper patch):\n> \n> ---\n> \n> merge-recursive.c | 2 +-\n> 1 files changed, 1 insertions(+), 1 deletions(-)\n> \n> diff --git a/merge-recursive.c b/merge-recursive.c\n> index 774bacd..b7ea3cd 100644\n> --- a/merge-recursive.c\n> +++ b/merge-recursive.c\n> @@ -343,7 +343,7 @@ static struct string_list *get_renames(struct merge_options *o, struct 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' || !re->pair->one->size) {\n> \t\t\tdiff_free_filepair(pair);\n> \t\t\tcontinue;\n> \t\t}\n\nThis doesn't work for me (actually, it segfaults, \"re\" has just been\ndeclared). However, removing \"re->\" solves the problem.\n\n---\n\n merge-recursive.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex d6f0582..9c2a77f 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -343,7 +343,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' || !pair->one->size) {\n \t\t\tdiff_free_filepair(pair);\n \t\t\tcontinue;\n \t\t}\n"},{"id":"111889","messageId":"20090421195434.3a01676d@gmail.com","threadId":"18957","inReplyTo":"alpine.DEB.1.00.0904211100350.10279@pacific.mpi-cbg.de","subject":"Re: correct git merge behavior or corner case?","fromName":"Michał Kiedrowicz","fromEmail":"michal.kiedrowicz@gmail.com","sentAt":"2009-04-21T17:54:34Z","receivedAt":"2009-04-21T17:54:34Z","isPatch":false,"sender":{"key":"michal.kiedrowicz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/14072847?v=4"},"body":"Johannes Schindelin wrote:\n\n> Hi,\n> \n> On Tue, 21 Apr 2009, Johannes Schindelin wrote:\n> \n> > I actually agree with Junio, though, that we want this special\n> > handling of empty files only in merge-recursive.\n> \n> And this _might_ be enough (not even compile-tested due to lack of\n> time; the OP did not provide the test as a proper patch):\n> \n> ---\n> \n> merge-recursive.c | 2 +-\n> 1 files changed, 1 insertions(+), 1 deletions(-)\n> \n> diff --git a/merge-recursive.c b/merge-recursive.c\n> index 774bacd..b7ea3cd 100644\n> --- a/merge-recursive.c\n> +++ b/merge-recursive.c\n> @@ -343,7 +343,7 @@ static struct string_list *get_renames(struct\n> merge_options *o, struct string_list_item *item;\n> \t\tstruct rename *re;\n> \t\tstruct diff_filepair *pair =\n> diff_queued_diff.queue[i];\n> -\t\tif (pair->status != 'R') {\n> +\t\tif (pair->status != 'R' || !re->pair->one->size) {\n> \t\t\tdiff_free_filepair(pair);\n> \t\t\tcontinue;\n> \t\t}\n> --\n\nAnd here is a test case:\n\n---\n\n t/t6035-merge-suspected-rename.sh | 33 +++++++++++++++++++++++++++++++++\n 1 files changed, 33 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t6035-merge-suspected-rename.sh b/t/t6035-merge-suspected-rename.sh\nnew file mode 100755\nindex 0000000..81615fd\n--- /dev/null\n+++ b/t/t6035-merge-suspected-rename.sh\n@@ -0,0 +1,33 @@\n+#!/bin/sh\n+\n+test_description='Merge-recursive merging suspected rename'\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' '\n+\techo \"hello\" > date &&\n+\tgit add date &&\n+\tgit commit -m initial &&\n+\n+\tgit branch parallel &&\n+\n+\techo \"hello\" > LICENSE &&\n+\tcp LICENSE LICENSE-copy &&\n+\tgit add LICENSE &&\n+\tgit commit -m LICENSE &&\n+\n+\tgit rm date &&\n+\tgit commit -m removed &&\n+\n+\tgit checkout parallel &&\n+\tdate > date &&\n+\tgit add date &&\n+\tgit commit -m date\n+'\n+\n+test_expect_success 'merge' '\n+\tgit checkout master &&\n+\ttest_must_fail git merge parallel &&\n+\ttest_cmp LICENSE LICENSE-copy\n+'\n+\n+test_done\n"},{"id":"111892","messageId":"20090421180529.GA7583@coredump.intra.peff.net","threadId":"18957","inReplyTo":"20090421195434.3a01676d@gmail.com","subject":"Re: correct git merge behavior or corner case?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-04-21T18:05:30Z","receivedAt":"2009-04-21T18:05:30Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 21, 2009 at 07:54:34PM +0200, Michał Kiedrowicz wrote:\n\n> And here is a test case:\n> [...]\n> +\techo \"hello\" > date &&\n> +\tgit add date &&\n> +\tgit commit -m initial &&\n> +\n> +\tgit branch parallel &&\n> +\n> +\techo \"hello\" > LICENSE &&\n> +\tcp LICENSE LICENSE-copy &&\n> +\tgit add LICENSE &&\n> +\tgit commit -m LICENSE &&\n\nI thought the point was about _empty_ renames. This is a small but\nnon-zero rename.\n\n-Peff\n"},{"id":"111896","messageId":"20090421204701.1e0115c0@gmail.com","threadId":"18957","inReplyTo":"20090421180529.GA7583@coredump.intra.peff.net","subject":"Re: correct git merge behavior or corner case?","fromName":"Michał Kiedrowicz","fromEmail":"michal.kiedrowicz@gmail.com","sentAt":"2009-04-21T18:47:01Z","receivedAt":"2009-04-21T18:47:01Z","isPatch":false,"sender":{"key":"michal.kiedrowicz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/14072847?v=4"},"body":"Jeff King wrote:\n\n> On Tue, Apr 21, 2009 at 07:54:34PM +0200, Michał Kiedrowicz wrote:\n> \n> > And here is a test case:\n> > [...]\n> > +\techo \"hello\" > date &&\n> > +\tgit add date &&\n> > +\tgit commit -m initial &&\n> > +\n> > +\tgit branch parallel &&\n> > +\n> > +\techo \"hello\" > LICENSE &&\n> > +\tcp LICENSE LICENSE-copy &&\n> > +\tgit add LICENSE &&\n> > +\tgit commit -m LICENSE &&\n> \n> I thought the point was about _empty_ renames. This is a small but\n> non-zero rename.\n> \n> -Peff\n\nYes, you are right. But change these echos to touch and you'll see that\nthis bug happens if date and LICENSE are the same, not necessary empty.\n\nMichał Kiedrowicz\n"},{"id":"111898","messageId":"20090421191126.GA7632@coredump.intra.peff.net","threadId":"18957","inReplyTo":"20090421204701.1e0115c0@gmail.com","subject":"Re: correct git merge behavior or corner case?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-04-21T19:11:26Z","receivedAt":"2009-04-21T19:11:26Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 21, 2009 at 08:47:01PM +0200, Michał Kiedrowicz wrote:\n\n> > I thought the point was about _empty_ renames. This is a small but\n> > non-zero rename.\n> \n> Yes, you are right. But change these echos to touch and you'll see that\n> this bug happens if date and LICENSE are the same, not necessary empty.\n\nRight, because it is not exactly a bug, as I explained here:\n\n http://article.gmane.org/gmane.comp.version-control.git/117078\n\nI say \"not exactly a bug\", because it is working as intended by the\nsoftware authors, but because the intended behavior is to make a\nheuristic guess (some content moved from one filename to another, so we\nguess that a patch against the original content should actually go\nagainst the new location), the guess may not follow the user's\nexpectations.\n\nIt is easy to see how the heuristic can be fooled in a toy example. But\nwhat we really care about is whether and how the heuristic is fooled in\nthe real world. In the real world, there seems to be some non-trivial\nprobability of removing an empty file, adding a new one elsewhere, and\nthen merging with somebody who touched the empty file. So it may be\nworth improving the heuristic for this special case, especially because\nthe harm done in a false negative is relatively small.\n\nBut what is the probability of doing the same thing to a file that has\nnon-trivial contents? I would guess it is much less likely, and by\nspecial-casing it as a conflict, you have a much higher chance of\nbothering users who were relying on actual rename detection for their\nnon-trivial case[1]. Of course, I don't have actual numbers, so I'm just\nguessing.\n\nSo my point is that while both are perhaps a failing of the heuristic,\nonly one is going to be worth tweaking the heuristic for. So that is the\none that should be included in the test case, since it is how somebody\nimplementing a proposed tweak can test their tweak.\n\n-Peff\n\n[1] On a side note, this got me thinking about how git handles rename\ndetection during merging. One of the things I like about git is that it\ntries to be very stupid: if something is questionable during a merge, it\ncalls attention to it and makes it _easy_ for the user to access the\nversions and resolve (and I love mergetool for this). But renames are\nnot like this: either they happen during auto-conflict-resolution, or\nthey don't. I wonder if it might be a better strategy to barf on\nconflicts due removed files that could be resolved by questionable\nrenames, but have a post-merge \"renametool\" that shows the user\npotential renames and lets them interactively specify the resolution.\nBut maybe that would just be annoying, since 99% of the time, the rename\ndetection gets it right.\n"}]}