Update CODING_STYLE to exploit modern bash features

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

diff --git a/CODING_STYLE b/CODING_STYLE
index a18b75d7b83bec9b481c0e07ca7159a937898ec1..8e192f8b45f8e618d0d9a13df4e60f7efabec188 100644 (file)
--- a/CODING_STYLE
+++ b/CODING_STYLE
@@ -5,9 +5,7 @@ RAISIN CODING STYLE
 Shell version compliance
 ------------------------
 
-Although we are requiring bash to run at the moment, please don't make
-assumption on the bash version available. Use modern features only when
-no older constructs do what you need.
+Raisin requires BASH 3.2 or newer, use BASH features accordingly.
 
 
 Indentation
@@ -33,12 +31,13 @@ function do_something () {
     echo something
 }
 
+Prepend _ to the function name if it is a local function.
+
 
 Tests
 -----
 
-Use "test" to make a test. Do not use [. Do not use [[ unless you have a
-good reason.
+Use [[ for tests. Do not use test or [.
 
 
 Subshell
@@ -50,13 +49,13 @@ Use `` to execute a command in a subshell.
 Numeric tests
 -------------
 
-Use expr for calculations.
+Use $(( )) for calculations.
 
 
 Awk
 ---
 
-Use cut, tr, bash 4+ and sed instead of awk when possible, in this order.
+Use bash 3 features, cut, tr and sed instead of awk when possible.
 
 
 Block structure
@@ -72,7 +71,7 @@ done
 
 if statements, place then on a new line:
 
-if test -z "$VAR"
+if [[ -z "$VAR" ]]
 then
     echo do something
 fi

diff --git a/common-functions.sh b/common-functions.sh
index 5b9c1639ac613c4f9687d03ed11dbf833d5e0d20..ff1c246a428b7e3310195d7abf489cdaac2fee39 100644 (file)
--- a/common-functions.sh
+++ b/common-functions.sh
@@ -12,18 +12,18 @@ function common_init() {
     INST_DIR=`readlink -f $INST_DIR`
     
     # execution
-    if test $EUID -eq 0
+    if [[ $EUID -eq 0 ]]
     then
         export SUDO=""
-    elif test ! -f `which sudo 2>/dev/null`
+    elif [[ ! -f `which sudo 2>/dev/null` ]]
     then
         echo "Raisin requires sudo to install build dependencies for you."
         echo "Please install sudo, then run this script again."
         exit 1
     fi
 
-    if test -z "$BASH_VERSINFO" || test ${BASH_VERSINFO[0]} -lt 3 ||
-       (test ${BASH_VERSINFO[0]} -eq 3 && test ${BASH_VERSINFO[1]} -lt 2)
+    if [[ -z "$BASH_VERSINFO" || ${BASH_VERSINFO[0]} -lt 3 ||
+        (${BASH_VERSINFO[0]} -eq 3 && ${BASH_VERSINFO[1]} -lt 2) ]]
     then
         echo "Raisin requires BASH 3.2 or newer."
         exit 1
@@ -39,13 +39,13 @@ function common_init() {
 }
 
 function get_distro() {
-    if test -x "`which lsb_release 2>/dev/null`"
+    if [[ -x "`which lsb_release 2>/dev/null`" ]]
     then
         os_VENDOR=`lsb_release -i -s`
         os_RELEASE=`lsb_release -r -s`
         os_CODENAME=`lsb_release -c -s`
         os_UPDATE=""
-    elif test -r /etc/redhat-release
+    elif [[ -r /etc/redhat-release ]]
     then
         # Red Hat Enterprise Linux Server release 5.5 (Tikanga)
         # Red Hat Enterprise Linux Server release 7.0 Beta (Maipo)
@@ -56,7 +56,7 @@ function get_distro() {
         os_CODENAME=""
         for r in "Red Hat" "CentOS" "Fedora" "XenServer"; do
             os_VENDOR="$r"
-            if test -n "`grep -i \"$r\" /etc/redhat-release`"
+            if [[ -n "`grep -i \"$r\" /etc/redhat-release`" ]]
             then
                 ver=`sed -e 's/^.* \([0-9].*\) (\(.*\)).*$/\1\|\2/' /etc/redhat-release`
                 os_CODENAME=${ver#*|}
@@ -66,13 +66,13 @@ function get_distro() {
                 break
             fi
         done
-    elif test -r /etc/SuSE-release
+    elif [[ -r /etc/SuSE-release ]]
     then
         for r in "openSUSE" "SUSE Linux"
         do
             os_VENDOR="$r"
 
-            if test -n "`grep -i \"$r\" /etc/SuSE-release`"
+            if [[ -n "`grep -i \"$r\" /etc/SuSE-release`" ]]
             then
                 os_CODENAME=`grep "CODENAME = " /etc/SuSE-release | \
                              sed 's:.* = ::g'`
@@ -84,7 +84,7 @@ function get_distro() {
             fi
         done
     # If lsb_release is not installed, we should be able to detect Debian OS
-    elif test -f /etc/debian_version && [[ `cat /proc/version` =~ "Debian" ]]
+    elif [[ -f /etc/debian_version && `cat /proc/version` =~ "Debian" ]]
     then
         os_VENDOR="Debian"
         os_CODENAME=`awk '/VERSION=/' /etc/os-release | sed 's/VERSION=//' | \
@@ -123,7 +123,7 @@ function get_arch() {
 }
 
 function install_dependencies() {
-    if test "$NO_DEPS" && test "$NO_DEPS" -eq 1
+    if [[ "$NO_DEPS" && "$NO_DEPS" -eq 1 ]]
     then
         echo "Not installing any dependencies, as requested."
         echo "Depency list: $*"
@@ -143,7 +143,7 @@ function install_dependencies() {
 }
 
 function start_initscripts() {
-    while test $# -ge 1
+    while [[ $# -ge 1 ]]
     do
         case $DISTRO in
             "Debian" )
@@ -162,7 +162,7 @@ function start_initscripts() {
 }
 
 function stop_initscripts() {
-    while test $# -ge 1
+    while [[ $# -ge 1 ]]
     do
         case $DISTRO in
             "Debian" )
@@ -184,7 +184,7 @@ function for_each_component () {
     for component in `cat "$BASEDIR"/components/series`
     do
         capital=`echo $component | tr '[:lower:]' '[:upper:]'`
-        if test "`eval echo \$"$capital"_UPSTREAM_URL`"
+        if [[ "`eval echo \$"$capital"_UPSTREAM_URL`" ]]
         then
             "$component"_"$1"
         fi
@@ -192,10 +192,10 @@ function for_each_component () {
 }
 
 function build_package() {
-    if test $DISTRO = "Debian"
+    if [[ $DISTRO = "Debian" ]]
     then
         ./mkdeb "$1"
-    elif test $DISTRO = "Fedora"
+    elif [[  $DISTRO = "Fedora" ]]
     then
         ./mkrpm "$1"
     else
@@ -204,10 +204,10 @@ function build_package() {
 }
 
 function install_package() {
-    if test $DISTRO = "Debian"
+    if [[ $DISTRO = "Debian" ]]
     then
         $SUDO dpkg -i "$1".deb
-    elif test $DISTRO = "Fedora"
+    elif [[  $DISTRO = "Fedora" ]]
     then
         $SUDO rpm -i --force "$1"-`git show --oneline | head -1 | cut -d " " -f 1`-0.$ARCH.rpm
     else
@@ -216,10 +216,10 @@ function install_package() {
 }
 
 function uninstall_package() {
-    if test $DISTRO = "Debian"
+    if [[ $DISTRO = "Debian" ]]
     then
         $SUDO dpkg -r "$1"
-    elif test $DISTRO = "Fedora"
+    elif [[ $DISTRO = "Fedora" ]]
     then
         $SUDO rpm -e "$1"
     else

diff --git a/components/grub b/components/grub
index c131179716a66f300e3e44c7e5ab4d0c860a7818..95f938249e4368ac2d392c8c21713d5e542cdbe6 100644 (file)
--- a/components/grub
+++ b/components/grub
@@ -4,7 +4,7 @@ source "$BASEDIR"/config
 source "$BASEDIR"/common-functions.sh
 
 
-function grub_install_dependencies() {
+function _grub_install_dependencies() {
     local DEP_Debian_common="build-essential tar autoconf bison flex"
     local DEP_Debian_x86_32="$DEP_Debian_common"
     local DEP_Debian_x86_64="$DEP_Debian_common libc6-dev-i386"
@@ -17,7 +17,7 @@ function grub_install_dependencies() {
     local DEP_Fedora_x86_64="$DEP_Fedora_common glibc-devel.i686"
 
 
-    if test $ARCH != "x86_64" && test $ARCH != "x86_32"
+    if [[ $ARCH != "x86_64" && $ARCH != "x86_32" ]]
     then
         echo grub is only supported on x86_32 and x86_64
         return
@@ -28,7 +28,7 @@ function grub_install_dependencies() {
 
 
 function grub_build() {
-    grub_install_dependencies
+    _grub_install_dependencies
 
     cd "$BASEDIR"
     rm -f memdisk.tar
@@ -43,7 +43,7 @@ function grub_build() {
       -m ../memdisk.tar -o grub-i386-xen grub-core/*mod
     cp grub-i386-xen "$INST_DIR"/$PREFIX/lib/xen/boot
     ## GRUB64
-    if test $ARCH = "x86_64"
+    if [[ $ARCH = "x86_64" ]]
     then
         $MAKE clean
         ./configure --target=amd64 --with-platform=xen
@@ -58,7 +58,7 @@ function grub_build() {
 function grub_clean() {
     cd "$BASEDIR"
     rm -rf memdisk.tar
-    if test -d grub-dir
+    if [[ -d grub-dir ]]
     then
         cd grub-dir
         $MAKE distclean

diff --git a/components/libvirt b/components/libvirt
index 6809b5ae6c1c4bfa754a73a0aac921d33964cdb1..b39f3fa65bc9d0732f0e7455020a4c84dce25e1a 100644 (file)
--- a/components/libvirt
+++ b/components/libvirt
@@ -4,7 +4,7 @@ source "$BASEDIR"/config
 source "$BASEDIR"/common-functions.sh
 
 
-function libvirt_install_dependencies() {
+function _libvirt_install_dependencies() {
     local DEP_Debian_common="build-essential libtool autoconf autopoint \
                              xsltproc libxml2-utils pkg-config python-dev   \
                              libxml-xpath-perl libyajl-dev libxml2-dev      \
@@ -27,7 +27,7 @@ function libvirt_install_dependencies() {
 }
 
 function libvirt_build() {
-    libvirt_install_dependencies
+    _libvirt_install_dependencies
 
     cd "$BASEDIR"
     ./git-checkout.sh $LIBVIRT_UPSTREAM_URL $LIBVIRT_UPSTREAM_REVISION libvirt-dir
@@ -41,12 +41,12 @@ function libvirt_build() {
         --with-yajl --without-macvtap --without-avahi  --prefix=$PREFIX
     $MAKE
     $MAKE --ignore-errors install DESTDIR="$INST_DIR" || true
-    if test $DISTRO = "Debian"
+    if [[ $DISTRO = "Debian" ]]
     then
         mkdir -p "$INST_DIR"/etc/init.d
         cat "$BASEDIR"/data/libvirt.debian.init | sed -e "s,@PREFIX,$PREFIX,g" > "$INST_DIR"/etc/init.d/libvirtd
         chmod +x "$INST_DIR"/etc/init.d/libvirtd
-    elif test $DISTRO = "Fedora" || test $DISTRO = "CentOS"
+    elif [[ $DISTRO = "Fedora" || $DISTRO = "CentOS" ]]
     then
         $MAKE -C daemon libvirtd.init
         mkdir -p "$INST_DIR"/etc/rc.d/init.d
@@ -60,7 +60,7 @@ function libvirt_build() {
 
 function libvirt_clean() {
     cd "$BASEDIR"
-    if test -d libvirt-dir
+    if [[ -d libvirt-dir ]]
     then
         cd libvirt-dir
         $MAKE distclean
@@ -71,7 +71,7 @@ function libvirt_clean() {
 
 function libvirt_configure() {
     start_initscripts libvirtd
-    if test $DISTRO = "Fedora" || test $DISTRO = "CentOS"
+    if [[ $DISTRO = "Fedora" || $DISTRO = "CentOS" ]]
     then
         start_initscripts libvirt-guests virtlockd
     fi
@@ -79,7 +79,7 @@ function libvirt_configure() {
 
 function libvirt_unconfigure() {
     stop_initscripts libvirtd
-    if test $DISTRO = "Fedora" || test $DISTRO = "CentOS"
+    if [[ $DISTRO = "Fedora" || $DISTRO = "CentOS" ]]
     then
         stop_initscripts libvirt-guests virtlockd
     fi

diff --git a/components/xen b/components/xen
index de360767b50a7fdd936aee200f9ecbee88d8ca6f..a4de7e4968577ae7f9eafc2ca9b1fa01b56f7830 100644 (file)
--- a/components/xen
+++ b/components/xen
@@ -4,7 +4,7 @@ source "$BASEDIR"/config
 source "$BASEDIR"/common-functions.sh
 
 
-function xen_install_dependencies() {
+function _xen_install_dependencies() {
     local DEP_Debian_common="build-essential python-dev gettext uuid-dev   \
              libncurses5-dev libyajl-dev libaio-dev pkg-config libglib2.0-dev  \
              libssl-dev libpixman-1-dev bridge-utils"
@@ -24,7 +24,7 @@ function xen_install_dependencies() {
 }
 
 function xen_build() {
-    xen_install_dependencies
+    _xen_install_dependencies
 
     cd "$BASEDIR"
     ./git-checkout.sh $XEN_UPSTREAM_URL $XEN_UPSTREAM_REVISION xen-dir
@@ -40,7 +40,7 @@ function xen_build() {
 
 function xen_clean() {
     cd "$BASEDIR"
-    if test -d xen-dir
+    if [[ -d xen-dir ]]
     then
         cd xen-dir
         $MAKE distclean
@@ -49,17 +49,17 @@ function xen_clean() {
     fi
 }
 
-function xen_create_bridge_Debian() {
+function _xen_create_bridge_Debian() {
     BRIDGE="xenbr0"
     IFACE=`grep "dhcp" /etc/network/interfaces | head -1 | awk '{print$2}'`
 
-    if test -z "$IFACE"
+    if [[ -z "$IFACE" ]]
     then
         echo "Please refer to the following page to setup networking:"
         echo "http://wiki.xenproject.org/wiki/Network_Configuration_Examples_(Xen_4.1%2B)"
         return 1
     fi
-    if test "`grep $BRIDGE /etc/network/interfaces`"
+    if [[ "`grep $BRIDGE /etc/network/interfaces`" ]]
     then
         echo "a network bridge seems to be already setup"
         return 0
@@ -76,16 +76,16 @@ function xen_create_bridge_Debian() {
     $SUDO mv -f $TMPFILE /etc/network/interfaces
 }
 
-function xen_create_bridge_Fedora() {
+function _xen_create_bridge_Fedora() {
     BRIDGE="xenbr0"
 
-    if test "`grep $BRIDGE /etc/sysconfig/network-scripts/*`"
+    if [[ "`grep $BRIDGE /etc/sysconfig/network-scripts/*`" ]]
     then
         return 0
     fi
 
     IFACE=`grep 'BOOTPROTO="dhcp"' /etc/sysconfig/network-scripts/* | head -1 | cut -d : -f 1`
-    if test -z "$IFACE"
+    if [[ -z "$IFACE" ]]
     then
         return 1
     fi
@@ -109,24 +109,24 @@ function xen_create_bridge_Fedora() {
     $SUDO service network start
 }
 
-function xen_update_bootloader_Debian() {
+function _xen_update_bootloader_Debian() {
     $SUDO update-grub
 }
 
-function xen_update_bootloader_Fedora() {
+function _xen_update_bootloader_Fedora() {
     $SUDO grub2-mkconfig -o /boot/grub2/grub.cfg
 }
 
 function xen_configure() {
-    xen_create_bridge_$DISTRO
+    _xen_create_bridge_$DISTRO
     start_initscripts xencommons xendomains xen-watchdog
-    xen_update_bootloader_$DISTRO
+    _xen_update_bootloader_$DISTRO
 }
 
 function xen_unconfigure() {
     # leave the bridge in place
     stop_initscripts xencommons xendomains xen-watchdog
-    if test "`grep -rIi xen /boot/grub* | head -1`"
+    if [[ "`grep -rIi xen /boot/grub* | head -1`" ]]
     then
         xen_update_bootloader_$DISTRO
     fi

diff --git a/git-checkout.sh b/git-checkout.sh
index 20ae31ff236ddb3f977afb7f03a7458ffd1ecfdf..912bfae8e128c29fb0336fe99da71616ac52616c 100755 (executable)
--- a/git-checkout.sh
+++ b/git-checkout.sh
@@ -1,6 +1,7 @@
-#!/bin/sh
+#!/usr/bin/env bash
 
-if test $# -lt 3; then
+if [[ $# -lt 3 ]]
+then
        echo "Usage: $0 <tree> <tag> <dir>"
        exit 1
 fi
@@ -11,11 +12,13 @@ DIR=$3
 
 set -e
 
-if test \! -d $DIR-remote; then
+if [[ ! -d $DIR-remote ]]
+then
        rm -rf $DIR-remote $DIR-remote.tmp
        mkdir -p $DIR-remote.tmp; rmdir $DIR-remote.tmp
        $GIT clone $TREE $DIR-remote.tmp
-       if test "$TAG" ; then
+       if [[ "$TAG" ]]
+       then
                cd $DIR-remote.tmp
                $GIT branch -D dummy >/dev/null 2>&1 ||:
                $GIT checkout -b dummy $TAG

diff --git a/mkdeb b/mkdeb
index fd147a7c8b99527ef8015ec6b620943ef2223163..368d2c5eafabdfb0fce7166756364b2f6a6a1fa5 100755 (executable)
--- a/mkdeb
+++ b/mkdeb
@@ -7,7 +7,7 @@
 
 set -e
 
-if test -z "$1"
+if [[ -z "$1" ]]
 then 
   echo "usage: $0 package_name"
   exit 1
@@ -31,7 +31,8 @@ rm -rf deb
 cp -a "$INST_DIR" deb
 
 # Debian doesn't use /usr/lib64 for 64-bit libraries
-if test -d deb/usr/lib64 ; then 
+if [[ -d deb/usr/lib64 ]]
+then 
   cp -a deb/usr/lib64/* deb/usr/lib/
   rm -rf deb/usr/lib64
 fi

diff --git a/mkrpm b/mkrpm
index 47e1814178632e8b02bcb2d47f77cb583588e12c..d5ecc7f00a1854a6a519df61f864981e613e1896 100755 (executable)
--- a/mkrpm
+++ b/mkrpm
@@ -7,7 +7,7 @@
 
 set -e
 
-if test -z "$1"
+if [[ -z "$1" ]]
 then 
   echo "usage: $0 package_name"
   exit 1

diff --git a/raise.sh b/raise.sh
index 9d743e4b40137101094144ac3a5070cc5c35389d..1e923a27d09b1cfeec5483f5e275a98e34c5f181 100755 (executable)
--- a/raise.sh
+++ b/raise.sh
@@ -21,17 +21,17 @@ common_init
 INST=0
 export NO_DEPS=0
 export VERBOSE=0
-while test $# -ge 1
+while [[ $# -ge 1 ]]
 do
-  if test "$1" = "-n" || test "$1" = "--no-deps"
+  if [[ "$1" = "-n" || "$1" = "--no-deps" ]]
   then
     NO_DEPS=1
     shift 1
-  elif test "$1" = "-v" || test "$1" = "--verbose"
+  elif [[ "$1" = "-v" || "$1" = "--verbose" ]]
   then
     VERBOSE=1
     shift 1
-  elif test "$1" = "-i" || test "$1" = "--install"
+  elif [[ "$1" = "-i" || "$1" = "--install" ]]
   then
     INST=1
     shift 1
@@ -43,7 +43,7 @@ done
 
 mkdir -p "$INST_DIR" &>/dev/null
 install_dependencies git
-if test $DISTRO = "Fedora"
+if [[ $DISTRO = "Fedora" ]]
 then
     install_dependencies rpm-build
 fi
@@ -54,7 +54,7 @@ for_each_component build
 
 build_package xen-system
 
-if test -z "$INST" || test "$INST" -eq 0
+if [[ -z "$INST" || "$INST" -eq 0 ]]
 then
     exit 0
 fi