conf/165817: [periodic] [patch] /etc/periodic reports misconfiguration when it shouldn't

[John Baldwin]

The following reply was made to PR conf/165817; it has been noted by GNATS.

From: John Baldwin <jhb at FreeBSD.org>
To: Volodymyr Kostyrko <c.kworr at gmail.com>
Cc: bug-followup at freebsd.org
Subject: Re: conf/165817: [periodic] [patch] /etc/periodic reports misconfiguration
 when it shouldn&#39;t
Date: Tue, 08 May 2012 10:52:37 -0400

 On 5/8/12 3:57 AM, Volodymyr Kostyrko wrote:
 > John Baldwin wrote:
 >> This doesn't make sense.  The various variables don't have a default
 >> value in
 >> /etc/defaults/rc.conf (e.g. daily_local), so they should just be
 >> empty, and
 >> the for loop should not do anything if the variable is empty.  For
 >> example, if
 >> you run this in /bin/sh:
 >>
 >> $ for script in $notexists
 >>> do
 >>> echo foo
 >>> done
 >> $
 >>
 >> You don't get any output at all.  Thus, having the default
 >> configuration of
 >> not having these variables set should never get into the loop to
 >> execute the
 >> line you are modifying.
 >>
 >> In your case you must have daily_local, etc. set to some absolute path
 >> that
 >> doesn't exist (e.g. daily_local="/nonexistent") in which case that is a
 >> misconfiguration that the scripts should warn you about.
 >>
 >> Or is the problem that you have daily_local set to "/etc/*.local" (the
 >> glob)
 >> and that isn't matching, so the shell runs the loop with the value
 >> "/etc/*.local"?  That is a bit harder to fix.
 >>
 >> Your patch would not be correct if someone set "daily_local" to
 >> "/nonexistent".  That is a case that _should_ be warned about.
 > 
 > Yes, I should probably rethink the patch. It definitely should warn
 > about /nonexistent.
 > 
 > That makes me somehow stuck. periodic scripts source
 > /etc/defaults/periodic.conf which defines this variables as
 > "/etc/*.local". So when it comes to checking whether this file is
 > available the variable is already set. The possible solution is to check
 > whether variable is set to the default path and skip warning if default
 > path is unexistent or adding empty placeholders for required files. I
 > personally prefer the latter to keep scripts as simple as possible and
 > strip all signs of artificial intelligence aka weird logic.
 
 Oof, now I see.  I was looking at /etc/defaults/rc.conf rather than
 /etc/defaults/periodic.conf for some reason.  We certainly should not
 warn in the default install.  I'll think about this some more.
 
 -- 
 John Baldwin