From be8449aa177473a834a5b2c401a8a3fcc61522b4 Mon Sep 17 00:00:00 2001 From: Jaroslav Rohel Date: Wed, 2 Dec 2020 08:00:07 +0100 Subject: [PATCH 1/9] Option: Add reset() method The method resets the option to its initial state. Can be used, for example, before reloading the configuration in daemon mode (PackageKit). --- libdnf/conf/Option.hpp | 2 ++ libdnf/conf/OptionBool.hpp | 9 ++++++++- libdnf/conf/OptionChild.hpp | 14 ++++++++++++++ libdnf/conf/OptionEnum.hpp | 15 +++++++++++++++ libdnf/conf/OptionNumber.hpp | 10 +++++++++- libdnf/conf/OptionString.cpp | 11 ++++++++--- libdnf/conf/OptionString.hpp | 8 ++++++++ libdnf/conf/OptionStringList.hpp | 9 ++++++++- 8 files changed, 72 insertions(+), 6 deletions(-) diff --git a/libdnf/conf/Option.hpp b/libdnf/conf/Option.hpp index e9a9dfc84..849871fe7 100644 --- a/libdnf/conf/Option.hpp +++ b/libdnf/conf/Option.hpp @@ -62,6 +62,8 @@ class Option { virtual void set(Priority priority, const std::string & value) = 0; virtual std::string getValueString() const = 0; virtual bool empty() const noexcept; + /// Resets the option to its initial state. + virtual void reset() = 0; virtual ~Option() = default; protected: diff --git a/libdnf/conf/OptionBool.hpp b/libdnf/conf/OptionBool.hpp index c27ab0b79..a5e647807 100644 --- a/libdnf/conf/OptionBool.hpp +++ b/libdnf/conf/OptionBool.hpp @@ -47,6 +47,7 @@ class OptionBool : public Option { std::string getValueString() const override; const char * const * getTrueValues() const noexcept; const char * const * getFalseValues() const noexcept; + void reset() override; protected: const char * const * const trueValues; @@ -84,7 +85,13 @@ inline const char * const * OptionBool::getTrueValues() const noexcept inline const char * const * OptionBool::getFalseValues() const noexcept { - return falseValues ? falseValues : defFalseValues; + return falseValues ? falseValues : defFalseValues; +} + +inline void OptionBool::reset() +{ + value = defaultValue; + priority = Priority::DEFAULT; } } diff --git a/libdnf/conf/OptionChild.hpp b/libdnf/conf/OptionChild.hpp index 5d1503cb6..3056345f9 100644 --- a/libdnf/conf/OptionChild.hpp +++ b/libdnf/conf/OptionChild.hpp @@ -39,6 +39,7 @@ class OptionChild : public Option { const typename ParentOptionType::ValueType getDefaultValue() const; std::string getValueString() const override; bool empty() const noexcept override; + void reset() override; private: const ParentOptionType * parent; @@ -56,6 +57,7 @@ class OptionChild::empty() const noexcept return priority == Priority::EMPTY && parent->empty(); } +template +inline void OptionChild::reset() +{ + priority = Priority::EMPTY; +} + template inline OptionChild::value>::type>::OptionChild(const ParentOptionType & parent) : parent(&parent) {} @@ -171,6 +179,12 @@ inline bool OptionChildempty(); } +template +inline void OptionChild::value>::type>::reset() +{ + priority = Priority::EMPTY; +} + } #endif diff --git a/libdnf/conf/OptionEnum.hpp b/libdnf/conf/OptionEnum.hpp index c63156cb3..d2f710f20 100644 --- a/libdnf/conf/OptionEnum.hpp +++ b/libdnf/conf/OptionEnum.hpp @@ -49,6 +49,7 @@ class OptionEnum : public Option { T getDefaultValue() const; std::string toString(ValueType value) const; std::string getValueString() const override; + void reset() override; protected: FromStringFunc fromStringUser; @@ -74,6 +75,7 @@ class OptionEnum : public Option { const std::string & getValue() const; const std::string & getDefaultValue() const; std::string getValueString() const override; + void reset() override; protected: FromStringFunc fromStringUser; @@ -88,6 +90,13 @@ inline OptionEnum * OptionEnum::clone() const return new OptionEnum(*this); } +template +inline void OptionEnum::reset() +{ + value = defaultValue; + priority = Priority::DEFAULT; +} + inline OptionEnum * OptionEnum::clone() const { return new OptionEnum(*this); @@ -108,6 +117,12 @@ inline std::string OptionEnum::getValueString() const return value; } +inline void OptionEnum::reset() +{ + value = defaultValue; + priority = Priority::DEFAULT; +} + } #endif diff --git a/libdnf/conf/OptionNumber.hpp b/libdnf/conf/OptionNumber.hpp index 98988fd50..f7a7b3d6e 100644 --- a/libdnf/conf/OptionNumber.hpp +++ b/libdnf/conf/OptionNumber.hpp @@ -50,6 +50,7 @@ class OptionNumber : public Option { T getDefaultValue() const; std::string toString(ValueType value) const; std::string getValueString() const override; + void reset() override; protected: FromStringFunc fromStringUser; @@ -80,7 +81,14 @@ inline T OptionNumber::getDefaultValue() const template inline std::string OptionNumber::getValueString() const { - return toString(value); + return toString(value); +} + +template +inline void OptionNumber::reset() +{ + value = defaultValue; + priority = Priority::DEFAULT; } extern template class OptionNumber; diff --git a/libdnf/conf/OptionString.cpp b/libdnf/conf/OptionString.cpp index d27194f7a..b42e6c633 100644 --- a/libdnf/conf/OptionString.cpp +++ b/libdnf/conf/OptionString.cpp @@ -27,18 +27,21 @@ namespace libdnf { OptionString::OptionString(const std::string & defaultValue) -: Option(Priority::DEFAULT), defaultValue(defaultValue), value(defaultValue) {} +: Option(Priority::DEFAULT), initPriority(Priority::DEFAULT), defaultValue(defaultValue), value(defaultValue) {} OptionString::OptionString(const char * defaultValue) { if (defaultValue) { this->value = this->defaultValue = defaultValue; - this->priority = Priority::DEFAULT; + this->initPriority = this->priority = Priority::DEFAULT; + } else { + this->initPriority = Priority::EMPTY; } } OptionString::OptionString(const std::string & defaultValue, const std::string & regex, bool icase) -: Option(Priority::DEFAULT), regex(regex), icase(icase), defaultValue(defaultValue), value(defaultValue) { test(defaultValue); } +: Option(Priority::DEFAULT), initPriority(Priority::DEFAULT), regex(regex), icase(icase) +, defaultValue(defaultValue), value(defaultValue) { test(defaultValue); } OptionString::OptionString(const char * defaultValue, const std::string & regex, bool icase) : regex(regex), icase(icase) @@ -48,6 +51,8 @@ OptionString::OptionString(const char * defaultValue, const std::string & regex, test(this->defaultValue); this->value = this->defaultValue; this->priority = Priority::DEFAULT; + } else { + this->initPriority = Priority::EMPTY; } } diff --git a/libdnf/conf/OptionString.hpp b/libdnf/conf/OptionString.hpp index 2e26305c4..03fef8bcf 100644 --- a/libdnf/conf/OptionString.hpp +++ b/libdnf/conf/OptionString.hpp @@ -42,8 +42,10 @@ class OptionString : public Option { const std::string & getValue() const; const std::string & getDefaultValue() const noexcept; std::string getValueString() const override; + void reset() override; protected: + Priority initPriority; std::string regex; bool icase; std::string defaultValue; @@ -70,6 +72,12 @@ inline std::string OptionString::fromString(const std::string & value) const return value; } +inline void OptionString::reset() +{ + value = defaultValue; + priority = initPriority; +} + } #endif diff --git a/libdnf/conf/OptionStringList.hpp b/libdnf/conf/OptionStringList.hpp index 942e56b16..20debaa8c 100644 --- a/libdnf/conf/OptionStringList.hpp +++ b/libdnf/conf/OptionStringList.hpp @@ -45,6 +45,7 @@ class OptionStringList : public Option { const ValueType & getDefaultValue() const; std::string toString(const ValueType & value) const; std::string getValueString() const override; + void reset() override; protected: std::string regex; @@ -70,7 +71,13 @@ inline const OptionStringList::ValueType & OptionStringList::getDefaultValue() c inline std::string OptionStringList::getValueString() const { - return toString(value); + return toString(value); +} + +inline void OptionStringList::reset() +{ + value = defaultValue; + priority = Priority::DEFAULT; } } From 372a000414875f323147cd342dd8b4c8c7ebe260 Mon Sep 17 00:00:00 2001 From: Jaroslav Rohel Date: Tue, 1 Dec 2020 08:29:53 +0100 Subject: [PATCH 2/9] Add OptionBinds::getOption() method Sometime we want direct access to the underlying Option. E.g. we want to get its original value (not just a string representation) or find out the Option type. --- libdnf/conf/OptionBinds.cpp | 9 +++++++++ libdnf/conf/OptionBinds.hpp | 2 ++ 2 files changed, 11 insertions(+) diff --git a/libdnf/conf/OptionBinds.cpp b/libdnf/conf/OptionBinds.cpp index f7c67540b..ab53518a3 100644 --- a/libdnf/conf/OptionBinds.cpp +++ b/libdnf/conf/OptionBinds.cpp @@ -66,6 +66,15 @@ bool OptionBinds::Item::getAddValue() const return addValue; } +const Option & OptionBinds::Item::getOption() const +{ + return *option; +} + +Option & OptionBinds::Item::getOption() +{ + return *option; +} // =========== OptionBinds class =============== diff --git a/libdnf/conf/OptionBinds.hpp b/libdnf/conf/OptionBinds.hpp index 715c37e26..515120b93 100644 --- a/libdnf/conf/OptionBinds.hpp +++ b/libdnf/conf/OptionBinds.hpp @@ -55,6 +55,8 @@ class OptionBinds { void newString(Option::Priority priority, const std::string & value); std::string getValueString() const; bool getAddValue() const; + const Option & getOption() const; + Option & getOption(); private: friend class OptionBinds; From 3a686c378978c90538a6ac5d9826d52ce7c8daf6 Mon Sep 17 00:00:00 2001 From: Jaroslav Rohel Date: Tue, 1 Dec 2020 08:37:14 +0100 Subject: [PATCH 3/9] [context] Add dnf_repo_conf_from_gkeyfile() and dnf_repo_conf_reset() dnf_repo_conf_from_gkeyfile(): The function reloads repository configuration from GKeyFile. dnf_repo_conf_reset(): Resets repository configuration options previously readed from repository configuration file to initial state. --- libdnf/dnf-repo.cpp | 64 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/libdnf/dnf-repo.cpp b/libdnf/dnf-repo.cpp index 00f4bbf7b..9f283df55 100644 --- a/libdnf/dnf-repo.cpp +++ b/libdnf/dnf-repo.cpp @@ -936,6 +936,70 @@ dnf_repo_get_boolean(GKeyFile *keyfile, return false; } +/* Resets repository configuration options previously readed from repository + * configuration file to initial state. */ +static void +dnf_repo_conf_reset(libdnf::ConfigRepo &config) +{ + for (auto & item : config.optBinds()) { + auto & itemOption = item.second; + if (itemOption.getPriority() == libdnf::Option::Priority::REPOCONFIG) { + itemOption.getOption().reset(); + } + } +} + +/* Loads repository configuration from GKeyFile */ +static void +dnf_repo_conf_from_gkeyfile(libdnf::ConfigRepo &config, const char *repoId, GKeyFile *gkeyFile) +{ + // Reset to the initial state before reloading the configuration. + dnf_repo_conf_reset(config); + + g_auto(GStrv) keys = g_key_file_get_keys(gkeyFile, repoId, NULL, NULL); + for (auto it = keys; *it != NULL; ++it) { + auto key = *it; + g_autofree gchar *str = g_key_file_get_value(gkeyFile, repoId, key, NULL); + if (str) { + try { + auto & optionItem = config.optBinds().at(key); + + if (dynamic_cast(&optionItem.getOption()) || + dynamic_cast*>(&optionItem.getOption()) + ) { + + // reload list option from gKeyFile using g_key_file_get_string_list() + // g_key_file_get_value () is problematic for multiline lists + g_auto(GStrv) list = g_key_file_get_string_list(gkeyFile, repoId, key, NULL, NULL); + if (list) { + // list can be ['value1', 'value2, value3'] therefore we first join + // to have 'value1, value2, value3' + g_autofree gchar * tmp_strval = g_strjoinv(",", list); + try { + optionItem.newString(libdnf::Option::Priority::REPOCONFIG, tmp_strval); + } catch (const std::exception & ex) { + g_debug("Invalid configuration value: %s = %s in %s; %s", key, str, repoId, ex.what()); + } + } + + } else { + + // process other (non list) options + try { + optionItem.newString(libdnf::Option::Priority::REPOCONFIG, str); + } catch (const std::exception & ex) { + g_debug("Invalid configuration value: %s = %s in %s; %s", key, str, repoId, ex.what()); + } + + } + + } catch (const std::exception &) { + g_debug("Unknown configuration option: %s = %s in %s", key, str, repoId); + } + } + } +} + /* Initialize (or potentially reset) repo & LrHandle from keyfile values. */ static gboolean dnf_repo_set_keyfile_data(DnfRepo *repo, GError **error) From 5f1c06a66fcdb2c2340c11c07c5ba0ea3abf4b77 Mon Sep 17 00:00:00 2001 From: Jaroslav Rohel Date: Wed, 2 Dec 2020 11:37:26 +0100 Subject: [PATCH 4/9] [context] Use dnf_repo_conf_from_gkeyfile() for repo configuration reload The dnf_repo_set_key_file_data() uses dnf_repo_conf_from_gkeyfile() now. All occurrences of the direct use 'repo->getConfig()->.*set' and newString() were removed. --- libdnf/dnf-repo.cpp | 121 +++++++++----------------------------------- 1 file changed, 25 insertions(+), 96 deletions(-) diff --git a/libdnf/dnf-repo.cpp b/libdnf/dnf-repo.cpp index 9f283df55..2837580f7 100644 --- a/libdnf/dnf-repo.cpp +++ b/libdnf/dnf-repo.cpp @@ -1006,7 +1006,6 @@ dnf_repo_set_keyfile_data(DnfRepo *repo, GError **error) { DnfRepoPrivate *priv = GET_PRIVATE(repo); guint cost; - gboolean module_hotfixes = false; g_autofree gchar *metadata_expire_str = NULL; g_autofree gchar *mirrorlist = NULL; g_autofree gchar *mirrorlisturl = NULL; @@ -1016,48 +1015,28 @@ dnf_repo_set_keyfile_data(DnfRepo *repo, GError **error) g_autofree gchar *usr = NULL; g_autofree gchar *usr_pwd = NULL; g_autofree gchar *usr_pwd_proxy = NULL; - g_auto(GStrv) baseurls; auto repoId = priv->repo->getId().c_str(); g_debug("setting keyfile data for %s", repoId); - /* skip_if_unavailable is optional */ - if (g_key_file_has_key(priv->keyfile, repoId, "skip_if_unavailable", NULL)) { - bool skip = dnf_repo_get_boolean(priv->keyfile, repoId, "skip_if_unavailable"); - priv->repo->getConfig()->skip_if_unavailable().set(libdnf::Option::Priority::REPOCONFIG, skip); - } + auto conf = priv->repo->getConfig(); - /* priority is optional */ - g_autofree gchar * priority_str = g_key_file_get_string(priv->keyfile, repoId, "priority", NULL); - if (priority_str) { - priv->repo->getConfig()->priority().set(libdnf::Option::Priority::REPOCONFIG, priority_str); - } + // Reload repository configuration from keyfile. + dnf_repo_conf_from_gkeyfile(*conf, repoId, priv->keyfile); /* cost is optional */ cost = g_key_file_get_integer(priv->keyfile, repoId, "cost", NULL); if (cost != 0) dnf_repo_set_cost(repo, cost); - module_hotfixes = g_key_file_get_boolean(priv->keyfile, repoId, "module_hotfixes", NULL); - priv->repo->getConfig()->module_hotfixes().set(libdnf::Option::Priority::REPOCONFIG, module_hotfixes); - /* baseurl is optional; if missing, unset it */ - baseurls = g_key_file_get_string_list(priv->keyfile, repoId, "baseurl", NULL, NULL); - if (baseurls) { - // baseruls can be ['value1', 'value2, value3'] therefore we first join to have 'value1, value2, value3' - g_autofree gchar * tmp_strval = g_strjoinv(",", baseurls); - - auto & bindBaseurls = priv->repo->getConfig()->optBinds().at("baseurl"); - bindBaseurls.newString(libdnf::Option::Priority::REPOCONFIG, tmp_strval); - - auto & repoBaseurls = priv->repo->getConfig()->baseurl(); - if (!repoBaseurls.getValue().empty()){ - auto len = repoBaseurls.getValue().size(); - g_strfreev(baseurls); - baseurls = g_new0(char *, len + 1); - for (size_t i = 0; i < len; ++i) { - baseurls[i] = g_strdup(repoBaseurls.getValue()[i].c_str()); - } + g_auto(GStrv) baseurls = NULL; + auto & repoBaseurls = conf->baseurl().getValue(); + if (!repoBaseurls.empty()){ + auto len = repoBaseurls.size(); + baseurls = g_new0(char *, len + 1); + for (size_t i = 0; i < len; ++i) { + baseurls[i] = g_strdup(repoBaseurls[i].c_str()); } } if (!lr_handle_setopt(priv->repo_handle, error, LRO_URLS, baseurls)) @@ -1093,18 +1072,6 @@ dnf_repo_set_keyfile_data(DnfRepo *repo, GError **error) if (!lr_handle_setopt(priv->repo_handle, error, LRO_METALINKURL, metalinkurl)) return FALSE; - /* needed in order for addCountmeFlag() to use the same persistdir as DNF - * would */ - if (metalinkurl) - priv->repo->getConfig()->metalink().set(libdnf::Option::Priority::REPOCONFIG, metalinkurl); - if (mirrorlisturl) - priv->repo->getConfig()->mirrorlist().set(libdnf::Option::Priority::REPOCONFIG, mirrorlisturl); - - if (g_key_file_has_key(priv->keyfile, repoId, "countme", NULL)) { - bool countme = dnf_repo_get_boolean(priv->keyfile, repoId, "countme"); - priv->repo->getConfig()->countme().set(libdnf::Option::Priority::REPOCONFIG, countme); - } - /* file:// */ if (baseurls != NULL && baseurls[0] != NULL && mirrorlisturl == NULL && metalinkurl == NULL) { @@ -1150,42 +1117,20 @@ dnf_repo_set_keyfile_data(DnfRepo *repo, GError **error) dnf_repo_set_location_tmp(repo, tmp->str); } - /* gpgkey is optional for gpgcheck=1, but required for repo_gpgcheck=1 */ + // Sync priv->gpgkeys g_strfreev(priv->gpgkeys); - priv->gpgkeys = NULL; - - g_auto(GStrv) gpgkeys; - gpgkeys = g_key_file_get_string_list(priv->keyfile, repoId, "gpgkey", NULL, NULL); - - if (gpgkeys) { - // gpgkeys can be ['value1', 'value2, value3'] therefore we first join to have 'value1, value2, value3' - g_autofree gchar * tmp_strval = g_strjoinv(",", gpgkeys); - - auto & bindGpgkeys = priv->repo->getConfig()->optBinds().at("gpgkey"); - bindGpgkeys.newString(libdnf::Option::Priority::REPOCONFIG, tmp_strval); - - auto & repoGpgkeys = priv->repo->getConfig()->gpgkey(); - if (!repoGpgkeys.getValue().empty()){ - auto len = repoGpgkeys.getValue().size(); - priv->gpgkeys = g_new0(char *, len + 1); - for (size_t i = 0; i < len; ++i) { - priv->gpgkeys[i] = g_strdup(repoGpgkeys.getValue()[i].c_str()); - } - } else { - /* Canonicalize the empty list to NULL for ease of checking elsewhere */ - g_strfreev(static_cast(g_steal_pointer(&priv->gpgkeys))); + auto & repoGpgkeys = conf->gpgkey().getValue(); + if (!repoGpgkeys.empty()){ + auto len = repoGpgkeys.size(); + priv->gpgkeys = g_new0(char *, len + 1); + for (size_t i = 0; i < len; ++i) { + priv->gpgkeys[i] = g_strdup(repoGpgkeys[i].c_str()); } + } else { + priv->gpgkeys = NULL; } - if (g_key_file_has_key(priv->keyfile, repoId, "gpgcheck", NULL)) { - auto gpgcheck_pkgs = dnf_repo_get_boolean(priv->keyfile, repoId, "gpgcheck"); - priv->repo->getConfig()->gpgcheck().set(libdnf::Option::Priority::REPOCONFIG, gpgcheck_pkgs); - } - - if (g_key_file_has_key(priv->keyfile, repoId, "repo_gpgcheck", NULL)) { - auto gpgcheck_md = dnf_repo_get_boolean(priv->keyfile, repoId, "repo_gpgcheck"); - priv->repo->getConfig()->repo_gpgcheck().set(libdnf::Option::Priority::REPOCONFIG, gpgcheck_md); - } + /* gpgkey is optional for gpgcheck=1, but required for repo_gpgcheck=1 */ auto gpgcheck_md = priv->repo->getConfig()->repo_gpgcheck().getValue(); if (gpgcheck_md && priv->gpgkeys == NULL) { g_set_error_literal(error, @@ -1199,35 +1144,19 @@ dnf_repo_set_keyfile_data(DnfRepo *repo, GError **error) if (!lr_handle_setopt(priv->repo_handle, error, LRO_GPGCHECK, (long)gpgcheck_md)) return FALSE; - auto & repoExcludepkgs = priv->repo->getConfig()->excludepkgs(); - repoExcludepkgs.set(libdnf::Option::Priority::REPOCONFIG, ""); - - auto & bindExcludepkgs = priv->repo->getConfig()->optBinds().at("excludepkgs"); - if (auto excludepkgs = g_key_file_get_string(priv->keyfile, repoId, "exclude", NULL)) { - bindExcludepkgs.newString(libdnf::Option::Priority::REPOCONFIG, excludepkgs); - g_free(excludepkgs); - } - if (auto excludepkgs = g_key_file_get_string(priv->keyfile, repoId, "excludepkgs", NULL)) { - bindExcludepkgs.newString(libdnf::Option::Priority::REPOCONFIG, excludepkgs); - g_free(excludepkgs); - } - + // Sync priv->exclude_packages g_strfreev(priv->exclude_packages); - if (!repoExcludepkgs.getValue().empty()) { - auto len = repoExcludepkgs.getValue().size(); + auto & repoExcludepkgs = conf->excludepkgs().getValue(); + if (!repoExcludepkgs.empty()) { + auto len = repoExcludepkgs.size(); priv->exclude_packages = g_new0(char *, len + 1); for (size_t i = 0; i < len; ++i) { - priv->exclude_packages[i] = g_strdup(repoExcludepkgs.getValue()[i].c_str()); + priv->exclude_packages[i] = g_strdup(repoExcludepkgs[i].c_str()); } } else { priv->exclude_packages = NULL; } - if (auto includepkgs = g_key_file_get_string(priv->keyfile, repoId, "includepkgs", NULL)) { - priv->repo->getConfig()->includepkgs().set(libdnf::Option::Priority::REPOCONFIG, includepkgs); - g_free(includepkgs); - } - /* proxy is optional */ proxy = g_key_file_get_string(priv->keyfile, repoId, "proxy", NULL); auto repoProxy = proxy ? (strcasecmp(proxy, "_none_") == 0 ? NULL : proxy) From c6afbb4f93eee480c68201297e9c5c7afdf05dd3 Mon Sep 17 00:00:00 2001 From: Jaroslav Rohel Date: Wed, 2 Dec 2020 13:26:51 +0100 Subject: [PATCH 5/9] [context] Fix: "cost" and "metadata_expire" repository options Changes in dnf_repo_set_keyfile_data(): Removed the dnf_repo_set_cost() call. Removed the "metadata_expire" parsing and dnf_repo_set_metadata_expire() call. The options were set earlier. The function calls were redundant and set the priority to the wrong RUNTIME value. --- libdnf/dnf-repo.cpp | 103 -------------------------------------------- 1 file changed, 103 deletions(-) diff --git a/libdnf/dnf-repo.cpp b/libdnf/dnf-repo.cpp index 2837580f7..61d496750 100644 --- a/libdnf/dnf-repo.cpp +++ b/libdnf/dnf-repo.cpp @@ -816,93 +816,6 @@ dnf_repo_set_metadata_expire(DnfRepo *repo, guint metadata_expire) priv->repo->getConfig()->metadata_expire().set(libdnf::Option::Priority::RUNTIME, metadata_expire); } -/** - * dnf_repo_parse_time_from_str - * @expression: a expression to be parsed - * @out_parsed_time: (out): return location for parsed time - * @error: error item - * - * Parse String into an integer value of seconds, or a human - * readable variation specifying days, hours, minutes or seconds - * until something happens. Note that due to historical president - * -1 means "never", so this accepts that and allows - * the word never, too. - * - * Valid inputs: 100, 1.5m, 90s, 1.2d, 1d, 0xF, 0.1, -1, never. - * Invalid inputs: -10, -0.1, 45.6Z, 1d6h, 1day, 1y. - - * Returns: integer value in seconds - **/ - -static gboolean -dnf_repo_parse_time_from_str(const gchar *expression, guint *out_parsed_time, GError **error) -{ - gint multiplier; - gdouble parsed_time; - gchar *endptr = NULL; - - if (!g_strcmp0(expression, "")) { - g_set_error_literal(error, - DNF_ERROR, - DNF_ERROR_FILE_INVALID, - "no metadata value specified"); - return FALSE; - } - - if (g_strcmp0(expression, "-1") == 0 || - g_strcmp0(expression,"never") == 0) { - *out_parsed_time = G_MAXUINT; - return TRUE; /* Note early return */ - } - - gchar last_char = expression[ strlen(expression) - 1 ]; - - /* check if the input ends with h, m ,d ,s as units */ - if (g_ascii_isalpha(last_char)) { - if (last_char == 'h') - multiplier = 60 * 60; - else if (last_char == 's') - multiplier = 1; - else if (last_char == 'm') - multiplier = 60; - else if (last_char == 'd') - multiplier = 60 * 60 * 24; - else { - g_set_error(error, DNF_ERROR, DNF_ERROR_FILE_INVALID, - "unknown unit %c", last_char); - return FALSE; - } - } - else - multiplier = 1; - - /* convert expression into a double*/ - parsed_time = g_ascii_strtod(expression, &endptr); - - /* failed to parse */ - if (expression == endptr) { - g_set_error(error, DNF_ERROR, DNF_ERROR_INTERNAL_ERROR, - "failed to parse time: %s", expression); - return FALSE; - } - - /* time can not be below zero */ - if (parsed_time < 0) { - g_set_error(error, DNF_ERROR, DNF_ERROR_INTERNAL_ERROR, - "seconds value must not be negative %s",expression ); - return FALSE; - } - - /* time too large */ - if (parsed_time > G_MAXDOUBLE || (parsed_time * multiplier) > G_MAXUINT){ - g_set_error(error, DNF_ERROR, DNF_ERROR_INTERNAL_ERROR, - "time too large"); - return FALSE; - } - - *out_parsed_time = (guint) (parsed_time * multiplier); - return TRUE; -} /** * dnf_repo_get_username_password_string: */ @@ -1005,8 +918,6 @@ static gboolean dnf_repo_set_keyfile_data(DnfRepo *repo, GError **error) { DnfRepoPrivate *priv = GET_PRIVATE(repo); - guint cost; - g_autofree gchar *metadata_expire_str = NULL; g_autofree gchar *mirrorlist = NULL; g_autofree gchar *mirrorlisturl = NULL; g_autofree gchar *metalinkurl = NULL; @@ -1024,11 +935,6 @@ dnf_repo_set_keyfile_data(DnfRepo *repo, GError **error) // Reload repository configuration from keyfile. dnf_repo_conf_from_gkeyfile(*conf, repoId, priv->keyfile); - /* cost is optional */ - cost = g_key_file_get_integer(priv->keyfile, repoId, "cost", NULL); - if (cost != 0) - dnf_repo_set_cost(repo, cost); - /* baseurl is optional; if missing, unset it */ g_auto(GStrv) baseurls = NULL; auto & repoBaseurls = conf->baseurl().getValue(); @@ -1042,15 +948,6 @@ dnf_repo_set_keyfile_data(DnfRepo *repo, GError **error) if (!lr_handle_setopt(priv->repo_handle, error, LRO_URLS, baseurls)) return FALSE; - /* metadata_expire is optional, if shown, we parse the string to add the time */ - metadata_expire_str = g_key_file_get_string(priv->keyfile, repoId, "metadata_expire", NULL); - if (metadata_expire_str) { - guint metadata_expire; - if (!dnf_repo_parse_time_from_str(metadata_expire_str, &metadata_expire, error)) - return FALSE; - dnf_repo_set_metadata_expire(repo, metadata_expire); - } - /* the "mirrorlist" entry could be either a real mirrorlist, or a metalink entry */ mirrorlist = g_key_file_get_string(priv->keyfile, repoId, "mirrorlist", NULL); if (mirrorlist) { From b11ac5204dc4c7048a7b6880813f2f9b1d8eb242 Mon Sep 17 00:00:00 2001 From: Jaroslav Rohel Date: Wed, 2 Dec 2020 13:21:35 +0100 Subject: [PATCH 6/9] [context] Fix: username, password, proxy, proxy_username, proxy_password - Uses global configuration options when they are not defined in the repository configuration. - proxy_username and proxy_password is urlEncoded before passing to librepo. --- libdnf/dnf-repo.cpp | 78 ++++++++++++++++++++++++++------------------- 1 file changed, 46 insertions(+), 32 deletions(-) diff --git a/libdnf/dnf-repo.cpp b/libdnf/dnf-repo.cpp index 61d496750..005721ef6 100644 --- a/libdnf/dnf-repo.cpp +++ b/libdnf/dnf-repo.cpp @@ -817,18 +817,22 @@ dnf_repo_set_metadata_expire(DnfRepo *repo, guint metadata_expire) } /** - * dnf_repo_get_username_password_string: - */ -static gchar * -dnf_repo_get_username_password_string(const gchar *user, const gchar *pass) -{ - if (user == NULL && pass == NULL) - return NULL; - if (user != NULL && pass == NULL) - return g_strdup(user); - if (user == NULL && pass != NULL) - return g_strdup_printf(":%s", pass); - return g_strdup_printf("%s:%s", user, pass); +* @brief Format user password string +* +* Returns user and password in user:password form. If encode is True, +* special characters in user and password are URL encoded. +* +* @param user Username +* @param passwd Password +* @param encode If quote is True, special characters in user and password are URL encoded. +* @return User and password in user:password form +*/ +static std::string formatUserPassString(const std::string & user, const std::string & passwd, bool encode) +{ + if (encode) + return libdnf::urlEncode(user) + ":" + libdnf::urlEncode(passwd); + else + return user + ":" + passwd; } static gboolean @@ -921,11 +925,8 @@ dnf_repo_set_keyfile_data(DnfRepo *repo, GError **error) g_autofree gchar *mirrorlist = NULL; g_autofree gchar *mirrorlisturl = NULL; g_autofree gchar *metalinkurl = NULL; - g_autofree gchar *proxy = NULL; - g_autofree gchar *pwd = NULL; - g_autofree gchar *usr = NULL; - g_autofree gchar *usr_pwd = NULL; - g_autofree gchar *usr_pwd_proxy = NULL; + std::string tmp_str; + const char *tmp_cstr; auto repoId = priv->repo->getId().c_str(); g_debug("setting keyfile data for %s", repoId); @@ -1054,26 +1055,39 @@ dnf_repo_set_keyfile_data(DnfRepo *repo, GError **error) priv->exclude_packages = NULL; } - /* proxy is optional */ - proxy = g_key_file_get_string(priv->keyfile, repoId, "proxy", NULL); - auto repoProxy = proxy ? (strcasecmp(proxy, "_none_") == 0 ? NULL : proxy) - : dnf_context_get_http_proxy(priv->context); - if (!lr_handle_setopt(priv->repo_handle, error, LRO_PROXY, repoProxy)) + tmp_str = conf->proxy().getValue(); + tmp_cstr = tmp_str.empty() ? dnf_context_get_http_proxy(priv->context) : tmp_str.c_str(); + if (!lr_handle_setopt(priv->repo_handle, error, LRO_PROXY, tmp_cstr)) return FALSE; - /* both parts of the proxy auth are optional */ - usr = g_key_file_get_string(priv->keyfile, repoId, "proxy_username", NULL); - pwd = g_key_file_get_string(priv->keyfile, repoId, "proxy_password", NULL); - usr_pwd_proxy = dnf_repo_get_username_password_string(usr, pwd); - if (!lr_handle_setopt(priv->repo_handle, error, LRO_PROXYUSERPWD, usr_pwd_proxy)) + // setup proxy username and password + tmp_cstr = NULL; + if (!conf->proxy_username().empty()) { + tmp_str = conf->proxy_username().getValue(); + if (!tmp_str.empty()) { + if (conf->proxy_password().empty()) { + g_set_error(error, DNF_ERROR, DNF_ERROR_FILE_INVALID, + "repo '%s': 'proxy_username' is set but not 'proxy_password'", repoId); + return FALSE; + } + tmp_str = formatUserPassString(tmp_str, conf->proxy_password().getValue(), true); + tmp_cstr = tmp_str.c_str(); + } + } + if (!lr_handle_setopt(priv->repo_handle, error, LRO_PROXYUSERPWD, tmp_cstr)) return FALSE; - /* both parts of the HTTP auth are optional */ - usr = g_key_file_get_string(priv->keyfile, repoId, "username", NULL); - pwd = g_key_file_get_string(priv->keyfile, repoId, "password", NULL); - usr_pwd = dnf_repo_get_username_password_string(usr, pwd); - if (!lr_handle_setopt(priv->repo_handle, error, LRO_USERPWD, usr_pwd)) + // setup username and password + tmp_cstr = NULL; + tmp_str = conf->username().getValue(); + if (!tmp_str.empty()) { + // TODO Use URL encoded form, needs support in librepo + tmp_str = formatUserPassString(tmp_str, conf->password().getValue(), false); + tmp_cstr = tmp_str.c_str(); + } + if (!lr_handle_setopt(priv->repo_handle, error, LRO_USERPWD, tmp_cstr)) return FALSE; + return TRUE; } From fd07b29ccaec2648cfc050122e16e4846d7ac4be Mon Sep 17 00:00:00 2001 From: Jaroslav Rohel Date: Wed, 2 Dec 2020 13:53:45 +0100 Subject: [PATCH 7/9] [context] Add support for options: minrate, throttle, bandwidth, timeout --- libdnf/dnf-repo.cpp | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/libdnf/dnf-repo.cpp b/libdnf/dnf-repo.cpp index 005721ef6..c6dd027be 100644 --- a/libdnf/dnf-repo.cpp +++ b/libdnf/dnf-repo.cpp @@ -1055,6 +1055,35 @@ dnf_repo_set_keyfile_data(DnfRepo *repo, GError **error) priv->exclude_packages = NULL; } + auto minrate = conf->minrate().getValue(); + if (!lr_handle_setopt(priv->repo_handle, error, LRO_LOWSPEEDLIMIT, static_cast(minrate))) + return FALSE; + + auto maxspeed = conf->throttle().getValue(); + if (maxspeed > 0 && maxspeed <= 1) + maxspeed *= conf->bandwidth().getValue(); + if (maxspeed != 0 && maxspeed < minrate) { + g_set_error_literal(error, DNF_ERROR, DNF_ERROR_FILE_INVALID, + "Maximum download speed is lower than minimum. " + "Please change configuration of minrate or throttle"); + return FALSE; + } + if (!lr_handle_setopt(priv->repo_handle, error, LRO_MAXSPEED, static_cast(maxspeed))) + return FALSE; + + long timeout = conf->timeout().getValue(); + if (timeout > 0) { + if (!lr_handle_setopt(priv->repo_handle, error, LRO_CONNECTTIMEOUT, timeout)) + return FALSE; + if (!lr_handle_setopt(priv->repo_handle, error, LRO_LOWSPEEDTIME, timeout)) + return FALSE; + } else { + if (!lr_handle_setopt(priv->repo_handle, error, LRO_CONNECTTIMEOUT, LRO_CONNECTTIMEOUT_DEFAULT)) + return FALSE; + if (!lr_handle_setopt(priv->repo_handle, error, LRO_LOWSPEEDTIME, LRO_LOWSPEEDTIME_DEFAULT)) + return FALSE; + } + tmp_str = conf->proxy().getValue(); tmp_cstr = tmp_str.empty() ? dnf_context_get_http_proxy(priv->context) : tmp_str.c_str(); if (!lr_handle_setopt(priv->repo_handle, error, LRO_PROXY, tmp_cstr)) From c484de218699dff834fb32133cf502b2d0c64162 Mon Sep 17 00:00:00 2001 From: Jaroslav Rohel Date: Thu, 3 Dec 2020 10:55:02 +0100 Subject: [PATCH 8/9] [context] Remove g_key_file_get_string() from dnf_repo_set_keyfile_data() Removes the remaining usage of g_key_file_get_string() from dnf_repo_set_keyfile_data(). Use the values from ConfigRepo instead. --- libdnf/dnf-repo.cpp | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/libdnf/dnf-repo.cpp b/libdnf/dnf-repo.cpp index c6dd027be..c5c50d55c 100644 --- a/libdnf/dnf-repo.cpp +++ b/libdnf/dnf-repo.cpp @@ -922,9 +922,6 @@ static gboolean dnf_repo_set_keyfile_data(DnfRepo *repo, GError **error) { DnfRepoPrivate *priv = GET_PRIVATE(repo); - g_autofree gchar *mirrorlist = NULL; - g_autofree gchar *mirrorlisturl = NULL; - g_autofree gchar *metalinkurl = NULL; std::string tmp_str; const char *tmp_cstr; @@ -949,20 +946,22 @@ dnf_repo_set_keyfile_data(DnfRepo *repo, GError **error) if (!lr_handle_setopt(priv->repo_handle, error, LRO_URLS, baseurls)) return FALSE; + const char *mirrorlisturl = NULL; + const char *metalinkurl = NULL; + /* the "mirrorlist" entry could be either a real mirrorlist, or a metalink entry */ - mirrorlist = g_key_file_get_string(priv->keyfile, repoId, "mirrorlist", NULL); - if (mirrorlist) { - if (strstr(mirrorlist, "metalink")) - metalinkurl = static_cast(g_steal_pointer(&mirrorlist)); + tmp_cstr = conf->mirrorlist().empty() ? NULL : conf->mirrorlist().getValue().c_str(); + if (tmp_cstr) { + if (strstr(tmp_cstr, "metalink")) + metalinkurl = tmp_cstr; else /* it really is a mirrorlist */ - mirrorlisturl = static_cast(g_steal_pointer(&mirrorlist)); + mirrorlisturl = tmp_cstr; } /* let "metalink" entry override metalink-as-mirrorlist entry */ - if (g_key_file_has_key(priv->keyfile, repoId, "metalink", NULL)) { - g_free(metalinkurl); - metalinkurl = g_key_file_get_string(priv->keyfile, repoId, "metalink", NULL); - } + tmp_cstr = conf->metalink().empty() ? NULL : conf->metalink().getValue().c_str(); + if (tmp_cstr) + metalinkurl = tmp_cstr; /* now set the final values (or unset them) */ if (!lr_handle_setopt(priv->repo_handle, error, LRO_MIRRORLISTURL, mirrorlisturl)) From ce44d3dced4b800e3b7f80556fac1daf7e7fa49d Mon Sep 17 00:00:00 2001 From: Jaroslav Rohel Date: Thu, 3 Dec 2020 11:43:18 +0100 Subject: [PATCH 9/9] [context] Remove the extra gpgkey member from DnfRepoPrivate The value stored in ConfigRepo can be used directly. --- libdnf/dnf-repo.cpp | 81 ++++++++++++++++++--------------------------- 1 file changed, 33 insertions(+), 48 deletions(-) diff --git a/libdnf/dnf-repo.cpp b/libdnf/dnf-repo.cpp index c5c50d55c..193999902 100644 --- a/libdnf/dnf-repo.cpp +++ b/libdnf/dnf-repo.cpp @@ -63,7 +63,6 @@ typedef struct { DnfRepoEnabled enabled; - gchar **gpgkeys; gchar **exclude_packages; gchar *filename; /* /etc/yum.repos.d/updates.repo */ gchar *location; /* /var/cache/PackageKit/metadata/fedora */ @@ -97,7 +96,6 @@ dnf_repo_finalize(GObject *object) DnfRepoPrivate *priv = GET_PRIVATE(repo); g_free(priv->filename); - g_strfreev(priv->gpgkeys); g_strfreev(priv->exclude_packages); g_free(priv->location_tmp); g_free(priv->location); @@ -225,16 +223,13 @@ gchar ** dnf_repo_get_public_keys(DnfRepo *repo) { DnfRepoPrivate *priv = GET_PRIVATE(repo); - g_autoptr(GPtrArray) ret = g_ptr_array_new(); - for (char **iter = priv->gpgkeys; iter && *iter; iter++) { - const char *key = *iter; - g_autofree gchar *key_bn = g_path_get_basename(key); - /* transfer ownership to ptrarray */ - g_ptr_array_add(ret, g_build_filename(priv->location, key_bn, NULL)); + const auto & keys = priv->repo->getConfig()->gpgkey().getValue(); + gchar **ret = g_new0(gchar *, keys.size() + 1); + for (size_t i = 0; i < keys.size(); ++i) { + g_autofree gchar *key_bn = g_path_get_basename(keys[i].c_str()); + ret[i] = g_build_filename(priv->location, key_bn, NULL); } - g_ptr_array_add(ret, NULL); - /* transfer ownership of container and elements to caller */ - return (gchar**)g_ptr_array_free(static_cast(g_steal_pointer(&ret)), FALSE); + return ret; } /** @@ -1014,22 +1009,9 @@ dnf_repo_set_keyfile_data(DnfRepo *repo, GError **error) dnf_repo_set_location_tmp(repo, tmp->str); } - // Sync priv->gpgkeys - g_strfreev(priv->gpgkeys); - auto & repoGpgkeys = conf->gpgkey().getValue(); - if (!repoGpgkeys.empty()){ - auto len = repoGpgkeys.size(); - priv->gpgkeys = g_new0(char *, len + 1); - for (size_t i = 0; i < len; ++i) { - priv->gpgkeys[i] = g_strdup(repoGpgkeys[i].c_str()); - } - } else { - priv->gpgkeys = NULL; - } - /* gpgkey is optional for gpgcheck=1, but required for repo_gpgcheck=1 */ - auto gpgcheck_md = priv->repo->getConfig()->repo_gpgcheck().getValue(); - if (gpgcheck_md && priv->gpgkeys == NULL) { + auto repo_gpgcheck = conf->repo_gpgcheck().getValue(); + if (repo_gpgcheck && conf->gpgkey().getValue().empty()) { g_set_error_literal(error, DNF_ERROR, DNF_ERROR_FILE_INVALID, @@ -1038,7 +1020,7 @@ dnf_repo_set_keyfile_data(DnfRepo *repo, GError **error) } /* XXX: setopt() expects a long, so we need a long on the stack */ - if (!lr_handle_setopt(priv->repo_handle, error, LRO_GPGCHECK, (long)gpgcheck_md)) + if (!lr_handle_setopt(priv->repo_handle, error, LRO_GPGCHECK, (long)repo_gpgcheck)) return FALSE; // Sync priv->exclude_packages @@ -1750,28 +1732,31 @@ dnf_repo_update(DnfRepo *repo, goto out; } - if (priv->gpgkeys && - (priv->repo->getConfig()->repo_gpgcheck().getValue() || priv->repo->getConfig()->gpgcheck().getValue())) { - for (char **iter = priv->gpgkeys; iter && *iter; iter++) { - const char *gpgkey = *iter; - g_autofree char *gpgkey_name = g_path_get_basename(gpgkey); - g_autofree char *key_tmp = g_build_filename(priv->location_tmp, gpgkey_name, NULL); - - /* download and import public key */ - if ((g_str_has_prefix(gpgkey, "https://") || - g_str_has_prefix(gpgkey, "file://"))) { - g_debug("importing public key %s", gpgkey); - - ret = dnf_repo_download_import_public_key(repo, gpgkey, key_tmp, error); - if (!ret) - goto out; - } + { + const auto & gpgkeys = priv->repo->getConfig()->gpgkey().getValue(); + if (!gpgkeys.empty() && + (priv->repo->getConfig()->repo_gpgcheck().getValue() || priv->repo->getConfig()->gpgcheck().getValue())) { + for (const auto & key : gpgkeys) { + const char *gpgkey = key.c_str(); + g_autofree char *gpgkey_name = g_path_get_basename(gpgkey); + g_autofree char *key_tmp = g_build_filename(priv->location_tmp, gpgkey_name, NULL); + + /* download and import public key */ + if ((g_str_has_prefix(gpgkey, "https://") || + g_str_has_prefix(gpgkey, "file://"))) { + g_debug("importing public key %s", gpgkey); + + ret = dnf_repo_download_import_public_key(repo, gpgkey, key_tmp, error); + if (!ret) + goto out; + } - /* do we autoimport this into librpm? */ - if ((flags & DNF_REPO_UPDATE_FLAG_IMPORT_PUBKEY) > 0) { - ret = dnf_repo_add_public_key(repo, key_tmp, error); - if (!ret) - goto out; + /* do we autoimport this into librpm? */ + if ((flags & DNF_REPO_UPDATE_FLAG_IMPORT_PUBKEY) > 0) { + ret = dnf_repo_add_public_key(repo, key_tmp, error); + if (!ret) + goto out; + } } } }