[Toaster] [review-request] ed/toaster/shells
Damian, Alexandru
alexandru.damian at intel.com
Tue Jun 9 03:01:15 PDT 2015
Hi,
Sorry for the delay, patches ok, with amendments.
Cheers,
Alex
On Thu, May 21, 2015 at 4:16 PM, Ed Bartosh <ed.bartosh at linux.intel.com>
wrote:
> On Wed, May 20, 2015 at 11:22:17AM +0100, Damian, Alexandru wrote:
> >
> > It's the command line start that breaks. If I change the
> >
> > #!/bin/bash
> >
> > to
> >
> > #!/bin/sh
> >
> >
> > My output is:
> >
> > yocto$ ./poky/bitbake/bin/toaster
> > ./poky/bitbake/bin/toaster: 1: ./poky/bitbake/bin/toaster: Bad
> substitution
> > ./poky/bitbake/bin/toaster: 201: ./poky/bitbake/bin/toaster: Syntax
> error:
> > "(" unexpected (expecting "fi")
> >
> Thank you for the info. I didn't see this as my /bin/sh points to bash
> even if
> I run dash :)
>
> >
> > Why shouldn't we use the bash as the shebang script ? I suspect that all
> > reasonably modern Linuxes have bash installed, and it doesn't conflict
> with
> > whatever shell the user actually uses.
>
> Sure they have. I just wanted to make it run under zsh and clean up
> bashisms.
> Cleaning up bashsms and leaving bash in shebang doesn't look very logical
> to me.
> If it's clean it should work in any shell. If it can work only in bash
> then what's
> the point of cleaning it from bashisms?
>
> Anyway, I made it to work in dash too. Please, review ed/toaster/shells
>
> Regards,
> Ed
>
> >
> > On Wed, May 20, 2015 at 8:15 AM, Ed Bartosh <ed.bartosh at linux.intel.com>
> > wrote:
> >
> > > ping
> > >
> > > On Mon, May 18, 2015 at 07:16:11PM +0300, Ed Bartosh wrote:
> > > > 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 at intel.com> wrote:
> > > > >
> > > > > > Taken for submission,
> > > > > >
> > > > > > Thank you,
> > > > > > Alex
> > > > > >
> > > > > > On Mon, May 4, 2015 at 11:42 AM, Ed Bartosh <
> > > ed.bartosh at 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 at 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 at 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 at 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 at yoctoproject.org
> > > > > >> https://lists.yoctoproject.org/listinfo/toaster
> > > > > >>
> > > > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > > Alex Damian
> > > > > > Yocto Project
> > > > > > SSG / OTC
> > > > > >
> > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Alex Damian
> > > > > Yocto Project
> > > > > SSG / OTC
> > > >
> > > > --
> > > > --
> > > > Regards,
> > > > Ed
> > > > --
> > > > _______________________________________________
> > > > toaster mailing list
> > > > toaster at yoctoproject.org
> > > > https://lists.yoctoproject.org/listinfo/toaster
> > >
> > > --
> > > --
> > > Regards,
> > > Ed
> > >
> >
> >
> >
> > --
> > Alex Damian
> > Yocto Project
> > SSG / OTC
>
> --
> --
> Regards,
> Ed
>
--
Alex Damian
Yocto Project
SSG / OTC
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.yoctoproject.org/pipermail/toaster/attachments/20150609/f0c6f2fe/attachment-0001.html>
More information about the toaster
mailing list