{"thread":{"id":"22649","subject":"Bugs with detection of renames that are also overwrites","startedAt":"2010-02-13T23:46:55Z","lastAt":"2010-02-23T19:08:36Z","messageCount":5,"participants":["Anders Kaseorg","Jeff King","Jon Seymour"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"134503","messageId":"alpine.DEB.2.00.1002131820150.14055@dr-wily.mit.edu","threadId":"22649","inReplyTo":null,"subject":"Bugs with detection of renames that are also overwrites","fromName":"Anders Kaseorg","fromEmail":"andersk@mit.edu","sentAt":"2010-02-13T23:46:55Z","receivedAt":"2010-02-13T23:46:55Z","isPatch":false,"sender":{"key":"andersk@mit.edu","avatar":"https://avatars.githubusercontent.com/u/26471?v=4"},"body":"One can create a rename that also overwrites an existing file, for example \nwith ‘git mv -f’:\n\n$ git init\n$ seq 100 200 > a; seq 300 400 > b; git add a b\n$ git commit -m foo; git tag foo\n$ git mv -f a b\n$ git commit -m bar; git tag bar\n\nGit does not ordinarily detect this as a rename, even with -M.\n\n$ git diff --stat -M foo bar\n a |  101 ----------------------------------\n b |  202 ++++++++++++++++++++++++++++++++++----------------------------------\n 2 files changed, 101 insertions(+), 202 deletions(-)\n\nBut it can (sometimes*) be forced to detect the rename with -M -B.  \n\n$ git diff --stat -M -B foo bar\n a => b |    0\n 1 files changed, 0 insertions(+), 0 deletions(-)\n\nHowever, the resulting patch incorrectly omits the deletion of the \noverwritten file!\n\n$ git diff -M -B foo bar\ndiff --git a/a b/b\nsimilarity index 100%\nrename from a\nrename to b\n\nTherefore, the patch can’t be applied to its own source tree.\n\n$ git checkout foo\n$ git diff -M -B foo bar | git apply\nerror: b: already exists in working directory\n$ git format-patch --stdout -M -B foo..bar | git am\nApplying: bar\nerror: b: already exists in index\nPatch failed at 0001 bar\nWhen you have resolved this problem run \"git am --resolved\".\nIf you would prefer to skip this patch, instead run \"git am --skip\".\nTo restore the original branch and stop patching run \"git am --abort\".\n\nIf it matters, I’m using Git 1.7.0.\n\nAlso, a question: is it possible to get ‘git merge’ to recognize this \nrename?\n\nAnders\n\n(* I say “sometimes” because -B only detects the rewrite if it deems the \nrenamed file to be sufficiently different than the overwritten file.  If \nyou use 190 and 390 instead of 200 and 400 above, the rewrite and rename \nwill not be detected, even though the rename would be detected if it was \nnot an overwrite.  This also feels like a bug.)\n"},{"id":"135417","messageId":"20100223122257.GA1735@coredump.intra.peff.net","threadId":"22649","inReplyTo":"alpine.DEB.2.00.1002131820150.14055@dr-wily.mit.edu","subject":"Re: Bugs with detection of renames that are also overwrites","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-02-23T12:22:57Z","receivedAt":"2010-02-23T12:22:57Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Feb 13, 2010 at 06:46:55PM -0500, Anders Kaseorg wrote:\n\n> One can create a rename that also overwrites an existing file, for example \n> with ‘git mv -f’:\n> \n> $ git init\n> $ seq 100 200 > a; seq 300 400 > b; git add a b\n> $ git commit -m foo; git tag foo\n> $ git mv -f a b\n> $ git commit -m bar; git tag bar\n> \n> Git does not ordinarily detect this as a rename, even with -M.\n\nRight. Git looks at only a subset of the files when looking for renames.\nFor straight renames, the set of possible sources is the list of deleted\nfiles, and the possible destinations are the new files.\n\nFinding copies (\"-C\") is similar, except that we also consider files\nthat were modified but not deleted. And --find-copies-harder will look\nat _all_ files as sources, not just modified ones.\n\nBut what you are asking for is to expand the possible destination list\nto include files that were modified but are not new. I don't think there\nis currently a way to do that explicitly.\n\nAs you discovered, though, if either the source or destination file has\nchanged significantly, we should break them apart using \"-B\":\n\n> But it can (sometimes*) be forced to detect the rename with -M -B.  \n> \n> $ git diff --stat -M -B foo bar\n>  a => b |    0\n>  1 files changed, 0 insertions(+), 0 deletions(-)\n\nIn which case the rename engine sees the deletion and addition\nseparately (they just happen to have the same path name), and therefore\nthe path gets added to the source and destination lists.\n\n> However, the resulting patch incorrectly omits the deletion of the \n> overwritten file!\n> \n> $ git diff -M -B foo bar\n> diff --git a/a b/b\n> similarity index 100%\n> rename from a\n> rename to b\n>\n> Therefore, the patch can’t be applied to its own source tree.\n> \n> $ git checkout foo\n> $ git diff -M -B foo bar | git apply\n> error: b: already exists in working directory\n\nHmm. I think this is a conflict between what the user wants to see and\nwhat apply wants to see. From the user's perspective, thinking about the\ndiff representing a change, \"b\" was not really deleted. It was simply\noverwritten. But from apply's perspective, the diff is a set of\ninstructions, and those instructions do not mention that \"b\" previously\nexisted and was overwritten. So it is right to be cautious and declare a\nconflict.\n\nI'm not sure just throwing a \"delete\" line in there would be the best\nthing, though. The instructions for apply really need to be \"if 'b' has\nthis sha1, then it is safe to delete and rename a into place. Otherwise\nit is a conflict\". And I'm not sure how we would represent that (I guess\nwith a full diff and an \"index\" line).\n\n> Also, a question: is it possible to get ‘git merge’ to recognize this \n> rename?\n\nNo, I don't think there is a way currently. You would need to patch git\nto set opts.break_opt in merge-recursive.c:get_renames, I think.\n\n> (* I say “sometimes” because -B only detects the rewrite if it deems the \n> renamed file to be sufficiently different than the overwritten file.  If \n> you use 190 and 390 instead of 200 and 400 above, the rewrite and rename \n> will not be detected, even though the rename would be detected if it was \n> not an overwrite.  This also feels like a bug.)\n\nI think you are getting into a philosophical discussion of what is a\nrename, here. If \"a\" and \"b\" are very similar, \"a\" is removed, and \"b\"\nhas the same (or similar) content as \"a\", was it a rename from \"a\", or\nwas it simply that \"b\" was changed, possibly to incorporate the\nduplicated items in \"a\"?\n\nSo I don't think it is a bug. But that isn't to say you can't come up\nwith a case where it would be nice, during a diff or a merge, to show\nthings that way. I mentioned at the beginning of the message that what\nyou wanted was to expand the list of destination possibilities. You\ncould have a \"--find-overwrites\" which puts all of the modified files\ninto the destination list. You would not want it on by default, though,\nI think, as it would add a lot of computation time to find a somewhat\nrare case.\n\n-Peff\n"},{"id":"135420","messageId":"2cfc40321002230453q3a593fa6l43850207ebddfc99@mail.gmail.com","threadId":"22649","inReplyTo":"20100223122257.GA1735@coredump.intra.peff.net","subject":"Re: Bugs with detection of renames that are also overwrites","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2010-02-23T12:53:13Z","receivedAt":"2010-02-23T12:53:13Z","isPatch":false,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"I came across a similar case recently while testing my rebasing ideas\nwhich lead me to wonder - is there a way to suppress the rename and\noverwrite detection? For my purposes, I don't care if the diff is\nbulky, I just want it to apply reliably.\n\njon\n\nOn Tue, Feb 23, 2010 at 11:22 PM, Jeff King <peff@peff.net> wrote:\n> On Sat, Feb 13, 2010 at 06:46:55PM -0500, Anders Kaseorg wrote:\n>\n>> One can create a rename that also overwrites an existing file, for example\n>> with ‘git mv -f’:\n>>\n>> $ git init\n>> $ seq 100 200 > a; seq 300 400 > b; git add a b\n>> $ git commit -m foo; git tag foo\n>> $ git mv -f a b\n>> $ git commit -m bar; git tag bar\n>>\n>> Git does not ordinarily detect this as a rename, even with -M.\n>\n> Right. Git looks at only a subset of the files when looking for renames.\n> For straight renames, the set of possible sources is the list of deleted\n> files, and the possible destinations are the new files.\n>\n> Finding copies (\"-C\") is similar, except that we also consider files\n> that were modified but not deleted. And --find-copies-harder will look\n> at _all_ files as sources, not just modified ones.\n>\n> But what you are asking for is to expand the possible destination list\n> to include files that were modified but are not new. I don't think there\n> is currently a way to do that explicitly.\n>\n> As you discovered, though, if either the source or destination file has\n> changed significantly, we should break them apart using \"-B\":\n>\n>> But it can (sometimes*) be forced to detect the rename with -M -B.\n>>\n>> $ git diff --stat -M -B foo bar\n>>  a => b |    0\n>>  1 files changed, 0 insertions(+), 0 deletions(-)\n>\n> In which case the rename engine sees the deletion and addition\n> separately (they just happen to have the same path name), and therefore\n> the path gets added to the source and destination lists.\n>\n>> However, the resulting patch incorrectly omits the deletion of the\n>> overwritten file!\n>>\n>> $ git diff -M -B foo bar\n>> diff --git a/a b/b\n>> similarity index 100%\n>> rename from a\n>> rename to b\n>>\n>> Therefore, the patch can’t be applied to its own source tree.\n>>\n>> $ git checkout foo\n>> $ git diff -M -B foo bar | git apply\n>> error: b: already exists in working directory\n>\n> Hmm. I think this is a conflict between what the user wants to see and\n> what apply wants to see. From the user's perspective, thinking about the\n> diff representing a change, \"b\" was not really deleted. It was simply\n> overwritten. But from apply's perspective, the diff is a set of\n> instructions, and those instructions do not mention that \"b\" previously\n> existed and was overwritten. So it is right to be cautious and declare a\n> conflict.\n>\n> I'm not sure just throwing a \"delete\" line in there would be the best\n> thing, though. The instructions for apply really need to be \"if 'b' has\n> this sha1, then it is safe to delete and rename a into place. Otherwise\n> it is a conflict\". And I'm not sure how we would represent that (I guess\n> with a full diff and an \"index\" line).\n>\n>> Also, a question: is it possible to get ‘git merge’ to recognize this\n>> rename?\n>\n> No, I don't think there is a way currently. You would need to patch git\n> to set opts.break_opt in merge-recursive.c:get_renames, I think.\n>\n>> (* I say “sometimes” because -B only detects the rewrite if it deems the\n>> renamed file to be sufficiently different than the overwritten file.  If\n>> you use 190 and 390 instead of 200 and 400 above, the rewrite and rename\n>> will not be detected, even though the rename would be detected if it was\n>> not an overwrite.  This also feels like a bug.)\n>\n> I think you are getting into a philosophical discussion of what is a\n> rename, here. If \"a\" and \"b\" are very similar, \"a\" is removed, and \"b\"\n> has the same (or similar) content as \"a\", was it a rename from \"a\", or\n> was it simply that \"b\" was changed, possibly to incorporate the\n> duplicated items in \"a\"?\n>\n> So I don't think it is a bug. But that isn't to say you can't come up\n> with a case where it would be nice, during a diff or a merge, to show\n> things that way. I mentioned at the beginning of the message that what\n> you wanted was to expand the list of destination possibilities. You\n> could have a \"--find-overwrites\" which puts all of the modified files\n> into the destination list. You would not want it on by default, though,\n> I think, as it would add a lot of computation time to find a somewhat\n> rare case.\n>\n> -Peff\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n>\n"},{"id":"135422","messageId":"20100223125952.GA7638@coredump.intra.peff.net","threadId":"22649","inReplyTo":"2cfc40321002230453q3a593fa6l43850207ebddfc99@mail.gmail.com","subject":"Re: Bugs with detection of renames that are also overwrites","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-02-23T12:59:52Z","receivedAt":"2010-02-23T12:59:52Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 23, 2010 at 11:53:13PM +1100, Jon Seymour wrote:\n\n> I came across a similar case recently while testing my rebasing ideas\n> which lead me to wonder - is there a way to suppress the rename and\n> overwrite detection? For my purposes, I don't care if the diff is\n> bulky, I just want it to apply reliably.\n\nFor diff generation, it should be off by default (but you may have\nconfigured diff.renames, in which case you can pass --no-renames to\nsuppress it).\n\nFor the merge-recursive driver, it is on by default. You can hack it off\nby setting merge.renamelimit to something small like '1' (but don't use\n'0', which means \"no limit\").  Rebase uses \"git am -3\" internally, which\nin turn invokes merge-recursive.\n\n-Peff\n"},{"id":"135451","messageId":"alpine.DEB.2.00.1002231336310.14055@dr-wily.mit.edu","threadId":"22649","inReplyTo":"20100223122257.GA1735@coredump.intra.peff.net","subject":"Re: Bugs with detection of renames that are also overwrites","fromName":"Anders Kaseorg","fromEmail":"andersk@mit.edu","sentAt":"2010-02-23T19:08:36Z","receivedAt":"2010-02-23T19:08:36Z","isPatch":false,"sender":{"key":"andersk@mit.edu","avatar":"https://avatars.githubusercontent.com/u/26471?v=4"},"body":"On Tue, 23 Feb 2010, Jeff King wrote:\n> > Therefore, the patch can’t be applied to its own source tree.\n> > \n> > $ git checkout foo\n> > $ git diff -M -B foo bar | git apply\n> > error: b: already exists in working directory\n> \n> Hmm. I think this is a conflict between what the user wants to see and\n> what apply wants to see. From the user's perspective, thinking about the\n> diff representing a change, \"b\" was not really deleted. It was simply\n> overwritten. But from apply's perspective, the diff is a set of\n> instructions, and those instructions do not mention that \"b\" previously\n> existed and was overwritten. So it is right to be cautious and declare a\n> conflict.\n\nI agree; git apply has no choice given that input.\n\n> I'm not sure just throwing a \"delete\" line in there would be the best\n> thing, though. The instructions for apply really need to be \"if 'b' has\n> this sha1, then it is safe to delete and rename a into place. Otherwise\n> it is a conflict\". And I'm not sure how we would represent that (I guess\n> with a full diff and an \"index\" line).\n\nYeah, a full diff is the only sane solution, just like it is for plain \ndeletes.  Otherwise the patch could not be reverse-applied.  (If the user \nreally doesn’t want to see the deletion, they can use --diff-filter.)\n\ndiff --git a/b b/b\ndeleted file mode 100644\nindex 3d47ea7..0000000\n--- a/b\n+++ /dev/null\n@@ -1,101 +0,0 @@\n-300\n-301\n…\n-399\n-400\ndiff --git a/a b/b\nsimilarity index 100%\nrename from a\nrename to b\n\nThis patch is already handled correctly by git apply (and git apply -R), \nas long as the rename is listed after the delete.\n\nAnders\n"}]}