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

> 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.

_______________________________________________
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Reply via email to