Date: Tue, 30 Aug 2011 18:29:46 -0700 From: Doug Barton <dougb@FreeBSD.org> To: Anders Nordby <anders@FreeBSD.org> Cc: cvs-ports@FreeBSD.org, cvs-all@FreeBSD.org, ports-committers@FreeBSD.org Subject: Re: cvs commit: ports/www/varnish2 Makefile distinfo pkg-descr pkg-plist ports/www/varnish2/files pkg-message.in varnishd.in varnishlog.in varnishncsa.in Message-ID: <4E5D8E8A.6060309@FreeBSD.org> In-Reply-To: <201108292218.p7TMINcs047197@repoman.freebsd.org> References: <201108292218.p7TMINcs047197@repoman.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format. --------------050502030500060406090308 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Minor issues with the rc.d scripts. They should all REQUIRE: LOGIN because they run as an unprivileged user, and I did the minor optimization for the pidfile on all 3. For varnishd, the code to handle the pre-start stuff should be in a prestart method. I also tried to optimize out the common _flags arguments to make the code easier to read. Please double-check my work. I'm also not sure why you're using _flags here instead of command_args. The combination of allowing the user to specify a bunch of _options and using them to build the _flags argument is interesting (note, I'm not saying wrong) but it seems to me that it would make more sense to use those _options to build command_args, which would then allow the user to specify other things in _flags without having to recreate the entire string. hth, Doug -- Nothin' ever doesn't change, but nothin' changes much. -- OK Go Breadth of IT experience, and depth of knowledge in the DNS. Yours for the right price. :) http://SupersetSolutions.com/ --------------050502030500060406090308 Content-Type: text/plain; name="varnish-rcd.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="varnish-rcd.diff" Index: varnishd.in =================================================================== RCS file: /home/pcvs/ports/www/varnish2/files/varnishd.in,v retrieving revision 1.11 diff -u -r1.11 varnishd.in --- varnishd.in 29 Aug 2011 22:18:22 -0000 1.11 +++ varnishd.in 31 Aug 2011 01:13:34 -0000 @@ -4,7 +4,7 @@ # # PROVIDE: varnishd -# REQUIRE: DAEMON +# REQUIRE: LOGIN # KEYWORD: shutdown # @@ -57,6 +57,8 @@ command="%%PREFIX%%/sbin/${name}" +start_precmd=${name}_prestart + # read configuration and set defaults load_rc_config ${name} : ${varnishd_enable:="NO"} @@ -68,16 +70,22 @@ : ${varnishd_hash:="classic,16383"} : ${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} -h ${varnishd_hash} -u ${varnishd_user} -g ${varnishd_group}"} -else - : ${varnishd_flags:="-P ${varnishd_pidfile} -a ${varnishd_listen} -T ${varnishd_admin} -b ${varnishd_backend} -s ${varnishd_storage} -h ${varnishd_hash} -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_prestart() +{ + varnishd_flags="-P ${varnishd_pidfile} -a ${varnishd_listen} -T ${varnishd_admin} -s ${varnishd_storage} -h ${varnishd_hash} -u ${varnishd_user} -g ${varnishd_group}" + + if [ -n "${varnishd_config}" ] ; then + varnishd_flags="$varnishd_flags -f ${varnishd_config}" + else + varnishd_flags="$varnishd_flags -b ${varnishd_backend}" + 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 +} pidfile="${varnishd_pidfile}" run_rc_command "$1" Index: varnishlog.in =================================================================== RCS file: /home/pcvs/ports/www/varnish2/files/varnishlog.in,v retrieving revision 1.7 diff -u -r1.7 varnishlog.in --- varnishlog.in 29 Aug 2011 22:18:22 -0000 1.7 +++ varnishlog.in 31 Aug 2011 01:13:34 -0000 @@ -4,7 +4,7 @@ # # PROVIDE: varnishlog -# REQUIRE: DAEMON +# REQUIRE: LOGIN # KEYWORD: shutdown # @@ -40,10 +40,11 @@ # read configuration and set defaults load_rc_config ${name} + +pidfile=${varnishlog_pidfile:-"/var/run/${name}.pid"} + : ${varnishlog_enable:="NO"} -: ${varnishlog_pidfile:="/var/run/${name}.pid"} : ${varnishlog_file:="/var/log/varnish.log"} -: ${varnishlog_flags:="-P ${varnishlog_pidfile} -D -a -w ${varnishlog_file}"} +: ${varnishlog_flags:="-P $pidfile -D -a -w ${varnishlog_file}"} -pidfile=${varnishlog_pidfile} run_rc_command "$1" Index: varnishncsa.in =================================================================== RCS file: /home/pcvs/ports/www/varnish2/files/varnishncsa.in,v retrieving revision 1.5 diff -u -r1.5 varnishncsa.in --- varnishncsa.in 29 Aug 2011 22:18:22 -0000 1.5 +++ varnishncsa.in 31 Aug 2011 01:13:34 -0000 @@ -4,7 +4,7 @@ # # PROVIDE: varnishncsa -# REQUIRE: DAEMON +# REQUIRE: LOGIN # KEYWORD: shutdown # @@ -40,10 +40,11 @@ # read configuration and set defaults load_rc_config ${name} + +pidfile=${varnishncsa_pidfile:-"/var/run/${name}.pid"} + : ${varnishncsa_enable:="NO"} -: ${varnishncsa_pidfile:="/var/run/${name}.pid"} : ${varnishncsa_file:="/var/log/${name}.log"} -: ${varnishncsa_flags:="-P ${varnishncsa_pidfile} -D -a -c -w ${varnishncsa_file}"} +: ${varnishncsa_flags:="-P $pidfile -D -a -c -w ${varnishncsa_file}"} -pidfile=${varnishncsa_pidfile} run_rc_command "$1" --------------050502030500060406090308--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4E5D8E8A.6060309>