Prev: portmaster-with-package-support / pkg_add: illegal option -- -
Next: please submit ports/142210
From: Doug Barton on 31 Dec 2009 17:30
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/

From: "Philip M. Gollucci" on 31 Dec 2009 18:49
will review in 2010.

Thought most typically it runs as smtpd/smtpd
i.e. apache.org,perl.org,$work

I'm turn on which way to go here.


Glen Barber wrote:
> Hi Doug,
>
> I really appreciate the feedback.
>
> On Thu, Dec 31, 2009 at 5:30 PM, Doug Barton <dougb(a)freebsd.org> wrote:
>> Glen Barber wrote:
> [...]
>> 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.
>>
>
> Initially, they were the user/group I was using to run this service.
> I later found it was not necessary, and in changing the rc script, I
> had forgotten to remove them. This service runs fine with
> nobody/nogroup, which is reflected in an attached patch, which
> incorporates your changes.
>
> [...]
>
>> 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.) :)
>
> I'm glad embarrassment doesn't translate well over the internet... :-)
>
> A diff against the port version with your modifications and my
> nobody/nogroup changes is attached. If it is okay with the maintainer
> (cc'd), I think this can be committed.
>
> Thanks again for the feedback, and I really appreciate your explanations.
>
> Regards,
>
>


--
------------------------------------------------------------------------
1024D/DB9B8C1C B90B FBC3 A3A1 C71A 8E70 3F8C 75B8 8FFB DB9B 8C1C
Philip M. Gollucci (pgollucci(a)p6m7g8.com) c: 703.336.9354
VP Apache Infrastructure; Member, Apache Software Foundation
Committer, FreeBSD Foundation
Consultant, P6M7G8 Inc.
Sr. System Admin, Ridecharge Inc.

Work like you don't need the money,
love like you'll never get hurt,
and dance like nobody's watching.
_______________________________________________
freebsd-ports(a)freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-ports
To unsubscribe, send any mail to "freebsd-ports-unsubscribe(a)freebsd.org"

 | 
Pages: 1
Prev: portmaster-with-package-support / pkg_add: illegal option -- -
Next: please submit ports/142210