From: Ed Bartosh <ed.bartosh@linux.intel.com>
To: "Damian, Alexandru" <alexandru.damian@intel.com>
Cc: "toaster@yoctoproject.org" <toaster@yoctoproject.org>
Subject: Re: [review-request][PATCH] bitbake: toaster: Code cleanup: bashisms
Date: Mon, 18 May 2015 13:36:13 +0300 [thread overview]
Message-ID: <20150518103613.GA8218@linux.intel.com> (raw)
In-Reply-To: <CAJ2CSBtjGr6S0LFm0sWL09+DcomnJ6O1YQyC3kaHTC9eWeS1ig@mail.gmail.com>
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 ?
Unfortunately I can't find equivalent of $BASH_SOURCE for dash. `dirname $0` can be used
only if script is executed. However, if script is sourced $0 is pointing to dash itself.
Looks like there is no way to get name of sourced script in dash and probably in other simple shells.
Any ideas how to work around this?
Regards,
Ed
> 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
next prev parent reply other threads:[~2015-05-18 12:29 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-04 10:42 [review-request][PATCH] bitbake: toaster: Code cleanup: bashisms Ed Bartosh
2015-05-07 10:30 ` Damian, Alexandru
2015-05-07 14:29 ` Damian, Alexandru
2015-05-18 10:36 ` Ed Bartosh [this message]
2015-05-18 16:16 ` Ed Bartosh
[not found] ` <20150520071514.GA26418@linux.intel.com>
[not found] ` <CAJ2CSBsv+k=J8DCzbFg5DbhF24cdZB21r_KC_o4h4oE3kcbKaw@mail.gmail.com>
2015-05-21 15:16 ` [review-request] ed/toaster/shells Ed Bartosh
2015-06-09 10:01 ` Damian, Alexandru
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150518103613.GA8218@linux.intel.com \
--to=ed.bartosh@linux.intel.com \
--cc=alexandru.damian@intel.com \
--cc=toaster@yoctoproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.