Date: Mon, 09 Jan 2012 02:14:28 -0800 From: Doug Barton <dougb@FreeBSD.org> To: Stefan Esser <se@freebsd.org> Cc: Hiroki Sato <hrs@FreeBSD.org>, freebsd-rc@FreeBSD.org Subject: Re: Making use of set_rcvar. Message-ID: <4F0ABE04.5050503@FreeBSD.org> In-Reply-To: <4F0AB60E.7090601@freebsd.org> References: <4F079A76.3030306@FreeBSD.org> <20120107112538.GC1696@garage.freebsd.pl> <4F08C95F.6040808@FreeBSD.org> <20120108.081216.1547061187942402256.hrs@allbsd.org> <4F0A22D8.8090206@FreeBSD.org> <4F0AB60E.7090601@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 01/09/2012 01:40, Stefan Esser wrote:
> Am 09.01.2012 00:12, schrieb Doug Barton:
>> Attached is a patch that does what I suggested a long time ago, removes
>> set_rcvar() entirely and assigns each rcvar statically. I'll commit this
>> in a few days if no one objects. (Note, it can't be committed yet
>> because the scripts in ports that call set_rcvar() have to be modified
>> first.)
> [...]
>> Index: rc.d/nscd
>> ===================================================================
>> --- rc.d/nscd (revision 229825)
>> +++ rc.d/nscd (working copy)
>> @@ -19,7 +19,7 @@
>> . /etc/rc.subr
>>
>> name="nscd"
>> -rcvar=`set_rcvar`
>> +rcvar="nscd_enable"
>
> Why not generally use
>
> rcvar="${name}_enable"
>
> instead of (e.g.)
>
> rcvar="nscd_enable"
>
> in all scripts *in your patch set*, for which the outcome is the same?
Because there is no good reason to add an extra layer of indirection.
IMO we should actually dereference all instances of $name in the
scripts, but I don't want to create the churn without good reason.
> But for nfsd vs. nfs_server and cleartmp vs. clear_tmp it might be a
> good idea to modify $name to match the value of the _enable parameter,
> anyway.
> Then only the special cases sendmail_*_enable and moused_*_enable would
> persist.
That sounds great, are you going to handle all the compatibility shims,
through the next 2 major releases? These knobs are not just used
internally, they are in users' rc.conf[.local] files. Changing the
special cases shouldn't be done without a really good reason. The
purpose (in part) of having a distinct $rcvar value in the first place
is to deal with them.
> The use of "${name}_enable" does not add measurable overhead, but that
> way more of an existing script might be used as a prototype unchanged.
I understand what you're saying, and I know that the whole "use
variables wherever we can" thing is all '1337 and computer science'y,
but it's silly. The concept of a universal template that can be copied
and pasted for different services is a pipe dream. There are already
many things that need to be changed in the new script, and not updating
rcvar for a new script causes clear and obvious failure messages.
> But using ${name}_enable enforces the use of an enable parameter that
> actually is based on $name, not a slight variation thereof (ISTR such
> cases existed and had to be fixed to avoid confusion).
For all new scripts we enforce <file name> == PROVIDE == $name (and by
extension rcvar="${name}_enable"). That's all part and parcel of the
same code review, so there is no reason to believe that requiring use of
the variable is going to cause proper code review any better than
actually, you know, doing proper code review.
Doug
--
You can observe a lot just by watching. -- Yogi Berra
Breadth of IT experience, and depth of knowledge in the DNS.
Yours for the right price. :) http://SupersetSolutions.com/
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4F0ABE04.5050503>