{"thread":{"id":"31718","subject":"Fixing the p4merge launch shell script","startedAt":"2012-10-02T23:38:35Z","lastAt":"2012-10-04T06:59:03Z","messageCount":4,"participants":["Jeremy Morton","Junio C Hamano","David Aguilar"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"200376","messageId":"506B7AFB.1070700@game-point.net","threadId":"31718","inReplyTo":null,"subject":"Fixing the p4merge launch shell script","fromName":"Jeremy Morton","fromEmail":"admin@game-point.net","sentAt":"2012-10-02T23:38:35Z","receivedAt":"2012-10-02T23:38:35Z","isPatch":false,"sender":{"key":"admin@game-point.net","avatar":null},"body":"Hi,\n\nI've noticed that the p4merge shell script could do with some \nimprovement when it comes to merging. Because p4merge throws up an \nerror when one of the files it's given to diff is \"/dev/null\", git needs \nto create a temporary empty file and pass that to p4merge when diffing a \nfile that has been created/deleted (eg. create file, git add ., git diff \n--cached).\n\nAt the moment, the hack I'm using, which works, is to create a \nc:/blank.txt file which is an empty file, and modify \n\"libexec/git-core/mergetools/p4merge\" to start with this:\n\ndiff_cmd () {\n\tif [ ! -f \"$LOCAL\" ]\n\tthen\n\t\tLOCAL=\"C:/blank.txt\"\n\tfi\n\tif [ ! -f \"$REMOTE\" ]\n\tthen\n\t\tREMOTE=\"C:/blank.txt\"\n\tfi\n\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\"\n}\n\nObviously, this isn't good enough because c:/blank.txt probably won't \nexist on the system. It would be nice for the temporary blank file to \nhave the same extension as the one that's been added, so we could diff \n(say) c:/users/jez/Temp/abc123_newFile.foo \nc:/development/bar/newfile.foo. However, I can't see a way to do this \npurely from within the shell script (even `mktemp` doesn't exist on all \nsystems so that isn't good enough). I believe git needs to create this \ntemporary blank file itself, much like it creates a temporary file for \nthe previous version of a file that's been modified when it's being \ndiff'd. It then needs to expose the location of this temporary file as \na variable; say $LOCALBLANK. So, we would be able to modify the shell \nscript to this:\n\n\ndiff_cmd () {\n\tif [ ! -f \"$LOCAL\" ]\n\tthen\n\t\tLOCAL=$LOCALBLANK\n\tfi\n\tif [ ! -f \"$REMOTE\" ]\n\tthen\n\t\tREMOTE=$REMOTEBLANK\n\tfi\n\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\"\n}\n\nThoughts? Is there an easier way to do this?\n\n-- \nBest regards,\nJeremy Morton (Jez)\n"},{"id":"200377","messageId":"7vipas5qpu.fsf@alter.siamese.dyndns.org","threadId":"31718","inReplyTo":"506B7AFB.1070700@game-point.net","subject":"Re: Fixing the p4merge launch shell script","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-10-03T00:44:45Z","receivedAt":"2012-10-03T00:44:45Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeremy Morton writes:\n\n> I've noticed that the p4merge shell script could do with some\n> improvement when it comes to merging. Because p4merge throws up an\n> error when one of the files it's given to diff is \"/dev/null\", git\n> needs to create a temporary empty file and pass that to p4merge when\n> diffing a file that has been created/deleted (eg. create file, git add\n> ., git diff --cached).\n> ...\n> Thoughts? Is there an easier way to do this?\n\nWhich version of git? Perhaps you do not have ec245ba (mergetool:\nProvide an empty file when needed, 2012-01-19) yet?\n"},{"id":"200395","messageId":"loom.20121003T103315-553@post.gmane.org","threadId":"31718","inReplyTo":"7vipas5qpu.fsf@alter.siamese.dyndns.org","subject":"Re: Fixing the p4merge launch shell script","fromName":"Jeremy Morton","fromEmail":"admin@game-point.net","sentAt":"2012-10-03T08:34:46Z","receivedAt":"2012-10-03T08:34:46Z","isPatch":false,"sender":{"key":"admin@game-point.net","avatar":null},"body":"Junio C Hamano pobox.com> writes:\n\n> \n> Jeremy Morton game-point.net> writes:\n> \n> > I've noticed that the p4merge shell script could do with some\n> > improvement when it comes to merging. Because p4merge throws up an\n> > error when one of the files it's given to diff is \"/dev/null\", git\n> > needs to create a temporary empty file and pass that to p4merge when\n> > diffing a file that has been created/deleted (eg. create file, git add\n> > ., git diff --cached).\n> > ...\n> > Thoughts? Is there an easier way to do this?\n> \n> Which version of git? Perhaps you do not have ec245ba (mergetool:\n> Provide an empty file when needed, 2012-01-19) yet?\n> \n\nThat patch fixes the mergetool part, but the part I was referring to was the\ndifftool part, which still has this problem.\n\nBest regards,\nJeremy Morton (Jez)\n"},{"id":"200500","messageId":"CAJDDKr6qqo+a545bx=ygDmzLFAT5yU=ue+Fv0QpZeBGXuOeG6A@mail.gmail.com","threadId":"31718","inReplyTo":"loom.20121003T103315-553@post.gmane.org","subject":"Re: Fixing the p4merge launch shell script","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2012-10-04T06:59:03Z","receivedAt":"2012-10-04T06:59:03Z","isPatch":false,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Wed, Oct 3, 2012 at 1:34 AM, Jeremy Morton wrote:\n> Junio C Hamano pobox.com> writes:\n>\n>>\n>> Jeremy Morton game-point.net> writes:\n>>\n>> > I've noticed that the p4merge shell script could do with some\n>> > improvement when it comes to merging. Because p4merge throws up an\n>> > error when one of the files it's given to diff is \"/dev/null\", git\n>> > needs to create a temporary empty file and pass that to p4merge when\n>> > diffing a file that has been created/deleted (eg. create file, git add\n>> > ., git diff --cached).\n>> > ...\n>> > Thoughts? Is there an easier way to do this?\n>>\n>> Which version of git? Perhaps you do not have ec245ba (mergetool:\n>> Provide an empty file when needed, 2012-01-19) yet?\n>>\n>\n> That patch fixes the mergetool part, but the part I was referring to was the\n> difftool part, which still has this problem.\n>\n> Best regards,\n> Jeremy Morton (Jez)\n\nThanks for the heads-up, Jez.\n\nI should have some time to take a look this weekend.\nIf someone beats me to it I'll be happy to review the patches before\nthen (Jeremy? ;-)\n\nIs `git diff` giving us /dev/null as the path here,\nand p4merge doesn't like that, or is something else\ngoing on?\n\nIf that's indeed happening, then I would imagine\na sensible thing for git-difftool--helper to do\nwould be to check for /dev/null and create some\ntemporary empty file.\n\nDoes that sound like the right way to go?\nAnother option would be for git-diff's external diff\ncode to handle it at a much earlier point, perhaps\nguarded behind an option that git-difftool knows to\npass (changing existing behavior is off the table).\n\nThese both sound like pretty big changes given\nthat we're getting ready for 1.8.0.\n\nIf it's just p4merge that doesn't like this then\nperhaps the right decision is to paper over it\nin p4merge's mergetools/ scriptlet.\n\nIt's certainly the least-impactful option. Thoughts?\n-- \nDavid\n"}]}