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

login
register
mail settings
Submitter Olle Lundberg
Date March 26, 2014, 11:02 a.m.
Message ID <6e8b538637302e06a351.1395831764@SE-C02KQ0DADR55>
Download mbox | patch
Permalink /patch/4071/
State Accepted
Commit 864c56cb8945c8ea002c11a115fdf8206dd6c385
Headers show

Comments

Olle Lundberg - March 26, 2014, 11:02 a.m.
# HG changeset patch
# User Olle Lundberg <geek@nerd.sh>
# Date 1395831553 -3600
#      Wed Mar 26 11:59:13 2014 +0100
# Node ID 6e8b538637302e06a3510d15e36c30757f8501a2
# Parent  2a14a2e1ec78f2950da46fedb70686278d90620e
contrib: 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.
Julien Cristau - March 26, 2014, 11:17 a.m.
On Wed, Mar 26, 2014 at 12:02:44 +0100, Olle Lundberg wrote:

> # HG changeset patch
> # User Olle Lundberg <geek@nerd.sh>
> # Date 1395831553 -3600
> #      Wed Mar 26 11:59:13 2014 +0100
> # Node ID 6e8b538637302e06a3510d15e36c30757f8501a2
> # Parent  2a14a2e1ec78f2950da46fedb70686278d90620e
> contrib: 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.
> 
Why does that mean these scripts shouldn't use the bash version in /bin?
Do they need any fancy new bash features?

Cheers,
Julien
Olle Lundberg - March 26, 2014, 11:34 a.m.
On Wed, Mar 26, 2014 at 12:17 PM, Julien Cristau
<julien.cristau@logilab.fr>wrote:

> On Wed, Mar 26, 2014 at 12:02:44 +0100, Olle Lundberg wrote:
>
> > # HG changeset patch
> > # User Olle Lundberg <geek@nerd.sh>
> > # Date 1395831553 -3600
> > #      Wed Mar 26 11:59:13 2014 +0100
> > # Node ID 6e8b538637302e06a3510d15e36c30757f8501a2
> > # Parent  2a14a2e1ec78f2950da46fedb70686278d90620e
> > contrib: 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.
> >
> Why does that mean these scripts shouldn't use the bash version in /bin?
> Do they need any fancy new bash features?
>

Yes, readarray/mapfile arrived in bash 4.

We could probably bail out on version instead. But i see it as good
behaviour to honour what the sysadmin/user wants us to use as bash, as long
as it is bash (just as we do with the hg binary and python):
head -n1 hg
#!/usr/bin/env python

Yes i do know about setuptools and that is replaces it, with the one that
invoked setup.py, but it is still the same concept.

To hardcode the path to the interpreter is just bad behaviour (and stupid).
Whatever way we chose to go the invocation should be through env (be it sh
or bash), else your scripts become bad unix citizens.


> Cheers,
> Julien
> --
> Julien Cristau          <julien.cristau@logilab.fr>
> Logilab                 http://www.logilab.fr/
> Informatique scientifique & gestion de connaissances
>
Julien Cristau - March 26, 2014, 11:48 a.m.
On Wed, Mar 26, 2014 at 12:34:32 +0100, Olle wrote:

> To hardcode the path to the interpreter is just bad behaviour (and stupid).

I'm afraid I have to disagree with that...

Cheers,
Julien
David Soria Parra - March 26, 2014, 4:05 p.m.
Julien Cristau <julien.cristau@logilab.fr> writes:

> On Wed, Mar 26, 2014 at 12:02:44 +0100, Olle Lundberg wrote:
>
>> # HG changeset patch
>> # User Olle Lundberg <geek@nerd.sh>
>> # Date 1395831553 -3600
>> #      Wed Mar 26 11:59:13 2014 +0100
>> # Node ID 6e8b538637302e06a3510d15e36c30757f8501a2
>> # Parent  2a14a2e1ec78f2950da46fedb70686278d90620e
>> contrib: 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.
>> 
> Why does that mean these scripts shouldn't use the bash version in /bin?
> Do they need any fancy new bash features?

Operating systems like Solaris 10 don't necessrily have a /bin/bash.

Patch

diff --git a/contrib/editmerge b/contrib/editmerge
--- a/contrib/editmerge
+++ b/contrib/editmerge
@@ -1,6 +1,6 @@ 
-#!/bin/bash
+#!/usr/bin/env bash
 # A simple script for opening merge conflicts in the editor.
 # Use the following Mercurial settings to enable it.
 #
 # [ui]
 # merge = editmerge
diff --git a/contrib/revsetbenchmarks.sh b/contrib/revsetbenchmarks.sh
--- a/contrib/revsetbenchmarks.sh
+++ b/contrib/revsetbenchmarks.sh
@@ -1,6 +1,6 @@ 
-#!/bin/bash
+#!/usr/bin/env bash
 
 # Measure the performance of a list of revsets against multiple revisions
 # defined by parameter. Checkout one by one and run perfrevset with every
 # revset in the list to benchmark its performance.
 #