From owner-freebsd-ports@FreeBSD.ORG Thu Dec 31 22:30:33 2009 Return-Path: Delivered-To: ports@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 6D82E1065696 for ; Thu, 31 Dec 2009 22:30:33 +0000 (UTC) (envelope-from dougb@FreeBSD.org) Received: from mail2.fluidhosting.com (mx21.fluidhosting.com [204.14.89.4]) by mx1.freebsd.org (Postfix) with ESMTP id 134598FC21 for ; Thu, 31 Dec 2009 22:30:32 +0000 (UTC) Received: (qmail 5113 invoked by uid 399); 31 Dec 2009 22:30:31 -0000 Received: from localhost (HELO foreign.dougb.net) (dougb@dougbarton.us@127.0.0.1) by localhost with ESMTPAM; 31 Dec 2009 22:30:31 -0000 X-Originating-IP: 127.0.0.1 X-Sender: dougb@dougbarton.us Message-ID: <4B3D2606.4040605@FreeBSD.org> Date: Thu, 31 Dec 2009 14:30:30 -0800 From: Doug Barton Organization: http://SupersetSolutions.com/ User-Agent: Thunderbird 2.0.0.23 (X11/20091206) MIME-Version: 1.0 To: Glen Barber References: <200912100507.nBA577Q3033700@repoman.freebsd.org> <4B2330FB.70309@FreeBSD.org> <4B243FBC.8080507@p6m7g8.com> <4B26A861.2080805@FreeBSD.org> <4ad871310912141329r540d5cd9of33ec2963dbf999d@mail.gmail.com> <4ad871310912311115g5c821d0bhd5d6c03f167deef1@mail.gmail.com> In-Reply-To: <4ad871310912311115g5c821d0bhd5d6c03f167deef1@mail.gmail.com> X-Enigmail-Version: 0.96.0 OpenPGP: id=D5B2F0FB Content-Type: multipart/mixed; boundary="------------040407070406090509040200" Cc: ports@freebsd.org, "Philip M. Gollucci" Subject: Re: cvs commit: ports/mail/p5-qpsmtpd/files qpsmtpd.in X-BeenThere: freebsd-ports@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Porting software to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 31 Dec 2009 22:30:33 -0000 This is a multi-part message in MIME format. --------------040407070406090509040200 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Glen Barber wrote: > I have attached two diffs of the rc script, however I have run into a > few issues with both. First, please understand that I do appreciate you taking a look at this, so please don't take my comments below as criticism. It seems to me that perhaps you're not really that familiar with shell scripting, and you're certainly not familiar with the rc.d system, but that's ok, we're here to help. :) If you haven't already, please read the article at http://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/rc-scripts.html and perhaps even the articles it references. I think that will be a big help for you. I've attached a diff for a script that works, please test it and if you're satisfied I will commit it for you. However, there is apparently an additional problem with the port because when I installed it the "smtpd" user was not created. If that is mandatory for the script to work it should be added by the port. I could get it started if I specified another unprivileged user/group in rc.conf.local. I will try to address your questions in line, then explain a little more about what my suggested version of the script does at the end. > The first diff still uses 'stop_cmd(){kill `cat $pidfile`}' for the > following reasons: > > - adding ': qpsmtpd_pidfile="$pidfile"' would never create > /var/run/qpsmtpd.pid, It's up to the process to create its own pidfile. > thus the rc script would never find it to kill > the process. If you allow the user to specify the pid file's location (which frankly I think is not usually necessary) you have to set pidfile=${name}_pidfile in the script _after_ load_rc_config and the setting of the default values. For most cases, hard-coding the pidfile into the script is sufficient. The other problem that you're running into is that when you have a service that uses an interpreted language (like perl, python, ruby, etc.) you usually have to use the command_interpreter option in rc.d. > To work around this, I added the pidfile creation to > start_cmd(). Yeah, don't do that. :) Since the existence of the pid directory is a PRErequisite for the thing to start properly, it needs to be in the start_precmd. You did this part right in your original version, FYI. > - as a side effect of the above workaround, stopping the process > would kill the PID, however leave the pidfile. As a second > workaround, I have kept the stop_cmd() function, and forcibly removed > the pidfile after the process was killed. Also not necessary, see below. > The second replaces the pidfile line with this: > > [ -z "$pidfile" ] && pidfile="/var/run/${name}.pid" > > and adds a command_args line, however the pidfile is still not cleaned > after stopping the service, so stop_cmd() is used again. Is there a > more correct way to clean up the pidfile that what I have attached? Yeah, more confusion that's not needed. :) The script I attached does the following: 1. Adds a $FreeBSD cvs id 2. Changes the rcorder to REQUIRE: LOGIN. This is usually the way to do it for ports scripts, but since this service runs as an unprivileged user it's mandatory. 3. Adds the shutdown KEYWORD. Since this script is starting a persistent service, you want to shut it down cleanly. 4. Adds descriptions for the variables the way they are done in the example in the porter's handbook. 5. No quotes are needed around /etc/rc.subr 6. Adds command_interpreter (also adds this to the sub list in the Makefile) 7. Removes the start/stop methods 8. Renames the prestart method to match the convention. 9. Shrinks the prestart method down to just creating and updating permissions on /var/run/qpsmtpd 10. Moves load_rc_config down 11. Properly sets the defaults for the options. (FYI, in your patch you used: ': foo="bar"', spend a few minutes thinking about why that's bad.) :) 12. Adds a command_args variable with the right options. The beauty of the rc.d system is that it does a lot for you in the background. Of course, that's also one reason it's so hard to learn how to use. :) hth, Doug -- Improve the effectiveness of your Internet presence with a domain name makeover! http://SupersetSolutions.com/ --------------040407070406090509040200 Content-Type: text/plain; name="p5-qpsmtpd-rcd.diff" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="p5-qpsmtpd-rcd.diff" ? work Index: Makefile =================================================================== RCS file: /home/pcvs/ports/mail/p5-qpsmtpd/Makefile,v retrieving revision 1.16 diff -u -r1.16 Makefile --- Makefile 26 Dec 2009 22:20:35 -0000 1.16 +++ Makefile 31 Dec 2009 22:18:04 -0000 @@ -36,6 +36,7 @@ USE_RC_SUBR= qpsmtpd +SUB_LIST+= PERL=${PERL} SUB_LIST+= PORTNAME=${PORTNAME} SUB_FILES+= pkg-message Index: files/qpsmtpd.in =================================================================== RCS file: /home/pcvs/ports/mail/p5-qpsmtpd/files/qpsmtpd.in,v retrieving revision 1.2 diff -u -r1.2 qpsmtpd.in --- files/qpsmtpd.in 10 Dec 2009 05:07:07 -0000 1.2 +++ files/qpsmtpd.in 31 Dec 2009 22:18:04 -0000 @@ -1,100 +1,56 @@ #!/bin/sh +# $FreeBSD$ +# # PROVIDE: qpsmtpd -# REQUIRE: NETWORKING SERVERS -# BEFORE: securelevel +# REQUIRE: LOGIN +# KEYWORD: shutdown +# +# Add the following lines to /etc/rc.conf.local or /etc/rc.conf +# to enable this service: +# +# qpsmtpd_enable (bool): Set to NO by default +# Set it to YES to enable qpsmtpd +# qpsmtpd_user (string): Set to "smtpd" by default +# The user to run qpsmtpd-forkserver as +# qpsmtpd_group (string): Set to "smtpd" by default +# The group the pid dir will be chowned to +# qpsmtpd_port (int): Set to 2525 by default +# The port it should listen on +# qpsmtpd_max_per_ip (int): Set to 3 by default +# Max connections per IP +# qpsmtpd_max_connections (int): Set to 15 by default +# Maximum total connections +# qpsmtpd_listen_on (address): Set to 0.0.0.0 by default +# IP address to listen on -#variables -#qpsmtpd_user = the user to run qpsmtpd-forkserver under -#qpsmtpd_group = the group the pid dir will be chowned to -#qpsmtpd_port = the port it should listen on -#qpsmtpd_max_per_ip = max connections per IP -#qpsmtpd_max_connections = maximum total connections -#qpsmtpd_listen_on = IP to listen on - -. "/etc/rc.subr" +. /etc/rc.subr name="qpsmtpd" rcvar=`set_rcvar` -load_rc_config $name command="%%PREFIX%%/bin/qpsmtpd-forkserver" +command_interpreter=%%PERL%% pidfile="/var/run/qpsmtpd/qpsmtpd.pid" -start_precmd="start_precmd" -start_cmd="start_cmd" -stop_cmd="stop_cmd" +start_precmd=${name}_prestart -start_precmd() +qpsmtpd_prestart() { - #exits if no user is specified - if [ -z $qpsmtpd_user ]; then - echo "qpsmtpd_user not set" - exit 1 - fi - - #exits if no group is specified - if [ -z $qpsmtpd_group ]; then - echo "qpsmtpd_group not set" - exit 1 - fi - - #sets it to the default if the port is not specified - if [ -z $qpsmtpd_port ]; then - qpsmtpd_port="2525" - fi - - #set it to the default max per ip - if [ -z $qpsmtpd_max_per_ip ]; then - qpsmtpd_max_per_ip="5" - fi - - #set it do the max number of connections total - if [ -z $qpsmtpd_max_connections ]; then - qpsmtpd_max_connections="15" - fi - - #set the default listen on to everything - if [ -z $qpsmtpd_listen_on ]; then - qpsmtpd_listen_on="0.0.0.0" - fi - - if [ ! -d /var/run/qpsmtpd/ ] ; then - mkdir /var/run/qpsmtpd - fi - - chown $qpsmtpd_user:$qpsmtpd_group /var/run/qpsmtpd + [ -d /var/run/qpsmtpd ] || mkdir /var/run/qpsmtpd + chown $qpsmtpd_user:$qpsmtpd_group /var/run/qpsmtpd } -start_cmd() -{ - if [ -e $pidfile ]; then - echo "$name already running as PID `cat $pidfile`." - exit 1 - else - eval $command \ - -p $qpsmtpd_port \ - -c $qpsmtpd_max_connections \ - -u $qpsmtpd_user \ - -m $qpsmtpd_max_per_ip \ - -l $qpsmtpd_listen_on \ - --pid-file $pidfile \ - -d \ - && echo "$name started as PID `cat $pidfile`." \ - || echo "Failed to start $name" - fi -} +load_rc_config $name -stop_cmd() -{ - if [ -e $pidfile ]; then - kill `cat $pidfile` \ - && echo "$name stopped." \ - || echo "Could not stop `cat $pidfile`." - else - echo "Cannot find $pidfile - $name not running?" - exit 1 - fi -} +: ${qpsmtpd_enable="NO"} +: ${qpsmtpd_user="smtpd"} +: ${qpsmtpd_group="smtpd"} +: ${qpsmtpd_port="2525"} +: ${qpsmtpd_max_per_ip="3"} +: ${qpsmtpd_max_connections="15"} +: ${qpsmtpd_listen_on="0.0.0.0"} + +command_args="-d -p $qpsmtpd_port -c $qpsmtpd_max_connections -u $qpsmtpd_user -m $qpsmtpd_max_per_ip -l $qpsmtpd_listen_on --pid-file $pidfile" run_rc_command "$1" --------------040407070406090509040200--