Re: [PATCH] contrib: added git-diffall

From: Tim Henigan <tim.henigan@gmail.com>
Date: Feb 22, 2012, 20:14 UTC
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:

Show 22 quoted lines

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