From owner-freebsd-rc@FreeBSD.ORG Thu Dec 1 18:04:03 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 DC870106567A; Thu, 1 Dec 2011 18:04:02 +0000 (UTC) (envelope-from cvs-src@yandex.ru) Received: from forward13.mail.yandex.net (forward13.mail.yandex.net [IPv6:2a02:6b8:0:801::3]) by mx1.freebsd.org (Postfix) with ESMTP id 010EB8FC24; Thu, 1 Dec 2011 18:04:01 +0000 (UTC) Received: from smtp12.mail.yandex.net (smtp12.mail.yandex.net [95.108.131.191]) by forward13.mail.yandex.net (Yandex) with ESMTP id 537821447A4; Thu, 1 Dec 2011 22:04:00 +0400 (MSK) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yandex.ru; s=mail; t=1322762640; bh=l77LWnHDXlxnk4RPMyJhFHuhbGaa+F8WGMKDZ30paFA=; h=Message-ID:Date:From:MIME-Version:To:CC:Subject:References: In-Reply-To:Content-Type:Content-Transfer-Encoding; b=cmikkGH/abcBJi30KNe9bpDq6ezp3Q5zbghB02ycyIcJrG3zF9amc6CUGkSniOECL VGbt4Hb2uIvf7IAlF8Xvnnh64OhCCkU96oajdmRNxQApEpqcd9XilHggjWCyAWb41m Jn4RHYuXLKKZuIvRXLl4V3LVaiBBdhXcuShPtfJk= Received: from smtp12.mail.yandex.net (localhost [127.0.0.1]) by smtp12.mail.yandex.net (Yandex) with ESMTP id 2597E16A04C3; Thu, 1 Dec 2011 22:04:00 +0400 (MSK) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yandex.ru; s=mail; t=1322762640; bh=l77LWnHDXlxnk4RPMyJhFHuhbGaa+F8WGMKDZ30paFA=; h=Message-ID:Date:From:MIME-Version:To:CC:Subject:References: In-Reply-To:Content-Type:Content-Transfer-Encoding; b=cmikkGH/abcBJi30KNe9bpDq6ezp3Q5zbghB02ycyIcJrG3zF9amc6CUGkSniOECL VGbt4Hb2uIvf7IAlF8Xvnnh64OhCCkU96oajdmRNxQApEpqcd9XilHggjWCyAWb41m Jn4RHYuXLKKZuIvRXLl4V3LVaiBBdhXcuShPtfJk= Received: from unknown (unknown [178.76.224.133]) by smtp12.mail.yandex.net (nwsmtp/Yandex) with ESMTP id 3ximlN79-3xieqD7i; Thu, 1 Dec 2011 22:03:59 +0400 X-Yandex-Spam: 1 Message-ID: <4ED7C17B.2060709@yandex.ru> Date: Thu, 01 Dec 2011 22:03:39 +0400 From: Ruslan Mahmatkhanov User-Agent: Mozilla/5.0 (X11; FreeBSD i386; rv:8.0) Gecko/20111109 Thunderbird/8.0 MIME-Version: 1.0 To: Doug Barton References: <4ED66DCB.1040102@yandex.ru> <4ED67B8F.50109@yandex.ru> <4ED6BE87.4060408@FreeBSD.org> In-Reply-To: <4ED6BE87.4060408@FreeBSD.org> X-Enigmail-Version: undefined Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: freebsd-rc@freebsd.org 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 18:04:03 -0000 Doug Barton wrote on 01.12.2011 03:38: > On 11/30/2011 10:53, Ruslan Mahmatkhanov wrote: > >> 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? Yes, zopectl actually handling that - it drops privileges to user that's defined in ${instance}/etc/zope.conf (default `www'). I changed REQUIRE to LOGIN though. >> 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() I merged all the changes from you except this part: cmd="$1" -[ $# -gt 0 ] && shift -[ -n "$*" ] && zope213_instances="$*" +shift +zope213_instances="$@" Dunno why, but it didn't working - service just doesn't starting. To be honest, i'm half-understand your changes because i mooched shell-scripting lessons in the school. > 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 I understand what you talking about, and i'm agree that this is not much correct to not have default value for an instance. I'll try to follow you suggestions and kode to implement this behaviour, thank you! > 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 > -- Regards, Ruslan Tinderboxing kills... the drives.