Add specific envvar system #16868

See #16829

Description

This PR adds a way to set some CLI args with an environment variable. The syntax for the envvars are BITCOIN_<ARGNAME>=<VALUE> E.g BITCOIN_DATADIR=/home/emil/bitcoin/ Console arguments override setted envvars. The envvars are always uppercase.

Testing

Set some envvars On Unix:

export BITCOIN_DATADIR=/path/to/your/datadir
export BITCOIN_CONF=/path/to/your/conf
./bitcoin-qt

This will start Bitcoin-Qt using the datadir path with an english overlay.

List of supported envvars

• datadir
• conf
• <s>blocksdir</s>
• <s>debuclogfile</s>
• <s>includeconf</s>
• <s>loadblock</s>
• <s>pid</s>
• Others can be added by changing one line of code

The travis exception is there because all characters there are valid English Latin ASCII chars so there won't be any problems.

Ping @ryanofsky

Two issues I see and suggested fixes I'd make:

1. No documentation. Environment variables interpreted by bitcoin should be part of man pages and probably -help output. Suggested fix: explicitly list environment variables that affect bitcoin settings in relevant documentation, and say what the precedence is (seems to be less than command line settings and more than static config settings, which I think is a good choice).

2. Too many environment variables are read. I think the only environment variables that should be read are BITCOIN_DATADIR are BITCOIN_CONF, because these are sufficient to bring in full configurations, and because exposing more variables creates chances for confusion and inconsistency (why is BITCOIN_BLOCKSDIR interpreted but BITCOIN_WALLETDIR ignored?), bikeshedding, future incompatibility across bitcoin versions, and silently ignored configuration errors. Suggested fix: stop interpreting other variables besides BITCOIN_DATADIR and BITCOIN_CONF.

Probably if this is going to be merged there should also be a python test covering this functionality. But you might could wait for more concept acks before putting work into that.

I guess I don't feel strongly, but overall I'd be for this change with rough edges removed.

@ryanofsky

1. Sure, I will write the documentation later.
2. I added all variables which take a file as an argument. Sure I will remove the others because they are unnecessary variables. However I still think that interpreting pid is ok because it is often used as an argument by init systems.

@ryanofsky Done

what the precedence is (seems to be less than command line settings and more than static config settings, which I think is a good choice).

This will break the functional (and possibly unit tests?)

@MarcoFalke How? Config files or cmd args have a higher priority than envvars. See travis, no errors

Config files or cmd args have a higher priority than envvars.

Would be nice to document this

See travis, no errors

No environment variables are set on travis, so it can't possibly fail

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #16659 (refactoring: Remove unused includes by practicalswift)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

Added help

Concept ACK on BITCOIN_DATADIR an BITCOIN_CONF. I do not believe any of the others are necessary.

Ok, then I will remove BITCOIN_PID

Done 6ccb229

@emilengler Can you squash your chanegs into a single commit (dropping all intermediate commit messages etc), add a more descriptive commit message and body as well as update the PR text to reflect the current state of this PR.

I removed the help from the -help command to the docs because this was only causing a cricular dependency

@fanquake Done

Closed because of lack of interest