{"thread":{"id":"51761","subject":"git-diff passes <rev>:<path> args to GIT_EXTERNAL_DIFF incorrectly?","startedAt":"2019-08-27T18:24:15Z","lastAt":"2019-08-30T16:27:05Z","messageCount":12,"participants":["Dmitry Nikulin","Junio C Hamano","Phillip Wood","Jeff King","SZEDER Gábor"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"381377","messageId":"CAH53SykX12SN83=gey8KS_x3cGkXH758sfEieskXnnvos8DMcA@mail.gmail.com","threadId":"51761","inReplyTo":null,"subject":"git-diff passes <rev>:<path> args to GIT_EXTERNAL_DIFF incorrectly?","fromName":"Dmitry Nikulin","fromEmail":"pastafariant@gmail.com","sentAt":"2019-08-27T18:24:02Z","receivedAt":"2019-08-27T18:24:15Z","isPatch":false,"sender":{"key":"pastafariant@gmail.com","avatar":null},"body":"I wrote a very simple Python script to see which arguments git-diff\npasses to the external diff program when comparing files across\nbranches:\n\n$ env GIT_EXTERNAL_DIFF=./print_argv.py git diff\norigin/branch1:file1.txt origin/branch2:file2.txt\n['./print_argv.py',\n 'file1.txt',\n '/tmp/QRaIJ1_file1.txt',\n '802b1c4ed7b06162b2ce09b7db72a576695b96e5',\n '100644',\n '/tmp/AZuOJ1_file2.txt',\n '076e8e37a712d8a66c0c3d1a103050dc509ca6ff',\n '100644',\n 'file2.txt',\n 'index 802b1c4..076e8e3 100644\\n']\n\nAccording to the docs\n(https://www.git-scm.com/docs/git/2.22.0#Documentation/git.txt-codeGITEXTERNALDIFFcode),\ngit-diff is supposed to pass 7 parameters:\n\npath old-file old-hex old-mode new-file new-hex new-mode\n\nThis is not what I am seeing here. Is this a bug or\nincorrect/incomplete documentation?\n\nTested with git 2.22.0 and 2.17.1.\n"},{"id":"381403","messageId":"CAH53Sy=zcP=DRg5WQFkaBp9CoHP+phCA_NasM5OqOdfwQGkQyQ@mail.gmail.com","threadId":"51761","inReplyTo":"CAH53SykX12SN83=gey8KS_x3cGkXH758sfEieskXnnvos8DMcA@mail.gmail.com","subject":"Re: git-diff passes <rev>:<path> args to GIT_EXTERNAL_DIFF incorrectly?","fromName":"Dmitry Nikulin","fromEmail":"pastafariant@gmail.com","sentAt":"2019-08-27T22:25:57Z","receivedAt":"2019-08-27T22:26:10Z","isPatch":false,"sender":{"key":"pastafariant@gmail.com","avatar":null},"body":"I have put up a demo repo here: https://github.com/dniku/git-external-diff-argv\n\nOn Tue, 27 Aug 2019 at 21:24, Dmitry Nikulin <pastafariant@gmail.com> wrote:\n>\n> I wrote a very simple Python script to see which arguments git-diff\n> passes to the external diff program when comparing files across\n> branches:\n>\n> $ env GIT_EXTERNAL_DIFF=./print_argv.py git diff\n> origin/branch1:file1.txt origin/branch2:file2.txt\n> ['./print_argv.py',\n>  'file1.txt',\n>  '/tmp/QRaIJ1_file1.txt',\n>  '802b1c4ed7b06162b2ce09b7db72a576695b96e5',\n>  '100644',\n>  '/tmp/AZuOJ1_file2.txt',\n>  '076e8e37a712d8a66c0c3d1a103050dc509ca6ff',\n>  '100644',\n>  'file2.txt',\n>  'index 802b1c4..076e8e3 100644\\n']\n>\n> According to the docs\n> (https://www.git-scm.com/docs/git/2.22.0#Documentation/git.txt-codeGITEXTERNALDIFFcode),\n> git-diff is supposed to pass 7 parameters:\n>\n> path old-file old-hex old-mode new-file new-hex new-mode\n>\n> This is not what I am seeing here. Is this a bug or\n> incorrect/incomplete documentation?\n>\n> Tested with git 2.22.0 and 2.17.1.\n"},{"id":"381493","messageId":"xmqqtva0zkvu.fsf@gitster-ct.c.googlers.com","threadId":"51761","inReplyTo":"CAH53SykX12SN83=gey8KS_x3cGkXH758sfEieskXnnvos8DMcA@mail.gmail.com","subject":"Re: git-diff passes <rev>:<path> args to GIT_EXTERNAL_DIFF incorrectly?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-08-29T03:54:45Z","receivedAt":"2019-08-29T03:54:50Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dmitry Nikulin <pastafariant@gmail.com> writes:\n\n> $ env GIT_EXTERNAL_DIFF=./print_argv.py git diff\n> origin/branch1:file1.txt origin/branch2:file2.txt\n\nI didn't even know external-diff driver is called (and does not even segfaut)\nin the \"compare two blobs\" hack codepath.\n\nThe syntax <tree-ish>:<path-in-the-tree> you have on the command\nline resolves to a blob object name.  There is no leading directory\nname, there is no permission bits or executable bit, there is no\nfilename, when \"diff\" is told to compare two blob objects this way.\nTHe \"diff\" machinery that drives the external (or internal for that\nmatter) diff will only get two 40-hex blob object names and nothing\nelse.  The only pieces of information you can trust among those the\nexternal program may receive are the blob object name(s) and the\ncontents stored in the temporary files given to it.  The location of\nthese temporary files or their mode bits have no relation to the\n\"files\" in some tree in the original repository, as that information\nis long lost when you write <tree-ish>:<path-in-the-tree> to tell\nGit to use that as a blob object name.\n\n    $ git diff -M branch1 branch2 -- file1 file2\n\nif file1 and file2 have similar-enough contents, may have a better\nchance of what you wanted to ask Git (if I am guessing what it is,\nthat is).\n\n\n\n\n\n\n"},{"id":"381514","messageId":"CAH53SymNwjrh_CzXVVtU5xABuGQWMsXhRDYyRzyHEwuxLWA2NQ@mail.gmail.com","threadId":"51761","inReplyTo":"xmqqtva0zkvu.fsf@gitster-ct.c.googlers.com","subject":"Re: git-diff passes <rev>:<path> args to GIT_EXTERNAL_DIFF incorrectly?","fromName":"Dmitry Nikulin","fromEmail":"pastafariant@gmail.com","sentAt":"2019-08-29T14:36:00Z","receivedAt":"2019-08-29T14:36:14Z","isPatch":false,"sender":{"key":"pastafariant@gmail.com","avatar":null},"body":"Thank you for the reply.\n\nOn Thu, 29 Aug 2019 at 06:54, Junio C Hamano <gitster@pobox.com> wrote:\n>     $ git diff -M branch1 branch2 -- file1 file2\n>\n> if file1 and file2 have similar-enough contents, may have a better\n> chance of what you wanted to ask Git (if I am guessing what it is,\n> that is).\n\nThe context here is that I am trying to diff two Jupyter notebooks\nusing an external tool (git-nbdiffdriver in my case). Therefore, for\nme it is crucial to use the external tool, and not Git's internal\nmachinery.\n\nFor the particular command that you suggested as the replacement, on\nmy demo repository it does not produce anything interesting, as it\ndoes not detect renames and calls my honeypot twice:\n\n$ env GIT_EXTERNAL_DIFF=./print_argv.py git diff -M origin/branch1\norigin/branch2 -- file1.txt file2.txt\n['./print_argv.py',\n 'file1.txt',\n '/tmp/2IEKCw_file1.txt',\n '802b1c4ed7b06162b2ce09b7db72a576695b96e5',\n '100644',\n '/dev/null',\n '.',\n '.']\n['./print_argv.py',\n 'file2.txt',\n '/dev/null',\n '.',\n '.',\n '/tmp/oAMdDx_file2.txt',\n '076e8e37a712d8a66c0c3d1a103050dc509ca6ff',\n '100644']\n\nHowever, for the original repository where I first faced this problem\n(https://github.com/yandexdataschool/Practical_RL), Git passes a very\nweird set of args to the external diff:\n\n$ env GIT_EXTERNAL_DIFF=./print_argv.py git diff -M master coursera --\nweek02_value_based/seminar_vi.ipynb\nweek2_model_based/practice_vi.ipynb\n['./print_argv.py',\n 'week02_value_based/seminar_vi.ipynb',\n '/tmp/amudWz_seminar_vi.ipynb',\n '8f8016963c888b7dd8dd20f60b7d6fdb41b26c1d',\n '100644',\n '/tmp/Ub7zPz_practice_vi.ipynb',\n '21db80f53b632d975a9af0acbaf397eb717cde2c',\n '100644',\n 'week2_model_based/practice_vi.ipynb',\n 'similarity index 82%\\n'\n 'rename from week02_value_based/seminar_vi.ipynb\\n'\n 'rename to week2_model_based/practice_vi.ipynb\\n'\n 'index 8f80169..21db80f 100644\\n']\n\nI would guess that this is a bug. There can clearly be a hotfix (after\nall, Git passes all of the information to the external that it should\nper the spec, that is, <old|new>-path, <old|new>-hex, <old|new>-mode;\nadding, however, some garbage). I do not know though to what extent\nthis information is correct. You say that this information is lost\nwhen I use the <tree-ish>:<path> notation; however, Git seems to pass\npaths and hexes correctly. This only leaves open the question of file\nmode. Perhaps it could be preserved at least for some cases, such as\nwhen the blob is retrieved from a path in a tree?\n"},{"id":"381570","messageId":"9c280a3f-6a1a-61d1-a255-1dcf0671d39c@gmail.com","threadId":"51761","inReplyTo":"CAH53SymNwjrh_CzXVVtU5xABuGQWMsXhRDYyRzyHEwuxLWA2NQ@mail.gmail.com","subject":"Re: git-diff passes <rev>:<path> args to GIT_EXTERNAL_DIFF incorrectly?","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2019-08-30T10:16:54Z","receivedAt":"2019-08-30T10:17:15Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Dmitry\n\nOn 29/08/2019 15:36, Dmitry Nikulin wrote:\n> Thank you for the reply.\n> [...]\n> However, for the original repository where I first faced this problem\n> (https://github.com/yandexdataschool/Practical_RL), Git passes a very\n> weird set of args to the external diff:\n> \n> $ env GIT_EXTERNAL_DIFF=./print_argv.py git diff -M master coursera --\n> week02_value_based/seminar_vi.ipynb\n> week2_model_based/practice_vi.ipynb\n> ['./print_argv.py',\n>   'week02_value_based/seminar_vi.ipynb',\n>   '/tmp/amudWz_seminar_vi.ipynb',\n>   '8f8016963c888b7dd8dd20f60b7d6fdb41b26c1d',\n>   '100644',\n>   '/tmp/Ub7zPz_practice_vi.ipynb',\n>   '21db80f53b632d975a9af0acbaf397eb717cde2c',\n>   '100644',\n>   'week2_model_based/practice_vi.ipynb',\n>   'similarity index 82%\\n'\n>   'rename from week02_value_based/seminar_vi.ipynb\\n'\n>   'rename to week2_model_based/practice_vi.ipynb\\n'\n>   'index 8f80169..21db80f 100644\\n']\n> \n> I would guess that this is a bug. There can clearly be a hotfix (after\n> all, Git passes all of the information to the external that it should\n> per the spec, that is, <old|new>-path, <old|new>-hex, <old|new>-mode;\n> adding, however, some garbage). \n\nWhen git detects a rename it adds two parameters to the end of the \nargument list for the external diff program. The first extra argument is \nthe new name and the second is the header with the similarity \ninformation[1]. I'm not sure why the last argument is being split in \nyour example. It is not split in the example below\n\n$git mv git.c renamed.c\n$env GIT_EXTERNAL_DIFF='printf \"|%s|\\\\n\"' git diff HEAD\n|git.c|\n|/tmp/lMQpP8_git.c|\n|c1ee7124edcfb0417539134d50212e997dc71c1f|\n|100644|\n|renamed.c|\n|c1ee7124edcfb0417539134d50212e997dc71c1f|\n|100644|\n|renamed.c|\n|similarity index 100%\nrename from git.c\nrename to renamed.c\n|\n\nBest Wishes\n\nPhillip\n\n[1] https://github.com/git/git/blob/master/diff.c#L4204\n\n>I do not know though to what extent\n> this information is correct. You say that this information is lost\n> when I use the <tree-ish>:<path> notation; however, Git seems to pass\n> paths and hexes correctly. This only leaves open the question of file\n> mode. Perhaps it could be preserved at least for some cases, such as\n> when the blob is retrieved from a path in a tree?\n> \n"},{"id":"381573","messageId":"CAH53SykQWLtjt0gWVrz5KyH-9WyqaQ0GtkhmyLt09QEqcAS_dw@mail.gmail.com","threadId":"51761","inReplyTo":"9c280a3f-6a1a-61d1-a255-1dcf0671d39c@gmail.com","subject":"Re: git-diff passes <rev>:<path> args to GIT_EXTERNAL_DIFF incorrectly?","fromName":"Dmitry Nikulin","fromEmail":"pastafariant@gmail.com","sentAt":"2019-08-30T13:23:13Z","receivedAt":"2019-08-30T13:23:26Z","isPatch":false,"sender":{"key":"pastafariant@gmail.com","avatar":null},"body":"On Fri, 30 Aug 2019 at 13:16, Phillip Wood <phillip.wood123@gmail.com> wrote:\n> I'm not sure why the last argument is being split in\n> your example. It is not split in the example below\n\nI have replicated the splitting issue on my small demo repo [1]:\n\n$ env GIT_EXTERNAL_DIFF=./print_argv.py git diff -M origin/branch1\norigin/branch1-mv -- file1.txt file1-mv.txt\n['./print_argv.py',\n 'file1.txt',\n '/tmp/EWaCSc_file1.txt',\n '2bef330804cb3f6962e45a72a12a3071ee9b5888',\n '100644',\n '/tmp/mtEiSc_file1-mv.txt',\n 'f8fd6737fbe5a45c97ba9c9de495dc46ff11eccd',\n '100644',\n 'file1-mv.txt',\n 'similarity index 90%\\n'\n 'rename from file1.txt\\n'\n 'rename to file1-mv.txt\\n'\n 'index 2bef330..f8fd673 100644\\n']\n\nThis is, however, tangential to the original problem: documenting the\nexternal diff CLI interface for diffing two blobs. Here is what I am\nseeing:\n\n$ env GIT_EXTERNAL_DIFF=./print_argv.py git diff\norigin/branch1:file1.txt origin/branch1-mv:file1-mv.txt\n['./print_argv.py',\n 'file1.txt',\n '/tmp/n9USvy_file1.txt',\n '2bef330804cb3f6962e45a72a12a3071ee9b5888',\n '100644',\n '/tmp/Zst0uy_file1-mv.txt',\n 'f8fd6737fbe5a45c97ba9c9de495dc46ff11eccd',\n '100644',\n 'file1-mv.txt',\n 'index 2bef330..f8fd673 100644\\n']\n\nThe meaning and origin of the last arg remains mysterious, and the\nother args do not conform to the published documentation[2], which\nstates that the args should be:\n\n    path old-file old-hex old-mode new-file new-hex new-mode\n\nInstead the args that are passed are:\n\n    path old-filename old-file old-hex old-mode new-file new-hex\nnew-mode new-filename something\n\n[1]: https://github.com/dniku/git-external-diff-argv\n[2]: https://www.git-scm.com/docs/git#Documentation/git.txt-codeGITEXTERNALDIFFcode\n"},{"id":"381577","messageId":"968253c6-d517-3391-4790-9a07a03fb980@gmail.com","threadId":"51761","inReplyTo":"CAH53SykQWLtjt0gWVrz5KyH-9WyqaQ0GtkhmyLt09QEqcAS_dw@mail.gmail.com","subject":"Re: git-diff passes <rev>:<path> args to GIT_EXTERNAL_DIFF incorrectly?","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2019-08-30T14:17:13Z","receivedAt":"2019-08-30T14:17:19Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 30/08/2019 14:23, Dmitry Nikulin wrote:\n> On Fri, 30 Aug 2019 at 13:16, Phillip Wood <phillip.wood123@gmail.com> wrote:\n>> I'm not sure why the last argument is being split in\n>> your example. It is not split in the example below\n> \n> I have replicated the splitting issue on my small demo repo [1]:\n> \n> $ env GIT_EXTERNAL_DIFF=./print_argv.py git diff -M origin/branch1\n> origin/branch1-mv -- file1.txt file1-mv.txt\n> ['./print_argv.py',\n>   'file1.txt',\n>   '/tmp/EWaCSc_file1.txt',\n>   '2bef330804cb3f6962e45a72a12a3071ee9b5888',\n>   '100644',\n>   '/tmp/mtEiSc_file1-mv.txt',\n>   'f8fd6737fbe5a45c97ba9c9de495dc46ff11eccd',\n>   '100644',\n>   'file1-mv.txt',\n>   'similarity index 90%\\n'\n>   'rename from file1.txt\\n'\n>   'rename to file1-mv.txt\\n'\n>   'index 2bef330..f8fd673 100644\\n']\n\nThat's strange - What OS are you using? Does python do any \npre-processing of arguments with newlines in them?\n\n> This is, however, tangential to the original problem: documenting the\n> external diff CLI interface for diffing two blobs. Here is what I am\n> seeing:\n> \n> $ env GIT_EXTERNAL_DIFF=./print_argv.py git diff\n> origin/branch1:file1.txt origin/branch1-mv:file1-mv.txt\n> ['./print_argv.py',\n>   'file1.txt',\n>   '/tmp/n9USvy_file1.txt',\n>   '2bef330804cb3f6962e45a72a12a3071ee9b5888',\n>   '100644',\n>   '/tmp/Zst0uy_file1-mv.txt',\n>   'f8fd6737fbe5a45c97ba9c9de495dc46ff11eccd',\n>   '100644',\n>   'file1-mv.txt',\n>   'index 2bef330..f8fd673 100644\\n']\n> \n> The meaning and origin of the last arg remains mysterious, and the\n> other args do not conform to the published documentation[2], which\n> states that the args should be:\n\nThe documentation is incomplete it should document the extra fields \npassed when it detects renames.\n\n>      path old-file old-hex old-mode new-file new-hex new-mode\n> \n> Instead the args that are passed are:\n> \n>      path old-filename old-file old-hex old-mode new-file new-hex\n> new-mode new-filename something\n\nI think what is happening is that because you're passing different \nfilenames in to get the blobs the 'pass rename information' code is \nbeing triggered so it shows the new filename as the name used to get the \n  second blob and then passes the header which only has an index line as \nthere isn't any real rename information.\n\nBest Wishes\n\nPhillip\n\n> [1]: https://github.com/dniku/git-external-diff-argv\n> [2]: https://www.git-scm.com/docs/git#Documentation/git.txt-codeGITEXTERNALDIFFcode\n> \n"},{"id":"381578","messageId":"20190830142730.GB16327@sigill.intra.peff.net","threadId":"51761","inReplyTo":"CAH53SykQWLtjt0gWVrz5KyH-9WyqaQ0GtkhmyLt09QEqcAS_dw@mail.gmail.com","subject":"Re: git-diff passes <rev>:<path> args to GIT_EXTERNAL_DIFF incorrectly?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-08-30T14:27:30Z","receivedAt":"2019-08-30T14:27:33Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Aug 30, 2019 at 04:23:13PM +0300, Dmitry Nikulin wrote:\n\n> On Fri, 30 Aug 2019 at 13:16, Phillip Wood <phillip.wood123@gmail.com> wrote:\n> > I'm not sure why the last argument is being split in\n> > your example. It is not split in the example below\n> \n> I have replicated the splitting issue on my small demo repo [1]:\n> \n> $ env GIT_EXTERNAL_DIFF=./print_argv.py git diff -M origin/branch1\n> origin/branch1-mv -- file1.txt file1-mv.txt\n> ['./print_argv.py',\n>  'file1.txt',\n>  '/tmp/EWaCSc_file1.txt',\n>  '2bef330804cb3f6962e45a72a12a3071ee9b5888',\n>  '100644',\n>  '/tmp/mtEiSc_file1-mv.txt',\n>  'f8fd6737fbe5a45c97ba9c9de495dc46ff11eccd',\n>  '100644',\n>  'file1-mv.txt',\n>  'similarity index 90%\\n'\n>  'rename from file1.txt\\n'\n>  'rename to file1-mv.txt\\n'\n>  'index 2bef330..f8fd673 100644\\n']\n\nInteresting. I _don't_ see that splitting when I run the same command in\nyour demo repo (nor, looking at Git's code, do I see how it could\nhappen; we always add the metainfo as a single argument).\n\n> This is, however, tangential to the original problem: documenting the\n> external diff CLI interface for diffing two blobs. Here is what I am\n> seeing:\n> \n> $ env GIT_EXTERNAL_DIFF=./print_argv.py git diff\n> origin/branch1:file1.txt origin/branch1-mv:file1-mv.txt\n> ['./print_argv.py',\n>  'file1.txt',\n>  '/tmp/n9USvy_file1.txt',\n>  '2bef330804cb3f6962e45a72a12a3071ee9b5888',\n>  '100644',\n>  '/tmp/Zst0uy_file1-mv.txt',\n>  'f8fd6737fbe5a45c97ba9c9de495dc46ff11eccd',\n>  '100644',\n>  'file1-mv.txt',\n>  'index 2bef330..f8fd673 100644\\n']\n> \n> The meaning and origin of the last arg remains mysterious, and the\n> other args do not conform to the published documentation[2], which\n> states that the args should be:\n> \n>     path old-file old-hex old-mode new-file new-hex new-mode\n> \n> Instead the args that are passed are:\n> \n>     path old-filename old-file old-hex old-mode new-file new-hex\n> new-mode new-filename something\n\nI think you have an extra \"old-filename\" in the second list. Without\nthat, the first 7 arguments are as documented.\n\nThe last two are:\n\n  - when the destination path differs from the source path, we append\n    it. This is generally a sign of a rename/copy, though it triggers\n    in the blob case because Git has been manually given a pair of files\n    with non-matching names.\n\n  - the final one is metainfo that Git typically prints between the\n    \"diff\" header and the diff itself. This is added only when we add\n    the filename, and would generally carry information about the rename\n    (but of course since there isn't one, it has only the index line).\n\nThese were both added by 427dcb4bca ([PATCH] Diff overhaul, adding half\nof copy detection., 2005-05-21). I think the intent was that existing\ndiff commands would/could ignore the extra arguments.\n\nCertainly the world would be a better place if those were described in\nthe external-diff documentation (specifically in relation to renames,\nwhich is their intended use).\n\nAs for the behavior in the blob-diff case, I think it's _pretty_\nreasonable. It's certainly useful to give the new-filename. The metainfo\nis mostly useless in this case, and potentially could be suppressed if\nthe pair is not an actual rename/copy. But I also think it's not hurting\nmuch (and a script can tell that's what's going on by the lack of rename\nlines in the metainfo).\n\n-Peff\n"},{"id":"381579","messageId":"20190830142932.GJ8571@szeder.dev","threadId":"51761","inReplyTo":"CAH53SykQWLtjt0gWVrz5KyH-9WyqaQ0GtkhmyLt09QEqcAS_dw@mail.gmail.com","subject":"Re: git-diff passes <rev>:<path> args to GIT_EXTERNAL_DIFF incorrectly?","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-08-30T14:29:32Z","receivedAt":"2019-08-30T14:29:38Z","isPatch":false,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Fri, Aug 30, 2019 at 04:23:13PM +0300, Dmitry Nikulin wrote:\n> On Fri, 30 Aug 2019 at 13:16, Phillip Wood <phillip.wood123@gmail.com> wrote:\n> > I'm not sure why the last argument is being split in\n> > your example. It is not split in the example below\n> \n> I have replicated the splitting issue on my small demo repo [1]:\n> \n> $ env GIT_EXTERNAL_DIFF=./print_argv.py git diff -M origin/branch1\n> origin/branch1-mv -- file1.txt file1-mv.txt\n> ['./print_argv.py',\n>  'file1.txt',\n>  '/tmp/EWaCSc_file1.txt',\n>  '2bef330804cb3f6962e45a72a12a3071ee9b5888',\n>  '100644',\n>  '/tmp/mtEiSc_file1-mv.txt',\n>  'f8fd6737fbe5a45c97ba9c9de495dc46ff11eccd',\n>  '100644',\n>  'file1-mv.txt',\n>  'similarity index 90%\\n'\n>  'rename from file1.txt\\n'\n>  'rename to file1-mv.txt\\n'\n>  'index 2bef330..f8fd673 100644\\n']\n\nAnother Python 2 vs 3 issue, perhaps?\n\n  # Python2, good:\n  $ GIT_EXTERNAL_DIFF='python2 ./print_argv.py' git --no-pager diff -M origin/branch1 origin/branch1-mv  -- file1.txt file1-mv.txt\n  ['./print_argv.py',\n   'file1.txt',\n   '/tmp/ViB58B_file1.txt',\n   '2bef330804cb3f6962e45a72a12a3071ee9b5888',\n   '100644',\n   '/tmp/p6OH9B_file1-mv.txt',\n   'f8fd6737fbe5a45c97ba9c9de495dc46ff11eccd',\n   '100644',\n   'file1-mv.txt',\n   'similarity index 90%\\nrename from file1.txt\\nrename to\n  file1-mv.txt\\nindex 2bef330..f8fd673 100644\\n']\n  # Python3, bad:\n  $ GIT_EXTERNAL_DIFF='python3 ./print_argv.py' git --no-pager diff -M origin/branch1 origin/branch1-mv  -- file1.txt file1-mv.txt\n  ['./print_argv.py',\n   'file1.txt',\n   '/tmp/5xD2DS_file1.txt',\n   '2bef330804cb3f6962e45a72a12a3071ee9b5888',\n   '100644',\n   '/tmp/vvHQGS_file1-mv.txt',\n   'f8fd6737fbe5a45c97ba9c9de495dc46ff11eccd',\n   '100644',\n   'file1-mv.txt',\n   'similarity index 90%\\n'\n   'rename from file1.txt\\n'\n   'rename to file1-mv.txt\\n'\n   'index 2bef330..f8fd673 100644\\n']\n\nLet's just stick to plain old printf for now, as suggested by Phillip\nearlier, to reduce unnecessary confusion.\n\n"},{"id":"381587","messageId":"CAH53SyniovbvYoU1nhMobRq7nZREENiagJqN9LAYmZsGMpc+ww@mail.gmail.com","threadId":"51761","inReplyTo":"20190830142730.GB16327@sigill.intra.peff.net","subject":"Re: git-diff passes <rev>:<path> args to GIT_EXTERNAL_DIFF incorrectly?","fromName":"Dmitry Nikulin","fromEmail":"pastafariant@gmail.com","sentAt":"2019-08-30T15:28:05Z","receivedAt":"2019-08-30T15:28:19Z","isPatch":false,"sender":{"key":"pastafariant@gmail.com","avatar":null},"body":"On Fri, 30 Aug 2019 at 17:27, Jeff King <peff@peff.net> wrote:\n\n> I think you have an extra \"old-filename\" in the second list.\nAgreed. My misunderstanding was that I had thought that when\ndocumentation said \"path\", it meant \"argv[0], the path to the diff\nexecutable\".\n\n> Interesting. I _don't_ see that splitting when I run the same command in\n> your demo repo (nor, looking at Git's code, do I see how it could\n> happen; we always add the metainfo as a single argument).\nSzeder Gábor below has guessed at the cause. I have tested it a little more:\n\n# Test script to ensure that shell does not get in the way\n$ cat test.py\nimport sys\nimport subprocess\n\nsubprocess.check_call([\n    'python%s' % sys.argv[1],\n    './print_argv.py',\n    'similarity index 90%\\nrename from file1.txt\\nrename to\nfile1-mv.txt\\nindex 2bef330..f8fd673 100644\\n',\n])\n\n# Python 2, expected behavior\n$ python3 test.py 2\n['./print_argv.py',\n 'similarity index 90%\\nrename from file1.txt\\nrename to\nfile1-mv.txt\\nindex 2bef330..f8fd673 100644\\n']\n\n# Python 3, broken behavior observed before\n$ python3 test.py 3\n['./print_argv.py',\n 'similarity index 90%\\n'\n 'rename from file1.txt\\n'\n 'rename to file1-mv.txt\\n'\n 'index 2bef330..f8fd673 100644\\n']\n\n# Directly via shell. Python 2, expected behavior\n$ python2 print_argv.py 'similarity index 90%\\nrename from\nfile1.txt\\nrename to file1-mv.txt\\nindex 2bef330..f8fd673 100644\\n'\n['print_argv.py',\n 'similarity index 90%\\\\nrename from file1.txt\\\\nrename to\nfile1-mv.txt\\\\nindex 2bef330..f8fd673 100644\\\\n']\n\n# Python 3, WTF\n$ python3 print_argv.py 'similarity index 90%\\nrename from\nfile1.txt\\nrename to file1-mv.txt\\nindex 2bef330..f8fd673 100644\\n'\n['print_argv.py',\n 'similarity index 90%\\\\nrename from file1.txt\\\\nrename to '\n 'file1-mv.txt\\\\nindex 2bef330..f8fd673 100644\\\\n']\n"},{"id":"381588","messageId":"CAH53SykYvushLUtjKK7x5p8dPxe4EwyJJpia==v6W1NAdFpy=Q@mail.gmail.com","threadId":"51761","inReplyTo":"CAH53SyniovbvYoU1nhMobRq7nZREENiagJqN9LAYmZsGMpc+ww@mail.gmail.com","subject":"Re: git-diff passes <rev>:<path> args to GIT_EXTERNAL_DIFF incorrectly?","fromName":"Dmitry Nikulin","fromEmail":"pastafariant@gmail.com","sentAt":"2019-08-30T15:51:46Z","receivedAt":"2019-08-30T15:51:59Z","isPatch":false,"sender":{"key":"pastafariant@gmail.com","avatar":null},"body":"I've figured it out. It's not a bug, it's a peculiarity of pprint().\nIt splits strings to avoid long lines and abuses the fact that in\nPython strings split with whitespace are concatenated by the parser.\n\nAlso, in my example newlines are not parsed correctly in the shell.\nEscaping them, I get the exact same behavior as in\nsubprocess.check_call():\n\n$ python2 print_argv.py $'similarity index 90%\\nrename from\nfile1.txt\\nrename to file1-mv.txt\\nindex 2bef330..f8fd673 100644\\n'\n['print_argv.py',\n 'similarity index 90%\\nrename from file1.txt\\nrename to\nfile1-mv.txt\\nindex 2bef330..f8fd673 100644\\n']\n$ python3 print_argv.py $'similarity index 90%\\nrename from\nfile1.txt\\nrename to file1-mv.txt\\nindex 2bef330..f8fd673 100644\\n'\n['print_argv.py',\n 'similarity index 90%\\n'\n 'rename from file1.txt\\n'\n 'rename to file1-mv.txt\\n'\n 'index 2bef330..f8fd673 100644\\n']\n"},{"id":"381589","messageId":"xmqqlfvazkj1.fsf@gitster-ct.c.googlers.com","threadId":"51761","inReplyTo":"20190830142932.GJ8571@szeder.dev","subject":"Re: git-diff passes <rev>:<path> args to GIT_EXTERNAL_DIFF incorrectly?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-08-30T16:26:58Z","receivedAt":"2019-08-30T16:27:05Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor <szeder.dev@gmail.com> writes:\n\n> Another Python 2 vs 3 issue, perhaps?\n>\n>   # Python2, good:\n>    ...\n>    'f8fd6737fbe5a45c97ba9c9de495dc46ff11eccd',\n>    '100644',\n>    'file1-mv.txt',\n>    'similarity index 90%\\nrename from file1.txt\\nrename to\n>   file1-mv.txt\\nindex 2bef330..f8fd673 100644\\n']\n>   # Python3, bad:\n>   ...\n>    'f8fd6737fbe5a45c97ba9c9de495dc46ff11eccd',\n>    '100644',\n>    'file1-mv.txt',\n>    'similarity index 90%\\n'\n>    'rename from file1.txt\\n'\n>    'rename to file1-mv.txt\\n'\n>    'index 2bef330..f8fd673 100644\\n']\n>\n> Let's just stick to plain old printf for now, as suggested by Phillip\n> earlier, to reduce unnecessary confusion.\n\nI notice that the latter is pretty-printed.  Pay attention to the\nlast few lines that end without a trailing comma.  Literal string\nconcatenation taking place to form a single string here, in which\ncase both are giving us the same string?\n\nBy the way, sorry for an earlier response based on what I\nmisremembered, which may have confused the discussion unnecessarily.\nEver since 65056021 (\"built-in diff.\", 2006-04-28) made \"git diff\" a\nbuilt-in, we had used (or at least attempted to use) <path> from\n<treeish>:<path>, so it is not unexpected to see paths in two blob\ndiff output.\n"}]}