A note on modern git plus ancient meld ("wrong number of arguments")

I note this just in case it helps someone else track down a similar problem, not because I think any change needs to be made to git, as a version of meld new enough to not be affected by this problem is 5 years old.

At $DAYJOB, I recently encountered a problem after upgrading from (don't laugh) git 1.7.1 to 1.7.8.3: one developer stated that meld failed to run, instead displaying the error 'Wrong number of arguments (Got 5)'.

We determined that this user was running a very old version of meld (1.1.1) from his home directory, as opposed to the also very old system version of meld (1.1.5). It turns out that the check added in
f61bd9c mergetools/meld: Use '--output' when available
fails on meld 1.1.1, leading git to incorrectly believe the --output flag is supporrted:

    $ meld-1.1.1 --output /dev/null --help >/dev/null 2>&1; echo $?
    0   # i.e., detected as supported
The test as written gives the correct ("not supported") result with meld
1.1.5:
    $ meld-1.1.5 --output /dev/null --help >/dev/null 2>&1; echo $?
    2   # i.e., detected as supported

so if you encounter the message 'Wrong number of arguments (Got 5)' from meld, then check whether you have an ancient version of meld. If for some reason you can't upgrade to at least 1.1.5, maybe you'd find the following configuration flags useful:

    [merge]
        tool = ancientmeld
    [mergetool "ancientmeld"]
        cmd = meld-1.1.1 \"$LOCAL\" \"$MERGED\" \"$REMOTE\"

Jeff

We originally used the --output test so that we wouldn't have to check for a specific version. Does your meld support meld --version, and what does it output?

I'm thinking that maybe we should just try and parse the version number since it seems like we cannot depend on ancient meld's return code.

Thanks Jeff. I'll see what we can do about it.

David

[...]

My bad. How about something like this patch (untested)?

-- >8 --
Subject: mergetools/meld: Use version number to detect '--output' support

In v1.7.7-rc0~3^2 (2011-08-19), git mergetool's "meld" support learned to use the --output option when calling versions of meld (1.5.0 and later) that support it.

Alas, it misdetects old versions (before 1.1.5, 2006-06-11) of meld as supporting the option, so on systems with such meld, instead of getting a nice merge helper, the operator gets a dialog box with the text "Wrong number of arguments (Got 5)". (Version 1.1.5 is when meld switched to using optparse. One consequence of that change was that errors in usage are detected and signalled through the exit status even when --help was passed.)

Just parse version numbers instead. We can detect the version number by running "meld --version" and postprocessing it. As a futureproofing measure, we are careful to handle all three --version output formats encountered so far. When confused, the mergetool falls back to assuming the --output option is not usable.

- [0.1, 0.8.5): "GNOME Meld 0.1".
- [0.8.5, 0.9.4.1):
"Meld 0.8.5
Written by Stephen Kennedy <[email protected]>"
- [0.9.4.1, 1.1.3): "GNOME Meld 0.9.4.1" again.
- [1.1.3, 1.1.5): back to the two-line form.
- [1.1.5, present): "$0 1.1.5". ($0 is typically "meld".)

---
 mergetools/meld |   17 +++++++++++------
 1 files changed, 11 insertions(+), 6 deletions(-)

index eaa115cc..3de29629 100644
@@ -23,10 +23,15 @@ check_meld_for_output_version () {
 	meld_path="$(git config mergetool.meld.path)"
 	meld_path="${meld_path:-meld}"
 
-	if "$meld_path" --output /dev/null --help >/dev/null 2>&1
-	then
-		meld_has_output_option=true
-	else
-		meld_has_output_option=false
-	fi
+	# "GNOME Meld 0.8.4" -> "0.8.4"
+	meld_version=$("$meld_path" --version 2>/dev/null)
+	meld_version=${meld_version#GNOME }
+	meld_version=${meld_version#* }
+
+	case $meld_version in
+	[2-9].* | [1-9][0-9]* | 1.[5-9]* | 1.[1-9][0-9]*)	# >= 1.5.0
+		meld_has_output_option=true ;;
+	*)
+		meld_has_output_option=false ;;
+	esac
 }

1.7.9

Hmm. I am debating myself if it may be more efficient, less error prone and simpler for the users if we gave them "mergetool.meld.useOutput" configuration option to tweak.

When an older meld fails when given --output for real (not with the dry run current code tries with --help), can we sanely detect that particular failure? If we can do so, another possibility may be to do something like this:

merge_cmd () {
	meld_has_output_option=$(git config --bool mergetool.meld.useOutput)
	case "$meld_has_output_option" in
        false)
		... do the non-output thing ...
		;;
	true)
		"$merge_tool_path" --output "$MERGED" "$LOCAL" "$BASE" "$REMOTE"
		;;
	*)
		"$merge_tool_path" --output "$MERGED" "$LOCAL" "$BASE" "$REMOTE"
		if it failed due to missing --output support?
		then
			meld_has_output_option=no
                        git config mergetool.meld.useOutput false
			merge_cmd
		fi
                ;;
	esac
}

In v1.7.7-rc0~3^2 (2011-08-19), git mergetool's "meld" support learned to use the --output option when calling versions of meld that are detected to support it (1.5.0 and newer, hopefully).

Alas, it misdetects old versions (before 1.1.5, 2006-06-11) of meld as supporting the option, so on systems with such meld, instead of getting a nice merge helper, the operator gets a dialog box with the text "Wrong number of arguments (Got 5)". (Version 1.1.5 is when meld switched to using optparse. One consequence of that change was that errors in usage are detected and signalled through the exit status even when --help was passed.)

Luckily there is a simpler check that is more reliable: the usage string printed by "meld --help" reliably reflects whether --output is supported in a given version. Use it.

---

Unfortunately it just pops up a GUI with a modal dialog box like this:

	 ___________________________________
	|                                   |
	| Wrong number of arguments (Got 5) |
	|                                   |
	|                     [Quit] [OK]   |
	|___________________________________|

If I choose "Quit", the exit status is 0.

But how about this? "meld --help | grep -e --output" seems to detect support for the option reliably. With 2>&1 on the upstream of the pipe, this even seems futureproof. ;-)

 mergetools/meld |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

index eaa115cc..cb672a55 100644
@@ -23,7 +23,7 @@ check_meld_for_output_version () {
 	meld_path="$(git config mergetool.meld.path)"
 	meld_path="${meld_path:-meld}"
 
-	if "$meld_path" --output /dev/null --help >/dev/null 2>&1
+	if "$meld_path" --help 2>&1 | grep -e --output >/dev/null
 	then
 		meld_has_output_option=true
 	else

1.7.9

I appreciate the interest you've all taken in my report, but I really don't think there's any need to do anything about this "problem" besides let people find this thread, in which they can learn to try upgrading their meld to one that's only 5 1/2 years old.

That said, another possibility is to test whether

    meld --commandline-option-that-cannot-possibly-exist --help

exits with status 0; if it does, then exit-status based probing of meld's capabilities won't work. In this case, assume --output is not available.

Jeff

I can confirm that this iteration of the patch (meld --help | grep) worked for me on meld 1.1.1, meld 1.1.5, and meld 1.3.0. Note however that none of these are versions of meld that do support the --output flag.

Jeff

Hmm, I might be mistaken, but aren't these string operations Bash-only? And AFAIK Git is striving for standard sh compatibility ...

Sebastian Schuberth

Hi,

They are widely supported in POSIX-style shells. See [1] and Documentation/CodingGuidelines:

- We use POSIX compliant parameter substitutions and avoid bashisms; namely:

- We use ${parameter-word} and its [-=?+] siblings, and their colon'ed "unset or null" form.

- We use ${parameter#word} and its [#%] siblings, and their doubled "longest matching" form.

A good way to catch these things is to try with dash or posh, which are a little less full-featured than bash and ksh.

Thanks for looking it over.
Jonathan

[1] http://pubs.opengroup.org/onlinepubs/9699919799/
Search for "sh -".