threads / patch / 10195

patchmergetool: add support for ECMerge

Subject: [PATCH] mergetool: add support for ECMerge

## tl;dr

15 messages between Oct 8, 2007 and Oct 9, 2007. Diffs are folded; open one to read it.

replies: 14people: 5as markdown or json

Steffen Prohaska· Oct 8, 2007, 21:22 UTC · lore

[PATCH] mergetool: support absolute paths to tools by git config merge.<tool>path

This commit adds a mechanism to provide absolute paths to the commands called by 'git mergetool'. A path can be specified in the configuation variable merge.<toolname>path.

This mechanism is especially useful on Windows, where external programs are unlikely to be in the path.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 Documentation/git-mergetool.txt |    5 ++
 git-mergetool.sh                |   81 ++++++++++++++++++++++-----------------
 2 files changed, 51 insertions(+), 35 deletions(-)
Show changes to 2 files +51 −35

Documentation/git-mergetool.txt, git-mergetool.sh

diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index 6c32c6d..6b97aaa 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -31,6 +31,11 @@ If a merge resolution program is not specified, 'git mergetool'
 will use the configuration variable merge.tool.  If the
 configuration variable merge.tool is not set, 'git mergetool'
 will pick a suitable default.
++
+You can explicitly provide a full path to the tool by setting the
+configuration variable merge.<tool>path. For example, you
+can configure the absolute path to kdiff3 by setting merge.kdiff3path.
+Otherise, 'git mergetool' assumes the tool is available in PATH.
 
 Author
 ------
diff --git a/git-mergetool.sh b/git-mergetool.sh
index 9f4f313..7f6c16f 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -192,10 +192,10 @@ merge_file () {
     case "$merge_tool" in
 	kdiff3)
 	    if base_present ; then
-		(kdiff3 --auto --L1 "$path (Base)" --L2 "$path (Local)" --L3 "$path (Remote)" \
+		("$merge_tool_path" --auto --L1 "$path (Base)" --L2 "$path (Local)" --L3 "$path (Remote)" \
 		    -o "$path" -- "$BASE" "$LOCAL" "$REMOTE" > /dev/null 2>&1)
 	    else
-		(kdiff3 --auto --L1 "$path (Local)" --L2 "$path (Remote)" \
+		("$merge_tool_path" --auto --L1 "$path (Local)" --L2 "$path (Remote)" \
 		    -o "$path" -- "$LOCAL" "$REMOTE" > /dev/null 2>&1)
 	    fi
 	    status=$?
@@ -203,35 +203,35 @@ merge_file () {
 	    ;;
 	tkdiff)
 	    if base_present ; then
-		tkdiff -a "$BASE" -o "$path" -- "$LOCAL" "$REMOTE"
+		"$merge_tool_path" -a "$BASE" -o "$path" -- "$LOCAL" "$REMOTE"
 	    else
-		tkdiff -o "$path" -- "$LOCAL" "$REMOTE"
+		"$merge_tool_path" -o "$path" -- "$LOCAL" "$REMOTE"
 	    fi
 	    status=$?
 	    save_backup
 	    ;;
 	meld|vimdiff)
 	    touch "$BACKUP"
-	    $merge_tool -- "$LOCAL" "$path" "$REMOTE"
+	    "$merge_tool_path" -- "$LOCAL" "$path" "$REMOTE"
 	    check_unchanged
 	    save_backup
 	    ;;
 	gvimdiff)
 		touch "$BACKUP"
-		gvimdiff -f -- "$LOCAL" "$path" "$REMOTE"
+		"$merge_tool_path" -f -- "$LOCAL" "$path" "$REMOTE"
 		check_unchanged
 		save_backup
 		;;
 	xxdiff)
 	    touch "$BACKUP"
 	    if base_present ; then
-		xxdiff -X --show-merged-pane \
+		"$merge_tool_path" -X --show-merged-pane \
 		    -R 'Accel.SaveAsMerged: "Ctrl-S"' \
 		    -R 'Accel.Search: "Ctrl+F"' \
 		    -R 'Accel.SearchForward: "Ctrl-G"' \
 		    --merged-file "$path" -- "$LOCAL" "$BASE" "$REMOTE"
 	    else
-		xxdiff -X --show-merged-pane \
+		"$merge_tool_path" -X --show-merged-pane \
 		    -R 'Accel.SaveAsMerged: "Ctrl-S"' \
 		    -R 'Accel.Search: "Ctrl+F"' \
 		    -R 'Accel.SearchForward: "Ctrl-G"' \
@@ -243,18 +243,18 @@ merge_file () {
 	opendiff)
 	    touch "$BACKUP"
 	    if base_present; then
-		opendiff "$LOCAL" "$REMOTE" -ancestor "$BASE" -merge "$path" | cat
+		"$merge_tool_path" "$LOCAL" "$REMOTE" -ancestor "$BASE" -merge "$path" | cat
 	    else
-		opendiff "$LOCAL" "$REMOTE" -merge "$path" | cat
+		"$merge_tool_path" "$LOCAL" "$REMOTE" -merge "$path" | cat
 	    fi
 	    check_unchanged
 	    save_backup
 	    ;;
 	emerge)
 	    if base_present ; then
-		emacs -f emerge-files-with-ancestor-command "$LOCAL" "$REMOTE" "$BASE" "$(basename "$path")"
+		"$merge_tool_path" -f emerge-files-with-ancestor-command "$LOCAL" "$REMOTE" "$BASE" "$(basename "$path")"
 	    else
-		emacs -f emerge-files-command "$LOCAL" "$REMOTE" "$(basename "$path")"
+		"$merge_tool_path" -f emerge-files-command "$LOCAL" "$REMOTE" "$(basename "$path")"
 	    fi
 	    status=$?
 	    save_backup
@@ -297,17 +297,23 @@ do
     shift
 done
 
+valid_tool() {
+	case "$1" in
+		kdiff3 | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | gvimdiff)
+			;; # happy
+		*)
+			return 1
+			;;
+	esac
+}
+
 if test -z "$merge_tool"; then
     merge_tool=`git config merge.tool`
-    case "$merge_tool" in
-	kdiff3 | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | gvimdiff | "")
-	    ;; # happy
-	*)
+    test -n "$merge_tool" || valid_tool "$merge_tool" || {
 	    echo >&2 "git config option merge.tool set to unknown tool: $merge_tool"
 	    echo >&2 "Resetting to default..."
 	    unset merge_tool
-	    ;;
-    esac
+    }
 fi
 
 if test -z "$merge_tool" ; then
@@ -345,24 +351,29 @@ if test -z "$merge_tool" ; then
     fi
 fi
 
-case "$merge_tool" in
-    kdiff3|tkdiff|meld|xxdiff|vimdiff|gvimdiff|opendiff)
-	if ! type "$merge_tool" > /dev/null 2>&1; then
-	    echo "The merge tool $merge_tool is not available"
-	    exit 1
-	fi
-	;;
-    emerge)
-	if ! type "emacs" > /dev/null 2>&1; then
-	    echo "Emacs is not available"
-	    exit 1
-	fi
-	;;
-    *)
-	echo "Unknown merge tool: $merge_tool"
+valid_tool "$merge_tool" || {
+	echo >&2 "Unknown merge_tool $merge_tool"
 	exit 1
-	;;
-esac
+}
+
+if test -z "$merge_tool_path" ; then
+	merge_tool_path=`git config merge.${merge_tool}path`
+	if test -z "$merge_tool_path" ; then
+		case "$merge_tool" in
+			emerge)
+				merge_tool_path=emacs
+				;;
+			*)
+				merge_tool_path=$merge_tool
+				;;
+		esac
+	fi
+fi
+
+if ! type "$merge_tool_path" > /dev/null 2>&1; then
+    echo "The merge tool $merge_tool is not available as '$merge_tool_path'"
+    exit 1
+fi
 
 if test $# -eq 0 ; then
 	files=`git ls-files -u | sed -e 's/^[^	]*	//' | sort -u`
-- 
1.5.3.3.127.g40d17
Steffen Prohaska· Oct 8, 2007, 21:22 UTC · re: Steffen Prohaska · lore

Add support for ECMerge available from http://www.elliecomputing.com/Products/merge_overview.asp

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 Documentation/git-mergetool.txt |    2 +-
 git-mergetool.sh                |   12 +++++++++++-
 2 files changed, 12 insertions(+), 2 deletions(-)
Show changes to 2 files +12 −2

Documentation/git-mergetool.txt, git-mergetool.sh

diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index 6b97aaa..ece7718 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -25,7 +25,7 @@ OPTIONS
 -t or --tool=<tool>::
 	Use the merge resolution program specified by <tool>.
 	Valid merge tools are:
-	kdiff3, tkdiff, meld, xxdiff, emerge, vimdiff, gvimdiff, and opendiff
+	kdiff3, tkdiff, meld, xxdiff, emerge, vimdiff, gvimdiff, ecmerge, and opendiff
 +
 If a merge resolution program is not specified, 'git mergetool'
 will use the configuration variable merge.tool.  If the
diff --git a/git-mergetool.sh b/git-mergetool.sh
index 7f6c16f..39f6595 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -250,6 +250,16 @@ merge_file () {
 	    check_unchanged
 	    save_backup
 	    ;;
+	ecmerge)
+	    touch "$BACKUP"
+	    if base_present; then
+		"$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" --mode=merge3 --to="$path"
+	    else
+		"$merge_tool_path" "$LOCAL" "$REMOTE" --mode=merge2 --to="$path"
+	    fi
+	    check_unchanged
+	    save_backup
+	    ;;
 	emerge)
 	    if base_present ; then
 		"$merge_tool_path" -f emerge-files-with-ancestor-command "$LOCAL" "$REMOTE" "$BASE" "$(basename "$path")"
@@ -299,7 +309,7 @@ done
 
 valid_tool() {
 	case "$1" in
-		kdiff3 | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | gvimdiff)
+		kdiff3 | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | gvimdiff | ecmerge)
 			;; # happy
 		*)
 			return 1
-- 
1.5.3.3.127.g40d17
Theodore Tso· Oct 8, 2007, 21:44 UTC · re: Steffen Prohaska · lore

Re: [PATCH] mergetool: add support for ECMerge

On Mon, Oct 08, 2007 at 11:22:41PM +0200, Steffen Prohaska wrote:
> Add support for ECMerge available from
> http://www.elliecomputing.com/Products/merge_overview.asp
Hmm.  A propietary merge tool.  It's not that much code, so I guess....

I note though that it claims on the web page that they are integrated with "most famous SCM's", but they don't include git. If we add this support, are they going to change their web page? :-)

Also, ECmerge is supported on Linux, Solaris, MacOS X, and Windows. Which platforms have you tested on? My guess is that if it works on Linux, it'll probably work on Solaris and MacOS, but is it a fair guess you haven't tested under Windows? Not that most Windows systems are going to be able to use git-mergetool since it's a bash script, unless they are using Cygwin or some such.

       	    	      	     	     - Ted
Steffen Prohaska· Oct 9, 2007, 06:14 UTC · re: Theodore Tso · lore

Re: [PATCH] mergetool: add support for ECMerge

On Oct 8, 2007, at 11:44 PM, Theodore Tso wrote:
Show 10 quoted lines
> On Mon, Oct 08, 2007 at 11:22:41PM +0200, Steffen Prohaska wrote:
>> Add support for ECMerge available from
>> http://www.elliecomputing.com/Products/merge_overview.asp
>
> Hmm.  A propietary merge tool.  It's not that much code, so I  
> guess....
>
> I note though that it claims on the web page that they are integrated
> with "most famous SCM's", but they don't include git.  If we add this
> support, are they going to change their web page?  :-)

You may even apply for a free (as beer) license if you contribute to open source projects:

http://www.elliecomputing.com/Community/opensource.asp
I didn't try this, so I can't say how this works in practice.
> Also, ECmerge is supported on Linux, Solaris, MacOS X, and Windows.
> Which platforms have you tested on?
Windows. Haven't tested Unix systems yet.
Show 5 quoted lines
> My guess is that if it works on
> Linux, it'll probably work on Solaris and MacOS, but is it a fair
> guess you haven't tested under Windows?  Not that most Windows systems
> are going to be able to use git-mergetool since it's a bash script,
> unless they are using Cygwin or some such.

I work on the msysgit project and I'd like to have mergetool available before I advertise git on Windows. It makes merging so much easier ;)

	Steffen
Johannes Schindelin· Oct 9, 2007, 12:37 UTC · re: Steffen Prohaska · lore

Re: [PATCH] mergetool: add support for ECMerge

Hi,
On Tue, 9 Oct 2007, Steffen Prohaska wrote:
> I work on the msysgit project and I'd like to have mergetool available 
> before I advertise git on Windows. It makes merging so much easier ;)

I would _hate_ to rely on a closed source program (in addition to Windows itself) in msysGit.

And it seems that you cannot even get ECMerge for free in general.
What does TortoiseCVS use?

Ciao, Dscho

Steffen Prohaska· Oct 9, 2007, 12:49 UTC · re: Johannes Schindelin · lore

Re: [PATCH] mergetool: add support for ECMerge

On Oct 9, 2007, at 2:37 PM, Johannes Schindelin wrote:
Show 9 quoted lines
> On Tue, 9 Oct 2007, Steffen Prohaska wrote:
>
>> I work on the msysgit project and I'd like to have mergetool  
>> available
>> before I advertise git on Windows. It makes merging so much easier ;)
>
> I would _hate_ to rely on a closed source program (in addition to  
> Windows
> itself) in msysGit.

It's only an option. You can also choose kdiff3 if you like, or vimdiff or emacs merge. But personally, I prefer ECMerge because is seems to support a faster and safer workflow (at least for me).

Before submitted this patch I thought about a general mechanism to easily specify any tool that supports command line argument. But just adding them tool by tool as needed looks easier to me, and hopefully to the user. We can include the correct command line in git-mergetool and only require the location of the program from the user.

> And it seems that you cannot even get ECMerge for free in general.

True. But does it matter? I can't get Windows for free and git runs on it.

> What does TortoiseCVS use?
Don't know.
	Steffen
Alan Hadsell· Oct 9, 2007, 13:03 UTC · re: Johannes Schindelin · lore

Re: [PATCH] mergetool: add support for ECMerge

On 10/9/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> What does TortoiseCVS use?

They don't actually include it in the package, but they recommend WinMerge http://winmerge.org/, which is free (GPL).

-- 
Alan Hadsell
Steffen Prohaska· Oct 9, 2007, 13:17 UTC · re: Alan Hadsell · lore

Re: [PATCH] mergetool: add support for ECMerge

On Oct 9, 2007, at 3:03 PM, Alan Hadsell wrote:
Show 6 quoted lines
> On 10/9/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>
>> What does TortoiseCVS use?
>
> They don't actually include it in the package, but they recommend
> WinMerge http://winmerge.org/, which is free (GPL).

WinMerge doesn't support 3-way merges. At least I cannot find any indication for 3-way in the manual [1] and the comparison at [2] also tells that WinMerge doesn't support 3-way.

	Steffen

[1] http://winmerge.org/2.6/manual/CompareFiles.html [2] http://en.wikipedia.org/wiki/Comparison_of_file_comparison_tools

Alan Hadsell· Oct 9, 2007, 14:21 UTC · re: Steffen Prohaska · lore

Re: [PATCH] mergetool: add support for ECMerge

Steffen Prohaska writes:
> WinMerge doesn't support 3-way merges. At least I cannot find any
> indication for 3-way in the manual [1] and the comparison at [2] also
> tells that WinMerge doesn't support 3-way.
>From their FAQ:

Does WinMerge support merging three files? Sometimes it is called 3-way merge, where one file is ancestor. This would be useful for version control!

Unfortunately not. We acknowledge it would be good feature but have no plans to implement it in near future. This can be somewhat worked around by using two WinMerge instances. (One developer has begun preliminary work on this feature, but it is not yet available, even in experimental releases.)

Theodore Tso· Oct 8, 2007, 21:57 UTC · re: Steffen Prohaska · lore

Re: [PATCH] mergetool: support absolute paths to tools by git config merge.<tool>path

On Mon, Oct 08, 2007 at 11:22:40PM +0200, Steffen Prohaska wrote:
> This commit adds a mechanism to provide absolute paths to the
> commands called by 'git mergetool'. A path can be specified
> in the configuation variable merge.<toolname>path.

This patch doesn't work the config file doesn't specify an explicit mergetool via merge.tool. The reason for that is this loop:

    for i in $merge_tool_candidates; do
        if test $i = emerge ; then
            cmd=emacs
        else
            cmd=$i
        fi
        if type $cmd > /dev/null 2>&1; then
            merge_tool=$i
            break
        fi
    done

is only checking to see if $cmd is in the path; it's not looking up the merge.<toolname>path variable in this loop.

I guess the other question is whether we would be better off simply telling the user to specify an absolute pathname in merge.tool, and then having git-mergetool strip off the directory path via basename, and then on window systems, stripping off the .EXE or .COM suffix, and then downcasing the name so that something like "C:\Program Files\ECMerge\ECMerge.exe" gets translated to "ecmerge". Would I be right in guessing that the reason why you used merge.<toolname>path approach was to avoid this messy headache?

					- Ted
Theodore Tso· Oct 8, 2007, 22:01 UTC · re: Theodore Tso · lore

Re: [PATCH] mergetool: support absolute paths to tools by git config merge.<tool>path

On Mon, Oct 08, 2007 at 05:57:29PM -0400, Theodore Tso wrote:
> This patch doesn't work the config file doesn't specify an explicit
> mergetool via merge.tool.   The reason for that is this loop:

Err, let me try that again. "This patch doesn't work IF the config file doesn't specify an explicit > mergetool via merge.tool."

Sorry about the missing conjuction; the sentence doesn't make much sense without it...

						- Ted
Steffen Prohaska· Oct 9, 2007, 06:30 UTC · re: Theodore Tso · lore

Re: [PATCH] mergetool: support absolute paths to tools by git config merge.<tool>path

On Oct 8, 2007, at 11:57 PM, Theodore Tso wrote:
Show 22 quoted lines
> On Mon, Oct 08, 2007 at 11:22:40PM +0200, Steffen Prohaska wrote:
>> This commit adds a mechanism to provide absolute paths to the
>> commands called by 'git mergetool'. A path can be specified
>> in the configuation variable merge.<toolname>path.
>
> This patch doesn't work if the config file doesn't specify an explicit
> mergetool via merge.tool.   The reason for that is this loop:
>
>     for i in $merge_tool_candidates; do
>         if test $i = emerge ; then
>             cmd=emacs
>         else
>             cmd=$i
>         fi
>         if type $cmd > /dev/null 2>&1; then
>             merge_tool=$i
>             break
>         fi
>     done
>
> is only checking to see if $cmd is in the path; it's not looking up
> the merge.<toolname>path variable in this loop.

I didn't change the automatic detection. It should work as before. That is it continues to assume that merge tools are in PATH.

Is you expectation that git-mergetool should also consider the absolute paths provided in merge.<toolname>path?

When I wrote the patch I had in mind that people will set the merge.tool explicitly if they provide an absolute path. Automatic detection would only be used if nothing is configured. In this case a tool must be in PATH or would not be found.

Show 8 quoted lines
> I guess the other question is whether we would be better off simply
> telling the user to specify an absolute pathname in merge.tool, and
> then having git-mergetool strip off the directory path via basename,
> and then on window systems, stripping off the .EXE or .COM suffix, and
> then downcasing the name so that something like "C:\Program
> Files\ECMerge\ECMerge.exe" gets translated to "ecmerge".  Would I be
> right in guessing that the reason why you used merge.<toolname>path
> approach was to avoid this messy headache?

Yes. The program to start ECMerge on Windows is called 'guimerge.exe'. Hard to derive a sensible short name from this.

So I don't think that an automatic translation is an option. I prefer to provide the absolute paths.

Absolute paths have another advantage. You can set several of them and choose a tool on the command line. Maybe you have several tools you want to try. Or you hacking with someone else who preferes a different tool. Or you just want to give a demo. I see merge.<toolname>path more as a database associating absolute paths with the shortnames.

My mental model is as follows:
1) merge.tool selects the mechanism needed to call the tool, that is
command line arguments, how merge result is passed, ...
2) merge.<toolname>path provides additional information how to locate
the selected tool in the filesystem.
The two points are somewhat orthogonal. I'd not fuse them into one.
	Steffen
Frank Lichtenheld· Oct 8, 2007, 22:00 UTC · re: Steffen Prohaska · lore

Re: [PATCH] mergetool: support absolute paths to tools by git config merge.<tool>path

On Mon, Oct 08, 2007 at 11:22:40PM +0200, Steffen Prohaska wrote:
> This commit adds a mechanism to provide absolute paths to the
> commands called by 'git mergetool'. A path can be specified
> in the configuation variable merge.<toolname>path.

Why not merge.<toolname>.path? This would it make easy to introduce new variables of the form merge.<toolname>.<var> later if needed. I also think it is more consistent with names of existing configuration variables. (think branch.<name>.<var> or remote.<name>.<var>)

Gruesse,
-- 
Frank Lichtenheld <frank@lichtenheld.de>
www: http://www.djpig.de/
Steffen Prohaska· Oct 9, 2007, 06:42 UTC · re: Frank Lichtenheld · lore

Re: [PATCH] mergetool: support absolute paths to tools by git config merge.<tool>path

On Oct 9, 2007, at 12:00 AM, Frank Lichtenheld wrote:
Show 11 quoted lines
> On Mon, Oct 08, 2007 at 11:22:40PM +0200, Steffen Prohaska wrote:
>> This commit adds a mechanism to provide absolute paths to the
>> commands called by 'git mergetool'. A path can be specified
>> in the configuation variable merge.<toolname>path.
>
> Why not merge.<toolname>.path?
>
> This would it make easy to introduce new variables of the form
> merge.<toolname>.<var> later if needed. I also think it is more
> consistent with names of existing configuration variables.
> (think branch.<name>.<var> or remote.<name>.<var>)

Looks more beautiful on the command line but a bit more complex in the config file. But I like the idea.

Maybe mergetool.<tool>.path is even a better choice? It clearly indicate the relationship with mergetool. We could explain:

'The variable merge.tool = <tool> refers to a tool that can be further specified in mergetool.<tool>.path.'

	Steffen
Johannes Schindelin· Oct 9, 2007, 12:35 UTC · re: Steffen Prohaska · lore

Re: [PATCH] mergetool: support absolute paths to tools by git config merge.<tool>path

Hi,
On Mon, 8 Oct 2007, Steffen Prohaska wrote:
> This commit adds a mechanism to provide absolute paths to the
> commands called by 'git mergetool'. A path can be specified
> in the configuation variable merge.<toolname>path.

Would it not be more in line with all other config variables if it was written merge.<toolname>.path? (Indeed, I thought your subject line contained a typo when I read it first.)

Ciao, Dscho

← back to recent threads