[openstack-dev] [tripleo] /bin/bash vs. /bin/sh

Ben Nemec openstack at nemebean.com
Tue Apr 15 20:16:38 UTC 2014


On 04/15/2014 02:44 PM, Clint Byrum wrote:
> Excerpts from Ben Nemec's message of 2014-04-14 09:26:17 -0700:
>> tldr: I propose we use bash explicitly for all diskimage-builder scripts
>> (at least for the short-term - see details below).
>>
>> This is something that was raised on my linting changes to enable set -o
>> pipefail.  That is a bash-ism, so it could break in the
>> diskimage-builder scripts that are run using /bin/sh.  Two possible
>> fixes for that: switch to /bin/bash, or don't use -o pipefail
>>
>
> What about this:
>
> if ! [ "$SHEBANG" = "#!/bin/bash" ] ; then
>    report_warning Non bash shebang, skipping script lint
> fi

I was thinking along the same lines, although at least for the moment I 
would like to leave the +x check enabled for all shebangs since it still 
doesn't make sense to have a shebang without +x.

>
>> But I think this raises a bigger question - does diskimage-builder
>> require bash?  If so, I think we should just add a rule to enforce that
>> /bin/bash is the shell used for everything.  I know we have a bunch of
>> bash-isms in the code already, so at least in the short-term I think
>> this is probably the way to go, so we can get the benefits of things
>> like -o pipefail and lose the ambiguity we have right now.  For
>> reference, a quick grep of the diskimage-builder source shows we have
>> 150 scripts using bash explicitly and only 24 that are plain sh, so
>> making the code truly shell-agnostic is likely to be a significant
>> amount of work.
>
> Yes, diskimage-builder is bash, not posix shell. We're not masochists.
> ;)
>
>>
>> In the long run it might be nice to have cross-shell compatibility, but
>> if we're going to do that I think we need a couple of things: 1) Someone
>> to do the work (I don't have a particular need to run dib in not-bash,
>> so I'm not signing up for that :-) 2) Testing in other shells -
>> obviously just changing /bin/bash to /bin/sh doesn't mean we actually
>> support anything but bash.  We really need to be gating on other shells
>> if we're going to make a significant effort to support them.  It's not
>> good to ask reviewers to try to catch every bash-ism proposed in a
>> change.  This also relates to some of the unit testing work that is
>> going on right now too - if we had better unit test coverage of the
>> scripts we would be able to do this more easily.
>>
>
> I suggest that diskimage-builder's included elements should be /bin/bash
> only. When we have an element linting tool, non bash shebangs should be
> warnings we should enforce "no warnings". For t-i-e, we can strive for
> no warnings, but that would be a stretch goal and may involve refining
> the warnings.

This doesn't seem to be a problem in dib - almost everything was 
explicitly bash already, and the scripts that weren't are pretty trivial.

The ramdisk init script remains a thorn in my side for this though - 
based on our conversation last Friday we don't want that to have the 
same set -eu behavior as the other scripts (since init failing causes a 
kernel panic), and based on Chris's comment in 
http://lists.openstack.org/pipermail/openstack-dev/2014-April/032753.html we 
don't even want that to be explicitly a bash script, except that it is 
today.  So I think we need an exception mechanism like the pep8 #noqa 
tag (or something similar) to note that init should basically be ignored 
by the lint checks since it needs to violate most of the current ones.

For the moment, I'm thinking I'll silently ignore /bin/sh scripts, 
convert init to be one of those, and work on the warning/exception 
mechanism in the future.

-Ben



More information about the OpenStack-dev mailing list