Dag-Erling Smorgrav wrote:
> des 2009-10-13 18:51:11 UTC
>
> FreeBSD ports repository
>
> Modified files:
> www/varnish Makefile
> www/varnish/files varnishd.in varnishlog.in varnishncsa.in
> Added files:
> www/varnish/files pkg-message.in
> Log:
> Reorganize the rc scripts; there were several things about the old ones
> that simply didn't make sense. Add a pkg-message containing a very brief
> quick-start guide and a warning to existing users about the rc changes.
>
> Revision Changes Path
> 1.28 +6 -0 ports/www/varnish/Makefile
> 1.1 +18 -0 ports/www/varnish/files/pkg-message.in (new)
> 1.7 +49 -21 ports/www/varnish/files/varnishd.in
> 1.5 +23 -13 ports/www/varnish/files/varnishlog.in
> 1.3 +23 -13 ports/www/varnish/files/varnishncsa.in
>
> http://www.FreeBSD.org/cgi/cvsweb.cgi/ports/www/varnish/Makefile.diff?&r1=1.27&r2=1.28&f=h
> http://www.FreeBSD.org/cgi/cvsweb.cgi/ports/www/varnish/files/pkg-message.in
> http://www.FreeBSD.org/cgi/cvsweb.cgi/ports/www/varnish/files/varnishd.in.diff?&r1=1.6&r2=1.7&f=h
> http://www.FreeBSD.org/cgi/cvsweb.cgi/ports/www/varnish/files/varnishlog.in.diff?&r1=1.4&r2=1.5&f=h
> http://www.FreeBSD.org/cgi/cvsweb.cgi/ports/www/varnish/files/varnishncsa.in.diff?&r1=1.2&r2=1.3&f=h
Overall the scripts now look quite good. :) I have a few notes, only
the first of which is really significant. Since the varnishd script
uses a specific user and group it should REQUIRE: LOGIN rather than
DAEMON. In fact, unless there is a good reason to start before LOGIN
we generally prefer that all ports scripts REQUIRE it.
Also in varnishd, the test for the existence of $varnishd_config
should really be encased in a pre_start method since that's the only
time it's relevant (unless I'm missing something). I've also
simplified it a bit to make it more clear what is different. If there
is some reason that the command line flags have to be in a certain
order I'm sure you can adapt it as you like.
I'd also suggest using varnishd_[ug]id instead of _user and _group,
since as you point out rc.subr special cases those two variables. I
used _uid in rc.d/named to good effect.
hth,
Doug
--
Improve the effectiveness of your Internet presence with
a domain name makeover! http://SupersetSolutions.com/
Index: varnishd.in
===================================================================
RCS file: /home/pcvs/ports/www/varnish/files/varnishd.in,v
retrieving revision 1.7
diff -u -r1.7 varnishd.in
--- varnishd.in 13 Oct 2009 18:51:10 -0000 1.7
+++ varnishd.in 13 Oct 2009 20:08:41 -0000
@@ -4,7 +4,7 @@
#
# PROVIDE: varnishd
-# REQUIRE: DAEMON
+# REQUIRE: LOGIN
# KEYWORD: shutdown
#
@@ -34,15 +34,18 @@
# varnishd_storage - storage method and parameters.
# default: "file,/tmp,50%"
#
-# varnishd_user - unprivileged user for the child process.
+# varnishd_uid - unprivileged user for the child process.
# default: "www"
#
-# varnishd_group - unprivileged group for the child process.
+# varnishd_gid - unprivileged group for the child process.
# default: "www"
#
# varnishd_flags - complete command line arguments.
-# default if varnishd_config is unset: "-P ${varnishd_pidfile} -a
${varnishd_listen} -T ${varnishd_admin} -b ${varnishd_backend} -s
${varnishd_storage} -u ${varnishd_user} -g ${varnishd_group}"
-# default if varnishd_config is set: "-P ${varnishd_pidfile} -a
${varnishd_listen} -T ${varnishd_admin} -f ${varnishd_config} -s
${varnishd_storage} -u ${varnishd_user} -g ${varnishd_group}"
+# Common flags: -P ${varnishd_pidfile} -a ${varnishd_listen}
+# -T ${varnishd_admin} -s ${varnishd_storage}
+# -u ${varnishd_uid} -g ${varnishd_gid}
+# default if varnishd_config is unset: "<Common flags> -b ${varnishd_backend}"
+# default if varnishd_config is set: "<Common flags> -f ${varnishd_config}"
#
# See varnishd(1) for a detailed overview of command-line options.
#
@@ -54,6 +57,21 @@
command="%%PREFIX%%/sbin/${name}"
+start_precmd=${name}_prestart
+
+varnishd_prestart()
+{
+ local common
+
+ common="-P ${varnishd_pidfile} -a ${varnishd_listen} -T
${varnishd_admin} -s ${varnishd_storage} -u ${varnishd_uid} -g
${varnishd_group}"
+
+ if [ -n "${varnishd_config}" ] ; then
+ varnishd_flags="$common -f ${varnishd_config}"
+ else
+ varnishd_flags="$common -b ${varnishd_backend}"
+ fi
+}
+
# read configuration and set defaults
load_rc_config ${name}
: ${varnishd_enable:="NO"}
@@ -63,18 +81,8 @@
: ${varnishd_backend:="localhost:8080"}
: ${varnishd_config:=""}
: ${varnishd_storage:="file,/tmp,50%"}
-: ${varnishd_user:="www"}
-: ${varnishd_group:="www"}
-if [ -n "${varnishd_config}" ] ; then
- : ${varnishd_flags:="-P ${varnishd_pidfile} -a ${varnishd_listen} -T
${varnishd_admin} -f ${varnishd_config} -s ${varnishd_storage} -u
${varnishd_user} -g ${varnishd_group}"}
-else
- : ${varnishd_flags:="-P ${varnishd_pidfile} -a ${varnishd_listen} -T
${varnishd_admin} -b ${varnishd_backend} -s ${varnishd_storage} -u
${varnishd_user} -g ${varnishd_group}"}
-fi
-
-# If we leave these set, rc.subr will su to them before starting
-# varnishd, which is not what we want.
-unset varnishd_user
-unset varnishd_group
+: ${varnishd_uid:="www"}
+: ${varnishd_gid:="www"}
pidfile="${varnishd_pidfile}"
run_rc_command "$1"
Index: varnishlog.in
===================================================================
RCS file: /home/pcvs/ports/www/varnish/files/varnishlog.in,v
retrieving revision 1.5
diff -u -r1.5 varnishlog.in
--- varnishlog.in 13 Oct 2009 18:51:10 -0000 1.5
+++ varnishlog.in 13 Oct 2009 20:08:41 -0000
@@ -4,7 +4,7 @@
#
# PROVIDE: varnishlog
-# REQUIRE: DAEMON
+# REQUIRE: LOGIN
# KEYWORD: shutdown
#
Index: varnishncsa.in
===================================================================
RCS file: /home/pcvs/ports/www/varnish/files/varnishncsa.in,v
retrieving revision 1.3
diff -u -r1.3 varnishncsa.in
--- varnishncsa.in 13 Oct 2009 18:51:10 -0000 1.3
+++ varnishncsa.in 13 Oct 2009 20:08:41 -0000
@@ -4,7 +4,7 @@
#
# PROVIDE: varnishncsa
-# REQUIRE: DAEMON
+# REQUIRE: LOGIN
# KEYWORD: shutdown
#
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/cvs-all
To unsubscribe, send any mail to "[email protected]"
