Patchwork [2,of,2,V2] tests: don't hardcode path to bash interpreter

login
register
mail settings
Submitter Olle Lundberg
Date March 26, 2014, 11:02 a.m.
Message ID <c0c158e277f7b7a33c51.1395831765@SE-C02KQ0DADR55>
Download mbox | patch
Permalink /patch/4072/
State Accepted
Commit 5d57b2101ab119bce762025b8d83a00db7263e3b
Headers show

Comments

Olle Lundberg - March 26, 2014, 11:02 a.m.
# HG changeset patch
# User Olle Lundberg <geek@nerd.sh>
# Date 1395785272 -3600
#      Tue Mar 25 23:07:52 2014 +0100
# Node ID c0c158e277f7b7a33c51d0ee163c42345c70d84d
# Parent  6e8b538637302e06a3510d15e36c30757f8501a2
tests: don't hardcode path to bash interpreter

Use the env binary to figure out the correct bash to use.
Certain systems ships with an ancient version of bash, but the
user might have installed a newer one that is earlier in $PATH.

For example the current version of Mac OS X ships version 3.2.51
of bash, which does not understand new fancy builtins such as
readarray. A user might install a newer version of bash, use that
as their shell and add that path before bin.
Martin Geisler - March 26, 2014, 11:19 a.m.
Olle Lundberg <olle.lundberg@gmail.com> writes:

> # HG changeset patch
> # User Olle Lundberg <geek@nerd.sh>
> # Date 1395785272 -3600
> #      Tue Mar 25 23:07:52 2014 +0100
> # Node ID c0c158e277f7b7a33c51d0ee163c42345c70d84d
> # Parent  6e8b538637302e06a3510d15e36c30757f8501a2
> tests: don't hardcode path to bash interpreter
>
> Use the env binary to figure out the correct bash to use.
> Certain systems ships with an ancient version of bash, but the
> user might have installed a newer one that is earlier in $PATH.
>
> For example the current version of Mac OS X ships version 3.2.51
> of bash, which does not understand new fancy builtins such as
> readarray. A user might install a newer version of bash, use that
> as their shell and add that path before bin.

Okay, but couldn't one object that the scripts we distribute shouldn't
use new fancy features?

Infact, it seems like it would be better if we used #!/bin/sh instead
and wrote the scripts so they work on a wide vararity of machines.

I haven't really looked at the shell scripts, so I don't know if we
already use non-standard features or if it would be difficult to rewrite
them to a simpler form of shell script.
Olle Lundberg - March 26, 2014, 11:37 a.m.
On Wed, Mar 26, 2014 at 12:19 PM, Martin Geisler <martin@geisler.net> wrote:

> Olle Lundberg <olle.lundberg@gmail.com> writes:
>
> > # HG changeset patch
> > # User Olle Lundberg <geek@nerd.sh>
> > # Date 1395785272 -3600
> > #      Tue Mar 25 23:07:52 2014 +0100
> > # Node ID c0c158e277f7b7a33c51d0ee163c42345c70d84d
> > # Parent  6e8b538637302e06a3510d15e36c30757f8501a2
> > tests: don't hardcode path to bash interpreter
> >
> > Use the env binary to figure out the correct bash to use.
> > Certain systems ships with an ancient version of bash, but the
> > user might have installed a newer one that is earlier in $PATH.
> >
> > For example the current version of Mac OS X ships version 3.2.51
> > of bash, which does not understand new fancy builtins such as
> > readarray. A user might install a newer version of bash, use that
> > as their shell and add that path before bin.
>
> Okay, but couldn't one object that the scripts we distribute shouldn't
> use new fancy features?
>
> Yes and no. The most important part is actually about being a good unix
citizen and honour the environment a sysadmin have setup.
We should really invoke {ba,}sh through /usr/bin/env {ba,}sh and not
hardcode an interpreter.


> Infact, it seems like it would be better if we used #!/bin/sh instead
> and wrote the scripts so they work on a wide vararity of machines.
>
Sure, when will you patch that?


>
> I haven't really looked at the shell scripts, so I don't know if we
> already use non-standard features or if it would be difficult to rewrite
> them to a simpler form of shell script.
>
> --
> Martin Geisler
>
David Soria Parra - March 26, 2014, 4:18 p.m.
Martin Geisler <martin@geisler.net> writes:

> Olle Lundberg <olle.lundberg@gmail.com> writes:
>
>> # HG changeset patch
>> # User Olle Lundberg <geek@nerd.sh>
>> # Date 1395785272 -3600
>> #      Tue Mar 25 23:07:52 2014 +0100
>> # Node ID c0c158e277f7b7a33c51d0ee163c42345c70d84d
>> # Parent  6e8b538637302e06a3510d15e36c30757f8501a2
>> tests: don't hardcode path to bash interpreter
>>
>> Use the env binary to figure out the correct bash to use.
>> Certain systems ships with an ancient version of bash, but the
>> user might have installed a newer one that is earlier in $PATH.
>>
>> For example the current version of Mac OS X ships version 3.2.51
>> of bash, which does not understand new fancy builtins such as
>> readarray. A user might install a newer version of bash, use that
>> as their shell and add that path before bin.
>
> Okay, but couldn't one object that the scripts we distribute shouldn't
> use new fancy features?
Yes we should try to be /bin/sh compatible.

> Infact, it seems like it would be better if we used #!/bin/sh instead
> and wrote the scripts so they work on a wide vararity of machines.
>
> I haven't really looked at the shell scripts, so I don't know if we
> already use non-standard features or if it would be difficult to rewrite
> them to a simpler form of shell script.
revsetbenchmark.sh does use bash arrays which is a bash 4. I agree
we should work on more compatible shell scripts as at least some unixes
like Solaris & co won't have a /bin/bash. Also there are plenty of
pre-bash 4 RHEL, etc around. I think olles approach is sensible
though. Until we fix the scripts to not use bash, we should use
/usr/bin/env to find bash.
David Soria Parra - March 26, 2014, 4:50 p.m.
Olle Lundberg <olle.lundberg@gmail.com> writes:

> # HG changeset patch
> # User Olle Lundberg <geek@nerd.sh>
> # Date 1395785272 -3600
> #      Tue Mar 25 23:07:52 2014 +0100
> # Node ID c0c158e277f7b7a33c51d0ee163c42345c70d84d
> # Parent  6e8b538637302e06a3510d15e36c30757f8501a2
> tests: don't hardcode path to bash interpreter
>
> Use the env binary to figure out the correct bash to use.
> Certain systems ships with an ancient version of bash, but the
> user might have installed a newer one that is earlier in $PATH.
>
> For example the current version of Mac OS X ships version 3.2.51
> of bash, which does not understand new fancy builtins such as
> readarray. A user might install a newer version of bash, use that
> as their shell and add that path before bin.
>
> diff --git a/tests/bundles/rebase.sh b/tests/bundles/rebase.sh
> --- a/tests/bundles/rebase.sh
> +++ b/tests/bundles/rebase.sh
> @@ -1,6 +1,6 @@
> -#!/bin/bash
> +#!/usr/bin/env bash
>  hg init rebase
>  cd rebase
>  
>  #  @  7: 'H'
>  #  |
> diff --git a/tests/bundles/remote.sh b/tests/bundles/remote.sh
> --- a/tests/bundles/remote.sh
> +++ b/tests/bundles/remote.sh
> @@ -1,6 +1,6 @@
> -#!/bin/bash
> +#!/usr/bin/env bash
>  hg init remote
>  cd remote
>  
>  echo "0" >> afile
>  hg add afile

As discussed, I think the sensible thing is to get rid of /bin/bash
wherever possible and use sh instead. Nevertheless we should use
/usr/bin/env for portability anyway. So I queued that. Thank you!

Patch

diff --git a/tests/bundles/rebase.sh b/tests/bundles/rebase.sh
--- a/tests/bundles/rebase.sh
+++ b/tests/bundles/rebase.sh
@@ -1,6 +1,6 @@ 
-#!/bin/bash
+#!/usr/bin/env bash
 hg init rebase
 cd rebase
 
 #  @  7: 'H'
 #  |
diff --git a/tests/bundles/remote.sh b/tests/bundles/remote.sh
--- a/tests/bundles/remote.sh
+++ b/tests/bundles/remote.sh
@@ -1,6 +1,6 @@ 
-#!/bin/bash
+#!/usr/bin/env bash
 hg init remote
 cd remote
 
 echo "0" >> afile
 hg add afile