-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 2/16/13 10:24 AM, Chris Rees wrote:
> On 16 February 2013 18:08, Gary Palmer <gpal...@freebsd.org>
> wrote:
>> On Sat, Feb 16, 2013 at 05:38:56PM +0000, Chris Rees wrote:
>>> On 16 February 2013 17:05, Paul Mather
>>> <p...@gromit.dlib.vt.edu> wrote:
>>>> On Feb 16, 2013, at 4:21 AM, Jeremy Chadwick wrote:
>>>> 
>>>>> On Sat, Feb 16, 2013 at 12:23:33PM +0400, Boris Samorodov
>>>>> wrote:
>>>>>> 16.02.2013 01:32, Jeremy Chadwick ??????????:
>>>>>> 
>>>>>>> Follow up -- I read Alfred's most recent mail.  Lo and
>>>>>>> behold, I find this in /var/log/messages (but such did
>>>>>>> not come to my terminal):
>>>>>>> 
>>>>>>> Feb 15 13:26:20 icarus jdc: /usr/sbin/service: WARNING:
>>>>>>> $svnserve_enable is not set properly - see rc.conf(5). 
>>>>>>> Feb 15 13:26:20 icarus jdc: /usr/sbin/service: WARNING:
>>>>>>> $smartd_enable is not set properly - see rc.conf(5). 
>>>>>>> Feb 15 13:26:20 icarus jdc: /usr/sbin/service: WARNING:
>>>>>>> $rsyncd_enable is not set properly - see rc.conf(5). 
>>>>>>> Feb 15 13:26:20 icarus jdc: /usr/sbin/service: WARNING:
>>>>>>> $htcacheclean_enable is not set properly - see
>>>>>>> rc.conf(5). Feb 15 13:26:20 icarus jdc:
>>>>>>> /usr/sbin/service: WARNING: $fetchmail_enable is not
>>>>>>> set properly - see rc.conf(5).
>>>>>>> 
>>>>>>> Cute.  Agreed -- this is unacceptable on two levels (as
>>>>>>> I see it):
>>>>>>> 
>>>>>>> 1) These messages should be going to stdout or stderr
>>>>>>> in some way, so honestly logger(8) should be called
>>>>>>> with the "-s" flag (IMO).
>>>>>> 
>>>>>> Fully agreed here.
>>>>> 
>>>>> It turns out logger -s has no effect, just like how the
>>>>> echo 1>&2 statements in warn() and err() have no effect
>>>>> either (these should be outputting the warnings in question
>>>>> to stderr) -- see rc.subr's source for what I'm referring
>>>>> to.
>>>>> 
>>>>> Gary and I have been discussing this off-list and the
>>>>> reason has been found: service(8) has this code in it:
>>>>> 
>>>>> checkyesno $rcvar 2>/dev/null && echo $file
>>>>> 
>>>>> This explains why there's no warn() or err() output on the
>>>>> terminal -- it's being redirected to /dev/null prior.
>>>>> 
>>>>> I do not know who maintains the rc(8) and rc.subr(8)
>>>>> framework, but they've got their work cut out for them.
>>>>> 
>>>>> (Note: the echo statements in warn() and err() could be
>>>>> replaced with "logger -s" as I said; this would allow the
>>>>> "echo 1>&2" to be removed)
>>>>> 
>>>>>>> 2) These messages should not be displayed at all (i.e.
>>>>>>> lack of an xxx_enable variable should imply
>>>>>>> xxx_enable="no").
>>>>>> 
>>>>>> I see this message as one more level of supervision.
>>>>>> 
>>>>>> If undefined at /etc/make.conf the value of xxx_enable is
>>>>>> "no" from the system's POV (i.e. the service is not
>>>>>> strarted). From the admininstrators's POV the port was
>>>>>> installed BUT is not used. It's up to admininstrator
>>>>>> whether it's OK or not -- just let him remind.
>>>>> 
>>>>> I believe the point you're trying to make is that the
>>>>> warning in question should 'act as a reminder to the
>>>>> administrator that they need to set xxx_enable="yes" in
>>>>> rc.conf'.
>>>>> 
>>>>> If not: please explain if you could what you mean, because
>>>>> I don't understand.
>>>>> 
>>>>> If so: I strongly disagree with this method of approach, as
>>>>> what you've proposed is a borderline straw man argument.
>>>>> 
>>>>> Reminding the admin to set xxx_enable is presently done
>>>>> inside most ports' pkg-message.  IMO, this should really be
>>>>> done inside bsd.port.mk when USE_RC_SUBR is used, emitting
>>>>> a message during install that says something like:
>>>>> 
>>>>> To enable the xxx service, please add the following to
>>>>> /etc/rc.conf: xxx_enable="yes"
>>>>> 
>>>>> Of course, I don't know if this would work for packages.
>>>>> 
>>>>> The current message for <missing xxx_enable in rc.conf> is
>>>>> this:
>>>>> 
>>>>> WARNING: $xxx_enable is not set properly - see rc.conf(5).
>>>>> 
>>>>> The message is entirely misleading for this specific
>>>>> situation; it isn't "reminding" an administrator -- if
>>>>> anything it's confusing them (thread is case in point).  If
>>>>> we're going to cater to ignorance, then the message should
>>>>> reflect the situation.
>>>>> 
>>>>> Thus IMO, this is what ***should*** happen:
>>>>> 
>>>>> Definition in rc.conf    Behaviour/result 
>>>>> -----------------------
>>>>> ------------------------------------------- 
>>>>> myprog_enable="yes"      emit no warnings, service should
>>>>> run myprog_enable="no"       emit no warnings, service
>>>>> should not run myprog_enable="abc123"   emit a warning,
>>>>> service should not run <no definition>          emit no
>>>>> warnings, service should not run
>>>> 
>>>> 
>>>> I think case 4 ("<no definition>") is a case where a warning
>>>> should be emitted because it is arguably not immediately
>>>> apparent what will actually happen if no definition is
>>>> present.  In the case of services in the base OS it is
>>>> well-defined: every service should have an explicit default
>>>> in /etc/defaults/rc.conf that you can easily consult to know
>>>> definitively what will happen with that service.  (If it
>>>> doesn't, that is a bug, IMHO.)
>>>> 
>>>> For ports, the case is not so clear.  There is a general
>>>> trend for the port rc.d script to default its respective
>>>> xxx_enable explicitly to "NO".  But it is not a universal
>>>> rule that "no definition" = default to "NO".  The
>>>> net/avahi-app port, for example, doesn't default to "NO" if
>>>> xxx_enable is not set: it defaults to whatever the
>>>> gnome_enable setting is defined to be.
>>> 
>>> With few exceptions, it should be considered a rule that ports
>>> rc scripts contain:
>>> 
>>> : ${xxx_enable=no}
>>> 
>>> to avoid this.  If you see any ports that don't define the
>>> _enable variable at all, they are wrong and need fixing.
>> 
>> Except the 'service' command doesn't parse the individual ports
>> rc.d script so the default is never found.  It relies purely on
>> rc.conf and rc.conf.local settings.
>> 
>> So no matter if the port has
>> 
>> : ${xxx_enable=no}
>> 
>> or not, 'service -e' will still spit out the warnings
>> 
>> Not loading the individual ports files is probably done for two
>> reasons I can think of - performance, and safety (in case the
>> rc.d file is bad for some reason).  However it leads to these
>> rather irritating warnings, especially when you find them later
>> and you don't know what you did to cause them.
>> 
>> I was away to suggest that having /usr/local/etc/rc.conf.d would
>> help as ports could then specify their defaults safely without
>> having to do file rewrites, however that appears to be only
>> looked at in load_rc_config(), and 'service -e' bypasses that as
>> well by doing
>> 
>> load_rc_config 'XXX'
>> 
>> so only /usr/local/etc/rc.conf.d/xxx would be looked at.
> 
> Yes, OK.  A variation on Alfred's checkyes patch is at [1].
> 
> I have not made it into a function deliberately because this is 
> already rc idiom as used in other rc scripts where a warning is
> not necessary, and also because it is very rarely the right thing
> to do (i.e. it would be tempting to overuse it if it were a
> function).

Actually, I think the approach used in service(8) is bogus.  It seems
that it's intended to be fast?

I *think* the code should be actually changed in a way like this:

Index: service.sh
===================================================================
- --- service.sh  (revision 246844)
+++ service.sh  (working copy)
@@ -69,15 +69,15 @@ if [ -n "$RESTART" ]; then

        for file in `reverse_list ${files}`; do
                if grep -q ^rcvar $file; then
- -                       eval `grep ^name= $file`
                        eval `grep ^rcvar $file`
+                       eval `$file rcvar | grep ^${rcvar}`
                        checkyesno $rcvar 2>/dev/null && run_rc_script
${file} stop
                fi
        done
        for file in $files; do
                if grep -q ^rcvar $file; then
- -                       eval `grep ^name= $file`
                        eval `grep ^rcvar $file`
+                       eval `$file rcvar | grep ^${rcvar}`
                        checkyesno $rcvar 2>/dev/null && run_rc_script
${file} start
                fi
        done
@@ -98,8 +98,8 @@ fi
 if [ -n "$ENABLED" ]; then
        for file in $files; do
                if grep -q ^rcvar $file; then
- -                       eval `grep ^name= $file`
                        eval `grep ^rcvar $file`
+                       eval `$file rcvar | grep ^${rcvar}`
                        checkyesno $rcvar 2>/dev/null && echo $file
                fi
        done

However, that this would reveal some issues with the existing rc.d
scripts, e.g. some scripts execute commands regardless it's start,
stop or rcvar (securelevel come to mind; another problem is that
rc.subr checks pid when it doesn't need to do so, e.g. when doing
rcvar), and that should be fixed first in my opinion.

Cheers,
-----BEGIN PGP SIGNATURE-----

iQEcBAEBCAAGBQJRIIzAAAoJEG80Jeu8UPuzC8MIALcFCEXOWwqvqFbTJTphpSwx
2i28r829yoi3cwI1RYByfGBnUfscCGFXEUd13pUvx1vx/1h4kKipQAsS0jiIg2a9
gchtleoUGmw4XN4TNQJfUuyh2eM/yJH1EV7PwvKWhOHJ+iyvYqA9pust2ABxT0Dt
n90ioqMsGI7r7qFt8QBrUE0gfR2mBFKCsV1VFFujC20mrJEJqKDLlfJHA61g1VMP
O+6BexKydN3k/j4/KX1PYJ54KLu2nCJHXFp0SWwVMp8F4s0Ln+lg2C7sE4lSMI8f
UJ2I90ZynNS7uBr03/7x//AMCcsa2tZoddXnPQWf1T2JxaZzk31+msRcYsvsjoo=
=3ewP
-----END PGP SIGNATURE-----
_______________________________________________
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"

Reply via email to