New cvsd rc.d script (Was: Re: cvs commit: ports/devel/cvsd Makefile
ports/devel/cvsd/files cvsd.sh.in extra-cvsd-buildroot.in)
Doug Barton
dougb at FreeBSD.org
Tue Jun 6 10:56:27 PDT 2006
Ying-Chieh Liao wrote:
> ijliao 2006-06-06 09:33:07 UTC
>
> FreeBSD ports repository
>
> Modified files:
> devel/cvsd Makefile
> Added files:
> devel/cvsd/files cvsd.sh.in extra-cvsd-buildroot.in
> Log:
> Take maintainership
> Add rc.subr support
>
> PR: 98582 http://www.FreeBSD.org/cgi/query-pr.cgi?pr=98582
> Submitted by: "Andrey V. Elsukov" <bu7cher at yandex.ru> (new maintainer)
>
> Revision Changes Path
> 1.39 +12 -5 ports/devel/cvsd/Makefile
> 1.1 +52 -0 ports/devel/cvsd/files/cvsd.sh.in (new)
> 1.1 +42 -0 ports/devel/cvsd/files/extra-cvsd-buildroot.in (new)
>
> http://www.FreeBSD.org/cgi/cvsweb.cgi/ports/devel/cvsd/Makefile.diff?&r1=1.38&r2=1.39&f=h
> http://www.FreeBSD.org/cgi/cvsweb.cgi/ports/devel/cvsd/files/cvsd.sh.in
> http://www.FreeBSD.org/cgi/cvsweb.cgi/ports/devel/cvsd/files/extra-cvsd-buildroot.in
First off let me thank you for taking the initiative to move this port to
the rc.d framework. I have some suggestions for you, and I hope that you
will not take them as a criticism in any way. This script provides a good
educational opportunity, so although I'm offering a fairly extensive patch
here, it's still based on your good work. In addition to the attached patch,
I'm going to provide a bit of commentary here to explain my suggestions.
First, the name of the .in file and the USE_RC_SUBR variable should be cvsd,
not cvsd.sh. The Porter's Handbook had the .sh example until just a second
ago when I fixed it, so I apologize if you used that as a reference. It's
not a big enough problem to justify changing the name now, as bsd.port.mk
will do the right thing. Just something to keep in mind for future reference.
Index: cvsd.sh.in
===================================================================
RCS file: /home/pcvs/ports/devel/cvsd/files/cvsd.sh.in,v
retrieving revision 1.1
diff -u -r1.1 cvsd.sh.in
--- cvsd.sh.in 6 Jun 2006 09:33:07 -0000 1.1
+++ cvsd.sh.in 6 Jun 2006 17:37:03 -0000
@@ -2,24 +2,22 @@
# $FreeBSD:
#
# PROVIDE: cvsd
-# REQUIRE: NETWORKING
-# KEYWORD: nojail
+# REQUIRE: LOGIN
Most services should REQUIRE LOGIN unless there is a good reason to have
them start sooner. In this case, your service will almost certainly need
LOGIN as it runs with an unprivileged user account.
+# KEYWORD: nojail shutdown
If you're starting a daemon, you should include the shutdown KEYWORD so that
it will be disabled in an orderly fashion when the system is shut down.
. %%RC_SUBR%%
name="cvsd"
-rcvar=`set_rcvar`
-command="%%PREFIX%%/sbin/$name"
-
-load_rc_config $name
+rcvar=${name}_enable
Minor issue, but saves one layer of indirection.
-: ${cvsd_enable="NO"}
-: ${cvsd_config="%%PREFIX%%/etc/$name/$name.conf"}
+command="%%PREFIX%%/sbin/$name"
+command_args='-f $cvsd_config'
+required_files=$cvsd_config
-command_args="-f $cvsd_config"
+start_precmd=${name}_prestart
+stop_postcmd=${name}_poststop
-start_precmd="cvsd_prestart"
-stop_postcmd="cvsd_poststop"
+osreldate=`sysctl -n kern.osreldate`
These changes are more in keeping with typical rc.d style. Also, one little
shell trick for you. If you enclose the name of the variable you want to use
in single quotes as I did above for command_args, you can define
command_args before cvsd_config gets define, and the variable will be filled
in at the appropriate time in run_rc_command.
cvsd_prestart()
{
@@ -29,6 +27,16 @@
devfs -m $jail/dev rule apply path null unhide
devfs -m $jail/dev rule apply path zero unhide
fi
+
+ jail=`sed -n 's/^ *RootJail *\([^ ]*\) *$/\1/p' < $cvsd_config`
+ if [ -z "$jail" ]; then
+ err 1 "RootJail is not specified in $cvsd_config"
+ fi
+
+ pidfile=`sed -n 's/^ *PidFile *\([^ ]*\) *$/\1/p' < $cvsd_config`
+ if [ -z "$pidfile" ]; then
+ err 1 "PidFile is not specified in $cvsd_config"
+ fi
Two things here, first you should never perform script actions
unconditionally in an rc.d script. The test for this is to run the script
with for example './cvsd rcvar' and make sure that the only output will be
the values of the variables. Second, the -z test is the preferred way to
test if a variable is empty.
I hope this is useful for you. If you approve the changes, I'll be glad to
commit them for you, or you can submit a new PR.
Regards,
Doug
--
This .signature sanitized for your protection
-------------- next part --------------
Index: cvsd.sh.in
===================================================================
RCS file: /home/pcvs/ports/devel/cvsd/files/cvsd.sh.in,v
retrieving revision 1.1
diff -u -r1.1 cvsd.sh.in
--- cvsd.sh.in 6 Jun 2006 09:33:07 -0000 1.1
+++ cvsd.sh.in 6 Jun 2006 17:37:03 -0000
@@ -2,24 +2,22 @@
# $FreeBSD:
#
# PROVIDE: cvsd
-# REQUIRE: NETWORKING
-# KEYWORD: nojail
+# REQUIRE: LOGIN
+# KEYWORD: nojail shutdown
. %%RC_SUBR%%
name="cvsd"
-rcvar=`set_rcvar`
-command="%%PREFIX%%/sbin/$name"
-
-load_rc_config $name
+rcvar=${name}_enable
-: ${cvsd_enable="NO"}
-: ${cvsd_config="%%PREFIX%%/etc/$name/$name.conf"}
+command="%%PREFIX%%/sbin/$name"
+command_args='-f $cvsd_config'
+required_files=$cvsd_config
-command_args="-f $cvsd_config"
+start_precmd=${name}_prestart
+stop_postcmd=${name}_poststop
-start_precmd="cvsd_prestart"
-stop_postcmd="cvsd_poststop"
+osreldate=`sysctl -n kern.osreldate`
cvsd_prestart()
{
@@ -29,6 +27,16 @@
devfs -m $jail/dev rule apply path null unhide
devfs -m $jail/dev rule apply path zero unhide
fi
+
+ jail=`sed -n 's/^ *RootJail *\([^ ]*\) *$/\1/p' < $cvsd_config`
+ if [ -z "$jail" ]; then
+ err 1 "RootJail is not specified in $cvsd_config"
+ fi
+
+ pidfile=`sed -n 's/^ *PidFile *\([^ ]*\) *$/\1/p' < $cvsd_config`
+ if [ -z "$pidfile" ]; then
+ err 1 "PidFile is not specified in $cvsd_config"
+ fi
}
cvsd_poststop()
@@ -38,15 +46,9 @@
fi
}
-jail=`sed -n 's/^ *RootJail *\([^ ]*\) *$/\1/p' < $cvsd_config`
-pidfile=`sed -n 's/^ *PidFile *\([^ ]*\) *$/\1/p' < $cvsd_config`
-osreldate=`sysctl -n kern.osreldate`
-if [ "$jail" = "X$jail" ]; then
- err 1 "RootJail is not specified in $cvsd_config"
-fi
-if [ "$pidfile" = "X$pidfile" ]; then
- err 1 "PidFile is not specified in $cvsd_config"
-fi
+load_rc_config $name
-run_rc_command "$1"
+: ${cvsd_enable="NO"}
+: ${cvsd_config="%%PREFIX%%/etc/$name/$name.conf"}
+run_rc_command "$1"
More information about the freebsd-ports
mailing list