Periodic 999.local scripts reports misconfiguration if they fail to find /etc/*.local file. However the absence of this file is not misconfiguration but is rather an option to specify local configuration additions in netboot environment. After that if anyone sets: *_show_badconfig=yes *_show_success=no The minimal mail would be emailed anyway regardless of other tasks: /etc/*.local: No such file -- End of * output -- Attached patch fixes this by not modifying `rc` when `*.local` file is not found. Generated output remains the same and the only change would be not generating mail when `*.local` file is not found locally. Fix: Patch attached with submission follows:
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. -- John Baldwin
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. -- Sphinx of black quartz judge my vow.
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
Fixed by r313069 in head. MFCed to stable/11 by r316359