Date: Wed, 30 Nov 2011 15:38:47 -0800 From: Doug Barton <dougb@FreeBSD.org> To: Ruslan Mahmatkhanov <cvs-src@yandex.ru> Cc: freebsd-rc@freebsd.org Subject: Re: rc-script review request Message-ID: <4ED6BE87.4060408@FreeBSD.org> In-Reply-To: <4ED67B8F.50109@yandex.ru> References: <4ED66DCB.1040102@yandex.ru> <CADLo83-RR945MKbdvpdghHsHpO1_MA4OA21WkA_3xCTjOOhDsQ@mail.gmail.com> <4ED67B8F.50109@yandex.ru>
next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format. --------------040004080509000701030605 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit On 11/30/2011 10:53, Ruslan Mahmatkhanov wrote: > Chris Rees wrote on 30.11.2011 22:15: >> I could get yelled at for this, but normally I'd prefer: > > Please yell, i'm not experienced in rc at all, so i'll be glad any > guidance (in any form) to raise it (experience) :). But i thought that > it's safe to use existing scripts from the tree. Ruslan, the problem is that there are a lot of bad examples already in the tree. :) >> start_cmd="${name}_start" >> >> over >> >> start_cmd="zope213_start". Yes, using variables where it's clear what's being done is preferred, since that will facilitate reuse of *good* examples. > Fixed, thank you. I also added `KEYWORD: shutdown' per PH, because of > zope starting under non-root user. You use 'shutdown' because it starts a persistent service, and we want to shut those down cleanly and in order. If the service runs under a non-root user it needs REQUIRE: LOGIN instead of DAEMON. However, I don't see that it runs as a non-root user, unless zopectl handles that for you? > Is there still any problems in the script? 1. Always use tabs 2. Make the start/stop/restart printouts fit rc.d style a little more 3. Simplify the shell code for dealing with command line arguments 4. $@ should be used there instead of $* because the former treats the elements as discrete, which is what you want to feed a for loop. 5. Move the default for _enable up to where we like it to be. 6. Localize the variable in zope213ctl() But there is a more fundamental problem. You seem to be requiring the user to supply an instance argument for the script to work at all. That's contrary to how we generally do things, and I'm fairly confident that this is not going to work on startup. I think that what you need is to provide at least one default, so after the default for _enable you'd have something like this: : ${zope213_instances:=%%PREFIX%%} (assuming that /usr/local is the default Then you need an additional function: zope213_check_instances () { cmd="$1" shift if [ -n "$@" ]; then zope213_instances="$@" elif [ -z "$zope213_instances" ]; then err 1 "No value for zope213_instances, so nothing to do" fi } And call that function first in each of your start/stop/restart functions. You should test that of course. :) hth, Doug -- "We could put the whole Internet into a book." "Too practical." Breadth of IT experience, and depth of knowledge in the DNS. Yours for the right price. :) http://SupersetSolutions.com/ --------------040004080509000701030605 Content-Type: text/plain; name="zope213.in" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="zope213.in" #!/bin/sh # # Startup script for Zope server. # # $FreeBSD: ports/www/zope211/files/zope211.in,v 1.3 2011/05/15 02:49:17 dougb Exp $ # # PROVIDE: zope213 # REQUIRE: DAEMON # KEYWORD: shutdown # Define these zope213_* variables in one of these files: # /etc/rc.conf # /etc/rc.conf.local # /etc/rc.conf.d/zope213 # # zope213_enable : bool # Enable Zope ("YES") or not ("NO", the default). # # zope213_instances : list # List of dirs with Zope's instances ("" by default). # . /etc/rc.subr name="zope213" rcvar=`set_rcvar` load_rc_config $name : ${zope213_enable:="NO"} zope213ctl () { local instance for instance in $zope213_instances; do if [ -d ${instance} ]; then echo -n " Zope instance ${instance} -> " ${instance}/bin/zopectl "$1" fi done } zope213_start () { echo -n 'Starting Zope 2.13:' zope213ctl "start" echo '.' } zope213_stop () { echo -n 'Stopping Zope 2.13:' zope213ctl "stop" echo '.' } zope213_restart () { echo -n 'Restarting Zope 2.13:' zope213ctl "restart" echo '.' } start_cmd="${name}_start" stop_cmd="${name}_stop" restart_cmd="${name}_restart" cmd="$1" shift zope213_instances="$@" run_rc_command "${cmd}" --------------040004080509000701030605--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4ED6BE87.4060408>