{"thread":{"id":"11483","subject":"git diff -B: splitting up complete renames","startedAt":"2008-01-05T17:18:38Z","lastAt":"2008-01-05T19:18:20Z","messageCount":3,"participants":["Jakub Narebski","Linus Torvalds","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"64538","messageId":"200801051818.40009.jnareb@gmail.com","threadId":"11483","inReplyTo":null,"subject":"git diff -B: splitting up complete renames","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-01-05T17:18:38Z","receivedAt":"2008-01-05T17:18:38Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"First: in Documentation/diffcore.txt there is an example of broken pair\n\n  diffcore-break: For Splitting Up \"Complete Rewrites\"\n  ----------------------------------------------------\n\n  The second transformation in the chain is diffcore-break, and is\n  controlled by the -B option to the git-diff-* commands.  This is\n  used to detect a filepair that represents \"complete rewrite\" and\n  break such filepair into two filepairs that represent delete and\n  create.  E.g.  If the input contained this filepair:\n\n  ------------------------------------------------\n  :100644 100644 bcd1234... 0123456... M file0\n  ------------------------------------------------\n\n  and if it detects that the file \"file0\" is completely rewritten,\n  it changes it to:\n\n  ------------------------------------------------\n  :100644 000000 bcd1234... 0000000... D file0\n  :000000 100644 0000000... 0123456... A file0\n  ------------------------------------------------\n\nShouldn't the last block read for the modern git read:\n\n  ------------------------------------------------\n  :100644 000000 bcd1234... 0123456... M99 file0\n  ------------------------------------------------\n\nor is it only for '-B' togehter with '-M' or '-C'?\n\n\nSecond: do you have per chance good examples of broken (and perhaps \nmerged together) pair, such that it is affected by --diff-filter=B;\nwith -B<num> or -B<num1>/<num2> parameters needed, if required. COPYING \nand Makefile are one good example, but do you have literal files with \nsimilar behavior? \n\nThird: Do \"git diff --no-index\" (filesystem diff) can show breaking / \nuse dissimilarity? I couldn't make it work...\n\n-- \nJakub Narebski\nPoland\n"},{"id":"64539","messageId":"alpine.LFD.1.00.0801050926180.2811@woody.linux-foundation.org","threadId":"11483","inReplyTo":"200801051818.40009.jnareb@gmail.com","subject":"Re: git diff -B: splitting up complete renames","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-01-05T17:37:20Z","receivedAt":"2008-01-05T17:37:20Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 5 Jan 2008, Jakub Narebski wrote:\n> \n> Shouldn't the last block read for the modern git read:\n> \n>   ------------------------------------------------\n>   :100644 000000 bcd1234... 0123456... M99 file0\n>   ------------------------------------------------\n> \n> or is it only for '-B' togehter with '-M' or '-C'?\n\nWithout -M/C, git re-joins them, so yeah, either the documentation (or \nthat re-joining logic) is wrong.\n\n> Second: do you have per chance good examples of broken (and perhaps \n> merged together) pair, such that it is affected by --diff-filter=B;\n> with -B<num> or -B<num1>/<num2> parameters needed, if required. COPYING \n> and Makefile are one good example, but do you have literal files with \n> similar behavior? \n\nHmm. A kernel example that shows *something* is\n\n\tgit show -B --raw -r acbbbe9f5ab52da90a8edec02ec973e7f44dae81\n\nwhich shows \n\n\t:100644 100644 eb14b18... 1f2d6d5... M093       include/asm-x86/byteorder.h\n\neven though that 093 is the *dissimilarity* index, so according to the \ndocumentation you quote you'd expect it to be\n\n\t:100644 000000 eb14b18... 0000000... D\tinclude/asm-x86/byteorder.h\n\t:000000 100644 0000000... 1f2d6d5... A\tinclude/asm-x86/byteorder.h\n\ninstead of a M. Whether it's the docs or the behavior that should be \nchanged, I dunno.\n\nEspecially as using -B -M makes it all sane again, because then it shows \nit as the real rename:\n\n\t:100644 100644 435074a... 26b0dcd... M  include/asm-x86/Kbuild\n\t:100644 100644 a45470a... 1f2d6d5... R061       include/asm-x86/byteorder_32.h  include/asm-x86/byteorde\n\t:100644 000000 5e86c86... 0000000... D  include/asm-x86/byteorder_64.h\n\nand it's all good.\n\nMy personal opinion is that -B doesn't make much sense without M (or C), \nbut at least you can see an example of the breaking behaviour in the \nkernel for a real case.\n\n\t\t\tLinus\n"},{"id":"64541","messageId":"7vprwg84jn.fsf@gitster.siamese.dyndns.org","threadId":"11483","inReplyTo":"200801051818.40009.jnareb@gmail.com","subject":"Re: git diff -B: splitting up complete renames","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-05T19:18:20Z","receivedAt":"2008-01-05T19:18:20Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> ...\n>   and if it detects that the file \"file0\" is completely rewritten,\n>   it changes it to:\n>\n>   ------------------------------------------------\n>   :100644 000000 bcd1234... 0000000... D file0\n>   :000000 100644 0000000... 0123456... A file0\n>   ------------------------------------------------\n>\n> Shouldn't the last block read for the modern git read:\n>\n>   ------------------------------------------------\n>   :100644 000000 bcd1234... 0123456... M99 file0\n>   ------------------------------------------------\n\nCorrect.  The description is based on an earlier round of the\ncode before 366175ef8c3b1e145f4ba846e63a1dea3ec3cacc (Rework -B\noutput).  \"git show\" of that patch has description in its\nchanges to Documentation/diffcore.txt.\n\nThe original motivation of -B was not about helping three way\nrenames it allows you when used with -M/-C.  It was about\nshowing textual diff more readably for quite dissimilar\nfilepairs.\n\n> Third: Do \"git diff --no-index\" (filesystem diff) can show breaking / \n> use dissimilarity? I couldn't make it work...\n\nIt doesn't, for two reasons.\n\n * Look at diffcore_break().  It breaks filepairs with identical\n   paths and nothing else.\n\n   The reason for this check is that the caller can call\n   diffcore_rename() _before_ diffcore_break() [*1*] and a\n   filepair that was detected as renamed pair ought to be\n   already found similar enough.  These days, I think you can\n   replace the check with p->renamed_pair.\n\n * Look at diffcore_merge_broken(), which is responsible for\n   re-merging a broken pair.  Because we match them by looking\n   at their names (iow, we do not use pointers to link halves of\n   a broken pair pointing at each other).\n\n   This was correct before \"no-index\" stuff because \"git diff\"\n   was about comparing pairs of blobs found at corresponding\n   paths in two tree-like things (i.e. \"a/one\" and \"b/one\"\n   corresponds to each other when you do \"git diff -- one\").\n   The modification to introduce no-index forgot to update this\n   logic.\n\nIf you wanted to fix this, you can change p->broken_pair from a\nboolean to a pointer that points at the other half of the broken\npair (and record that when we break a pair in diffcore_break()),\nand look at that pointer to decide which one to match up with\nwhich other one in diffcore_merge_broken().  Together with a\nchange to look at p->renamed_pair instead of paths I mentioned,\nI think it would work more like the regular git diff for\n\"no-index\" case.\n\nI consider that \"no-index\" frontend more or less a bolted on\nhalf-baked hack that covers only minimally necessary cases to\nserve as non-git \"diff\" replacement, without sharing enough with\nthe real \"git diff\" internals; I would not be surprised at all\nif there are more corner cases like this that does not work.\nBut I do not think it is fundamentally unfixable.  The change to\nadd \"no-index\" support just needed to be more careful, and it is\nnot too late to make it so.\n\n\n[Footnote]\n\n*1* diffcore was designed as a generic library to allow\nexperimenting more combinations of transformations than what\nthen-current git used.  All of our existing callers ended up\ncalling the transformations in the same order (i.e. the order\ndiffcore_std() calls them), but individual transformations were\nwritten not to assume too much about the order they would be\ncalled.\n"}]}