From: Tim Henigan <tim.henigan@gmail.com>
Date: Wed, 22 Feb 2012 20:14:43 GMT
Subject: Re: [PATCH] contrib: added git-diffall
Message-ID: <CAFouetgyYRKT7h1h4hv40Kxp=ibq1Hw-1mzFe6DsyAUknXKyYQ@mail.gmail.com>
In-Reply-To: <CAJDDKr72rPqp1z-12Ht3Q2UaeUVFutKwoOgkD1G+SbhsBs+p1A@mail.gmail.com>

On Wed, Feb 22, 2012 at 4:15 AM, David Aguilar <davvid@gmail.com> wrote:
> On Tue, Feb 21, 2012 at 6:41 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Tim Henigan <tim.henigan@gmail.com> writes:
>>
>> David, any idea on this?
>
> I don't see any bash-isms there myself, either.  We should keep this
> stuff without bash-isms.
> I haven't had time to read these patches in depth yet but will try to
> read the re-roll.
>
> Can we ask the github user to elaborate on what exactly was erroring out?
> Does dash not handle || inside $()?  We can only make wild guesses
> without their help.
>
> The only hint from the pull request is "silent exit with no results".
> Do we do that?
> There are a few code paths where we do "exit 1" but that's only under
> error conditions.
>
> We haven't had any reports about git-mergetool/difftool, which use
> these functions...
> are we certain the problem was not some other bash-isms in the script?

I have not heard back from the user, but I tested on Ubuntu earlier
today.  I found that when using an older version of the script
(fbedb7a in the GitHub repo), I could repeat the error.  It is the
'git-diffall' script that fails silently, I believe due to a bash-ism
that was fixed in a subsequent commit.

In my latest local version of the script, I have switched back to
#!/bin/sh.  It runs successfully, so I will squash the change into v2
of the patch.

For what it is worth, since that bug was originally reported, I have
been running "checkbashisms" [1] on the git-diffall script.  That
utility reports that the script is clean.

-Tim

[1]: http://sourceforge.net/projects/checkbaskisms/