[PATCH] mergetool: export variables for use by custom mergetools

[PATCH] mergetool: export variables for use by custom mergetools

[David Aguilar]

The MERGED, BACKUP, LOCAL, REMOTE and BASE variables were not being
exported from the git-mergetool.sh script.  This prevented custom
mergetools from being able to use them.

We now export them so that arbitrary mergetools can easily interact
with git mergetool.

This problem was Reported-By: Evgeny <evgeny.zislis@gmail.com>

Signed-off-by: David Aguilar <davvid@gmail.com>
---
 git-mergetool.sh |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/git-mergetool.sh b/git-mergetool.sh
index fcdec4a..c4f31ee 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -146,6 +146,12 @@ merge_file () {
     REMOTE="$MERGED.REMOTE.$ext"
     BASE="$MERGED.BASE.$ext"
 
+    export MERGED
+    export BACKUP
+    export LOCAL
+    export REMOTE
+    export BASE
+
     mv -- "$MERGED" "$BACKUP"
     cp -- "$BACKUP" "$MERGED"
 
-- 
1.5.5.1

Re: [PATCH] mergetool: export variables for use by custom mergetools

[Charles Bailey]

On Sat, May 17, 2008 at 01:39:26PM -0700, David Aguilar wrote:
> The MERGED, BACKUP, LOCAL, REMOTE and BASE variables were not being
> exported from the git-mergetool.sh script.  This prevented custom
> mergetools from being able to use them.
> 
> We now export them so that arbitrary mergetools can easily interact
> with git mergetool.
> 
> This problem was Reported-By: Evgeny <evgeny.zislis@gmail.com>
> 
> Signed-off-by: David Aguilar <davvid@gmail.com>

I think there must be a misunderstanding here. The variables are not
designed to be exported. The point of the custom merge tool patch to
git mergetool was to support custom mergetools without the need for a
wrapper script, just a git config variable.

I know that (with the right config) git mergetool supports p4merge as
it is one of a selection of tools that I tested it with. You should be
able to get p4 merge to work by setting the config variable:

mergetool.p4merge.cmd

to the value (IIRC, and I don't have the p4merge documentation to hand):

p4merge "$BASE" "$REMOTE" "$LOCAL" "$MERGED"

Getting the quoting correct on this depends on whether you set it via
commandline or via editing a git config file and can be a little
tricky.

If you have a wrapper shell script then you can easily pass these as
positional parameters and re-export them as necessary.

-- 
Charles Bailey
http://ccgi.hashpling.plus.com/blog/

Re: [PATCH] mergetool: export variables for use by custom mergetools

[Evgeny]

Ohh, so the documentation is not clear then. Now that you say it this
way, I finally understant that I should have in my ~/.gitconfig
something like:

[mergetool "p4merge"]
	cmd = p4merge.sh "$PWD/$BASE" "$PWD/$REMOTE" "$PWD/$LOCAL" "$PWD/$MERGED"
	keepBackup = false

[merge]
	tool = p4merge


Thanks for the explanation!
I believe that an example in the documentation could be a nice
addition, to clear things up in the future.

PS: The latest version of P4Merge did not work without the $PWD/
(fullpath). strange, but an older version worked okay even without it.


No patch is needed -- all is great, thank you great people! :)

Thank you Charles!



-
Evgeny


On Sun, May 18, 2008 at 2:34 AM, Charles Bailey <charles@hashpling.org> wrote:
> On Sat, May 17, 2008 at 01:39:26PM -0700, David Aguilar wrote:
>> The MERGED, BACKUP, LOCAL, REMOTE and BASE variables were not being
>> exported from the git-mergetool.sh script.  This prevented custom
>> mergetools from being able to use them.
>>
>> We now export them so that arbitrary mergetools can easily interact
>> with git mergetool.
>>
>> This problem was Reported-By: Evgeny <evgeny.zislis@gmail.com>
>>
>> Signed-off-by: David Aguilar <davvid@gmail.com>
>
> I think there must be a misunderstanding here. The variables are not
> designed to be exported. The point of the custom merge tool patch to
> git mergetool was to support custom mergetools without the need for a
> wrapper script, just a git config variable.
>
> I know that (with the right config) git mergetool supports p4merge as
> it is one of a selection of tools that I tested it with. You should be
> able to get p4 merge to work by setting the config variable:
>
> mergetool.p4merge.cmd
>
> to the value (IIRC, and I don't have the p4merge documentation to hand):
>
> p4merge "$BASE" "$REMOTE" "$LOCAL" "$MERGED"
>
> Getting the quoting correct on this depends on whether you set it via
> commandline or via editing a git config file and can be a little
> tricky.
>
> If you have a wrapper shell script then you can easily pass these as
> positional parameters and re-export them as necessary.
>
> --
> Charles Bailey
> http://ccgi.hashpling.plus.com/blog/
>

Re: [PATCH] mergetool: export variables for use by custom mergetools

[Charles Bailey]

On Sun, May 18, 2008 at 02:55:10AM +0300, Evgeny wrote:
> Ohh, so the documentation is not clear then.

That's a pity, I did make an effort with the documentation, but
explaining things is not my strongest ability.

> 
> [mergetool "p4merge"]
> 	cmd = p4merge.sh "$PWD/$BASE" "$PWD/$REMOTE" "$PWD/$LOCAL" "$PWD/$MERGED"
> 	keepBackup = false
> 
> [merge]
> 	tool = p4merge
> 

"$PWD/" looks very wrong. Perhaps p4merge.sh is changing its working
directory and not handling the passed paths correctly?

> Thanks for the explanation!
> I believe that an example in the documentation could be a nice
> addition, to clear things up in the future.

Yes, there were a few examples in the original patch mail text (note
that 'path' has sinces changed to 'MERGED'), but they never made it to
the documentation. There's also some discussion about eval vs. leaking
environment variables:

http://thread.gmane.org/gmane.comp.version-control.git/74059

-- 
Charles Bailey
http://ccgi.hashpling.plus.com/blog/

Re: [PATCH] mergetool: export variables for use by custom mergetools

[Evgeny]

I also tried directly with
  cmd = $HOME/Applications/p4merge.app/Contents/Resources/launchp4merge
"$PWD/$BASE" .....

It also did not work without $PWD, but it also did not work when I
used it from the command line. Some weird error, saying :

Incorrect parameters:
'./test.BASE.18308' is (or points to) an invalid file.
'./test.LOCAL.18308' is (or points to) an invalid file.
'./test.REMOTE.18308' is (or points to) an invalid file.

Use 'launchp4merge -h' for more help.


I think it's a p4merge bug, they did change it to Qt4 lately. Probably
broke a couple of things.


On Sun, May 18, 2008 at 3:07 AM, Charles Bailey <charles@hashpling.org> wrote:
> On Sun, May 18, 2008 at 02:55:10AM +0300, Evgeny wrote:
>> Ohh, so the documentation is not clear then.
>
> That's a pity, I did make an effort with the documentation, but
> explaining things is not my strongest ability.
>
>>
>> [mergetool "p4merge"]
>>       cmd = p4merge.sh "$PWD/$BASE" "$PWD/$REMOTE" "$PWD/$LOCAL" "$PWD/$MERGED"
>>       keepBackup = false
>>
>> [merge]
>>       tool = p4merge
>>
>
> "$PWD/" looks very wrong. Perhaps p4merge.sh is changing its working
> directory and not handling the passed paths correctly?
>
>> Thanks for the explanation!
>> I believe that an example in the documentation could be a nice
>> addition, to clear things up in the future.
>
> Yes, there were a few examples in the original patch mail text (note
> that 'path' has sinces changed to 'MERGED'), but they never made it to
> the documentation. There's also some discussion about eval vs. leaking
> environment variables:
>
> http://thread.gmane.org/gmane.comp.version-control.git/74059
>
> --
> Charles Bailey
> http://ccgi.hashpling.plus.com/blog/
>