From 2c111bc6a1e13050aaa5e7bc5546f0b2f2c5e9f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=A1clav=20Kubern=C3=A1t?= Date: Thu, 5 Jan 2023 18:45:43 +0100 Subject: [PATCH 1/3] Fix CI build --- .github/workflows/run-tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index cf5bc1251..41f509c9b 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -17,7 +17,7 @@ jobs: submodules: true - name: Install/cache ninja, doctest, and Qt's dependencies - uses: awalsh128/cache-apt-pkgs-action@latest + uses: awalsh128/cache-apt-pkgs-action@v1.2.1 with: packages: ninja-build doctest-dev libgl1-mesa-dev libpulse-dev libxcb-icccm4 libxcb-image0 libxcb-keysyms1 libxcb-randr0 libxcb-render-util0 libxcb-shape0 libxcb-util1 libxcb-xinerama0 libxkbcommon-x11-0 version: 1.0 From 1a61dedf2b4edacb2d652e3501281ba7ef5d004c Mon Sep 17 00:00:00 2001 From: Fanda Vacek Date: Thu, 5 Jan 2023 17:30:52 +0100 Subject: [PATCH 2/3] ConfigCLIOptions::configFile: --config relative paths are relative to the location of the program binary #217 --- libshvcore/CMakeLists.txt | 1 + libshvcore/src/utils/clioptions.cpp | 143 +++++++++++++++------------ libshvcore/src/utils/clioptions.h | 1 + libshvcore/tests/test_clioptions.cpp | 108 ++++++++++++++++++++ 4 files changed, 188 insertions(+), 65 deletions(-) create mode 100644 libshvcore/tests/test_clioptions.cpp diff --git a/libshvcore/CMakeLists.txt b/libshvcore/CMakeLists.txt index 1656d24d4..230124791 100644 --- a/libshvcore/CMakeLists.txt +++ b/libshvcore/CMakeLists.txt @@ -45,6 +45,7 @@ if(BUILD_TESTING) add_shvcore_test(crypt) add_shvcore_test(shvlog) add_shvcore_test(join_path) + add_shvcore_test(clioptions) endif() install(TARGETS libshvcore) diff --git a/libshvcore/src/utils/clioptions.cpp b/libshvcore/src/utils/clioptions.cpp index 851f087e2..3667f6d7e 100644 --- a/libshvcore/src/utils/clioptions.cpp +++ b/libshvcore/src/utils/clioptions.cpp @@ -6,23 +6,27 @@ #include #include +#include +#include #include #include #include #include -namespace cp = shv::chainpack; +using namespace shv::chainpack; +using namespace std::string_literals; +using namespace std; namespace shv::core::utils { CLIOptions::Option& CLIOptions::Option::setValueString(const std::string &val_str) { - cp::RpcValue::Type t = type(); + RpcValue::Type t = type(); switch(t) { - case(cp::RpcValue::Type::Invalid): + case(RpcValue::Type::Invalid): shvWarning() << "Setting value:" << val_str << "to an invalid type option."; break; - case(cp::RpcValue::Type::Bool): + case(RpcValue::Type::Bool): { if(val_str.empty()) { setValue(true); @@ -46,8 +50,8 @@ CLIOptions::Option& CLIOptions::Option::setValueString(const std::string &val_st } break; } - case cp::RpcValue::Type::Int: - case cp::RpcValue::Type::UInt: + case RpcValue::Type::Int: + case RpcValue::Type::UInt: { bool ok; setValue(String::toInt(val_str, &ok)); @@ -55,7 +59,7 @@ CLIOptions::Option& CLIOptions::Option::setValueString(const std::string &val_st shvWarning() << "Value:" << val_str << "cannot be converted to Int."; break; } - case(cp::RpcValue::Type::Double): + case(RpcValue::Type::Double): { bool ok; setValue(String::toDouble(val_str, &ok)); @@ -72,8 +76,8 @@ CLIOptions::Option& CLIOptions::Option::setValueString(const std::string &val_st CLIOptions::CLIOptions() { - addOption("abortOnException").setType(cp::RpcValue::Type::Bool).setNames("--abort-on-exception").setComment("Abort application on exception"); - addOption("help").setType(cp::RpcValue::Type::Bool).setNames("-h", "--help").setComment("Print help"); + addOption("abortOnException").setType(RpcValue::Type::Bool).setNames("--abort-on-exception").setComment("Abort application on exception"); + addOption("help").setType(RpcValue::Type::Bool).setNames("-h", "--help").setComment("Print help"); } CLIOptions::~CLIOptions() = default; @@ -115,23 +119,23 @@ CLIOptions::Option& CLIOptions::optionRef(const std::string& name) return m_options[name]; } -cp::RpcValue::Map CLIOptions::values() const +RpcValue::Map CLIOptions::values() const { - cp::RpcValue::Map ret; + RpcValue::Map ret; for(const auto &kv : m_options) ret[kv.first] = value(kv.first); return ret; } -cp::RpcValue CLIOptions::value(const std::string &name) const +RpcValue CLIOptions::value(const std::string &name) const { - cp::RpcValue ret = value_helper(name, shv::core::Exception::Throw); + RpcValue ret = value_helper(name, shv::core::Exception::Throw); return ret; } -cp::RpcValue CLIOptions::value(const std::string& name, const cp::RpcValue default_value) const +RpcValue CLIOptions::value(const std::string& name, const RpcValue default_value) const { - cp::RpcValue ret = value_helper(name, !shv::core::Exception::Throw); + RpcValue ret = value_helper(name, !shv::core::Exception::Throw); if(!ret.isValid()) ret = default_value; return ret; @@ -142,16 +146,16 @@ bool CLIOptions::isValueSet(const std::string &name) const return option(name, !shv::core::Exception::Throw).isSet(); } -cp::RpcValue CLIOptions::value_helper(const std::string &name, bool throw_exception) const +RpcValue CLIOptions::value_helper(const std::string &name, bool throw_exception) const { Option opt = option(name, throw_exception); if(!opt.isValid()) - return cp::RpcValue(); - cp::RpcValue ret = opt.value(); + return RpcValue(); + RpcValue ret = opt.value(); if(!ret.isValid()) ret = opt.defaultValue(); if(!ret.isValid()) - ret = cp::RpcValue::fromType(opt.type()); + ret = RpcValue::fromType(opt.type()); return ret; } @@ -160,7 +164,7 @@ bool CLIOptions::optionExists(const std::string &name) const return option(name, !shv::core::Exception::Throw).isValid(); } -bool CLIOptions::setValue(const std::string& name, const cp::RpcValue val, bool throw_exc) +bool CLIOptions::setValue(const std::string& name, const RpcValue val, bool throw_exc) { Option o = option(name, false); if(optionExists(name)) { @@ -237,7 +241,7 @@ void CLIOptions::parse(const StringList& cmd_line_args) } else if((!arg.empty() && arg[0] == '-')) { // might be negative number or next switch - if(opt.type() != cp::RpcValue::Type::Int) + if(opt.type() != RpcValue::Type::Int) arg = std::string(); } else { @@ -315,14 +319,14 @@ void CLIOptions::printHelp(std::ostream &os) const for(const auto &kv : m_options) { const Option &opt = kv.second; os << String::join(opt.names(), ", "); - if(opt.type() != cp::RpcValue::Type::Bool) { - if(opt.type() == cp::RpcValue::Type::Int - || opt.type() == cp::RpcValue::Type::UInt - || opt.type() == cp::RpcValue::Type::Double) os << " " << "number"; + if(opt.type() != RpcValue::Type::Bool) { + if(opt.type() == RpcValue::Type::Int + || opt.type() == RpcValue::Type::UInt + || opt.type() == RpcValue::Type::Double) os << " " << "number"; else os << " " << "'string'"; } //os << ':'; - cp::RpcValue def_val = opt.defaultValue(); + RpcValue def_val = opt.defaultValue(); if(def_val.isValid()) os << " DEFAULT=" << def_val.toStdString(); if(opt.isMandatory()) @@ -367,8 +371,8 @@ void CLIOptions::addParseError(const std::string& err) ConfigCLIOptions::ConfigCLIOptions() { - addOption("config").setType(cp::RpcValue::Type::String).setNames("--config").setComment("Application config name, it is loaded from {config}[.conf] if file exists in {config-dir}, deault value is {app-name}.conf"); - addOption("configDir").setType(cp::RpcValue::Type::String).setNames("--config-dir").setComment("Directory where application config fiels are searched, default value: {app-dir-path}."); + addOption("config").setType(RpcValue::Type::String).setNames("--config").setComment("Application config name, it is loaded from {config}[.conf] if file exists in {config-dir}, deault value is {app-name}.conf"); + addOption("configDir").setType(RpcValue::Type::String).setNames("--config-dir").setComment("Directory where application config fiels are searched, default value: {app-dir-path}."); } void ConfigCLIOptions::parse(const StringList &cmd_line_args) @@ -400,53 +404,62 @@ bool ConfigCLIOptions::loadConfigFile() } return true; } - -std::string ConfigCLIOptions::configFile() +namespace { +bool is_absolute_path(const std::string &path) { - std::string config = "config"; - std::string conf_ext = ".conf"; - std::string config_file; - if(isValueSet(config)) { - config_file = value(config).toString(); - if(config_file.empty()) { - /// explicitly set empty config means DO NOT load config from any file - return std::string(); - } - } - else { - config_file = applicationName() + conf_ext; - } #ifdef _WIN32 - bool is_absolute_path = config_file.size() > 2 && config_file[1] == ':'; + return path.size() > 2 && path[1] == ':'; #else - bool is_absolute_path = !config_file.empty() && config_file[0] == '/'; + return !path.empty() && path[0] == '/'; #endif - if(!is_absolute_path) { - std::string config_dir = configDir(); - if(config_dir.empty()) - config_dir = applicationDir(); - config_file = config_dir + '/' + config_file; - } - if(!String::endsWith(config_file, conf_ext)) { - std::ifstream f(config_file); - if(!f) - config_file += conf_ext; - } - return config_file; +} +} +std::string ConfigCLIOptions::configFile() +{ + auto [abs_config_dir, abs_config_file] = absoluteConfigPaths(configDir(), config()); + return abs_config_file; } std::string ConfigCLIOptions::effectiveConfigDir() { - if(!configDir().empty()) - return configDir(); - String cfg = config(); - if(!cfg.empty() && cfg[0] == '/') { - auto ix = cfg.lastIndexOf('/'); - if(ix != std::string::npos) { - return cfg.mid(0, ix); + auto [abs_config_dir, abs_config_file] = absoluteConfigPaths(configDir(), config()); + return abs_config_dir; +} + +std::tuple ConfigCLIOptions::absoluteConfigPaths(const std::string &config_dir, const std::string &config_file) const +{ + using shv::core::utils::joinPath; + static const auto conf_ext = ".conf"s; + auto cwd = std::filesystem::current_path().string(); + auto default_config_name = applicationName() + conf_ext; + if(config_file.empty()) { + if(config_dir.empty()) + return make_tuple(cwd, joinPath(cwd, default_config_name)); + else if(is_absolute_path(config_dir)) + return make_tuple(config_dir, joinPath(config_dir, default_config_name)); + else + return make_tuple(joinPath(cwd, config_dir), joinPath(cwd, config_dir, default_config_name)); + } + else if(is_absolute_path(config_file)) { + if(config_dir.empty()) { + auto sep_pos = config_file.find_last_of('/'); + // absolute config file must contain '/' + assert(sep_pos != std::string::npos); + return make_tuple(config_file.substr(0, sep_pos), config_file); } + else if(is_absolute_path(config_dir)) + return make_tuple(config_dir, config_file); + else + return make_tuple(joinPath(cwd, config_dir), config_file); + } + else /* relative config_file */ { + if(config_dir.empty()) + return make_tuple(cwd, joinPath(cwd, config_file)); + else if(is_absolute_path(config_dir)) + return make_tuple(config_dir, joinPath(config_dir, config_file)); + else + return make_tuple(joinPath(cwd, config_dir), joinPath(cwd, config_dir, config_file)); } - return applicationDir(); } void ConfigCLIOptions::mergeConfig_helper(const std::string &key_prefix, const shv::chainpack::RpcValue &config_map) diff --git a/libshvcore/src/utils/clioptions.h b/libshvcore/src/utils/clioptions.h index 40da3ad01..378bbe19c 100644 --- a/libshvcore/src/utils/clioptions.h +++ b/libshvcore/src/utils/clioptions.h @@ -133,6 +133,7 @@ class SHVCORE_DECL_EXPORT ConfigCLIOptions : public CLIOptions void mergeConfig(const shv::chainpack::RpcValue &config_map); std::string configFile(); std::string effectiveConfigDir(); + std::tupleabsoluteConfigPaths(const std::string &config_dir, const std::string &config_file) const; protected: void mergeConfig_helper(const std::string &key_prefix, const shv::chainpack::RpcValue &config_map); }; diff --git a/libshvcore/tests/test_clioptions.cpp b/libshvcore/tests/test_clioptions.cpp new file mode 100644 index 000000000..396650711 --- /dev/null +++ b/libshvcore/tests/test_clioptions.cpp @@ -0,0 +1,108 @@ +#include +#include +#include +#include +#include + +#include + +#define DOCTEST_CONFIG_IMPLEMENT +#include + +using namespace shv::core::utils; +using namespace shv::core; +using namespace std; + +vector cmdline; + +int main(int argc, char** argv) +{ + //NecroLog::setFileLogTresholds("test_shvlog:D"); + cmdline = NecroLog::setCLIOptions(argc, argv); + + return doctest::Context(argc, argv).run(); +} + +DOCTEST_TEST_CASE("CliOptions") +{ + DOCTEST_SUBCASE("Make config dir and config file absolute") + { + string req_abs_config_dir; + string req_abs_config_file; + string config_dir; + string config_file; + const auto cwd = std::filesystem::current_path().string(); + const auto default_config_file_name = "test_core_clioptions.conf"s; + ConfigCLIOptions cliopts; + cliopts.parse(cmdline); + DOCTEST_SUBCASE("Empty config dir") + { + config_dir = {}; + DOCTEST_SUBCASE("Empty config file") + { + config_file = {}; + req_abs_config_dir = cwd; + req_abs_config_file = joinPath(cwd, default_config_file_name); + } + DOCTEST_SUBCASE("Relative config file") + { + config_file = "foo/bar/baz.conf"s; + req_abs_config_dir = cwd; + req_abs_config_file = joinPath(cwd, config_file); + } + DOCTEST_SUBCASE("Absolute config file") + { + config_file = "/foo/bar/baz.conf"s; + req_abs_config_dir = "/foo/bar"s; + req_abs_config_file = config_file; + } + } + DOCTEST_SUBCASE("Relative config dir") + { + config_dir = "foo/bar"s; + DOCTEST_SUBCASE("Empty config file") + { + config_file = {}; + req_abs_config_dir = joinPath(cwd, config_dir); + req_abs_config_file = joinPath(cwd, config_dir, default_config_file_name); + } + DOCTEST_SUBCASE("Relative config file") + { + config_file = "foo/bar/baz.conf"s; + req_abs_config_dir = joinPath(cwd, config_dir); + req_abs_config_file = joinPath(cwd, config_dir, config_file); + } + DOCTEST_SUBCASE("Absolute config file") + { + config_file = "/abs/foo/bar/baz.conf"s; + req_abs_config_dir = joinPath(cwd, config_dir); + req_abs_config_file = config_file; + } + } + DOCTEST_SUBCASE("Absolute config dir") + { + config_dir = "/a/foo/bar"s; + DOCTEST_SUBCASE("Empty config file") + { + config_file = {}; + req_abs_config_dir = config_dir; + req_abs_config_file = joinPath(config_dir, default_config_file_name); + } + DOCTEST_SUBCASE("Relative config file") + { + config_file = "foo/bar/baz.conf"s; + req_abs_config_dir = config_dir; + req_abs_config_file = joinPath(config_dir, config_file); + } + DOCTEST_SUBCASE("Absolute config file") + { + config_file = "/abs/foo/bar/baz.conf"s; + req_abs_config_dir = config_dir; + req_abs_config_file = config_file; + } + } + auto [abs_config_dir, abs_config_file] = cliopts.absoluteConfigPaths(config_dir, config_file); + REQUIRE(abs_config_dir == req_abs_config_dir); + REQUIRE(abs_config_file == req_abs_config_file); + } +} From b5b9c80a67b46a18facefe37ca2146a810a44377 Mon Sep 17 00:00:00 2001 From: Fanda Vacek Date: Thu, 5 Jan 2023 21:41:23 +0100 Subject: [PATCH 3/3] superfluous assert() removed --- libshvcore/src/utils/clioptions.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/libshvcore/src/utils/clioptions.cpp b/libshvcore/src/utils/clioptions.cpp index 3667f6d7e..64f53eede 100644 --- a/libshvcore/src/utils/clioptions.cpp +++ b/libshvcore/src/utils/clioptions.cpp @@ -444,7 +444,6 @@ std::tuple ConfigCLIOptions::absoluteConfigPaths(const if(config_dir.empty()) { auto sep_pos = config_file.find_last_of('/'); // absolute config file must contain '/' - assert(sep_pos != std::string::npos); return make_tuple(config_file.substr(0, sep_pos), config_file); } else if(is_absolute_path(config_dir))