Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions doc/release-notes-pr10267.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
Changed command-line options
----------------------------

- `-includeconf=<file>` can be used to include additional configuration files.
Only works inside the `bitcoin.conf` file, not inside included files or from
command-line. Multiple files may be included. Can be disabled from command-
line via `-noincludeconf`. Note that multi-argument commands like
`-includeconf` will override preceding `-noincludeconf`, i.e.

noincludeconf=1
includeconf=relative.conf

as bitcoin.conf will still include `relative.conf`.
2 changes: 1 addition & 1 deletion src/bitcoin-cli.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ static int AppInitRPC(int argc, char* argv[])
return EXIT_FAILURE;
}
try {
gArgs.ReadConfigFile(gArgs.GetArg("-conf", BITCOIN_CONF_FILENAME));
gArgs.ReadConfigFiles();
} catch (const std::exception& e) {
fprintf(stderr,"Error reading configuration file: %s\n", e.what());
return EXIT_FAILURE;
Expand Down
2 changes: 1 addition & 1 deletion src/bitcoind.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ bool AppInit(int argc, char* argv[])
}
try
{
gArgs.ReadConfigFile(gArgs.GetArg("-conf", BITCOIN_CONF_FILENAME));
gArgs.ReadConfigFiles();
} catch (const std::exception& e) {
fprintf(stderr,"Error reading configuration file: %s\n", e.what());
return false;
Expand Down
1 change: 1 addition & 0 deletions src/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,7 @@ std::string HelpMessage(HelpMessageMode mode)
strUsage += HelpMessageOpt("-debuglogfile=<file>", strprintf(_("Specify location of debug log file. Relative paths will be prefixed by a net-specific datadir location. (default: %s)"), DEFAULT_DEBUGLOGFILE));
if (showDebug)
strUsage += HelpMessageOpt("-feefilter", strprintf("Tell other nodes to filter invs to us by our mempool min fee (default: %u)", DEFAULT_FEEFILTER));
strUsage += HelpMessageOpt("-includeconf=<file>", _("Specify additional configuration file, relative to the -datadir path (only useable from configuration file, not command line)"));
strUsage += HelpMessageOpt("-loadblock=<file>", _("Imports blocks from external blk000??.dat file on startup"));
strUsage += HelpMessageOpt("-maxmempool=<n>", strprintf(_("Keep the transaction memory pool below <n> megabytes (default: %u)"), DEFAULT_MAX_MEMPOOL_SIZE));
strUsage += HelpMessageOpt("-maxorphantx=<n>", strprintf(_("Keep at most <n> unconnectable transactions in memory (default: %u)"), DEFAULT_MAX_ORPHAN_TRANSACTIONS));
Expand Down
2 changes: 1 addition & 1 deletion src/interfaces/node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class NodeImpl : public Node
{
gArgs.ParseParameters(argc, argv);
}
void readConfigFile(const std::string& conf_path) override { gArgs.ReadConfigFile(conf_path); }
void readConfigFiles() override { gArgs.ReadConfigFiles(); }
bool softSetArg(const std::string& arg, const std::string& value) override { return gArgs.SoftSetArg(arg, value); }
bool softSetBoolArg(const std::string& arg, bool value) override { return gArgs.SoftSetBoolArg(arg, value); }
void selectParams(const std::string& network) override { SelectParams(network); }
Expand Down
2 changes: 1 addition & 1 deletion src/interfaces/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class Node
virtual bool softSetBoolArg(const std::string& arg, bool value) = 0;

//! Load settings from configuration file.
virtual void readConfigFile(const std::string& conf_path) = 0;
virtual void readConfigFiles() = 0;

//! Choose network parameters.
virtual void selectParams(const std::string& network) = 0;
Expand Down
2 changes: 1 addition & 1 deletion src/qt/bitcoin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -616,7 +616,7 @@ int main(int argc, char *argv[])
return EXIT_FAILURE;
}
try {
node->readConfigFile(gArgs.GetArg("-conf", BITCOIN_CONF_FILENAME));
node->readConfigFiles();
} catch (const std::exception& e) {
QMessageBox::critical(0, QObject::tr(PACKAGE_NAME),
QObject::tr("Error: Cannot parse configuration file: %1. Only use key=value syntax.").arg(e.what()));
Expand Down
35 changes: 34 additions & 1 deletion src/util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,17 @@ void ArgsManager::ParseParameters(int argc, const char* const argv[])
m_override_args[key].push_back(val);
}
}

// we do not allow -includeconf from command line, so we clear it here
auto it = m_override_args.find("-includeconf");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a loop, otherwise only the first -includeconf is removed from m_override_args. Also update comment above accordingly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is defined as a std::map<std::string, std::vector<std::string>>, which may only have one entry per key. It removes the vector itself, which may contain multiple values. (I am only showing the first for simplicity in the error output.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. Either log all or nothing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Added loop over vector.

if (it != m_override_args.end()) {
if (it->second.size() > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be nice if we could be more aggressive here and fail to start if a -includeconf argument is provided on the command line (since continuing to startup would be continuing with config that the user has specified that they don't want).

ParseParameters() doesn't currently have a way to return errors, and callers don't wrap calls with try-except (unlike GetChainName() for example). Perhaps a future PR could change the function to return a bool, so we could indicate an error to the caller and abort startup.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. I'm leaving it for now as it sounds like a future thing, I think.

for (const auto& ic : it->second) {
fprintf(stderr, "warning: -includeconf cannot be used from commandline; ignoring -includeconf=%s\n", ic.c_str());
}
m_override_args.erase(it);
}
}
}

std::vector<std::string> ArgsManager::GetArgs(const std::string& strArg) const
Expand Down Expand Up @@ -706,18 +717,40 @@ void ArgsManager::ReadConfigStream(std::istream& stream)
}
}

void ArgsManager::ReadConfigFile(const std::string& confPath)
void ArgsManager::ReadConfigFiles()
{
{
LOCK(cs_args);
m_config_args.clear();
}

const std::string confPath = GetArg("-conf", BITCOIN_CONF_FILENAME);
fs::ifstream stream(GetConfigFile(confPath));

// ok to not have a config file
if (stream.good()) {
ReadConfigStream(stream);
// if there is an -includeconf in the override args, but it is empty, that means the user
// passed '-noincludeconf' on the command line, in which case we should not include anything
if (m_override_args.count("-includeconf") == 0) {
std::vector<std::string> includeconf(GetArgs("-includeconf"));
{
// We haven't set m_network yet (that happens in SelectParams()), so manually check
// for network.includeconf args.
std::vector<std::string> includeconf_net(GetArgs(std::string("-") + GetChainName() + ".includeconf"));
includeconf.insert(includeconf.end(), includeconf_net.begin(), includeconf_net.end());
}

for (const std::string& to_include : includeconf) {
fs::ifstream include_config(GetConfigFile(to_include));
if (include_config.good()) {
ReadConfigStream(include_config);
LogPrintf("Included configuration file %s\n", to_include.c_str());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

printf?

} else {
fprintf(stderr, "Failed to include configuration file %s\n", to_include.c_str());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: return false on error?

}
}
}
}

// If datadir is changed in .conf file:
Expand Down
2 changes: 1 addition & 1 deletion src/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ class ArgsManager
void SelectConfigNetwork(const std::string& network);

void ParseParameters(int argc, const char*const argv[]);
void ReadConfigFile(const std::string& confPath);
void ReadConfigFiles();

/**
* Log warnings for options in m_section_only_args when
Expand Down
78 changes: 78 additions & 0 deletions test/functional/feature_includeconf.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
#!/usr/bin/env python3
# Copyright (c) 2018 The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
"""Tests the includeconf argument

Verify that:

1. adding includeconf to the configuration file causes the includeconf
file to be loaded in the correct order.
2. includeconf cannot be used as a command line argument.
3. includeconf cannot be used recursively (ie includeconf can only
be used from the base config file).
4. multiple includeconf arguments can be specified in the main config
file.
"""
import os
import tempfile

from test_framework.test_framework import BitcoinTestFramework, assert_equal

class IncludeConfTest(BitcoinTestFramework):
def set_test_params(self):
self.setup_clean_chain = False
self.num_nodes = 1

def setup_chain(self):
super().setup_chain()
# Create additional config files
# - tmpdir/node0/relative.conf
with open(os.path.join(self.options.tmpdir, "node0", "relative.conf"), "w", encoding="utf8") as f:
f.write("uacomment=relative\n")
# - tmpdir/node0/relative2.conf
with open(os.path.join(self.options.tmpdir, "node0", "relative2.conf"), "w", encoding="utf8") as f:
f.write("uacomment=relative2\n")
with open(os.path.join(self.options.tmpdir, "node0", "bitcoin.conf"), "a", encoding='utf8') as f:
f.write("uacomment=main\nincludeconf=relative.conf\n")

def run_test(self):
self.log.info("-includeconf works from config file. subversion should end with 'main; relative)/'")

subversion = self.nodes[0].getnetworkinfo()["subversion"]
assert subversion.endswith("main; relative)/")

self.log.info("-includeconf cannot be used as command-line arg. subversion should still end with 'main; relative)/'")
self.stop_node(0)
with tempfile.SpooledTemporaryFile(max_size=2**16) as log_stderr:
self.start_node(0, extra_args=["-includeconf=relative2.conf"], stderr=log_stderr)

subversion = self.nodes[0].getnetworkinfo()["subversion"]
assert subversion.endswith("main; relative)/")
log_stderr.seek(0)
stderr = log_stderr.read().decode('utf-8').strip()
assert_equal(stderr, 'warning: -includeconf cannot be used from commandline; ignoring -includeconf=relative2.conf')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: should probably be an (init)error, since it is clearly an invalid command line


self.log.info("-includeconf cannot be used recursively. subversion should end with 'main; relative)/'")
with open(os.path.join(self.options.tmpdir, "node0", "relative.conf"), "a", encoding="utf8") as f:
f.write("includeconf=relative2.conf\n")

self.restart_node(0)

subversion = self.nodes[0].getnetworkinfo()["subversion"]
assert subversion.endswith("main; relative)/")

self.log.info("multiple -includeconf args can be used from the base config file. subversion should end with 'main; relative; relative2)/'")
with open(os.path.join(self.options.tmpdir, "node0", "relative.conf"), "w", encoding="utf8") as f:
f.write("uacomment=relative\n")

with open(os.path.join(self.options.tmpdir, "node0", "bitcoin.conf"), "a", encoding='utf8') as f:
f.write("includeconf=relative2.conf\n")

self.restart_node(0)

subversion = self.nodes[0].getnetworkinfo()["subversion"]
assert subversion.endswith("main; relative; relative2)/")

if __name__ == '__main__':
IncludeConfTest().main()
1 change: 1 addition & 0 deletions test/functional/test_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@
'p2p_fingerprint.py',
'feature_uacomment.py',
'p2p_unrequested_blocks.py',
'feature_includeconf.py',
'feature_logging.py',
'p2p_node_network_limited.py',
'feature_blocksdir.py',
Expand Down