The function get_pidfile_from_conf was added to base before 8.3-RELEASE, and so it needed wrapping in tests to ensure it was available. It is now available on all supported releases, but uptake has been poor. I suspect it may be something to do with the if type get_pidfile_from_conf incantation, so I've done a sweep of the ports tree and removed all occurrences. Fix: (mostly cosmetic changes to please rclint, hence no revision bump) -- This message has been scanned for viruses and dangerous content by MailScanner, and is believed to be clean.--CSjRTqWNmBnq96zfbT96WbuSzURIMgS76izeR7utAdSiWr1R Content-Type: text/plain; name="patch.txt" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="patch.txt" Index: audio/musicpd/files/musicpd.in =================================================================== --- audio/musicpd/files/musicpd.in (revision 316967) +++ audio/musicpd/files/musicpd.in (working copy) @@ -27,8 +27,7 @@ musicpd_getpidfile() { - if type get_pidfile_from_conf >/dev/null 2>&1 && - get_pidfile_from_conf pid_file %%PREFIX%%/etc/musicpd.conf ; then + if get_pidfile_from_conf pid_file %%PREFIX%%/etc/musicpd.conf ; then pidfile="$_pidfile_from_conf" else pidfile="%%MPDDIR%%/.mpd/pid" Index: net-mgmt/hawk/files/hawk.in =================================================================== --- net-mgmt/hawk/files/hawk.in (revision 316967) +++ net-mgmt/hawk/files/hawk.in (working copy) @@ -19,23 +19,21 @@ load_rc_config hawk -: ${hawk_enable:="NO"} +: ${hawk_enable:=NO} command=%%PREFIX%%/sbin/hawk command_interpreter=%%PERL%% command_args='&' +start_precmd=find_pidfile +stop_precmd=find_pidfile find_pidfile() { - if type get_pidfile_from_conf >/dev/null 2>&1 && - get_pidfile_from_conf pidfile %%PREFIX%%/etc/hawk/daemon.conf; then + if get_pidfile_from_conf pidfile %%PREFIX%%/etc/hawk/daemon.conf; then pidfile="$_pidfile_from_conf" else - pidfile='/var/run/hawk.pid' + pidfile=/var/run/hawk.pid fi } -start_precmd=find_pidfile -stop_precmd=find_pidfile - -run_rc_command "$1" +run_rc_command $1 Index: net-mgmt/nrpe2/files/nrpe2.in =================================================================== --- net-mgmt/nrpe2/files/nrpe2.in (revision 316967) +++ net-mgmt/nrpe2/files/nrpe2.in (working copy) @@ -17,7 +17,7 @@ name=nrpe2 rcvar=nrpe2_enable -load_rc_config "${name}" +load_rc_config nrpe2 : ${nrpe2_enable:=NO} : ${nrpe2_configfile:=%%PREFIX%%/etc/nrpe.cfg} @@ -26,7 +26,7 @@ command="%%PREFIX%%/sbin/nrpe2" command_args="-c ${nrpe2_configfile} -d" -extra_commands="reload" +extra_commands=reload sig_reload=HUP start_precmd=nrpe2_prestart @@ -37,8 +37,7 @@ [ -n "$nrpe2_pidfile" ] && warn "No longer necessary to set nrpe2_pidfile in rc.conf[.local]" - if type get_pidfile_from_conf >/dev/null 2>&1 && - get_pidfile_from_conf pid_file %%PREFIX%%/etc/nrpe.cfg; then + if get_pidfile_from_conf pid_file %%PREFIX%%/etc/nrpe.cfg; then pidfile="$_pidfile_from_conf" else pidfile='/var/run/nrpe2/nrpe2.pid' @@ -51,4 +50,4 @@ install -d -o ${nrpe_user:-nagios} ${pidfile%/*} } -run_rc_command "$1" +run_rc_command $1 Index: sysutils/munin-node/files/munin-node.in =================================================================== --- sysutils/munin-node/files/munin-node.in (revision 316967) +++ sysutils/munin-node/files/munin-node.in (working copy) @@ -15,7 +15,7 @@ . /etc/rc.subr -name="munin_node" +name=munin_node rcvar=munin_node_enable load_rc_config $name @@ -31,12 +31,8 @@ find_pidfile() { - if type get_pidfile_from_conf >/dev/null 2>&1 && - get_pidfile_from_conf pid_file $munin_node_config; then - pidfile="$_pidfile_from_conf" - else - pidfile=`awk '$1 == "pid_file" { print $2 }' $munin_node_config` - fi + get_pidfile_from_conf pid_file $munin_node_config + pidfile="$_pidfile_from_conf" } -run_rc_command "$1" +run_rc_command $1 Index: sysutils/munin-node/files/munin-sched.in =================================================================== --- sysutils/munin-node/files/munin-sched.in (revision 316967) +++ sysutils/munin-node/files/munin-sched.in (working copy) @@ -15,7 +15,7 @@ . /etc/rc.subr -name="munin_sched" +name=munin_sched rcvar=munin_sched_enable load_rc_config $name @@ -30,13 +30,9 @@ find_pidfile() { - if type get_pidfile_from_conf >/dev/null 2>&1 && - get_pidfile_from_conf pid_file $munin_sched_config; then + get_pidfile_from_conf pid_file $munin_sched_config pidfile="${_pidfile_from_conf%node*}sched" pidfile="${pidfile}${_pidfile_from_conf#*node}" - else - pidfile=`awk '$1 == "pid_file" { gsub("node","sched",$2); print $2; }' $munin_sched_config` - fi } run_rc_command "$1" Index: x11/slim/files/slim.in =================================================================== --- x11/slim/files/slim.in (revision 316967) +++ x11/slim/files/slim.in (working copy) @@ -17,31 +17,29 @@ . /etc/rc.subr -name="slim" +name=slim rcvar=slim_enable +load_rc_config slim + +: ${slim_enable:=NO} + +command=%%PREFIX%%/bin/slim +command_args=-d start_precmd=${name}_rmfile stop_precmd=${name}_prestop stop_postcmd=${name}_rmfile -load_rc_config $name - -: ${slim_enable="NO"} - -command=%%PREFIX%%/bin/slim -command_args="-d" - find_pidfile() { - if type get_pidfile_from_conf >/dev/null 2>&1 && - get_pidfile_from_conf lockfile %%PREFIX%%/etc/${name}.conf; then + if get_pidfile_from_conf lockfile %%PREFIX%%/etc/${name}.conf; then pidfile="$_pidfile_from_conf" else pidfile="/var/run/${name}.pid" fi } -slim_rmfile () +slim_rmfile() { local file @@ -55,7 +53,7 @@ return 0 } -slim_prestop () +slim_prestop() { local xpid @@ -65,4 +63,4 @@ [ -n "$xpid" ] && kill $xpid } -run_rc_command "$1" +run_rc_command $1
Hi Jarrod, Florian and Henry, Please take note of ports/178269, containing a totally trivial change to the rc scripts of net-mgmt/nrpe2, sysutils/munin-node and x11/slim. I'm trying to reduce the apparent fear around the use of get_pidfile_from_conf, so this simplification should help. Are the changes OK with you? Chris
You've an okay from me to commit the changes to the net-mgmt/nrpe2 port, = but I don't understand why cosmetic changes are required or even = considered. Is the preference not to quote some options documented = anywhere? Jarrod.=
I'm fine with the get_pidfile_from_conf change. But I'm not sure whether the style changes are needed. For the quotes, with a quick grep in /etc/rc.d, it seems like that the standard ones have them. Maybe better leave them as-is. On Tue, Apr 30, 2013 at 5:52 PM, Chris Rees <utisoft@gmail.com> wrote: > Hi Jarrod, Florian and Henry, > > Please take note of ports/178269, containing a totally trivial change > to the rc scripts of net-mgmt/nrpe2, sysutils/munin-node and x11/slim. > > I'm trying to reduce the apparent fear around the use of > get_pidfile_from_conf, so this simplification should help. > > Are the changes OK with you? > > Chris > -- Cheers, Henry
Wow, sorry for the long delay in replying. I'll leave out the quotes changes if you prefer (I'm fixing up the style guide for rc scripts, and we're going to remove the requirement for quotes where not necessary). The other changes to slim's function spacing I would like to push in however; the rc style guide is more explicit about those. Do you mind if I commit that part, and leave the quotes in? Chris
Responsible Changed From-To: freebsd-ports-bugs->crees I'll take it.
Fine, please go ahead. On Thu, May 30, 2013 at 7:33 AM, Chris Rees <utisoft@gmail.com> wrote: > Wow, sorry for the long delay in replying. > > I'll leave out the quotes changes if you prefer (I'm fixing up the > style guide for rc scripts, and we're going to remove the requirement > for quotes where not necessary). > > The other changes to slim's function spacing I would like to push in > however; the rc style guide is more explicit about those. > > Do you mind if I commit that part, and leave the quotes in? > > Chris > -- Cheers, Henry
Author: crees Date: Fri May 31 11:54:01 2013 New Revision: 319487 URL: http://svnweb.freebsd.org/changeset/ports/319487 Log: Stop checking for get_pidfile_from_conf function in rc.subr. It is present in all supported versions of FreeBSD, and has had poor takeup. I strongly suspect the strange-looking checks are partially to blame for scaring maintainers off. Go forth and please use it! PR: ports/178269 Approved by: maintainers of all ports involved Modified: head/audio/musicpd/files/musicpd.in head/net-mgmt/hawk/files/hawk.in head/net-mgmt/nrpe2/files/nrpe2.in head/sysutils/munin-node/files/munin-node.in head/sysutils/munin-node/files/munin-sched.in head/x11/slim/files/slim.in Modified: head/audio/musicpd/files/musicpd.in ============================================================================== --- head/audio/musicpd/files/musicpd.in Fri May 31 11:33:41 2013 (r319486) +++ head/audio/musicpd/files/musicpd.in Fri May 31 11:54:01 2013 (r319487) @@ -27,8 +27,7 @@ stop_precmd=${name}_getpidfile musicpd_getpidfile() { - if type get_pidfile_from_conf >/dev/null 2>&1 && - get_pidfile_from_conf pid_file %%PREFIX%%/etc/musicpd.conf ; then + if get_pidfile_from_conf pid_file %%PREFIX%%/etc/musicpd.conf ; then pidfile="$_pidfile_from_conf" else pidfile="%%MPDDIR%%/.mpd/pid" Modified: head/net-mgmt/hawk/files/hawk.in ============================================================================== --- head/net-mgmt/hawk/files/hawk.in Fri May 31 11:33:41 2013 (r319486) +++ head/net-mgmt/hawk/files/hawk.in Fri May 31 11:54:01 2013 (r319487) @@ -24,18 +24,16 @@ load_rc_config hawk command=%%PREFIX%%/sbin/hawk command_interpreter=%%PERL%% command_args='&' +start_precmd=find_pidfile +stop_precmd=find_pidfile find_pidfile() { - if type get_pidfile_from_conf >/dev/null 2>&1 && - get_pidfile_from_conf pidfile %%PREFIX%%/etc/hawk/daemon.conf; then + if get_pidfile_from_conf pidfile %%PREFIX%%/etc/hawk/daemon.conf; then pidfile="$_pidfile_from_conf" else pidfile='/var/run/hawk.pid' fi } -start_precmd=find_pidfile -stop_precmd=find_pidfile - run_rc_command "$1" Modified: head/net-mgmt/nrpe2/files/nrpe2.in ============================================================================== --- head/net-mgmt/nrpe2/files/nrpe2.in Fri May 31 11:33:41 2013 (r319486) +++ head/net-mgmt/nrpe2/files/nrpe2.in Fri May 31 11:54:01 2013 (r319487) @@ -37,8 +37,7 @@ find_pidfile() [ -n "$nrpe2_pidfile" ] && warn "No longer necessary to set nrpe2_pidfile in rc.conf[.local]" - if type get_pidfile_from_conf >/dev/null 2>&1 && - get_pidfile_from_conf pid_file %%PREFIX%%/etc/nrpe.cfg; then + if get_pidfile_from_conf pid_file %%PREFIX%%/etc/nrpe.cfg; then pidfile="$_pidfile_from_conf" else pidfile='/var/run/nrpe2/nrpe2.pid' Modified: head/sysutils/munin-node/files/munin-node.in ============================================================================== --- head/sysutils/munin-node/files/munin-node.in Fri May 31 11:33:41 2013 (r319486) +++ head/sysutils/munin-node/files/munin-node.in Fri May 31 11:54:01 2013 (r319487) @@ -15,7 +15,7 @@ . /etc/rc.subr -name="munin_node" +name=munin_node rcvar=munin_node_enable load_rc_config $name @@ -31,12 +31,8 @@ stop_precmd=find_pidfile find_pidfile() { - if type get_pidfile_from_conf >/dev/null 2>&1 && - get_pidfile_from_conf pid_file $munin_node_config; then - pidfile="$_pidfile_from_conf" - else - pidfile=`awk '$1 == "pid_file" { print $2 }' $munin_node_config` - fi + get_pidfile_from_conf pid_file $munin_node_config + pidfile="$_pidfile_from_conf" } -run_rc_command "$1" +run_rc_command $1 Modified: head/sysutils/munin-node/files/munin-sched.in ============================================================================== --- head/sysutils/munin-node/files/munin-sched.in Fri May 31 11:33:41 2013 (r319486) +++ head/sysutils/munin-node/files/munin-sched.in Fri May 31 11:54:01 2013 (r319487) @@ -15,7 +15,7 @@ . /etc/rc.subr -name="munin_sched" +name=munin_sched rcvar=munin_sched_enable load_rc_config $name @@ -30,13 +30,9 @@ stop_precmd=find_pidfile find_pidfile() { - if type get_pidfile_from_conf >/dev/null 2>&1 && - get_pidfile_from_conf pid_file $munin_sched_config; then - pidfile="${_pidfile_from_conf%node*}sched" - pidfile="${pidfile}${_pidfile_from_conf#*node}" - else - pidfile=`awk '$1 == "pid_file" { gsub("node","sched",$2); print $2; }' $munin_sched_config` - fi + get_pidfile_from_conf pid_file $munin_sched_config + pidfile="${_pidfile_from_conf%node*}sched" + pidfile="${pidfile}${_pidfile_from_conf#*node}" } run_rc_command "$1" Modified: head/x11/slim/files/slim.in ============================================================================== --- head/x11/slim/files/slim.in Fri May 31 11:33:41 2013 (r319486) +++ head/x11/slim/files/slim.in Fri May 31 11:54:01 2013 (r319487) @@ -19,28 +19,26 @@ name="slim" rcvar=slim_enable -start_precmd=${name}_rmfile -stop_precmd=${name}_prestop -stop_postcmd=${name}_rmfile - -load_rc_config $name +load_rc_config slim -: ${slim_enable="NO"} +: ${slim_enable:="NO"} command=%%PREFIX%%/bin/slim command_args="-d" +start_precmd=${name}_rmfile +stop_precmd=${name}_prestop +stop_postcmd=${name}_rmfile find_pidfile() { - if type get_pidfile_from_conf >/dev/null 2>&1 && - get_pidfile_from_conf lockfile %%PREFIX%%/etc/${name}.conf; then + if get_pidfile_from_conf lockfile %%PREFIX%%/etc/${name}.conf; then pidfile="$_pidfile_from_conf" else pidfile="/var/run/${name}.pid" fi } -slim_rmfile () +slim_rmfile() { local file @@ -54,7 +52,7 @@ slim_rmfile () return 0 } -slim_prestop () +slim_prestop() { local xpid _______________________________________________ svn-ports-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-ports-all To unsubscribe, send any mail to "svn-ports-all-unsubscribe@freebsd.org"
State Changed From-To: open->closed Thanks for all your quick responses. To those who objkected to the quote removals, I've left them as-is. I am working on the rc script style guide to remove quotes where unnecessary, but it's not a problem if they are there.