diff mbox

support: properly check for bash as a dependency

Message ID 1395052954-1567-1-git-send-email-yann.morin.1998@free.fr
State Rejected
Headers show

Commit Message

Yann E. MORIN March 17, 2014, 10:42 a.m. UTC
From: "Yann E. MORIN" <yann.morin.1998@free.fr>

The way we are checking for bash is to look if "$SHELL --version"
will return a string containing "^GNU bash".

In case the system shell is dash (eg. /bin/sh -> /bin/dash), but
the user's login shell is bash, dash will not override the SHELL
variable:

    $ echo $SHELL
    /bin/bash
    $ /bin/dash
    $$ echo $SHELL
    /bin/bash

The same happens when called as the interpreter for a shell script:

    $ cat foo.sh
    #!/bin/dash
    echo $SHELL

    $ echo $SHELL
    /bin/bash
    $ ./foo.sh
    /bin/bash

So, calling "$SHELL --version" will still return "^GNU bash" no matter
what shell is actually running.

Since quite a lot of #!/bin/sh scripts are in fact bash scripts, we
really want to ensure that /bin/sh is bash.

Reported-by: Andrew Barnes <andy@outsideglobe.com> (on IRC)
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Andrew Barnes <andy@outsideglobe.com>
---
 support/dependencies/dependencies.sh | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

Comments

Peter Korsgaard March 17, 2014, 9:28 p.m. UTC | #1
>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:

 > From: "Yann E. MORIN" <yann.morin.1998@free.fr>
 > The way we are checking for bash is to look if "$SHELL --version"
 > will return a string containing "^GNU bash".

 > In case the system shell is dash (eg. /bin/sh -> /bin/dash), but
 > the user's login shell is bash, dash will not override the SHELL
 > variable:

 >     $ echo $SHELL
 >     /bin/bash
 >     $ /bin/dash
 >     $$ echo $SHELL
 >     /bin/bash

 > The same happens when called as the interpreter for a shell script:

 >     $ cat foo.sh
 >     #!/bin/dash
 >     echo $SHELL

 >     $ echo $SHELL
 >     /bin/bash
 >     $ ./foo.sh
 >     /bin/bash

 > So, calling "$SHELL --version" will still return "^GNU bash" no matter
 > what shell is actually running.

 > Since quite a lot of #!/bin/sh scripts are in fact bash scripts, we
 > really want to ensure that /bin/sh is bash.

Is that really still an issue? I just checked a few of the machines I
often do buildroot builds on and they all have /bin/sh == dash.


 > -# Check bash
 > -if ! $SHELL --version 2>&1 | grep -q '^GNU bash'; then
 > +# Check bash is the system shell
 > +if ! /bin/sh --version 2>&1 | grep -q '^GNU bash'; then

FYI, dash doesn't even understand a --version argument:

sh --version
sh: 0: Illegal option --
Yann E. MORIN March 17, 2014, 9:36 p.m. UTC | #2
Peter, All,

On 2014-03-17 22:28 +0100, Peter Korsgaard spake thusly:
> >>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:
> 
>  > From: "Yann E. MORIN" <yann.morin.1998@free.fr>
>  > The way we are checking for bash is to look if "$SHELL --version"
>  > will return a string containing "^GNU bash".
> 
>  > In case the system shell is dash (eg. /bin/sh -> /bin/dash), but
>  > the user's login shell is bash, dash will not override the SHELL
>  > variable:
> 
>  >     $ echo $SHELL
>  >     /bin/bash
>  >     $ /bin/dash
>  >     $$ echo $SHELL
>  >     /bin/bash
> 
>  > The same happens when called as the interpreter for a shell script:
> 
>  >     $ cat foo.sh
>  >     #!/bin/dash
>  >     echo $SHELL
> 
>  >     $ echo $SHELL
>  >     /bin/bash
>  >     $ ./foo.sh
>  >     /bin/bash
> 
>  > So, calling "$SHELL --version" will still return "^GNU bash" no matter
>  > what shell is actually running.
> 
>  > Since quite a lot of #!/bin/sh scripts are in fact bash scripts, we
>  > really want to ensure that /bin/sh is bash.
> 
> Is that really still an issue? I just checked a few of the machines I
> often do buildroot builds on and they all have /bin/sh == dash.

Some configure-y scripts will run with /bin/sh, but have bashisms in them.

It was reported on IRC by Andrew that switching the system shell from
dash to bash fixed an issue (Andrew, was that with libxml2?).

>  > -# Check bash
>  > -if ! $SHELL --version 2>&1 | grep -q '^GNU bash'; then
>  > +# Check bash is the system shell
>  > +if ! /bin/sh --version 2>&1 | grep -q '^GNU bash'; then
> 
> FYI, dash doesn't even understand a --version argument:

Yes, but it surely does not return something matching "^GNU bash" which
is all we care about.

Regards,
Yann E. MORIN.
Peter Korsgaard March 17, 2014, 10:01 p.m. UTC | #3
>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:

 > Peter, All,

 >> Is that really still an issue? I just checked a few of the machines I
 >> often do buildroot builds on and they all have /bin/sh == dash.

 > Some configure-y scripts will run with /bin/sh, but have bashisms in them.

 > It was reported on IRC by Andrew that switching the system shell from
 > dash to bash fixed an issue (Andrew, was that with libxml2?).

I don't think it is libxml2 as the stuff I'm currently doing at $WORK
uses libxml2 and I haven't seen any issues with /bin/sh = dash.


 >> > -# Check bash
 >> > -if ! $SHELL --version 2>&1 | grep -q '^GNU bash'; then
 >> > +# Check bash is the system shell
 >> > +if ! /bin/sh --version 2>&1 | grep -q '^GNU bash'; then
 >> 
 >> FYI, dash doesn't even understand a --version argument:

 > Yes, but it surely does not return something matching "^GNU bash" which
 > is all we care about.

True.
Thomas Petazzoni March 18, 2014, 5:01 a.m. UTC | #4
Dear Yann E. MORIN,

On Mon, 17 Mar 2014 11:42:34 +0100, Yann E. MORIN wrote:

> So, calling "$SHELL --version" will still return "^GNU bash" no matter
> what shell is actually running.
> 
> Since quite a lot of #!/bin/sh scripts are in fact bash scripts, we
> really want to ensure that /bin/sh is bash.

I'm definitely against that. My system has /bin/sh pointing to dash,
and Buildroot works fine. One of the thing that annoyed me in
OpenEmbedded was its requirements to have /bin/sh be bash.

We clearly don't want that.

I've just checked the Free Electrons autobuilders, and there are also
using dash as /bin/sh. This means that if configure scripts were using
bashims unsupported by dash, we would have noticed.

Thomas
Yann E. MORIN March 18, 2014, 4:55 p.m. UTC | #5
Thomas, All,

On 2014-03-18 06:01 +0100, Thomas Petazzoni spake thusly:
> On Mon, 17 Mar 2014 11:42:34 +0100, Yann E. MORIN wrote:
> 
> > So, calling "$SHELL --version" will still return "^GNU bash" no matter
> > what shell is actually running.
> > 
> > Since quite a lot of #!/bin/sh scripts are in fact bash scripts, we
> > really want to ensure that /bin/sh is bash.
> 
> I'm definitely against that. My system has /bin/sh pointing to dash,
> and Buildroot works fine. One of the thing that annoyed me in
> OpenEmbedded was its requirements to have /bin/sh be bash.
> 
> We clearly don't want that.

I'm just fine with that. :-)

But the current check is broken anyway, as all it checks is that the
login shell of the user is bash.

  - if we want to work whith dash as the system shell, then the current
    check is unneeded;

  - if we want to work with dash as the system shell, we don't care what
    login shell the user is using, as we must also work when this is dash;

  - SHELL is not mandated by POSIX, so it may be empty on a
    POSIX-compliant shell anyway.

So, we should just remove this check altogether.

> I've just checked the Free Electrons autobuilders, and there are also
> using dash as /bin/sh. This means that if configure scripts were using
> bashims unsupported by dash, we would have noticed.

Andrew, what package was the breakage due to?

Regards,
Yann E. MORIN.
Thomas Petazzoni March 18, 2014, 6:27 p.m. UTC | #6
Dear Yann E. MORIN,

On Tue, 18 Mar 2014 17:55:18 +0100, Yann E. MORIN wrote:

> > I'm definitely against that. My system has /bin/sh pointing to dash,
> > and Buildroot works fine. One of the thing that annoyed me in
> > OpenEmbedded was its requirements to have /bin/sh be bash.
> > 
> > We clearly don't want that.
> 
> I'm just fine with that. :-)
> 
> But the current check is broken anyway, as all it checks is that the
> login shell of the user is bash.
> 
>   - if we want to work whith dash as the system shell, then the current
>     check is unneeded;
> 
>   - if we want to work with dash as the system shell, we don't care what
>     login shell the user is using, as we must also work when this is dash;
> 
>   - SHELL is not mandated by POSIX, so it may be empty on a
>     POSIX-compliant shell anyway.
> 
> So, we should just remove this check altogether.

Yes, I agree.

Thomas
Arnout Vandecappelle March 20, 2014, 8:43 p.m. UTC | #7
On 18/03/14 17:55, Yann E. MORIN wrote:
> Thomas, All,
> 
> On 2014-03-18 06:01 +0100, Thomas Petazzoni spake thusly:
>> On Mon, 17 Mar 2014 11:42:34 +0100, Yann E. MORIN wrote:
>>
>>> So, calling "$SHELL --version" will still return "^GNU bash" no matter
>>> what shell is actually running.

 Have you tested this? dependencies.sh is run from the Makefile, and that
says:

SHELL:=$(shell if [ -x "$$BASH" ]; then echo $$BASH; \
        else if [ -x /bin/bash ]; then echo /bin/bash; \
        else echo sh; fi; fi)
[snip]

export SHELL CONFIG_SHELL quiet Q KBUILD_VERBOSE VERBOSE

 So $SHELL _will_ be set to bash unless there is no bash.

>>>
>>> Since quite a lot of #!/bin/sh scripts are in fact bash scripts, we
>>> really want to ensure that /bin/sh is bash.
>>
>> I'm definitely against that. My system has /bin/sh pointing to dash,
>> and Buildroot works fine. One of the thing that annoyed me in
>> OpenEmbedded was its requirements to have /bin/sh be bash.
>>
>> We clearly don't want that.
> 
> I'm just fine with that. :-)
> 
> But the current check is broken anyway, as all it checks is that the
> login shell of the user is bash.
> 
>   - if we want to work whith dash as the system shell, then the current
>     check is unneeded;
> 
>   - if we want to work with dash as the system shell, we don't care what
>     login shell the user is using, as we must also work when this is dash;
> 
>   - SHELL is not mandated by POSIX, so it may be empty on a
>     POSIX-compliant shell anyway.
> 
> So, we should just remove this check altogether.

 Not remove, but rather replace it with a check for /bin/bash existence.
We do have quite a lot of stuff referring to /bin/bash. bash in PATH is
not good enough.


 Regards,
 Arnout

> 
>> I've just checked the Free Electrons autobuilders, and there are also
>> using dash as /bin/sh. This means that if configure scripts were using
>> bashims unsupported by dash, we would have noticed.
> 
> Andrew, what package was the breakage due to?
> 
> Regards,
> Yann E. MORIN.
>
diff mbox

Patch

diff --git a/support/dependencies/dependencies.sh b/support/dependencies/dependencies.sh
index a8261b3..1b1a825 100755
--- a/support/dependencies/dependencies.sh
+++ b/support/dependencies/dependencies.sh
@@ -138,11 +138,16 @@  if [ ! -z "$CXXCOMPILER" ] ; then
 	fi
 fi
 
-# Check bash
-if ! $SHELL --version 2>&1 | grep -q '^GNU bash'; then
+# Check bash is the system shell
+if ! /bin/sh --version 2>&1 | grep -q '^GNU bash'; then
 	echo
-	echo "You must install 'bash' on your build machine";
-	exit 1;
+	echo "You must install 'bash' on your build machine"
+	echo "and make it your default shell. On Debian-like"
+	echo "systems, this can be done with:"
+	echo "    sudo dpkg-reconfigure dash"
+	echo "and answer 'No' to the question:"
+	echo "    Use dash as the default system shell (/bin/sh)?"
+	exit 1
 fi;
 
 # Check that a few mandatory programs are installed