Page MenuHomeFreeBSD

Including rc_conf_directories to be loaded with rc(8)
Needs ReviewPublic

Authored by larafercue_gmail.com on Jun 7 2018, 2:05 PM.

Details

Reviewers
brooks
0mp
Summary

rc.conf.d directories contains configuration files to be used at
boot time. This patch allows the user to write configuration files
under /etc/rc.conf.d and being loaded at boot time.

Test Plan

Create configuration files under /etc/rc.conf.d and rebooting to
see if they were loaded. Also changing the rc_conf_dirs to include
/usr/local/etc/rc.conf.d and repeat the process

Sponsored by: Bally Wulff Games & Entertainment GmbH

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 26587
Build 24971: arc lint + arc unit

Event Timeline

rpokala added a subscriber: rpokala.Jun 7 2018, 2:22 PM
rpokala added inline comments.
etc/defaults/rc.conf
711 ↗(On Diff #43409)

You're not actually using "sourced_dirs" anywhere.

714 ↗(On Diff #43409)

This will find files or directories named f; that's probably not what you intended...

(Perhaps you wanted find ${i} -type f?)

Removing unused variables and fixing find

larafercue_gmail.com marked 2 inline comments as done.Jun 7 2018, 2:29 PM
larafercue_gmail.com added inline comments.
etc/defaults/rc.conf
714 ↗(On Diff #43409)

That's actually what I wanted, thanks

rpokala added inline comments.Jun 7 2018, 2:32 PM
etc/defaults/rc.conf
711 ↗(On Diff #43412)

Oh, do you need to declare d as a local?

larafercue_gmail.com marked 3 inline comments as done.

Declaring d as local variable

Fixing problems and submitting comments

Exchanging i and d so d is for directories and i for files. Also including them

rpokala added inline comments.Jun 7 2018, 2:44 PM
etc/defaults/rc.conf
731 ↗(On Diff #43414)

Does the new stuff need to be repeated here?

larafercue_gmail.com marked an inline comment as done.

Repeating the block to catch redefined rc_conf_dirs

Submitting commits

etc/defaults/rc.conf
731 ↗(On Diff #43414)

I didn't thought to be needed, but if the variable is defined in the configuration files, it might be a good idea.

I think this looks good now. Now you can point someone who actually knows `rc' at it. :-)

dab added a subscriber: dab.Jun 7 2018, 4:51 PM
dab added inline comments.
etc/defaults/rc.conf
714 ↗(On Diff #43418)

You could save the nested for loops by doing:

for i in $(find ${rc_conf_dirs} -type f) ; do

714 ↗(On Diff #43418)

Do you actually want to do a recursive search through the rc_conf_dirs or limit the scope to just a first-level search? To be honest, I'm not sure of the "correct" answer to this, but rc only searches the first level in /etc/rc.d and /usr/local/rc.d for scripts to execute.

714 ↗(On Diff #43418)

Do you want to source all files found, or limit to a certain subset, such as only those files that end with ".conf"? I think there is precedent for both. Also, should "dot" files (e.g., /etc/rc.conf.d/.example.conf) be excluded?

714 ↗(On Diff #43418)

OK, I think this is my last comment (for now, at least).

Do you want to find and source the files in lexicographic order so that someone could, for example, ensure that /etc/rc.conf.d/00-local.conf was sourced before /etc/rc.conf.d/99-local.conf?

dab edited the summary of this revision. (Show Details)Jun 7 2018, 4:52 PM

Adding inline comments to figure out the best way to proceed.

etc/defaults/rc.conf
714 ↗(On Diff #43418)

could it be done like this?

for i in $(find -s ${rc_conf_dirs} -type f -depth 1 -name '*.conf' -and -not -name '.*') ; do

Searching only on the first level makes sense, they are a collection of configuration files and, in the case that the user wants to sort them out in folders, they can be searched by overriding the rc_conf_dirs variable.

It also makes sense to sort them out in lexicographic order.

larafercue_gmail.com added a subscriber: brooks.

Adding @brooks because he is in the subversion's property $FreeBSD$

dab added inline comments.Jun 8 2018, 12:42 PM
etc/defaults/rc.conf
714 ↗(On Diff #43418)

Yes, I think that will do it.

larafercue_gmail.com marked 2 inline comments as done.

Limiting search to immediates files named '*.conf' not starting with a dot

larafercue_gmail.com marked 4 inline comments as done.Jun 8 2018, 2:29 PM
0mp added a subscriber: 0mp.Nov 5 2018, 5:46 PM
0mp requested changes to this revision.Jul 31 2019, 3:56 PM
  1. I'd think that files in rc.conf.d should override what is in $rc_conf_files as they are more specific. Is there a reason why you'd like $rc_conf_files to be able to override files in rc.conf.d instead?
  2. This change deserves an update to the manual page.
etc/defaults/rc.conf
705 ↗(On Diff #43457)

This comment might require an update to mention rc_conf_dirs as well.

713 ↗(On Diff #43457)

-depth 1 should be -maxdepth 1. Otherwise, find(1) is going to iterate over all the subdirectories anyway.

730 ↗(On Diff #43457)

-depth 1 should be -maxdepth 1. Otherwise, find(1) is going to iterate over all the subdirectories anyway.

This revision now requires changes to proceed.Jul 31 2019, 3:56 PM

Updating the script with the new location and including the changes proposed by 0mp

Remove unused variable