From owner-freebsd-rc@FreeBSD.ORG Sun Jun 10 04:07:06 2012 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 43A9F106566B; Sun, 10 Jun 2012 04:07:06 +0000 (UTC) (envelope-from sahil@tandon.net) Received: from cricket.hamla.org (cricket.hamla.org [206.251.255.31]) by mx1.freebsd.org (Postfix) with ESMTP id 25E0E8FC0C; Sun, 10 Jun 2012 04:07:06 +0000 (UTC) Received: from magic.hamla.org (cpe-68-174-134-215.nyc.res.rr.com [68.174.134.215]) by cricket.hamla.org (Postfix) with ESMTPSA id 0E1248A06A; Sun, 10 Jun 2012 00:06:59 -0400 (EDT) Date: Sun, 10 Jun 2012 00:06:57 -0400 From: Sahil Tandon To: Doug Barton Message-ID: <20120610040657.GA1415@magic.hamla.org> References: <20120609010405.GA295@magic.hamla.org> <4FD38ED0.7070803@FreeBSD.org> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="liOOAslEiF7prFVr" Content-Disposition: inline In-Reply-To: <4FD38ED0.7070803@FreeBSD.org> User-Agent: Mutt/1.5.21 (2010-09-15) X-Virus-Scanned: clamav-milter 0.97.3 at cricket.hamla.org X-Virus-Status: Clean Cc: freebsd-rc@freebsd.org Subject: Re: RESEND: [sahil@tandon.net: Request for review: mail/postfix-postfwd rc script] X-BeenThere: freebsd-rc@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Sahil Tandon 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: Sun, 10 Jun 2012 04:07:06 -0000 --liOOAslEiF7prFVr Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Sat, 2012-06-09 at 10:58:40 -0700, Doug Barton wrote: > ... > 1. Add a $FreeBSD$ > 2. Remove some trailing whitespace Cool, thanks for that as well as your other comments, to which I reply inline: > 3. pidfile= is magic, and needs to be included. It's also not usually > necessary to allow the user to change the pidfile location, but if you > really want to allow that, the way to do it is in the new patch. My (evidently inaccurate) read of /etc/rc.subr led me to believe that the pidfile global was not strictly required, especially because its magical qualities appear to be lacking in this case. :) Indeed, that is why I had to add the status_cmd kludge and - I just realized - another one for reload_cmd. > 4. It's not clear to me why there are so many items in the default > _flags option. Are any of those truly required? If so, they should be in > command_args. OTOH, if what this represents is a rational default > configuration that the user might want to twiddle, it's fine. You're spot on with the OTOH; that is the rationale. > 5. I used the same trick for the conf file as I did for pidfile > (utilizing required_files). It was already properly utilized in > command_args in your patch. > 6. Rather than having stop_cmd and stop_postcmd be separate functions I > just put them in line. Elegant, thank you. > 7. With pidfile= the check command you added should not be necessary, > 'service postfwd status' is what you want instead. That does not work, because of the peculiarities of this program: looking for its procname (with or without interpreter) is futile because the ps output shows '[perl5.10.1]' rather than something more useful. :( > Of course, none of this is tested, so you'll want to do that. :) > ... One thing I retained is start_precmd, which is needed because of an upstream bug (I've reported it) which causes the program to silently terminate if 'start' is issued when postfwd is already running! Let me know if there is a more suitable way to handle this until it is fixed upstream. I've attached a new patch -- let me know if it gets mangled again in transit! -- Sahil Tandon --liOOAslEiF7prFVr Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="postfwd.in.patch-new" Index: files/postfwd.in =================================================================== RCS file: /home/pcvs/ports/mail/postfix-postfwd/files/postfwd.in,v retrieving revision 1.4 diff -u -u -r1.4 postfwd.in --- files/postfwd.in 14 Jan 2012 08:56:01 -0000 1.4 +++ files/postfwd.in 10 Jun 2012 04:01:20 -0000 @@ -1,45 +1,63 @@ #!/bin/sh -# PROVIDE: postfwd +# $FreeBSD$ +# +# PROVIDE: postfwd # REQUIRE: LOGIN cleanvar # KEYWORD: shutdown # # Add the following lines to /etc/rc.conf.local or /etc/rc.conf # to enable this service: # -# postfwd_enable (bool): +# postfwd_enable (bool): # Set to "NO" by default. -# Set it to "YES" to enable postfwd. -# postfwd_config (path): Set to %%PREFIX%%/etc/postfwd.conf -# by default. -# +# Set it to "YES" to enable postfwd. +# postfwd_config (path): +# Set to %%PREFIX%%/etc/postfwd.conf +# by default. . /etc/rc.subr name=postfwd rcvar=postfwd_enable -command=%%PREFIX%%/bin/${name} -required_files=%%PREFIX%%/etc/${name}.conf -pidfile="/var/run/${name}.pid" +load_rc_config $name -stop_postcmd=stop_postcmd +: ${postfwd_enable:="NO"} +: ${postfwd_flags="--shortlog --summary=600 --cache=600 --cache-rbl-timeout=3600 --cleanup-requests=1200 --cleanup-rbls=1800 --cleanup-rates=1200"} -stop_postcmd() -{ - rm -f $pidfile -} +pidfile=${postfwd_pidfile:="/var/run/${name}.pid"} +required_files=${postfwd_config:="%%PREFIX%%/etc/${name}.conf"} -load_rc_config "$name" +command=%%PREFIX%%/bin/${name} +command_args="-d -f ${required_files} --pidfile=${pidfile} -i 127.0.0.1 -p 10040 -u nobody -g nobody" -case "$postfwd_enable" in - [Yy][Ee][Ss] | 1 | [Oo][Nn] | [Tt][Rr][Uu][Ee]) ;; - *) echo "To make use of $name you must first set $rcvar=\"YES\" in /etc/rc.conf" ;; -esac +start_precmd="${name}_check" +status_cmd="${name}_status" +stop_cmd="${command} -k --pidfile=${pidfile}" +stop_postcmd="rm -f ${pidfile}" +extra_commands="reload" +reload_cmd="${name}_reload" + +postfwd_check() { + if [ -f "${postfwd_pidfile}" ]; then + err 1 "${name} is already running." + fi +} + +postfwd_status() { + postfwd_pid=`cat ${pidfile} 2>/dev/null` + postfwd_run=`ps -U nobody | grep -m 1 ${postfwd_pid} 2>/dev/null` + if [ -n "${postfwd_pid}" -a -n "${postfwd_run}" ]; then + echo "$name is running as ${postfwd_pid}" + else + echo "$name is not running" + fi +} -: ${postfwd_enable="NO"} -: ${postfwd_config="%%PREFIX%%/etc/${name}.conf"} +postfwd_reload() { -command_args="--shortlog --summary=600 --cache=600 --cache-rbl-timeout=3600 --cleanup-requests=1200 --cleanup-rbls=1800 --cleanup-rates=1200 -d -f ${required_files} -i 127.0.0.1 -p 10040 -u nobody -g nobody" + kill -HUP `cat $pidfile` +} run_rc_command "$1" --liOOAslEiF7prFVr--