Re: [review-request][PATCH] bitbake: toaster: Code cleanup: bashisms

[Ed Bartosh <ed.bartosh@linux.intel.com>]

Hi Alex,

Can you show me how exactly it breaks?

It works for me just fine in dash:
$ echo $0
dash
$ ./bitbake/bin/toaster
Syncing...
Creating tables ...
Installing custom SQL ...
Installing indexes ...
Installed 0 object(s) from 0 fixture(s)

Synced:
 > django.contrib.auth
 > django.contrib.contenttypes
 > django.contrib.messages
 > django.contrib.sessions
 > django.contrib.admin
 > django.contrib.staticfiles
 > django.contrib.humanize
 > south

Not synced (use migrations):
 - orm
 - bldcontrol
(use ./manage.py migrate to migrate these)
Running migrations for orm:
- Nothing to migrate.
 - Loading initial data for orm.
Installed 0 object(s) from 0 fixture(s)
Running migrations for bldcontrol:
- Nothing to migrate.
 - Loading initial data for bldcontrol.
Installed 0 object(s) from 0 fixture(s)
Verifying the Build Environment. If the local Build Environment is not properly configured, you will be asked to configure it.
Starting webserver...
Webserver address:  http://0.0.0.0:8000/
Starting browser...
Toaster is now running. You can stop it with Ctrl-C

sourcing toaster will not work in dash for sure, but it doesn't depend on which shell is in shebang I believe.

Regards,
Ed

On Thu, May 07, 2015 at 03:29:45PM +0100, Damian, Alexandru wrote:
> Actually, I spoke too soon.
> 
> This patch doesn't pass testing - it breaks launching toaster on my system,
> where /bin/sh is actually dash. Since this is default on Ubuntu systems, we
> need to support it to run out of the box.
> 
> I will not be submitting this patch. Can you please debug it also with dash
> as /bin/sh ?
> 
> Thank you,
> Alex
> 
> On Thu, May 7, 2015 at 11:30 AM, Damian, Alexandru <
> alexandru.damian@intel.com> wrote:
> 
> > Taken for submission,
> >
> > Thank you,
> > Alex
> >
> > On Mon, May 4, 2015 at 11:42 AM, Ed Bartosh <ed.bartosh@linux.intel.com>
> > wrote:
> >
> >> Fixed the following bashisms:
> >>  replaced echo -e -> printf
> >>  removed 'function' from function definitions
> >>  replaced $(< ${file}) -> `cat ${file}`
> >>
> >> Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
> >> ---
> >>  bitbake/bin/toaster | 49
> >> +++++++++++++++++++++++++------------------------
> >>  1 file changed, 25 insertions(+), 24 deletions(-)
> >>
> >> diff --git a/bitbake/bin/toaster b/bitbake/bin/toaster
> >> index 162d4d9..dfbc58d 100755
> >> --- a/bitbake/bin/toaster
> >> +++ b/bitbake/bin/toaster
> >> @@ -1,4 +1,4 @@
> >> -#!/bin/bash
> >> +#!/bin/sh
> >>  # (c) 2013 Intel Corp.
> >>
> >>  # This program is free software; you can redistribute it and/or modify
> >> @@ -28,13 +28,14 @@
> >>
> >>  # Helper function to kill a background toaster development server
> >>
> >> -function webserverKillAll()
> >> +webserverKillAll()
> >>  {
> >>      local pidfile
> >>      for pidfile in ${BUILDDIR}/.toastermain.pid; do
> >>          if [ -f ${pidfile} ]; then
> >> -            while kill -0 $(< ${pidfile}) 2>/dev/null; do
> >> -                kill -SIGTERM -$(< ${pidfile}) 2>/dev/null
> >> +            pid=`cat ${pidfile}`
> >> +            while kill -0 $pid 2>/dev/null; do
> >> +                kill -SIGTERM -$pid 2>/dev/null
> >>                  sleep 1
> >>                  # Kill processes if they are still running - may happen
> >> in interactive shells
> >>                  ps fux | grep "python.*manage.py runserver" | awk
> >> '{print $2}' | xargs kill
> >> @@ -44,7 +45,7 @@ function webserverKillAll()
> >>      done
> >>  }
> >>
> >> -function webserverStartAll()
> >> +webserverStartAll()
> >>  {
> >>      # do not start if toastermain points to a valid process
> >>      if ! cat "${BUILDDIR}/.toastermain.pid" 2>/dev/null | xargs -I{}
> >> kill -0 {} ; then
> >> @@ -58,7 +59,7 @@ function webserverStartAll()
> >>      if [ $retval -eq 1 ]; then
> >>          echo "Failed db sync, stopping system start" 1>&2
> >>      elif [ $retval -eq 2 ]; then
> >> -        echo -e "\nError on migration, trying to recover... \n"
> >> +        printf "\nError on migration, trying to recover... \n"
> >>          python $BBBASEDIR/lib/toaster/manage.py migrate orm 0001_initial
> >> --fake
> >>          retval=0
> >>          python $BBBASEDIR/lib/toaster/manage.py migrate orm || retval=1
> >> @@ -83,7 +84,7 @@ function webserverStartAll()
> >>
> >>  # Helper functions to add a special configuration file
> >>
> >> -function addtoConfiguration()
> >> +addtoConfiguration()
> >>  {
> >>      file=$1
> >>      shift
> >> @@ -94,13 +95,13 @@ function addtoConfiguration()
> >>  INSTOPSYSTEM=0
> >>
> >>  # define the stop command
> >> -function stop_system()
> >> +stop_system()
> >>  {
> >>      # prevent reentry
> >>      if [ $INSTOPSYSTEM -eq 1 ]; then return; fi
> >>      INSTOPSYSTEM=1
> >>      if [ -f ${BUILDDIR}/.toasterui.pid ]; then
> >> -        kill $(< ${BUILDDIR}/.toasterui.pid ) 2>/dev/null
> >> +        kill `cat ${BUILDDIR}/.toasterui.pid` 2>/dev/null
> >>          rm ${BUILDDIR}/.toasterui.pid
> >>      fi
> >>      BBSERVER=0.0.0.0:-1 bitbake -m
> >> @@ -113,12 +114,12 @@ function stop_system()
> >>      INSTOPSYSTEM=0
> >>  }
> >>
> >> -function check_pidbyfile() {
> >> -    [ -e $1 ] && kill -0 $(< $1) 2>/dev/null
> >> +check_pidbyfile() {
> >> +    [ -e $1 ] && kill -0 `cat $1` 2>/dev/null
> >>  }
> >>
> >>
> >> -function notify_chldexit() {
> >> +notify_chldexit() {
> >>      if [ $NOTOASTERUI -eq 0 ]; then
> >>          check_pidbyfile ${BUILDDIR}/.toasterui.pid && return
> >>          stop_system
> >> @@ -126,16 +127,16 @@ function notify_chldexit() {
> >>  }
> >>
> >>
> >> -function verify_prereq() {
> >> +verify_prereq() {
> >>      # Verify prerequisites
> >>
> >>      if ! echo "import django; print (1,) == django.VERSION[0:1] and
> >> django.VERSION[1:2][0] in (6,)" | python 2>/dev/null | grep True
> >> >/dev/null; then
> >> -        echo -e "This program needs Django 1.6. Please install
> >> with\n\npip install django==1.6\n"
> >> +        printf "This program needs Django 1.6. Please install
> >> with\n\npip install django==1.6\n"
> >>          return 2
> >>      fi
> >>
> >>      if ! echo "import south; print reduce(lambda x, y: 2 if x==2 else 0
> >> if x == 0 else y, map(lambda x: 1+cmp(x[1]-x[0],0), zip([0,8,4],
> >> map(int,south.__version__.split(\".\"))))) > 0" | python 2>/dev/null | grep
> >> True >/dev/null; then
> >> -        echo -e "This program needs South 0.8.4. Please install
> >> with\n\npip install south==0.8.4\n"
> >> +        printf "This program needs South 0.8.4. Please install
> >> with\n\npip install south==0.8.4\n"
> >>          return 2
> >>      fi
> >>      return 0
> >> @@ -174,47 +175,47 @@ if [ `basename \"$0\"` = `basename \"${SRCFILE}\"`
> >> ]; then
> >>      # Start just the web server, point the web browser to the interface,
> >> and start any Django services.
> >>
> >>      if ! verify_prereq; then
> >> -        echo -e "Error: Could not verify that the needed dependencies
> >> are installed. Please use virtualenv and pip to install dependencies listed
> >> in toaster-requirements.txt" 1>&2
> >> +        echo "Error: Could not verify that the needed dependencies are
> >> installed. Please use virtualenv and pip to install dependencies listed in
> >> toaster-requirements.txt" 1>&2
> >>          exit 1
> >>      fi
> >>
> >>      if [ -n "$BUILDDIR" ]; then
> >> -        echo -e "Error: It looks like you sourced oe-init-build-env.
> >> Toaster cannot start in build mode from an oe-core build environment.\n You
> >> should be starting Toaster from a new terminal window." 1>&2
> >> +        printf "Error: It looks like you sourced oe-init-build-env.
> >> Toaster cannot start in build mode from an oe-core build environment.\n You
> >> should be starting Toaster from a new terminal window." 1>&2
> >>          exit 1
> >>      fi
> >>
> >>      if ! which daemon >/dev/null 2>&1; then
> >> -        echo -e "Failed dependency; toaster needs the 'daemon' program
> >> in order to be able to start builds'. Please install the 'daemon' program
> >> from your distribution repositories or http://www.libslack.org/daemon/"
> >> 1>&2
> >> +        echo "Failed dependency; toaster needs the 'daemon' program in
> >> order to be able to start builds'. Please install the 'daemon' program from
> >> your distribution repositories or http://www.libslack.org/daemon/" 1>&2
> >>          exit 1
> >>      fi
> >>
> >>      # Define a fake builddir where only the pid files are actually
> >> created. No real builds will take place here.
> >>      BUILDDIR=/tmp/toaster_$$
> >>      if [ -d "$BUILDDIR" ]; then
> >> -        echo -e "Previous toaster run directory $BUILDDIR found,
> >> cowardly refusing to start. Please remove the directory when that toaster
> >> instance is over" 2>&1
> >> +        echo "Previous toaster run directory $BUILDDIR found, cowardly
> >> refusing to start. Please remove the directory when that toaster instance
> >> is over" 2>&1
> >>          exit 1
> >>      fi
> >>
> >>      mkdir -p "$BUILDDIR"
> >>
> >>      RUNNING=1
> >> -    function trap_ctrlc() {
> >> +    trap_ctrlc() {
> >>          echo "** Stopping system"
> >>          webserverKillAll
> >>          RUNNING=0
> >>      }
> >>
> >> -    function do_cleanup() {
> >> +    do_cleanup() {
> >>          find "$BUILDDIR" -type f | xargs rm
> >>          rmdir "$BUILDDIR"
> >>      }
> >> -    function cleanup() {
> >> +    cleanup() {
> >>          if grep -ir error "$BUILDDIR" >/dev/null; then
> >>              if grep -irn "That port is already in use" "$BUILDDIR"; then
> >>                  echo "You can use the \"webport=PORTNUMBER\" parameter
> >> to start Toaster on a different port (port $WEB_PORT is already in use)"
> >>                  do_cleanup
> >>              else
> >> -                echo -e "\nErrors found in the Toaster log files present
> >> in '$BUILDDIR'. Directory will not be cleaned.\n Please review the errors
> >> and notify toaster@yoctoproject.org or submit a bug
> >> https://bugzilla.yoctoproject.org/enter_bug.cgi?product=Toaster"
> >> +                printf "\nErrors found in the Toaster log files present
> >> in '$BUILDDIR'. Directory will not be cleaned.\n Please review the errors
> >> and notify toaster@yoctoproject.org or submit a bug
> >> https://bugzilla.yoctoproject.org/enter_bug.cgi?product=Toaster"
> >>              fi
> >>          else
> >>              echo "No errors found, removing the run directory
> >> '$BUILDDIR'"
> >> @@ -245,7 +246,7 @@ fi
> >>
> >>
> >>  if ! verify_prereq; then
> >> -    echo -e "Error: Could not verify that the needed dependencies are
> >> installed. Please use virtualenv and pip to install dependencies listed in
> >> toaster-requirements.txt" 1>&2
> >> +    echo "Error: Could not verify that the needed dependencies are
> >> installed. Please use virtualenv and pip to install dependencies listed in
> >> toaster-requirements.txt" 1>&2
> >>      return 1
> >>  fi
> >>
> >> --
> >> 2.1.4
> >>
> >> --
> >> _______________________________________________
> >> toaster mailing list
> >> toaster@yoctoproject.org
> >> https://lists.yoctoproject.org/listinfo/toaster
> >>
> >
> >
> >
> > --
> > Alex Damian
> > Yocto Project
> > SSG / OTC
> >
> 
> 
> 
> -- 
> Alex Damian
> Yocto Project
> SSG / OTC

-- 
--
Regards,
Ed