Date: Fri, 12 Feb 2010 12:07:24 -0800 From: Doug Barton <dougb@FreeBSD.org> To: Pav Lucistnik <pav@FreeBSD.org> Cc: Sunpoet Hsieh <sunpoet@sunpoet.net>, cvs-ports@FreeBSD.org, Mykola Dzham <i@levsha.me>, cvs-all@FreeBSD.org, ports-committers@FreeBSD.org Subject: Re: cvs commit: ports/audio/icecast2 Makefile ports/audio/icecast2/files icecast2.sh.in Message-ID: <4B75B4FC.4080202@FreeBSD.org> In-Reply-To: <201002121043.o1CAhFJ0099892@repoman.freebsd.org> References: <201002121043.o1CAhFJ0099892@repoman.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 2/12/2010 2:43 AM, Pav Lucistnik wrote: > pav 2010-02-12 10:43:15 UTC > > FreeBSD ports repository > > Modified files: > audio/icecast2 Makefile > audio/icecast2/files icecast2.sh.in > Log: > - Correct login/LOGIN in rc script > - Minor cleanup > > PR: ports/143532 http://www.FreeBSD.org/cgi/query-pr.cgi?pr=143532 > Submitted by: Mykola Dzham <i@levsha.me> > Approved by: Sunpoet Hsieh <sunpoet@sunpoet.net> (maintainer) > > Revision Changes Path > 1.63 +1 -2 ports/audio/icecast2/Makefile > 1.5 +2 -2 ports/audio/icecast2/files/icecast2.sh.in > > http://www.FreeBSD.org/cgi/cvsweb.cgi/ports/audio/icecast2/Makefile.diff?&r1=1.62&r2=1.63&f=h > http://www.FreeBSD.org/cgi/cvsweb.cgi/ports/audio/icecast2/files/icecast2.sh.in.diff?&r1=1.4&r2=1.5&f=h The change to LOGIN is a good catch, thanks for doing that. There are a few other problems with this script however. Most importantly, if the service is running as a user other than root (which the comment implies) then the current REQUIRE/BEFORE should be changed to just "REQUIRE: LOGIN". Also, if the xml file really is required then the -c option should be added to command_args so that a user does not accidentally delete it. If your intention is to allow the user to specify a different configuration file that should be down with its own icecast_ option. You would then move the required_files definition to after load_rc_config and the default variable assignments. Something like this: load_rc_config $name : ${icecast_enable="NO"} : ${icecast_conf:="%%PREFIX%%/etc/$name.xml"} required_files="$icecast_conf" Less importantly it's preferred that the name of the script, the PROVIDE line, and $name all match. For some reason the first 2 here are icecast2, while name=icecast. Is there a reason they cannot match? If not, this should be adjusted at some point in the future, but it's not critical. hth, Doug -- ... and that's just a little bit of history repeating. -- Propellerheads Improve the effectiveness of your Internet presence with a domain name makeover! http://SupersetSolutions.com/
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4B75B4FC.4080202>