Code review comment for lp:~mdavidsaver/epics-base/epicsconf

Revision history for this message
Andrew Johnson (anj) wrote :

The Art of Unix Programming talks about application configuration here:
    http://www.faqs.org/docs/artu/configurationchapter.html
In particular, http://www.faqs.org/docs/artu/ch10s04.html#id2942882 says that environment variables are useful when:
 * A value varies across several contexts that share dotfiles,
 * A value varies too often for dotfiles, but doesn't change on every startup
Both of those definitely apply to Channel Access client programs, so I don't see myself as being the only person to disagree with your blanket statement "Environment variables are a poor way to do configuration."

I agree that it is not necessary to implement this feature for IOCs, but as written all Linux IOCs /will/ attempt to read those files while IOCs on other architectures would not; this will cause user confusion, so at minimum all workstation architectures would need to support this (at the APS we have soft IOCs that we can run on Linux, Darwin or Solaris as needed). Unlike the access security and dbStatic subsystems we don't have separate versions of libCom for Host vs. IOC programs, so we're stuck with only being able to distinguish by target architecture.

I think we should implement this for all architectures, but make the list of files to read be configurable from a file in base/configure/, and since we already have a mechanism that does some of the work we should modify that to implement the solution. Here's how I think this should work:

1. Modify base/src/libCom/env/bldEnvData.pl and any necessary build rules to read the optional files CONFIG_ENV.$(T_A) and CONFIG_SITE_ENV.$(T_A) from base/configure when generating the envData.c files in libCom/O.$(T_A). Obviously the architecture-specific files should override (i.e. be read after) the generic ones. This gives base and our sites the ability to specify architecture-specific defaults to the existing environment parameters.
2. Add a new enviroment parameter named something like EPICS_CONFIG_PATH or EPICS_RC_FILES that lists the config files to be read. You don't have to allow the environment variable to override the compiled-in value if you don't want to, although I don't see the point of preventing an override if a user has some need to do that (I personally hope to never need to set LD_PRELOAD, but I like that it's there if I do). The default value for this parameter will be given in an architecture-specific base/configure file as described above, so that on vxWorks and RTEMS it would be blank, and Windows can have a different setting than the Unix-like OSs.
3. The file parser should be integrated into base/src/libCom/env/envSubr.c (not using STL, although I don't object to C++ if you prefer it), and it should only accept settings which are known parameters. It reads the files in order and overrides the compiled-in pParam->pdflt strings with the values from the config files (this will need changes to envDefs.h to move the const).

Doing it this way any environment variables will still override but can be distinguished from the values read from the files. File settings don't pollute the environment variables, nor do we permit users to add other settings to the files which are not EPICS parameters. The epicsPrtEnvParams() routine displays the current parameters while epicsEnvShow (in libCom/osi/os/*/osdEnv.c) lists the environment variables.

If you want to reimplement the libCom/env stuff from scratch that's fine as long as the parameter query APIs and the configure/*_ENV files stay the same. I would like to see a more integrated solution than you are proposing, which feels like a last-minute addition which increases our technical debt.

« Back to merge proposal