{"thread":{"id":"31784","subject":"[PATCH] mergetools/p4merge: Handle \"/dev/null\"","startedAt":"2012-10-11T03:22:36Z","lastAt":"2012-12-20T04:41:31Z","messageCount":3,"participants":["David Aguilar","Jeremy Morton"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"200970","messageId":"1349925756-87801-1-git-send-email-davvid@gmail.com","threadId":"31784","inReplyTo":null,"subject":"[PATCH] mergetools/p4merge: Handle \"/dev/null\"","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2012-10-11T03:22:36Z","receivedAt":"2012-10-11T03:22:36Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"p4merge does not properly handle the case where \"/dev/null\"\nis passed as a filename.\n\nWorkaround it by creating a temporary file for this purpose.\n\nReported-by: Jeremy Morton \nSigned-off-by: David Aguilar \n---\nJeremy, can you test this?\n\n mergetools/p4merge | 25 +++++++++++++++++++++++++\n 1 file changed, 25 insertions(+)\n\ndiff --git a/mergetools/p4merge b/mergetools/p4merge\nindex 1a45c1b..295361a 100644\n--- a/mergetools/p4merge\n+++ b/mergetools/p4merge\n@@ -1,5 +1,30 @@\n diff_cmd () {\n+\t# p4merge does not like /dev/null\n+\trm_local=\n+\trm_remote=\n+\tif test \"/dev/null\" = \"$LOCAL\"\n+\tthen\n+\t\tLOCAL=\"./p4merge-dev-null.LOCAL.$$\"\n+\t\t>\"$LOCAL\"\n+\t\trm_local=true\n+\tfi\n+\tif test \"/dev/null\" = \"$REMOTE\"\n+\tthen\n+\t\tREMOTE=\"./p4merge-dev-null.REMOTE.$$\"\n+\t\t>\"$REMOTE\"\n+\t\trm_remote=true\n+\tfi\n+\n \t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\"\n+\n+\tif test -n \"$rm_local\"\n+\tthen\n+\t\trm -f \"$LOCAL\"\n+\tfi\n+\tif test -n \"$rm_remote\"\n+\tthen\n+\t\trm -f \"$REMOTE\"\n+\tfi\n }\n \n merge_cmd () {\n-- \n1.8.0.rc1\n"},{"id":"202002","messageId":"508B9F89.7050909@game-point.net","threadId":"31784","inReplyTo":"1349925756-87801-1-git-send-email-davvid@gmail.com","subject":"Re: [PATCH] mergetools/p4merge: Handle \"/dev/null\"","fromName":"Jeremy Morton","fromEmail":"admin@game-point.net","sentAt":"2012-10-27T08:47:05Z","receivedAt":"2012-10-27T08:47:05Z","isPatch":true,"sender":{"key":"admin@game-point.net","avatar":null},"body":"Sorry to be replying to this so late; I hadn't noticed the post until now!\n\nI've tried putting that code in my p4merge script and yes it does indeed \nwork fine. However, it puts a temporary file in the working directory \nwhich I'm not sure is a good idea? If we look at this patch which \nactually solved pretty much the same problem, but when merging and, \nduring a merge conflict, a file was created in both branches:\nhttps://github.com/git/git/commit/ec245ba\n\n... it is creating a temp file in a proper temp dir, rather than in the \nworking dir. I think that would be the proper solution here. However, \nI really want to get this fixed so I'd be happy for this band-aid fix of \nthe p4merge script to be checked in until we could get a patch more like \nthe aforementioned one, at a later date, to create empty files in a \nproper temp dir and pass them as $LOCAL and $REMOTE. :-)\n\n-- \nBest regards,\nJeremy Morton (Jez)\n\nOn 11/10/2012 04:22, David Aguilar wrote:\n> p4merge does not properly handle the case where \"/dev/null\"\n> is passed as a filename.\n>\n> Workaround it by creating a temporary file for this purpose.\n>\n> Reported-by: Jeremy Morton\n> Signed-off-by: David Aguilar\n> ---\n> Jeremy, can you test this?\n>\n> mergetools/p4merge | 25 +++++++++++++++++++++++++\n> 1 file changed, 25 insertions(+)\n>\n> diff --git a/mergetools/p4merge b/mergetools/p4merge\n> index 1a45c1b..295361a 100644\n> --- a/mergetools/p4merge\n> +++ b/mergetools/p4merge\n> @@ -1,5 +1,30 @@\n> diff_cmd () {\n> +\t# p4merge does not like /dev/null\n> +\trm_local=\n> +\trm_remote=\n> +\tif test \"/dev/null\" = \"$LOCAL\"\n> +\tthen\n> +\t\tLOCAL=\"./p4merge-dev-null.LOCAL.$$\"\n> +\t\t>\"$LOCAL\"\n> +\t\trm_local=true\n> +\tfi\n> +\tif test \"/dev/null\" = \"$REMOTE\"\n> +\tthen\n> +\t\tREMOTE=\"./p4merge-dev-null.REMOTE.$$\"\n> +\t\t>\"$REMOTE\"\n> +\t\trm_remote=true\n> +\tfi\n> +\n> \t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\"\n> +\n> +\tif test -n \"$rm_local\"\n> +\tthen\n> +\t\trm -f \"$LOCAL\"\n> +\tfi\n> +\tif test -n \"$rm_remote\"\n> +\tthen\n> +\t\trm -f \"$REMOTE\"\n> +\tfi\n> }\n>\n> merge_cmd () {\n"},{"id":"205243","messageId":"CAJDDKr5rxUPzqkiNAhPpEkK4ajyPNkNsGdPYWca3oggBrWybmg@mail.gmail.com","threadId":"31784","inReplyTo":"508B9F89.7050909@game-point.net","subject":"Re: [PATCH] mergetools/p4merge: Handle \"/dev/null\"","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2012-12-20T04:41:31Z","receivedAt":"2012-12-20T04:41:31Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Sat, Oct 27, 2012 at 1:47 AM, Jeremy Morton wrote:\n> Sorry to be replying to this so late; I hadn't noticed the post until now!\n>\n> I've tried putting that code in my p4merge script and yes it does indeed\n> work fine. However, it puts a temporary file in the working directory which\n> I'm not sure is a good idea? If we look at this patch which actually solved\n> pretty much the same problem, but when merging and, during a merge conflict,\n> a file was created in both branches:\n> https://github.com/git/git/commit/ec245ba\n>\n> ... it is creating a temp file in a proper temp dir, rather than in the\n> working dir. I think that would be the proper solution here. However, I\n> really want to get this fixed so I'd be happy for this band-aid fix of the\n> p4merge script to be checked in until we could get a patch more like the\n> aforementioned one, at a later date, to create empty files in a proper temp\n> dir and pass them as $LOCAL and $REMOTE. :-)\n\nI had the same thoughts when I wrote it, but I figured that following\nthe existing pattern used by mergetool for $REMOTE and $LOCAL when\nthey do exist was simpler as the first step.\n\nI have a patch that fixes this by using mktemp that I will send shortly.\nIt only does it for the /dev/null file since the existing behavior for\nfiles that do exist is fine.\n-- \nDavid\n"}]}