mailinabox
mailinabox copied to clipboard
Allow user to overwrite config using `$STORAGE_ROOT/mailinabox.conf`
Based on comments from https://github.com/mail-in-a-box/mailinabox/pull/2349#issuecomment-1987193731, this PR attempts to provide a convention on how mailinbox determines its configuration
- mailinbox comes with defaults stored in
/etc/mailinabox.conf - Everything in 1 can be overwritten by defining it again in
$STORAGE_ROOT/mailinabox.conf - Backwards compatible If
$STORAGE_ROOT/mailinabox.confdoes not exists
To make it easier for the reviewer(s),
- All code changes are found in https://github.com/mail-in-a-box/mailinabox/pull/2382/commits/a7beb721680d405f86567ba001caecdb5eb2fe7e, which tries to read from
$STORAGE_ROOT/mailinabox.confintoDEFAULT_xxx
I hope this can be accepted so that I can further proceed with introducing a configurable TTL and DKIM_SELECTOR
#2352 #2383
What is the purpose of having multiple files?
additionally, there are dozens of uses of /etc/mailinabox.conf in the codebase that would not use changes in $STORAGE_ROOT/mailinabox.conf.
Syntax changes to code only masks git blame's and are likely not needed in a change to $STORAGE_ROOT/mailinabox.conf
What is the purpose of having multiple files?
For a lack of better word, I will call it layering (or inheritance), the algo goes like this:
- read
/etc/mailinabox.confas default, out-of-box (OOB) settings - customize OOB settings by overwriting with whatever is inside
$STORAGE_ROOT/mailinabox.conf
VSCode loads its settings this way, and I found to be very flexible for all kinds of use cases And it is easy to tell what are the defaults, and what the user has changed
additionally, there are dozens of uses of /etc/mailinabox.conf in the codebase that would not use changes in
$STORAGE_ROOT/mailinabox.conf.
Thanks for pointing this out, I missed it.. downside is, this PR not going to work
Syntax changes to code only masks git blame's and are likely not needed in a change to
$STORAGE_ROOT/mailinabox.conf
Ok, I will omit them in future, but ideally we can pick a code formatter, this will allow folks like me to PR more easily and does not affect those who don't use a code formatter
@jvolkenant
I had a quick look, only utils.py needs to have a slight change in logic and all *.sh references are
source setup/functions.sh # load our functions
source /etc/mailinabox.conf # load global vars
I believe this can be solved by combining load global vars into load our functions (and global vars)
Before I proceed, are there any other concerns about this approach?
@dms00
Yes I agree that it breaks the blame part, a discussion here https://github.com/orgs/community/discussions/5033 led to GitHub's solution here https://docs.github.com/en/repositories/working-with-files/using-files/viewing-a-file#ignore-commits-in-the-blame-view, so I will be taking out the formatting commits into another PR.
Apart from the formatting changes, are there other concerns? I appreciate an explicit No further concerns as the time I spend on each PR is fairly high, don't want to keep redoing it.
This is a little too complicated for the problem. New options can be stored in STORAGE_ROOT anywhere without having to be in a merged configuration namespace with the settings in mailinabox.conf. The main purpose of mailinabox.conf is to set the location of STORAGE_ROOT and other system properties that aren't likely to be preserved when restoring a backup to a new machine.