{"thread":{"id":"41650","subject":"Possible bug: --ext-diff ignored with --cc in git log","startedAt":"2016-03-09T17:43:10Z","lastAt":"2016-03-12T01:08:12Z","messageCount":8,"participants":["Vadim Zeitlin","Junio C Hamano","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"280477","messageId":"E1adi8q-0005NJ-4G@smtp.tt-solutions.com","threadId":"41650","inReplyTo":null,"subject":"Possible bug: --ext-diff ignored with --cc in git log","fromName":"Vadim Zeitlin","fromEmail":"vz-git@zeitlins.org","sentAt":"2016-03-09T17:43:10Z","receivedAt":"2016-03-09T17:43:10Z","isPatch":false,"sender":{"key":"vz-git@zeitlins.org","avatar":null},"body":" Hello,\n\n I use a combination of git attributes and a custom diff driver to ignore\nthe changes to the generated files (that we unfortunately need to keep in\nour repository) from appearing in \"git diff\" and \"git log\" output, i.e.:\n\n\t% cat .gitattributes\n\t# Use a custom diff driver for bakefile generated files\n\t# This allows ignoring them in git diff for example by doing\n\t# $ git config diff.generated.command true\n\t# pass --no-ext-diff to git diff to ignore the custom driver\n\t*.sln diff=generated\n\t*.vcproj diff=generated\n\t*.vcxproj diff=generated\n\t*.vcxproj.filters diff=generated\n\t% git config diff.generated.command\n\techo Diff of generated file \"$1\" suppressed; true\n\n This works quite nicely most of the time provided you use \"--ext-diff\"\nwith \"git log\", but not when showing the merges with \"--cc\". I.e. the\ncommand \"git log --ext-diff -p --cc\" still outputs the real diff even for\nthe generated files, as if \"--ext-diff\" were not given. If I use \"git log\n--ext-diff -p -m\", the generated files are ignored, which is nice, but the\ndiff for the other files becomes much more verbose and less readable and\nI'd really like to combine \"--ext-diff\" with \"--cc\" if possible.\n\n Is the current behaviour intentional? I see it with all the git versions I\ntried (1.7.10, 2.1.0, 2.7.0 and v2.8.0-rc1), but I don't really see why\nwould it need to work like this, so I hope it's an oversight and could be\ncorrected.\n\n Or is there perhaps some other way to do what I want already?\n\n Thanks in advance for any help,\nVZ\n"},{"id":"280586","messageId":"xmqqlh5qc698.fsf@gitster.mtv.corp.google.com","threadId":"41650","inReplyTo":"E1adi8q-0005NJ-4G@smtp.tt-solutions.com","subject":"Re: Possible bug: --ext-diff ignored with --cc in git log","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-10T22:33:55Z","receivedAt":"2016-03-10T22:33:55Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Vadim Zeitlin writes:\n\n> I.e. the\n> command \"git log --ext-diff -p --cc\" still outputs the real diff even for\n> the generated files, as if \"--ext-diff\" were not given. ...\n> Is the current behaviour intentional? I see it with all the git versions I\n> tried (1.7.10, 2.1.0, 2.7.0 and v2.8.0-rc1), but I don't really see why\n> would it need to work like this, so I hope it's an oversight and could be\n> corrected.\n\nI think this is \"intentional\" in the sense that \"--cc\" feature is\nfundamentally and conceptually incompatible with \"--ext-diff\".\n\n - The \"external diff\" feature is to allow third-party tools to\n produce output that is vastly different from the usual \"diff\"\n output, e.g. unlike the usual \"diff\", the output may not even be\n line-oriented, and certainly would not have to follow the\n convention of denoting the contents on old and new lines with \"-\"\n and \"+\" prefixes.\n\n - The \"--cc\" feature is to show multiple \"diff\" outputs in the\n usual format with post-processing to coalesce them into a more\n concise form, and fundamentally depends on (1) the output being\n line-oriented and (2) the contents of old and new lines denoted\n by \"-\"/\"+\" prefixes to be able to do so.\n\nI haven't tried it myself, but if the contents you are using\next-diff on can be compared in a format that is easy-to-read for\nhumans by passing them first to \"textconv\" filter and then running\nthe normal \"diff\" on, that may be a viable approach to do what you\nare trying to do, as \"textconv\" feature is meant to still produce\nthe output that still follows the usual \"diff\" convention. Its\noutput should be usable by any tool (e.g. diffstat) meant to\npost-process patch output, and would be a better match for the\n\"--cc\" mechanism.\n"},{"id":"280603","messageId":"E1aeCRp-0005Jn-C1@smtp.tt-solutions.com","threadId":"41650","inReplyTo":"xmqqlh5qc698.fsf@gitster.mtv.corp.google.com","subject":"Re[2]: Possible bug: --ext-diff ignored with --cc in git log","fromName":"Vadim Zeitlin","fromEmail":"vz-git@zeitlins.org","sentAt":"2016-03-11T02:04:46Z","receivedAt":"2016-03-11T02:04:46Z","isPatch":false,"sender":{"key":"vz-git@zeitlins.org","avatar":null},"body":"On Thu, 10 Mar 2016 14:33:55 -0800 Junio C Hamano wrote:\n\nJCH> Vadim Zeitlin writes:\nJCH> \nJCH> > I.e. the\nJCH> > command \"git log --ext-diff -p --cc\" still outputs the real diff even for\nJCH> > the generated files, as if \"--ext-diff\" were not given. ...\nJCH> > Is the current behaviour intentional? I see it with all the git versions I\nJCH> > tried (1.7.10, 2.1.0, 2.7.0 and v2.8.0-rc1), but I don't really see why\nJCH> > would it need to work like this, so I hope it's an oversight and could be\nJCH> > corrected.\nJCH> \nJCH> I think this is \"intentional\" in the sense that \"--cc\" feature is\nJCH> fundamentally and conceptually incompatible with \"--ext-diff\".\n\n Thank you for your reply, Junio, I hadn't realized that --cc was dependent\non textual diff output format before, but now I understand why it can't\nrespect --ext-diff.\n\nJCH> I haven't tried it myself, but if the contents you are using\nJCH> ext-diff on can be compared in a format that is easy-to-read for\nJCH> humans by passing them first to \"textconv\" filter and then running\nJCH> the normal \"diff\" on, that may be a viable approach to do what you\nJCH> are trying to do, as \"textconv\" feature is meant to still produce\nJCH> the output that still follows the usual \"diff\" convention. Its\nJCH> output should be usable by any tool (e.g. diffstat) meant to\nJCH> post-process patch output, and would be a better match for the\nJCH> \"--cc\" mechanism.\n\n I can't think of a way to make the output as concise as it is now (i.e.\njust a single line saying that a generated file has been modified but the\nchanges to it are not being shown) with this approach.\n\n Maybe I'm clutching at straws here, but I wonder if it could be possible\nto have a file attribute specifying whether --cc or -m should be used for\nit when showing merges? Because this is, basically, what I want here: --cc\nfor normal files for readability but -m for the files I'm not interested\nin. It's probably too specific to my particular hack^H^H^H^H use case to\nadd support for it to Git itself, but I wanted to mention it on a chance\nthat somebody else might think it's a good idea.\n\n Anyhow, thanks again for your explanation,\nVZ\n"},{"id":"280629","messageId":"xmqqziu4anb9.fsf@gitster.mtv.corp.google.com","threadId":"41650","inReplyTo":"E1aeCRp-0005Jn-C1@smtp.tt-solutions.com","subject":"Re: Possible bug: --ext-diff ignored with --cc in git log","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-11T18:20:42Z","receivedAt":"2016-03-11T18:20:42Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Vadim Zeitlin writes:\n\n> On Thu, 10 Mar 2016 14:33:55 -0800 Junio C Hamano wrote:\n>\n> JCH> Vadim Zeitlin writes:\n> JCH> \n> JCH> > I.e. the\n> JCH> > command \"git log --ext-diff -p --cc\" still outputs the real diff even for\n> JCH> > the generated files, as if \"--ext-diff\" were not given. ...\n> JCH> > Is the current behaviour intentional? I see it with all the git versions I\n> JCH> > tried (1.7.10, 2.1.0, 2.7.0 and v2.8.0-rc1), but I don't really see why\n> JCH> > would it need to work like this, so I hope it's an oversight and could be\n> JCH> > corrected.\n> JCH> \n> JCH> I think this is \"intentional\" in the sense that \"--cc\" feature is\n> JCH> fundamentally and conceptually incompatible with \"--ext-diff\".\n>\n> Thank you for your reply, Junio, I hadn't realized that --cc was dependent\n> on textual diff output format before, but now I understand why it can't\n> respect --ext-diff.\n\nHaving established that, I should also add that \"--cc fundamentally\nis incompatible with --ext-diff\" does not justify that\n\"--cc when given with --ext-diff just ignores and uses the usual\ndiff\".\n\nAn equally (or even more) valid consequence could have been to\ndisable \"--cc\" processing for paths that would trigger an external\ndiff driver. After all, the user told us that the contents would\nnot compare well with the usual \"diff\"; we know that \"--cc\" output\nthat summarizes the usual diff output is useless.\n\nWhat we show instead is an interesting thing to think about.\n\nFor example, we _could_ also ignore what external diff driver\nproduces in this case (as we know it won't be producing an\nappropriate input to the \"--cc\" post-processing), and pretend\nas if comparing an old version of foo.sln with a new version of\nfoo.sln produced a diff like this:\n\n diff --git a/foo.sln b/foo.sln\n index d7ff46e,b829410\n --- a/foo.sln\n +++ b/foo.sln\n @@ 1,1 @@\n -d7ff46ec4a016c6ab7d233b9d4a196ecde623528 - generated file\n +b829410f6da0afc14353b4621d2fdf874181a9f7 - generated file\n\nthen you might see in a merge that merges two versions of foo.sln\nand result in another version of foo.sln in your \"--cc\" output a\nhunk that is like this:\n\n diff --cc foo.sln\n index d7ff46e,6c9aaa1..b829410\n --- a/foo.sln\n +++ b/foo.sln\n @@@ 1,1 @@@\n - d7ff46ec4a016c6ab7d233b9d4a196ecde623528 - generated file\n -6c9aaa1ae63a2255a215c1287e38e75fcc5fc5d3 - generated file\n ++b829410f6da0afc14353b4621d2fdf874181a9f7 - generated file\n\nwhich would at least tell you that there was a merge, and if the\nmerge took the full contents of the file from one of the commits and\nrecorded as the result of the merge, then you wouldn't see them in\nthe \"--cc\" output.\n\nIt happens that the above is fairly easily doable with today's Git\nwithout any modification. Here is how.\n\n(1) Have this in your .git/config\n\n [diff \"uninteresting\"]\n \ttextconv = /path/to/uninteresting-textconv-script\n\n(2) Mark your .sln paths as uninteresting in your .gitattributes\n\n *.sln\tdiff=uninteresting\n\n(3) Have this textconv filter in /path/to/uninteresting-textconv-script\n\n #!/bin/sh\n printf \"%s generated file\\n\" \"$(sha1sum <\"$1\")\"\n"},{"id":"280632","messageId":"20160311185828.GA31750@sigill.intra.peff.net","threadId":"41650","inReplyTo":"xmqqziu4anb9.fsf@gitster.mtv.corp.google.com","subject":"Re: Possible bug: --ext-diff ignored with --cc in git log","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-03-11T18:58:28Z","receivedAt":"2016-03-11T18:58:28Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 11, 2016 at 10:20:42AM -0800, Junio C Hamano wrote:\n\n> diff --cc foo.sln\n> index d7ff46e,6c9aaa1..b829410\n> --- a/foo.sln\n> +++ b/foo.sln\n> @@@ 1,1 @@@\n> - d7ff46ec4a016c6ab7d233b9d4a196ecde623528 - generated file\n> -6c9aaa1ae63a2255a215c1287e38e75fcc5fc5d3 - generated file\n> ++b829410f6da0afc14353b4621d2fdf874181a9f7 - generated file\n> \n> which would at least tell you that there was a merge, and if the\n> merge took the full contents of the file from one of the commits and\n> recorded as the result of the merge, then you wouldn't see them in\n> the \"--cc\" output.\n> \n> It happens that the above is fairly easily doable with today's Git\n> without any modification. Here is how.\n> [...]\n\nI think an even easier way is:\n\n git log --cc --raw\n\nI know that is somewhat beside the point you are making, which is how we\nshould handle \"--cc\" with ext-diff. But I would much rather have us\nshow nothing for that case, and let the user turn on \"--raw\", than to\ninvent a diff-looking format that does not actually represent the file\ncontents.\n\n-Peff\n"},{"id":"280633","messageId":"xmqqr3fgal3j.fsf@gitster.mtv.corp.google.com","threadId":"41650","inReplyTo":"20160311185828.GA31750@sigill.intra.peff.net","subject":"Re: Possible bug: --ext-diff ignored with --cc in git log","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-11T19:08:32Z","receivedAt":"2016-03-11T19:08:32Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> It happens that the above is fairly easily doable with today's Git\n>> without any modification. Here is how.\n>> [...]\n>\n> I think an even easier way is:\n>\n> git log --cc --raw\n>\n> I know that is somewhat beside the point you are making, which is how we\n> should handle \"--cc\" with ext-diff. But I would much rather have us\n> show nothing for that case, and let the user turn on \"--raw\", than to\n> invent a diff-looking format that does not actually represent the file\n> contents.\n\nSorry, but I am not sure where you are trying to go with this.\n\nI understand that the original issue was that Vadim wants to\nsuppress reams of differences for _some_ paths but still wants to\nbenefit from the textual summarized diff for all the other paths.\nGiving \"--raw\" would be global, and would affect other paths, no?\n"},{"id":"280634","messageId":"20160311194518.GA1269@sigill.intra.peff.net","threadId":"41650","inReplyTo":"xmqqr3fgal3j.fsf@gitster.mtv.corp.google.com","subject":"Re: Possible bug: --ext-diff ignored with --cc in git log","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-03-11T19:45:18Z","receivedAt":"2016-03-11T19:45:18Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 11, 2016 at 11:08:32AM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> >> It happens that the above is fairly easily doable with today's Git\n> >> without any modification. Here is how.\n> >> [...]\n> >\n> > I think an even easier way is:\n> >\n> > git log --cc --raw\n> >\n> > I know that is somewhat beside the point you are making, which is how we\n> > should handle \"--cc\" with ext-diff. But I would much rather have us\n> > show nothing for that case, and let the user turn on \"--raw\", than to\n> > invent a diff-looking format that does not actually represent the file\n> > contents.\n> \n> Sorry, but I am not sure where you are trying to go with this.\n> \n> I understand that the original issue was that Vadim wants to\n> suppress reams of differences for _some_ paths but still wants to\n> benefit from the textual summarized diff for all the other paths.\n> Giving \"--raw\" would be global, and would affect other paths, no?\n\nAh, sorry, I thought the problem was the opposite: that there was no\noutput for ext-diff paths, and we needed to add something back in. Doing\n\"--raw\" is a much easier way than textconv of the \"add back in\" part,\nbut it does not suppress the ordinary combined diff.\n\n-Peff\n"},{"id":"280654","messageId":"E1aeY2d-0006K7-6e@smtp.tt-solutions.com","threadId":"41650","inReplyTo":"xmqqziu4anb9.fsf@gitster.mtv.corp.google.com","subject":"Re[2]: Possible bug: --ext-diff ignored with --cc in git log","fromName":"Vadim Zeitlin","fromEmail":"vz-git@zeitlins.org","sentAt":"2016-03-12T01:08:12Z","receivedAt":"2016-03-12T01:08:12Z","isPatch":false,"sender":{"key":"vz-git@zeitlins.org","avatar":null},"body":"On Fri, 11 Mar 2016 10:20:42 -0800 Junio C Hamano wrote:\n\nJCH> Vadim Zeitlin writes:\nJCH> \nJCH> > Thank you for your reply, Junio, I hadn't realized that --cc was dependent\nJCH> > on textual diff output format before, but now I understand why it can't\nJCH> > respect --ext-diff.\nJCH> \nJCH> Having established that, I should also add that \"--cc fundamentally\nJCH> is incompatible with --ext-diff\" does not justify that\nJCH> \"--cc when given with --ext-diff just ignores and uses the usual\nJCH> diff\".\nJCH> \nJCH> An equally (or even more) valid consequence could have been to\nJCH> disable \"--cc\" processing for paths that would trigger an external\nJCH> diff driver.\n\n FWIW I agree that this would make more sense than the current behaviour.\nBut it still wouldn't be ideal if disabling \"--cc\" meant not showing any\noutput for these files at all, we still want to know that the file has been\nmodified as part of the commit, even if we don't care about its contents.\n\nJCH> After all, the user told us that the contents would not compare well\nJCH> with the usual \"diff\"; we know that \"--cc\" output that summarizes the\nJCH> usual diff output is useless.\n\n This is so logical that it made me check how did \"--cc\" behave with the\nbinary files because this argument seems to apply perfectly well to them\ntoo. And (unsurprisingly?) it already works just fine with them:\n\n\t# I have alias g=git and I also suppress all successful output\n\t$ g init\n\t$ echo 'Binary\\0file' > binary\n\t$ g add binary\n\t$ g commit -m 'Added'\n\t$ echo '2nd line' >> binary\n\t$ g commit -a -m 'Added 2nd line'\n\t$ g checkout -b another HEAD~\n\t$ echo 'another line' >> binary\n\t$ g commit -a -m 'Added another line'\n\t$ g checkout master\n\t$ g merge\n\twarning: Cannot merge binary files: binary (HEAD vs. another)\n\tAuto-merging binary\n\tCONFLICT (content): Merge conflict in binary\n\tAutomatic merge failed; fix conflicts and then commit the result.\n\t$ vi binary # combine both versions\n\t$ g commit\n\t$ g show # finally I can show what all this is about\n\tcommit d30ae002cb52974228d50723fc8c9d7077e760da\n\tMerge: ae542d2 3204f35\n\tAuthor: Vadim Zeitlin \n\tDate: Sat Mar 12 01:57:55 2016 +0100\n\n\t Merge branch 'another'\n\n\tdiff --cc binary\n\tindex 31499e2,1730dfd..1eda50a\n\tBinary files differ\n\nSo it looks like it shouldn't be too difficult to make it also output\n\"Files using custom diff viewer differ\", what do you think?\n\nJCH> For example, we could also ignore what external diff driver\nJCH> produces in this case (as we know it won't be producing an\nJCH> appropriate input to the \"--cc\" post-processing), and pretend\nJCH> as if comparing an old version of foo.sln with a new version of\nJCH> foo.sln produced a diff like this:\nJCH> \nJCH> diff --git a/foo.sln b/foo.sln\nJCH> index d7ff46e,b829410\nJCH> --- a/foo.sln\nJCH> +++ b/foo.sln\nJCH> @@ 1,1 @@\nJCH> -d7ff46ec4a016c6ab7d233b9d4a196ecde623528 - generated file\nJCH> +b829410f6da0afc14353b4621d2fdf874181a9f7 - generated file\nJCH> \nJCH> then you might see in a merge that merges two versions of foo.sln\nJCH> and result in another version of foo.sln in your \"--cc\" output a\nJCH> hunk that is like this:\nJCH> \nJCH> diff --cc foo.sln\nJCH> index d7ff46e,6c9aaa1..b829410\nJCH> --- a/foo.sln\nJCH> +++ b/foo.sln\nJCH> @@@ 1,1 @@@\nJCH> - d7ff46ec4a016c6ab7d233b9d4a196ecde623528 - generated file\nJCH> -6c9aaa1ae63a2255a215c1287e38e75fcc5fc5d3 - generated file\nJCH> ++b829410f6da0afc14353b4621d2fdf874181a9f7 - generated file\nJCH> \nJCH> which would at least tell you that there was a merge, and if the\nJCH> merge took the full contents of the file from one of the commits and\nJCH> recorded as the result of the merge, then you wouldn't see them in\nJCH> the \"--cc\" output.\n\n Interesting, but I admit I don't really see any advantage of showing the\nSHA-1s here compared to what already happens with the binary files. Is\nthere anything I'm missing?\n\nJCH> It happens that the above is fairly easily doable with today's Git\nJCH> without any modification. Here is how.\nJCH> \nJCH> (1) Have this in your .git/config\nJCH> \nJCH> [diff \"uninteresting\"]\nJCH> \ttextconv = /path/to/uninteresting-textconv-script\nJCH> \nJCH> (2) Mark your .sln paths as uninteresting in your .gitattributes\nJCH> \nJCH> *.sln\tdiff=uninteresting\nJCH> \nJCH> (3) Have this textconv filter in /path/to/uninteresting-textconv-script\nJCH> \nJCH> #!/bin/sh\nJCH> printf \"%s generated file\\n\" \"$(sha1sum <\"$1\")\"\n\n This is really ingenious, thanks! I'm probably indeed going to put this in\nplace at least for now for our mail notification script because it's just\ntoo annoying to receive emails with thousands of lines of diffs to the\ngenerated files.\n\n But I still think that it would make sense for \"--cc\" to behave as it does\nfor the binary files for the ext-diffable ones too. I've never touched git\ncode before but if you think it's a good idea and if you don't see any\ninsurmountable difficulties in implementing this, I could try to make a\npatch doing it, please let me know if you think it could be useful.\n\n And thanks again for your textconv hint!\nVZ\n"}]}