Bug 178269 - [PATCH] Remove checks for get_pidfile_from_conf function
Summary: [PATCH] Remove checks for get_pidfile_from_conf function
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: Normal Affects Only Me
Assignee: Chris Rees
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-04-30 22:50 UTC by Chris Rees
Modified: 2013-05-31 13:00 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Rees freebsd_committer 2013-04-30 22:50:00 UTC
	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
Comment 1 Chris Rees 2013-04-30 22:52:29 UTC
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
Comment 2 jarrod 2013-05-03 10:25:52 UTC
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.=
Comment 3 Henry Hu 2013-05-06 16:17:58 UTC
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
Comment 4 Chris Rees 2013-05-30 15:33:28 UTC
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
Comment 5 Chris Rees freebsd_committer 2013-05-30 15:36:00 UTC
Responsible Changed
From-To: freebsd-ports-bugs->crees

I'll take it.
Comment 6 Henry Hu 2013-05-30 18:18:09 UTC
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
Comment 7 dfilter service freebsd_committer 2013-05-31 12:54:19 UTC
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"
Comment 8 Chris Rees freebsd_committer 2013-05-31 12:55:06 UTC
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.