Scripting guidelines

Fri Feb 21 09:39:54 UTC 2014

El 21/02/14 00:37, Dan Kenigsberg escribió:
> On Thu, Feb 20, 2014 at 09:52:23PM +0100, David Caro wrote:
>> Hi everyone!
>>
>> Lately I've had a hard time to properly review some patches containing shell
>> scripts to manage our infrastructure because there's no guidelines. So I created
>> a wiki page with a proposal [1]. It's made up as a mix of some already existing
>> guidelines.
>>
>> The reason to wrote a bash style guide and not a shell stile guide is because I
>> think that bash is widely adopted (default GNU shell) and provides enough
>> advantages to sacrifice some portability. I think that most of our maintenance
>> and management scripts will never be run on non-GNU OSes.
> 
> What are the advantages in your opinion? I like the $() construct and
> local variables. Associative arrays can come up handy.

* Less prone to errors:
  - [[ ]] builtin constructs make it really easy to avoid having errors when
spaces are in the tested variable:
      myfile="/thi/is my/file"
      [[ -f $myfile ]]  <- this does not break and works as expected
      [ -f $myfile ]    <- this breaks
  - Long options are really nice and make code clearer
  - Arrays are useful
  - Associative arrays are useful too
  - String expansions with brackets
     mv myfile{,.bkp}
  - Pattern substitution:
     ${myvar/mypatt/mysub}
  - Substring expansion:
     ${myvar:offset:length}
  - Indirect variable expansion (don't use it if you don't need it, like eval):
     ${!myvar}
  - $PIPESTATUS array, with the return codes of the commands executed in a pipe
  - options in echo (echo -e, echo -n, ...)
  - local variables definition, there's no local in POSIX:
     local a b c  <- this is not POSIX
  - source command (a lot clearer than '.', harder to confuse with execution)
  - select command to create menus
  - the time builtin (not a builtin in POSIX)
  - Process substitution redirection:
	foo <( bar )
  - '==' equality test instead of '=' (A lot more readable and less confusing)

  - '<>' lexicographical comparison
  - Check if 2 files are the same hard link:
	[[ $file1 -ef $file2 ]]
  - Redirecting stdin and stdout:
    &>, >& and &|
  - duplication and closure of file descriptions:
     m>&n-
  - Herestrings ('<<<')
  - A lot of the shopt options
  - FUNCNAME to get current function name
  - REGEXP matching test (=~)

Those are a few, there are more differences but some of them I don't recommend
(that's why we are creating a style guide, to use only the ones that really help)

> 
>>
>> POSIX compliance should be only used when really needed, for example, scripts to
>> build a specific project, that might be run on non-GNU based systems in the far
>> future.
> 
> I have the opposite sentiment - diversions from the standard should be
> kept small and well-justified.

I heard this before from Alon, in my opinion, both of you have the wrong idea of
what a standard is and when to apply it.

POSIX standard is a standard created to allow portability between operating
systems, that's it's goal. The infrastructure scripts will most likely never be
run outside the GNU family of operating systems
(fedora/centos/ubuntu/debian/suse/gentoo ...) so the portability is not
something that matters.

Thus, the application of the POSIX standard is too restrictive. Imagine it like
building your house in Norway with the same standards that are used in Tokyo,
you don't need protection from eathquakes, you need heat efficiency.

The same way, we don't need portability, we need flexibility and error avoidance.

As I said also, at the end of the script and at the end of the wiki, if you are
gonna distribute a script with the product, then POSIX standard is the one that
should be applied.

> 
>>
>> This thread is to start a discussion about it so please, share your opinions and
>> concerns (and proposals).
>>
>>
>> [1] http://www.ovirt.org/Bash_style_guide
> 
> Could you detail where [1] is different from the
> http://wiki.bash-hackers.org/scripting/style that it cites?

Some of the differences are:
  - Indentation (I said 4 spaces and that the consistency in using tabs or
spaces throughout the script is more important than using spaces)
  - I let case constructs to fit in one line if they are short (one command)
  - Added the 'local' usage on the function variables
  - Recommended to use function parameters instead of global variables
  - Recommended to specify a return statement explicitly
  - Using builtins instead of external commands when possible
  - Added portability note

Most of those are from here, a german linux user group:
http://lug.fh-swf.de/vim/vim-bash/StyleGuideShell.en.pdf

And I want to finish looking that one up and incorporate the best advice from
there too (the testing section is nice, and the comments)

> 
> BTW, it's missing the most important rule, that can turn the shell into
> a programming language: the -e option. Not using it is basically
> equivalent to wrapping every python line within its own try-except-pass
> block.

-e option is useful yes, but not always so helpful, agree that in the best case
you should use it. But sometimes is not so simple and people end up writing '||
:' at the end of each command.

I planned on adding more about bash options to the wiki, and also some
debugging, but did not have the time. Point taken.

> When I educate people I do this to allow people to contribute to any project.
> In our case this will enable people to contribute to infra and product.
>
> Having your own conventions to infra which are not following product standard
> will make it difficult to reuse people skills.

Let's write everything in javascript so we can all contribute to engine and is
the only language that can fit everywhere (ui included)... the goal of the
product is different from the goal of infra, the tools should be the best suited
for the goals of the team, and if we can choose the same tools without loosing
focus towards our goals then we should.

I'm not sure either of the percentage of shell scripts that are there in
product, but I'd say that it's not that much, as most of it are make scripts and
relatives.

>
> POSIX code is not less readable, one just need to accept that we follow
> standards and everything else will be aligned.

I don't agree, POSIX standard makes code a lot less readable if not using
'extensions':

  ssh -tNL 1234:127.0.0.1:4321	
  ### Imaginary non-POSIX compliant ssh that uses GNU extensions
  ssh --force-tty --no-command --local-tunnel 1234:127.0.0.1:4321

  [[ "${myvar: -1}" == "/" ]] ## the variable ends with '/'
  [ "$(printf "$myvar" | tail -c 1)" = "/" ]  ## remember that echo -n is not
posix compliant

  do_something &>something.log
  do_something 1>something.log 2>&1

Or any case where you have to go to sed/awk/python/ruby to get rid of the POSIX
restrictions and use their own programming languages. Having to understand 2
languages to read a script is less readable than having to understand 1 imo.

Having to expose all the variables as global is also less readable and a lot
more prone to errors too.

  do_whatever() {
      for i in one two; do
         echo "inside i = $i"
      done
  }

  for i in three four; do
    do_whatever
    echo "outside i = $i"
  done

The output of that is:
inside i = one
inside i = two
outside i = two
inside i = one
inside i = two
outside i = two

The non-POSIX equivalent using local does what you expected:
  do_whatever() {
      local i
      for i in one two; do
         echo "inside i = $i"
      done
  }

  for i in three four; do
    do_whatever
    echo "outside i = $i"
  done

inside i = one
inside i = two
outside i = three
inside i = one
inside i = two
outside i = four

The fact that you expect the code to do something and it does something else
goes inside non readable to me (because it's not easy to understand what it does
reading it).

-- 
David Caro

Red Hat S.L.
Continuous Integration Engineer - EMEA ENG Virtualization R&D

Email: dcaro at redhat.com
Web: www.redhat.com
RHT Global #: 82-62605

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: OpenPGP digital signature
URL: <http://lists.ovirt.org/pipermail/infra/attachments/20140221/bc0f09d4/attachment.sig>