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

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

[Jeff Epler]

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

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

[David Aguilar]

On Thu, Feb 9, 2012 at 11:17 AM, Jeff Epler <jepler@unpythonic.net> wrote:
> 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\"

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

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

[Jonathan Nieder]

David Aguilar wrote:
> On Thu, Feb 9, 2012 at 11:17 AM, Jeff Epler <jepler@unpythonic.net> wrote:

>> 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).
[...]
> We originally used the --output test so that we wouldn't have to check
> for a specific version.

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 <steve9000@users.sf.net>"
 - [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".)

Reported-by: Jeff Epler <jepler@unpythonic.net>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 mergetools/meld |   17 +++++++++++------
 1 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/mergetools/meld b/mergetools/meld
index eaa115cc..3de29629 100644
--- a/mergetools/meld
+++ b/mergetools/meld
@@ -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

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

[Sebastian Schuberth]

On Fri, Feb 10, 2012 at 09:23, Jonathan Nieder <jrnieder@gmail.com> wrote:

> +       meld_version=${meld_version#GNOME }
> +       meld_version=${meld_version#* }

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

-- 
Sebastian Schuberth

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

[Jonathan Nieder]

Hi,

Sebastian Schuberth wrote:
> On Fri, Feb 10, 2012 at 09:23, Jonathan Nieder <jrnieder@gmail.com> wrote:

>> +       meld_version=${meld_version#GNOME }
>> +       meld_version=${meld_version#* }
>
> Hmm, I might be mistaken, but aren't these string operations
> Bash-only? And AFAIK Git is striving for standard sh compatibility ...

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 -".

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

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> Just parse version numbers instead.  We can detect the version number
> by running "meld --version" and postprocessing it.

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
}

[PATCH] mergetools/meld: Use --help output to detect --output support

[Jonathan Nieder]

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.

Reported-by: Jeff Epler <jepler@unpythonic.net>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Junio C Hamano wrote:

> 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?

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(-)

diff --git a/mergetools/meld b/mergetools/meld
index eaa115cc..cb672a55 100644
--- a/mergetools/meld
+++ b/mergetools/meld
@@ -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

Re: [PATCH] mergetools/meld: Use --help output to detect --output support

[Jeff Epler]

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

On Fri, Feb 10, 2012 at 03:57:55PM -0600, Jonathan Nieder wrote:
> 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.
> 
> Reported-by: Jeff Epler <jepler@unpythonic.net>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
> Junio C Hamano wrote:
> 
> > 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?
> 
> 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(-)
> 
> diff --git a/mergetools/meld b/mergetools/meld
> index eaa115cc..cb672a55 100644
> --- a/mergetools/meld
> +++ b/mergetools/meld
> @@ -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
> 
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mergetools/meld: Use --help output to detect --output support

[Jeff Epler]

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.

Tested-by: Jeff Epler <jepler@unpythonic.net>

Jeff