Re: [RFC/PATCH] tests: add initial bash completion tests

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Apr 6, 2012, 23:30 UTC
Message-ID: <CAMP44s3CA9e06QoJ_-QWoEevfY==CgqZrUu1EKn3zeVDdAyvOA@mail.gmail.com>
In-Reply-To: <7vk41szdev.fsf@alter.siamese.dyndns.org>

On Sat, Apr 7, 2012 at 1:46 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 15 quoted lines

> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Sat, Apr 7, 2012 at 12:34 AM, Jeff King <peff@peff.net> wrote:
>>
>>> But my point still stands that you cannot assume that you are running
>>> bash, and you need to either find bash, or gracefully exit the test
>>> script if it is not available. Anything else will cause "make test" to
>>> fail on some systems (and indeed, applying and running your test, it
>>> breaks "make test" on my debian box with dash as /bin/sh).
>>
>> So? 'make test' fails on my Arch Linux box which doesn't have
>> /usr/bin/python, which is presumably why there is NO_PYTHON.
>
> If you "git grep NO_PYTHON", you would notice that t/test-lib.sh does have
> a provision to set PYTHON prerequisite,

Yes, but you have to specify NO_PYTHON, otherwise it would just assume python is installed.

> so fixing it presumably is just
> the matter of marking appropriate tests with the prerequisite, no?

You would still have to specify NO_PYTHON.

> Which ones are broken for you?

In fact, I do have python, but the program name is python2, but if I do this:

 export PYTHON_PATH=/usr/bin/python2

The tests fail on remote-helpers.

> Complaining about it in a thread that does not
> directly related to that "on a box without python some tests are broken"
> issue is a very good way to leave it unfixed.

My point is that the argument "this is worst than nothing because tests would break on X systems" seems rather weak to me considering that tests are already broken on Y systems.

Show 10 quoted lines

>> Instead
>> of doing some nasty hacks such as 'bash <<\END_OF_BASH_SCRIPT', it
>> would be much easier to have a NO_BASH option. And in fact, when zsh
>> completion tests are available, NO_ZSH (probably on by default).
>
> "The box does not have bash" and "The builder does not wish to let the
> scripts run with bash" are two separate things.  SHELL_PATH is often set
> to /bin/dash even on a box that has /bin/bash because it is much faster to
> run scripted Porcelains with, but the end user of the resulting build may
> still want to use bash in her interactive session.

So? SHELL_PATH would not interfere with NO_BASH/NO_ZSH. But in any case, I've sent a patch that detects bash presence, similarly to Jeff's patch.

Cheers.

-- 
Felipe Contreras