{"thread":{"id":"13557","subject":"[PATCH] mergetool: export variables for use by custom mergetools","startedAt":"2008-05-17T20:39:26Z","lastAt":"2008-05-18T00:12:36Z","messageCount":5,"participants":["David Aguilar","Evgeny","Charles Bailey"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"77204","messageId":"8b4e944ac849029f31f403ab75198ed6fb4197c1.1211056369.git.davvid@gmail.com","threadId":"13557","inReplyTo":null,"subject":"[PATCH] mergetool: export variables for use by custom mergetools","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2008-05-17T20:39:26Z","receivedAt":"2008-05-17T20:39:26Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"The MERGED, BACKUP, LOCAL, REMOTE and BASE variables were not being\nexported from the git-mergetool.sh script. This prevented custom\nmergetools from being able to use them.\n\nWe now export them so that arbitrary mergetools can easily interact\nwith git mergetool.\n\nThis problem was Reported-By: Evgeny \n\nSigned-off-by: David Aguilar \n---\n git-mergetool.sh | 6 ++++++\n 1 files changed, 6 insertions(+), 0 deletions(-)\n\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex fcdec4a..c4f31ee 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -146,6 +146,12 @@ merge_file () {\n REMOTE=\"$MERGED.REMOTE.$ext\"\n BASE=\"$MERGED.BASE.$ext\"\n \n+ export MERGED\n+ export BACKUP\n+ export LOCAL\n+ export REMOTE\n+ export BASE\n+\n mv -- \"$MERGED\" \"$BACKUP\"\n cp -- \"$BACKUP\" \"$MERGED\"\n \n-- \n1.5.5.1\n"},{"id":"77213","messageId":"20080517233414.GA28196@hashpling.org","threadId":"13557","inReplyTo":"8b4e944ac849029f31f403ab75198ed6fb4197c1.1211056369.git.davvid@gmail.com","subject":"Re: [PATCH] mergetool: export variables for use by custom mergetools","fromName":"Charles Bailey","fromEmail":"charles@hashpling.org","sentAt":"2008-05-17T23:34:14Z","receivedAt":"2008-05-17T23:34:14Z","isPatch":true,"sender":{"key":"charles@hashpling.org","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"On Sat, May 17, 2008 at 01:39:26PM -0700, David Aguilar wrote:\n> The MERGED, BACKUP, LOCAL, REMOTE and BASE variables were not being\n> exported from the git-mergetool.sh script. This prevented custom\n> mergetools from being able to use them.\n> \n> We now export them so that arbitrary mergetools can easily interact\n> with git mergetool.\n> \n> This problem was Reported-By: Evgeny \n> \n> Signed-off-by: David Aguilar \n\nI think there must be a misunderstanding here. The variables are not\ndesigned to be exported. The point of the custom merge tool patch to\ngit mergetool was to support custom mergetools without the need for a\nwrapper script, just a git config variable.\n\nI know that (with the right config) git mergetool supports p4merge as\nit is one of a selection of tools that I tested it with. You should be\nable to get p4 merge to work by setting the config variable:\n\nmergetool.p4merge.cmd\n\nto the value (IIRC, and I don't have the p4merge documentation to hand):\n\np4merge \"$BASE\" \"$REMOTE\" \"$LOCAL\" \"$MERGED\"\n\nGetting the quoting correct on this depends on whether you set it via\ncommandline or via editing a git config file and can be a little\ntricky.\n\nIf you have a wrapper shell script then you can easily pass these as\npositional parameters and re-export them as necessary.\n\n-- \nCharles Bailey\nhttp://ccgi.hashpling.plus.com/blog/\n"},{"id":"77210","messageId":"b6840c770805171655y691fbd0vd2d65d0914757b63@mail.gmail.com","threadId":"13557","inReplyTo":"20080517233414.GA28196@hashpling.org","subject":"Re: [PATCH] mergetool: export variables for use by custom mergetools","fromName":"Evgeny","fromEmail":"evgeny.zislis@gmail.com","sentAt":"2008-05-17T23:55:10Z","receivedAt":"2008-05-17T23:55:10Z","isPatch":true,"sender":{"key":"evgeny.zislis@gmail.com","avatar":"https://gravatar.com/avatar/b2ebd794721c2632b56a50c51c93899ee97de11defaeabf26139d173210489ea?d=mp&s=160"},"body":"Ohh, so the documentation is not clear then. Now that you say it this\nway, I finally understant that I should have in my ~/.gitconfig\nsomething like:\n\n[mergetool \"p4merge\"]\n\tcmd = p4merge.sh \"$PWD/$BASE\" \"$PWD/$REMOTE\" \"$PWD/$LOCAL\" \"$PWD/$MERGED\"\n\tkeepBackup = false\n\n[merge]\n\ttool = p4merge\n\n\nThanks for the explanation!\nI believe that an example in the documentation could be a nice\naddition, to clear things up in the future.\n\nPS: The latest version of P4Merge did not work without the $PWD/\n(fullpath). strange, but an older version worked okay even without it.\n\n\nNo patch is needed -- all is great, thank you great people! :)\n\nThank you Charles!\n\n\n\n-\nEvgeny\n\n\nOn Sun, May 18, 2008 at 2:34 AM, Charles Bailey wrote:\n> On Sat, May 17, 2008 at 01:39:26PM -0700, David Aguilar wrote:\n>> The MERGED, BACKUP, LOCAL, REMOTE and BASE variables were not being\n>> exported from the git-mergetool.sh script. This prevented custom\n>> mergetools from being able to use them.\n>>\n>> We now export them so that arbitrary mergetools can easily interact\n>> with git mergetool.\n>>\n>> This problem was Reported-By: Evgeny \n>>\n>> Signed-off-by: David Aguilar \n>\n> I think there must be a misunderstanding here. The variables are not\n> designed to be exported. The point of the custom merge tool patch to\n> git mergetool was to support custom mergetools without the need for a\n> wrapper script, just a git config variable.\n>\n> I know that (with the right config) git mergetool supports p4merge as\n> it is one of a selection of tools that I tested it with. You should be\n> able to get p4 merge to work by setting the config variable:\n>\n> mergetool.p4merge.cmd\n>\n> to the value (IIRC, and I don't have the p4merge documentation to hand):\n>\n> p4merge \"$BASE\" \"$REMOTE\" \"$LOCAL\" \"$MERGED\"\n>\n> Getting the quoting correct on this depends on whether you set it via\n> commandline or via editing a git config file and can be a little\n> tricky.\n>\n> If you have a wrapper shell script then you can easily pass these as\n> positional parameters and re-export them as necessary.\n>\n> --\n> Charles Bailey\n> http://ccgi.hashpling.plus.com/blog/\n>\n"},{"id":"77211","messageId":"20080518000739.GA29689@hashpling.org","threadId":"13557","inReplyTo":"b6840c770805171655y691fbd0vd2d65d0914757b63@mail.gmail.com","subject":"Re: [PATCH] mergetool: export variables for use by custom mergetools","fromName":"Charles Bailey","fromEmail":"charles@hashpling.org","sentAt":"2008-05-18T00:07:39Z","receivedAt":"2008-05-18T00:07:39Z","isPatch":true,"sender":{"key":"charles@hashpling.org","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"On Sun, May 18, 2008 at 02:55:10AM +0300, Evgeny wrote:\n> Ohh, so the documentation is not clear then.\n\nThat's a pity, I did make an effort with the documentation, but\nexplaining things is not my strongest ability.\n\n> \n> [mergetool \"p4merge\"]\n> \tcmd = p4merge.sh \"$PWD/$BASE\" \"$PWD/$REMOTE\" \"$PWD/$LOCAL\" \"$PWD/$MERGED\"\n> \tkeepBackup = false\n> \n> [merge]\n> \ttool = p4merge\n> \n\n\"$PWD/\" looks very wrong. Perhaps p4merge.sh is changing its working\ndirectory and not handling the passed paths correctly?\n\n> Thanks for the explanation!\n> I believe that an example in the documentation could be a nice\n> addition, to clear things up in the future.\n\nYes, there were a few examples in the original patch mail text (note\nthat 'path' has sinces changed to 'MERGED'), but they never made it to\nthe documentation. There's also some discussion about eval vs. leaking\nenvironment variables:\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/74059\n\n-- \nCharles Bailey\nhttp://ccgi.hashpling.plus.com/blog/\n"},{"id":"77212","messageId":"b6840c770805171712j636c8f43tde9c0a0b74ec58b@mail.gmail.com","threadId":"13557","inReplyTo":"20080518000739.GA29689@hashpling.org","subject":"Re: [PATCH] mergetool: export variables for use by custom mergetools","fromName":"Evgeny","fromEmail":"evgeny.zislis@gmail.com","sentAt":"2008-05-18T00:12:36Z","receivedAt":"2008-05-18T00:12:36Z","isPatch":true,"sender":{"key":"evgeny.zislis@gmail.com","avatar":"https://gravatar.com/avatar/b2ebd794721c2632b56a50c51c93899ee97de11defaeabf26139d173210489ea?d=mp&s=160"},"body":"I also tried directly with\n cmd = $HOME/Applications/p4merge.app/Contents/Resources/launchp4merge\n\"$PWD/$BASE\" .....\n\nIt also did not work without $PWD, but it also did not work when I\nused it from the command line. Some weird error, saying :\n\nIncorrect parameters:\n'./test.BASE.18308' is (or points to) an invalid file.\n'./test.LOCAL.18308' is (or points to) an invalid file.\n'./test.REMOTE.18308' is (or points to) an invalid file.\n\nUse 'launchp4merge -h' for more help.\n\n\nI think it's a p4merge bug, they did change it to Qt4 lately. Probably\nbroke a couple of things.\n\n\nOn Sun, May 18, 2008 at 3:07 AM, Charles Bailey wrote:\n> On Sun, May 18, 2008 at 02:55:10AM +0300, Evgeny wrote:\n>> Ohh, so the documentation is not clear then.\n>\n> That's a pity, I did make an effort with the documentation, but\n> explaining things is not my strongest ability.\n>\n>>\n>> [mergetool \"p4merge\"]\n>> cmd = p4merge.sh \"$PWD/$BASE\" \"$PWD/$REMOTE\" \"$PWD/$LOCAL\" \"$PWD/$MERGED\"\n>> keepBackup = false\n>>\n>> [merge]\n>> tool = p4merge\n>>\n>\n> \"$PWD/\" looks very wrong. Perhaps p4merge.sh is changing its working\n> directory and not handling the passed paths correctly?\n>\n>> Thanks for the explanation!\n>> I believe that an example in the documentation could be a nice\n>> addition, to clear things up in the future.\n>\n> Yes, there were a few examples in the original patch mail text (note\n> that 'path' has sinces changed to 'MERGED'), but they never made it to\n> the documentation. There's also some discussion about eval vs. leaking\n> environment variables:\n>\n> http://thread.gmane.org/gmane.comp.version-control.git/74059\n>\n> --\n> Charles Bailey\n> http://ccgi.hashpling.plus.com/blog/\n>\n"}]}