From owner-freebsd-rc@FreeBSD.ORG Thu Dec 1 01:14:34 2011 Return-Path: Delivered-To: freebsd-rc@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 12A411065673; Thu, 1 Dec 2011 01:14:34 +0000 (UTC) (envelope-from yanegomi@gmail.com) Received: from mail-gx0-f182.google.com (mail-gx0-f182.google.com [209.85.161.182]) by mx1.freebsd.org (Postfix) with ESMTP id B38018FC16; Thu, 1 Dec 2011 01:14:33 +0000 (UTC) Received: by ggnk5 with SMTP id k5so2004116ggn.13 for ; Wed, 30 Nov 2011 17:14:33 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; bh=XbdiBKZleeQDVeXYR+9wG6/tN/dlYVDNiivlXRnWvFc=; b=x4NLQLuQvAJ8nA6VMxHEhtbKn7CE23d1xApQrpM+JpAzeaBz4/XuUz32gcojZjQvM4 RshGfW0DkTZ1KmAIeYbJ0mh5epR44LgQZgUc7ycJiAtFn10NIVaoY6uiJzJJYgvAL4ei f+9wrR18nFYFVHqhT6WlhafhZL+dEN1dwtj4U= MIME-Version: 1.0 Received: by 10.182.172.41 with SMTP id az9mr994681obc.42.1322702073209; Wed, 30 Nov 2011 17:14:33 -0800 (PST) Received: by 10.182.62.227 with HTTP; Wed, 30 Nov 2011 17:14:33 -0800 (PST) In-Reply-To: <4ED6BE87.4060408@FreeBSD.org> References: <4ED66DCB.1040102@yandex.ru> <4ED67B8F.50109@yandex.ru> <4ED6BE87.4060408@FreeBSD.org> Date: Wed, 30 Nov 2011 17:14:33 -0800 Message-ID: From: Garrett Cooper To: Doug Barton Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: freebsd-rc@freebsd.org, Ruslan Mahmatkhanov Subject: Re: rc-script review request X-BeenThere: freebsd-rc@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Discussion related to /etc/rc.d design and implementation." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 01 Dec 2011 01:14:34 -0000 On Wed, Nov 30, 2011 at 3:38 PM, Doug Barton wrote: > 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=3D"${name}_start" >>> >>> over >>> >>> start_cmd=3D"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:=3D%%PREFIX%%} > (assuming that /usr/local is the default > > Then you need an additional function: > > zope213_check_instances () { > =A0 =A0 =A0 =A0cmd=3D"$1" > =A0 =A0 =A0 =A0shift > > =A0 =A0 =A0 =A0if [ -n "$@" ]; then > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0zope213_instances=3D"$@" > =A0 =A0 =A0 =A0elif [ -z "$zope213_instances" ]; then > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0err 1 "No value for zope213_instances, so = nothing to do" > =A0 =A0 =A0 =A0fi > } > > And call that function first in each of your start/stop/restart functions= . > > You should test that of course. :) Crazy thought -- should a script be made for rc scripts, similar to portcheck? Thanks! -Garrett