From d2e16223ba2c8ff2bfd115df668bf079cc7f2fa9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Wed, 17 Jan 2024 07:04:56 +0100 Subject: [PATCH 1/8] Add pImpl to `libdnf5::Vars` --- include/libdnf5/conf/vars.hpp | 17 +++++----- libdnf5/conf/vars.cpp | 64 +++++++++++++++++++++++++++-------- 2 files changed, 58 insertions(+), 23 deletions(-) diff --git a/include/libdnf5/conf/vars.hpp b/include/libdnf5/conf/vars.hpp index a17a0c886..d1c95ddb7 100644 --- a/include/libdnf5/conf/vars.hpp +++ b/include/libdnf5/conf/vars.hpp @@ -23,7 +23,6 @@ along with libdnf. If not, see . #include "libdnf5/base/base_weak.hpp" #include -#include #include #include @@ -60,16 +59,18 @@ struct Vars { Priority priority; }; - Vars(const libdnf5::BaseWeakPtr & base) : base(base) {} + Vars(const libdnf5::BaseWeakPtr & base); Vars(libdnf5::Base & base); + ~Vars(); + /// @brief Substitute DNF vars in the input text. /// /// @param text The text for substitution /// @return The substituted text std::string substitute(const std::string & text) const; - const std::map & get_variables() const { return variables; } + const std::map & get_variables() const; /// @brief Set particular variable to a value /// @@ -89,17 +90,17 @@ struct Vars { /// /// @param name Name of the variable /// @return true if there is such an element, otherwise false - bool contains(const std::string & name) const { return variables.find(name) != variables.end(); } + bool contains(const std::string & name) const; /// @brief Get value of particular variable. /// /// @param name Name of the variable - const std::string & get_value(const std::string & name) const { return variables.at(name).value; } + const std::string & get_value(const std::string & name) const; /// @brief Get particular variable. /// /// @param name Name of the variable - const Variable & get(const std::string & name) const { return variables.at(name); } + const Variable & get(const std::string & name) const; static std::unique_ptr detect_release(const BaseWeakPtr & base, const std::string & install_root_path); @@ -160,8 +161,8 @@ struct Vars { /// @return releasever_major, releasever_minor static std::tuple split_releasever(const std::string & releasever); - BaseWeakPtr base; - std::map variables; + class Impl; + std::unique_ptr p_impl; }; } // namespace libdnf5 diff --git a/libdnf5/conf/vars.cpp b/libdnf5/conf/vars.cpp index 4e58e9763..9b387331a 100644 --- a/libdnf5/conf/vars.cpp +++ b/libdnf5/conf/vars.cpp @@ -41,7 +41,6 @@ along with libdnf. If not, see . #include #include #include -#include #define ASCII_LOWERCASE "abcdefghijklmnopqrstuvwxyz" #define ASCII_UPPERCASE "ABCDEFGHIJKLMNOPQRSTUVWXYZ" @@ -121,6 +120,19 @@ static std::string detect_arch() { // ================================================================== +class Vars::Impl { +public: + Impl(const BaseWeakPtr & base); + +private: + friend Vars; + + std::map variables; + BaseWeakPtr base; +}; + +Vars::Impl::Impl(const BaseWeakPtr & base) : base(base) {} + std::unique_ptr Vars::detect_release(const BaseWeakPtr & base, const std::string & install_root_path) { std::unique_ptr release_ver; @@ -160,6 +172,10 @@ std::unique_ptr Vars::detect_release(const BaseWeakPtr & base, cons Vars::Vars(Base & base) : Vars(base.get_weak_ptr()) {} +Vars::Vars(const libdnf5::BaseWeakPtr & base) : p_impl(std::make_unique(base)) {} + +Vars::~Vars() = default; + const unsigned int MAXIMUM_EXPRESSION_DEPTH = 32; // Expand variables in a subexpression @@ -223,7 +239,7 @@ std::pair Vars::substitute_expression(std::string_view text auto pos_after_variable = static_cast(std::distance(res.begin(), it)); // Find the substituting string and the end of the variable expression - auto variable_mapping = variables.find(res.substr(pos_variable, pos_after_variable - pos_variable)); + auto variable_mapping = p_impl->variables.find(res.substr(pos_variable, pos_after_variable - pos_variable)); const std::string * subst_str = nullptr; size_t pos_after_variable_expression; @@ -262,7 +278,7 @@ std::pair Vars::substitute_expression(std::string_view text // If variable is unset or empty, the expansion of word is // substituted. Otherwise, the value of variable is // substituted. - if (variable_mapping == variables.end() || variable_mapping->second.value.empty()) { + if (variable_mapping == p_impl->variables.end() || variable_mapping->second.value.empty()) { subst_str = &expanded_word; } else { subst_str = &variable_mapping->second.value; @@ -271,7 +287,7 @@ std::pair Vars::substitute_expression(std::string_view text // ${variable:+word} (alternate value) // If variable is unset or empty nothing is substituted. // Otherwise, the expansion of word is substituted. - if (variable_mapping == variables.end() || variable_mapping->second.value.empty()) { + if (variable_mapping == p_impl->variables.end() || variable_mapping->second.value.empty()) { const std::string empty{}; subst_str = ∅ } else { @@ -285,7 +301,7 @@ std::pair Vars::substitute_expression(std::string_view text pos_after_variable_expression = pos_after_word + 1; } else if (res[pos_after_variable] == '}') { // ${variable} - if (variable_mapping != variables.end()) { + if (variable_mapping != p_impl->variables.end()) { subst_str = &variable_mapping->second.value; } // Move past the closing '}' @@ -297,7 +313,7 @@ std::pair Vars::substitute_expression(std::string_view text } } else { // No braces, we have a $variable - if (variable_mapping != variables.end()) { + if (variable_mapping != p_impl->variables.end()) { subst_str = &variable_mapping->second.value; } pos_after_variable_expression = pos_after_variable; @@ -358,10 +374,10 @@ void Vars::set(const std::string & name, const std::string & value, Priority pri // set_unsafe sets the variable without checking whether it's read-only std::function set_unsafe = [&](const std::string & name, const std::string & value, Priority prio) { - auto it = variables.find(name); + auto it = p_impl->variables.find(name); // Do nothing if the var is already set with a higher priority - if (it != variables.end() && prio < it->second.priority) { + if (it != p_impl->variables.end() && prio < it->second.priority) { return; } @@ -376,8 +392,8 @@ void Vars::set(const std::string & name, const std::string & value, Priority pri } } - if (it == variables.end()) { - variables.insert({name, {value, prio}}); + if (it == p_impl->variables.end()) { + p_impl->variables.insert({name, {value, prio}}); } else { it->second.value = value; it->second.priority = prio; @@ -390,8 +406,8 @@ void Vars::set_lazy( const std::string & name, const std::function()> & get_value, const Priority prio) { - auto it = variables.find(name); - if (it == variables.end() || prio > it->second.priority) { + auto it = p_impl->variables.find(name); + if (it == p_impl->variables.end() || prio > it->second.priority) { const auto maybe_value = get_value(); if (maybe_value != nullptr) { set(name, *maybe_value, prio); @@ -419,13 +435,15 @@ void Vars::detect_vars(const std::string & installroot) { set_lazy( "basearch", [this]() -> auto { - auto base_arch = libdnf5::rpm::get_base_arch(variables["arch"].value); + auto base_arch = libdnf5::rpm::get_base_arch(p_impl->variables["arch"].value); return base_arch.empty() ? nullptr : std::make_unique(std::move(base_arch)); }, Priority::AUTO); set_lazy( - "releasever", [this, &installroot]() -> auto { return detect_release(base, installroot); }, Priority::AUTO); + "releasever", + [this, &installroot]() -> auto { return detect_release(p_impl->base, installroot); }, + Priority::AUTO); } static void dir_close(DIR * d) { @@ -433,7 +451,7 @@ static void dir_close(DIR * d) { } void Vars::load_from_dir(const std::string & directory) { - auto & logger = *base->get_logger(); + auto & logger = *p_impl->base->get_logger(); if (DIR * dir = opendir(directory.c_str())) { std::unique_ptr dir_guard(dir, &dir_close); while (auto ent = readdir(dir)) { @@ -482,4 +500,20 @@ void Vars::load_from_env() { } } +const std::map & Vars::get_variables() const { + return p_impl->variables; +} + +bool Vars::contains(const std::string & name) const { + return p_impl->variables.find(name) != p_impl->variables.end(); +} + +const std::string & Vars::get_value(const std::string & name) const { + return p_impl->variables.at(name).value; +} + +const Vars::Variable & Vars::get(const std::string & name) const { + return p_impl->variables.at(name); +} + } // namespace libdnf5 From 2910c11038b4271303a31d166d893173ea0e4e2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Wed, 17 Jan 2024 07:51:01 +0100 Subject: [PATCH 2/8] Add pImpl to `libdnf5::ConfigParser` --- include/libdnf5/conf/config_parser.hpp | 16 +++- libdnf5/conf/config_parser.cpp | 122 +++++++++++++++---------- 2 files changed, 87 insertions(+), 51 deletions(-) diff --git a/include/libdnf5/conf/config_parser.hpp b/include/libdnf5/conf/config_parser.hpp index e8e8c0e53..c3479e066 100644 --- a/include/libdnf5/conf/config_parser.hpp +++ b/include/libdnf5/conf/config_parser.hpp @@ -23,7 +23,6 @@ along with libdnf. If not, see . #include "libdnf5/common/exception.hpp" #include "libdnf5/common/preserve_order_map.hpp" -#include #include @@ -86,6 +85,15 @@ struct ConfigParser { public: using Container = PreserveOrderMap>; + ConfigParser(); + ~ConfigParser(); + + ConfigParser(const ConfigParser & src); + ConfigParser & operator=(const ConfigParser & src); + + ConfigParser(ConfigParser && src) noexcept; + ConfigParser & operator=(ConfigParser && src) noexcept; + /** * @brief Reads/parse one INI file * @@ -132,10 +140,8 @@ struct ConfigParser { Container & get_data() noexcept; private: - Container data; - int item_number{0}; - std::string header; - std::map raw_items; + class Impl; + std::unique_ptr p_impl; }; } // namespace libdnf5 diff --git a/libdnf5/conf/config_parser.cpp b/libdnf5/conf/config_parser.cpp index cc2f1d7b5..dfbc190fb 100644 --- a/libdnf5/conf/config_parser.cpp +++ b/libdnf5/conf/config_parser.cpp @@ -24,6 +24,7 @@ along with libdnf. If not, see . #include "libdnf5/utils/bgettext/bgettext-mark-domain.h" #include "libdnf5/utils/fs/file.hpp" +#include #include @@ -58,6 +59,35 @@ ConfigParserOptionNotFoundError::ConfigParserOptionNotFoundError( : ConfigParserError(M_("Section \"{}\" does not contain option \"{}\""), section, option) {} +class ConfigParser::Impl { +private: + friend ConfigParser; + Container data; + int item_number{0}; + std::string header; + std::map raw_items; +}; + +ConfigParser::ConfigParser() : p_impl(std::make_unique()) {} + +ConfigParser::~ConfigParser() = default; + +ConfigParser::ConfigParser(const ConfigParser & src) : p_impl(new Impl(*src.p_impl)) {} +ConfigParser::ConfigParser(ConfigParser && src) noexcept = default; + +ConfigParser & ConfigParser::operator=(const ConfigParser & src) { + if (this != &src) { + if (p_impl) { + *p_impl = *src.p_impl; + } else { + p_impl = std::make_unique(*src.p_impl); + } + } + + return *this; +} +ConfigParser & ConfigParser::operator=(ConfigParser && src) noexcept = default; + void ConfigParser::read(const std::string & file_path) try { IniParser parser(file_path); ::libdnf5::read(*this, parser); @@ -73,13 +103,13 @@ void ConfigParser::read(const std::string & file_path) try { bool ConfigParser::add_section(const std::string & section, const std::string & raw_line) { - if (data.find(section) != data.end()) { + if (p_impl->data.find(section) != p_impl->data.end()) { return false; } if (!raw_line.empty()) { - raw_items[section] = raw_line; + p_impl->raw_items[section] = raw_line; } - data[section] = {}; + p_impl->data[section] = {}; return true; } @@ -90,13 +120,13 @@ bool ConfigParser::add_section(const std::string & section) { bool ConfigParser::add_section(std::string && section, std::string && raw_line) { - if (data.find(section) != data.end()) { + if (p_impl->data.find(section) != p_impl->data.end()) { return false; } if (!raw_line.empty()) { - raw_items[section] = std::move(raw_line); + p_impl->raw_items[section] = std::move(raw_line); } - data[std::move(section)] = {}; + p_impl->data[std::move(section)] = {}; return true; } @@ -107,13 +137,13 @@ bool ConfigParser::add_section(std::string && section) { bool ConfigParser::has_section(const std::string & section) const noexcept { - return data.find(section) != data.end(); + return p_impl->data.find(section) != p_impl->data.end(); } bool ConfigParser::has_option(const std::string & section, const std::string & key) const noexcept { - auto section_iter = data.find(section); - return section_iter != data.end() && section_iter->second.find(key) != section_iter->second.end(); + auto section_iter = p_impl->data.find(section); + return section_iter != p_impl->data.end() && section_iter->second.find(key) != section_iter->second.end(); } @@ -130,91 +160,91 @@ static std::string create_raw_item(const std::string & value, const std::string void ConfigParser::set_value( const std::string & section, const std::string & key, const std::string & value, const std::string & raw_item) { - auto section_iter = data.find(section); - if (section_iter == data.end()) { + auto section_iter = p_impl->data.find(section); + if (section_iter == p_impl->data.end()) { throw ConfigParserSectionNotFoundError(section); } if (raw_item.empty()) { - raw_items.erase(section + ']' + key); + p_impl->raw_items.erase(section + ']' + key); } else { - raw_items[section + ']' + key] = raw_item; + p_impl->raw_items[section + ']' + key] = raw_item; } section_iter->second[key] = value; } void ConfigParser::set_value(const std::string & section, const std::string & key, const std::string & value) { - auto raw_iter = raw_items.find(section + ']' + key); - auto raw = create_raw_item(value, raw_iter != raw_items.end() ? raw_iter->second : ""); + auto raw_iter = p_impl->raw_items.find(section + ']' + key); + auto raw = create_raw_item(value, raw_iter != p_impl->raw_items.end() ? raw_iter->second : ""); set_value(section, key, value, raw); } void ConfigParser::set_value( const std::string & section, std::string && key, std::string && value, std::string && raw_item) { - auto section_iter = data.find(section); - if (section_iter == data.end()) { + auto section_iter = p_impl->data.find(section); + if (section_iter == p_impl->data.end()) { throw ConfigParserSectionNotFoundError(section); } if (raw_item.empty()) { - raw_items.erase(section + ']' + key); + p_impl->raw_items.erase(section + ']' + key); } else { - raw_items[section + ']' + key] = std::move(raw_item); + p_impl->raw_items[section + ']' + key] = std::move(raw_item); } section_iter->second[std::move(key)] = std::move(value); } void ConfigParser::set_value(const std::string & section, std::string && key, std::string && value) { - auto raw_iter = raw_items.find(section + ']' + key); - auto raw = create_raw_item(value, raw_iter != raw_items.end() ? raw_iter->second : ""); + auto raw_iter = p_impl->raw_items.find(section + ']' + key); + auto raw = create_raw_item(value, raw_iter != p_impl->raw_items.end() ? raw_iter->second : ""); set_value(section, std::move(key), std::move(value), std::move(raw)); } bool ConfigParser::remove_section(const std::string & section) { - auto removed = data.erase(section) > 0; + auto removed = p_impl->data.erase(section) > 0; if (removed) { - raw_items.erase(section); + p_impl->raw_items.erase(section); } return removed; } bool ConfigParser::remove_option(const std::string & section, const std::string & key) { - auto section_iter = data.find(section); - if (section_iter == data.end()) { + auto section_iter = p_impl->data.find(section); + if (section_iter == p_impl->data.end()) { return false; } auto removed = section_iter->second.erase(key) > 0; if (removed) { - raw_items.erase(section + ']' + key); + p_impl->raw_items.erase(section + ']' + key); } return removed; } void ConfigParser::add_comment_line(const std::string & section, const std::string & comment) { - auto section_iter = data.find(section); - if (section_iter == data.end()) { + auto section_iter = p_impl->data.find(section); + if (section_iter == p_impl->data.end()) { throw ConfigParserSectionNotFoundError(section); } - section_iter->second["#" + std::to_string(++item_number)] = comment; + section_iter->second["#" + std::to_string(++p_impl->item_number)] = comment; } void ConfigParser::add_comment_line(const std::string & section, std::string && comment) { - auto section_iter = data.find(section); - if (section_iter == data.end()) { + auto section_iter = p_impl->data.find(section); + if (section_iter == p_impl->data.end()) { throw ConfigParserSectionNotFoundError(section); } - section_iter->second["#" + std::to_string(++item_number)] = std::move(comment); + section_iter->second["#" + std::to_string(++p_impl->item_number)] = std::move(comment); } const std::string & ConfigParser::get_value(const std::string & section, const std::string & key) const { - auto sect = data.find(section); - if (sect == data.end()) { + auto sect = p_impl->data.find(section); + if (sect == p_impl->data.end()) { throw ConfigParserSectionNotFoundError(section); } auto key_val = sect->second.find(key); @@ -226,22 +256,22 @@ const std::string & ConfigParser::get_value(const std::string & section, const s const std::string & ConfigParser::get_header() const noexcept { - return header; + return p_impl->header; } std::string & ConfigParser::get_header() noexcept { - return header; + return p_impl->header; } const ConfigParser::Container & ConfigParser::get_data() const noexcept { - return data; + return p_impl->data; } ConfigParser::Container & ConfigParser::get_data() noexcept { - return data; + return p_impl->data; } @@ -307,15 +337,15 @@ void ConfigParser::write(const std::string & file_path, bool append) const { utils::fs::File file(file_path, append ? "a" : "w"); bool prepend_new_line = append; - if (!header.empty()) { + if (!p_impl->header.empty()) { if (prepend_new_line) { file.putc('\n'); } - file.write(header); - prepend_new_line = header.back() != '\n'; + file.write(p_impl->header); + prepend_new_line = p_impl->header.back() != '\n'; } - for (const auto & section : data) { - write_section(file, section.first, section.second, raw_items, prepend_new_line); + for (const auto & section : p_impl->data) { + write_section(file, section.first, section.second, p_impl->raw_items, prepend_new_line); } // Make sure file ends with newline character '\n'. @@ -326,15 +356,15 @@ void ConfigParser::write(const std::string & file_path, bool append) const { void ConfigParser::write(const std::string & file_path, bool append, const std::string & section) const { - auto sit = data.find(section); - if (sit == data.end()) { + auto sit = p_impl->data.find(section); + if (sit == p_impl->data.end()) { throw ConfigParserSectionNotFoundError(section); } utils::fs::File file(file_path, append ? "a" : "w"); bool prepend_new_line = append; - write_section(file, sit->first, sit->second, raw_items, prepend_new_line); + write_section(file, sit->first, sit->second, p_impl->raw_items, prepend_new_line); // Make sure file ends with newline character '\n'. if (prepend_new_line) { From 69e3fa2674c3ecc6125e45f5d61edfe16c1979d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Wed, 17 Jan 2024 08:52:19 +0100 Subject: [PATCH 3/8] Add pImpl to `libdnf5::base::LogEvent` --- include/libdnf5/base/log_event.hpp | 36 ++++++------ libdnf5/base/log_event.cpp | 92 ++++++++++++++++++++++++++++-- 2 files changed, 102 insertions(+), 26 deletions(-) diff --git a/include/libdnf5/base/log_event.hpp b/include/libdnf5/base/log_event.hpp index dba018c36..3386701ec 100644 --- a/include/libdnf5/base/log_event.hpp +++ b/include/libdnf5/base/log_event.hpp @@ -44,27 +44,29 @@ class LogEvent { const libdnf5::transaction::TransactionItemType spec_type, const std::string & spec); LogEvent(libdnf5::GoalProblem problem, const SolverProblems & solver_problems); - ~LogEvent() = default; + ~LogEvent(); + + LogEvent(const LogEvent & src); + LogEvent & operator=(const LogEvent & src); + + LogEvent(LogEvent && src) noexcept; + LogEvent & operator=(LogEvent && src) noexcept; /// @return GoalAction for which goal event was created - libdnf5::GoalAction get_action() const { return action; }; + libdnf5::GoalAction get_action() const; /// @return GoalProblem that specify the type of report - libdnf5::GoalProblem get_problem() const { return problem; }; + libdnf5::GoalProblem get_problem() const; /// @return Additional information (internal), that are required for formatted string - const std::set get_additional_data() const { return additional_data; }; + const std::set & get_additional_data() const; /// @return GoalJobSetting if it is relevant for the particular GoalProblem - const libdnf5::GoalJobSettings * get_job_settings() const { - return job_settings ? &job_settings.value() : nullptr; - }; + const libdnf5::GoalJobSettings * get_job_settings() const; /// @return SPEC if it is relevant for the particular GoalProblem - const std::string * get_spec() const { return spec ? &spec.value() : nullptr; }; + const std::string * get_spec() const; /// @return SolverProblems if they are relevant for the particular GoalProblem - const SolverProblems * get_solver_problems() const { return solver_problems ? &solver_problems.value() : nullptr; }; + const SolverProblems * get_solver_problems() const; /// Convert an element from resolve log to string; - std::string to_string() const { - return to_string(action, problem, additional_data, job_settings, spec_type, spec, solver_problems); - }; + std::string to_string() const; private: /// Convert an element from resolve log to string; @@ -77,14 +79,8 @@ class LogEvent { const std::optional & spec, const std::optional & solver_problems); - libdnf5::GoalAction action; - libdnf5::GoalProblem problem; - - std::set additional_data; - std::optional job_settings; - std::optional spec_type; - std::optional spec; - std::optional solver_problems; + class Impl; + std::unique_ptr p_impl; }; } // namespace libdnf5::base diff --git a/libdnf5/base/log_event.cpp b/libdnf5/base/log_event.cpp index 298b49a6e..da30444f0 100644 --- a/libdnf5/base/log_event.cpp +++ b/libdnf5/base/log_event.cpp @@ -27,8 +27,31 @@ along with libdnf. If not, see . namespace libdnf5::base { - -LogEvent::LogEvent( +class LogEvent::Impl { +public: + Impl( + libdnf5::GoalAction action, + libdnf5::GoalProblem problem, + const std::set & additional_data, + const libdnf5::GoalJobSettings & settings, + libdnf5::transaction::TransactionItemType spec_type, + const std::string & spec); + + Impl(libdnf5::GoalAction action, libdnf5::GoalProblem problem, const SolverProblems & solver_problems); + +private: + friend LogEvent; + + libdnf5::GoalAction action; + libdnf5::GoalProblem problem; + std::set additional_data; + std::optional job_settings; + std::optional spec_type; + std::optional spec; + std::optional solver_problems; +}; + +LogEvent::Impl::Impl( libdnf5::GoalAction action, libdnf5::GoalProblem problem, const std::set & additional_data, @@ -40,7 +63,21 @@ LogEvent::LogEvent( additional_data(additional_data), job_settings(settings), spec_type(spec_type), - spec(spec) { + spec(spec) {} + +LogEvent::Impl::Impl(libdnf5::GoalAction action, libdnf5::GoalProblem problem, const SolverProblems & solver_problems) + : action(action), + problem(problem), + solver_problems(solver_problems) {} + +LogEvent::LogEvent( + libdnf5::GoalAction action, + libdnf5::GoalProblem problem, + const std::set & additional_data, + const libdnf5::GoalJobSettings & settings, + const libdnf5::transaction::TransactionItemType spec_type, + const std::string & spec) + : p_impl(std::make_unique(action, problem, additional_data, settings, spec_type, spec)) { libdnf_assert( !(problem == libdnf5::GoalProblem::SOLVER_ERROR || problem == libdnf5::GoalProblem::SOLVER_PROBLEM_STRICT_RESOLVEMENT), @@ -50,9 +87,7 @@ LogEvent::LogEvent( } LogEvent::LogEvent(libdnf5::GoalProblem problem, const SolverProblems & solver_problems) - : action(libdnf5::GoalAction::RESOLVE), - problem(problem), - solver_problems(solver_problems) { + : p_impl(std::make_unique(libdnf5::GoalAction::RESOLVE, problem, solver_problems)) { libdnf_assert( problem == libdnf5::GoalProblem::SOLVER_ERROR || problem == libdnf5::GoalProblem::SOLVER_PROBLEM_STRICT_RESOLVEMENT, @@ -60,6 +95,23 @@ LogEvent::LogEvent(libdnf5::GoalProblem problem, const SolverProblems & solver_p "libdnf5::GoalProblem::SOLVER_PROBLEM_STRICT_RESOLVEMENT is supported"); } +LogEvent::~LogEvent() = default; + +LogEvent::LogEvent(const LogEvent & src) : p_impl(new Impl(*src.p_impl)) {} +LogEvent::LogEvent(LogEvent && src) noexcept = default; + +LogEvent & LogEvent::operator=(const LogEvent & src) { + if (this != &src) { + if (p_impl) { + *p_impl = *src.p_impl; + } else { + p_impl = std::make_unique(*src.p_impl); + } + } + + return *this; +} +LogEvent & LogEvent::operator=(LogEvent && src) noexcept = default; std::string LogEvent::to_string( libdnf5::GoalAction action, @@ -167,5 +219,33 @@ std::string LogEvent::to_string( return ret; } +libdnf5::GoalAction LogEvent::get_action() const { + return p_impl->action; +}; +libdnf5::GoalProblem LogEvent::get_problem() const { + return p_impl->problem; +}; +const std::set & LogEvent::get_additional_data() const { + return p_impl->additional_data; +}; +const libdnf5::GoalJobSettings * LogEvent::get_job_settings() const { + return p_impl->job_settings ? &p_impl->job_settings.value() : nullptr; +}; +const std::string * LogEvent::get_spec() const { + return p_impl->spec ? &p_impl->spec.value() : nullptr; +}; +const SolverProblems * LogEvent::get_solver_problems() const { + return p_impl->solver_problems ? &p_impl->solver_problems.value() : nullptr; +}; +std::string LogEvent::to_string() const { + return to_string( + p_impl->action, + p_impl->problem, + p_impl->additional_data, + p_impl->job_settings, + p_impl->spec_type, + p_impl->spec, + p_impl->solver_problems); +}; } // namespace libdnf5::base From a57751ffb86a53a7a66e6e9e7c99a6df111cf616 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Wed, 17 Jan 2024 13:25:48 +0100 Subject: [PATCH 4/8] Add pImpl to `libdnf5::base::SolverProblems` --- include/libdnf5/base/solver_problems.hpp | 9 ++--- libdnf5/base/solver_problems.cpp | 44 +++++++++++++++++++----- 2 files changed, 39 insertions(+), 14 deletions(-) diff --git a/include/libdnf5/base/solver_problems.hpp b/include/libdnf5/base/solver_problems.hpp index 109cfc249..3941cc5d6 100644 --- a/include/libdnf5/base/solver_problems.hpp +++ b/include/libdnf5/base/solver_problems.hpp @@ -24,8 +24,6 @@ along with libdnf. If not, see . #include "goal_elements.hpp" -#include "libdnf5/common/impl_ptr.hpp" - namespace libdnf5::base { @@ -52,9 +50,7 @@ class SolverProblems { /// be rendered into a string by the `problem_to_string()` method. // @replaces libdnf/Goal.describeProblemRules(unsigned i, bool pkgs); // @replaces libdnf/Goal.describeAllProblemRules(bool pkgs); - std::vector>>> get_problems() const { - return problems; - }; + std::vector>>> get_problems() const; /// Convert SolverProblems class to string representative; std::string to_string() const; @@ -65,7 +61,8 @@ class SolverProblems { private: friend class Transaction; - std::vector>>> problems; + class Impl; + std::unique_ptr p_impl; }; } // namespace libdnf5::base diff --git a/libdnf5/base/solver_problems.cpp b/libdnf5/base/solver_problems.cpp index 7aef947cd..6ca2f9c2b 100644 --- a/libdnf5/base/solver_problems.cpp +++ b/libdnf5/base/solver_problems.cpp @@ -65,6 +65,7 @@ static const std::map PKG_PROBLEMS_DICT = { M_("The operation would result in removing" " of running kernel: {}")}}; + std::string string_join( const std::vector>> & src, const std::string & delim) { if (src.empty()) { @@ -174,13 +175,36 @@ std::vector>> get_removal_of_pr } // namespace -SolverProblems::SolverProblems( +class SolverProblems::Impl { +public: + Impl(const std::vector>>> & problems); + +private: + friend SolverProblems; + std::vector>>> problems; +}; + +SolverProblems::Impl::Impl( const std::vector>>> & problems) : problems(problems) {} -SolverProblems::SolverProblems(const SolverProblems & src) = default; +SolverProblems::SolverProblems( + const std::vector>>> & problems) + : p_impl(std::make_unique(problems)) {} + +SolverProblems::SolverProblems(const SolverProblems & src) : p_impl(new Impl(*src.p_impl)) {} SolverProblems::SolverProblems(SolverProblems && src) noexcept = default; -SolverProblems & SolverProblems::operator=(const SolverProblems & src) = default; +SolverProblems & SolverProblems::operator=(const SolverProblems & src) { + if (this != &src) { + if (p_impl) { + *p_impl = *src.p_impl; + } else { + p_impl = std::make_unique(*src.p_impl); + } + } + + return *this; +} SolverProblems & SolverProblems::operator=(SolverProblems && src) noexcept = default; SolverProblems::~SolverProblems() = default; @@ -241,22 +265,22 @@ std::string SolverProblems::problem_to_string(const std::pairproblems.empty()) { return {}; } std::string output; - if (problems.size() == 1) { + if (p_impl->problems.size() == 1) { output.append(_("Problem: ")); - output.append(string_join(*problems.begin(), "\n - ")); + output.append(string_join(*p_impl->problems.begin(), "\n - ")); return output; } const char * problem_prefix = _("Problem {}: "); output.append(utils::sformat(problem_prefix, 1)); - output.append(string_join(*problems.begin(), "\n - ")); + output.append(string_join(*p_impl->problems.begin(), "\n - ")); int index = 2; - for (auto iter = std::next(problems.begin()); iter != problems.end(); ++iter) { + for (auto iter = std::next(p_impl->problems.begin()); iter != p_impl->problems.end(); ++iter) { output.append("\n "); output.append(utils::sformat(problem_prefix, index)); output.append(string_join(*iter, "\n - ")); @@ -361,5 +385,9 @@ std::vector>>> SolverProblems::get_problems() + const { + return p_impl->problems; +}; } // namespace libdnf5::base From 1dc7557190eca81e1e2419a847d35759e8b714de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Mon, 19 Feb 2024 09:43:06 +0100 Subject: [PATCH 5/8] Add pImpl to `libdnf5::repo::RepoSack` --- include/libdnf5/repo/repo_sack.hpp | 19 ++-- libdnf5/repo/repo_sack.cpp | 136 ++++++++++++++++++----------- 2 files changed, 93 insertions(+), 62 deletions(-) diff --git a/include/libdnf5/repo/repo_sack.hpp b/include/libdnf5/repo/repo_sack.hpp index 417412684..792514cfc 100644 --- a/include/libdnf5/repo/repo_sack.hpp +++ b/include/libdnf5/repo/repo_sack.hpp @@ -98,10 +98,10 @@ class RepoSack : public sack::Sack { const std::vector & paths, bool calculate_checksum = false); /// @return `true` if the system repository has been initialized (via `get_system_repo()`). - bool has_system_repo() const noexcept { return system_repo; } + bool has_system_repo() const noexcept; /// @return `true` if the command line repository has been initialized (via `get_cmdline_repo()`). - bool has_cmdline_repo() const noexcept { return cmdline_repo; } + bool has_cmdline_repo() const noexcept; /// Dumps libsolv's rpm debugdata of all loaded repositories. /// @param dir The directory into which to dump the debugdata. @@ -135,7 +135,7 @@ class RepoSack : public sack::Sack { /// @param import_keys If true, attempts to download and import keys for repositories that failed key validation void update_and_load_repos(libdnf5::repo::RepoQuery & repos, bool import_keys = true); - RepoSackWeakPtr get_weak_ptr() { return RepoSackWeakPtr(this, &sack_guard); } + RepoSackWeakPtr get_weak_ptr(); /// @return The `Base` object to which this object belongs. /// @since 5.0 @@ -152,31 +152,28 @@ class RepoSack : public sack::Sack { /// are created from info in system state. void fix_group_missing_xml(); + ~RepoSack(); + private: friend class libdnf5::Base; friend class RepoQuery; friend class rpm::PackageSack; - explicit RepoSack(const libdnf5::BaseWeakPtr & base) : base(base) {} + explicit RepoSack(const libdnf5::BaseWeakPtr & base); explicit RepoSack(libdnf5::Base & base); /// Loads repositories configuration overrides from drop-in directories. No new repositories are created. /// Only the configuration of the corresponding existing repositories is modified. void load_repos_configuration_overrides(); - WeakPtrGuard sack_guard; - /// If not created yet, creates the cmdline repository and returns it. /// @return The cmdline repository. libdnf5::repo::RepoWeakPtr get_cmdline_repo(); void internalize_repos(); - BaseWeakPtr base; - - repo::Repo * system_repo{nullptr}; - repo::Repo * cmdline_repo{nullptr}; - bool repos_updated_and_loaded{false}; + class Impl; + std::unique_ptr p_impl; }; } // namespace libdnf5::repo diff --git a/libdnf5/repo/repo_sack.cpp b/libdnf5/repo/repo_sack.cpp index abe771fc3..3f1240936 100644 --- a/libdnf5/repo/repo_sack.cpp +++ b/libdnf5/repo/repo_sack.cpp @@ -38,7 +38,6 @@ along with libdnf. If not, see . #include "libdnf5/comps/group/query.hpp" #include "libdnf5/conf/config_parser.hpp" #include "libdnf5/conf/const.hpp" -#include "libdnf5/conf/option_bool.hpp" #include "libdnf5/repo/file_downloader.hpp" #include "libdnf5/utils/bgettext/bgettext-mark-domain.h" #include "libdnf5/utils/fs/file.hpp" @@ -72,8 +71,27 @@ constexpr const char * CMDLINE_REPO_NAME = "@commandline"; namespace libdnf5::repo { +class RepoSack::Impl { +public: + Impl(const libdnf5::BaseWeakPtr & base); + +private: + BaseWeakPtr base; + WeakPtrGuard sack_guard; + repo::Repo * system_repo{nullptr}; + repo::Repo * cmdline_repo{nullptr}; + bool repos_updated_and_loaded{false}; + friend RepoSack; +}; + + +RepoSack::Impl::Impl(const libdnf5::BaseWeakPtr & base) : base(base) {} + RepoSack::RepoSack(libdnf5::Base & base) : RepoSack(base.get_weak_ptr()) {} +RepoSack::RepoSack(const libdnf5::BaseWeakPtr & base) : p_impl(std::make_unique(base)) {} + +RepoSack::~RepoSack() = default; RepoWeakPtr RepoSack::create_repo(const std::string & id) { for (const auto & existing_repo : get_data()) { @@ -82,7 +100,7 @@ RepoWeakPtr RepoSack::create_repo(const std::string & id) { M_("Failed to create repo \"{}\": Id is present more than once in the configuration"), id); } } - auto repo = std::make_unique(base, id, Repo::Type::AVAILABLE); + auto repo = std::make_unique(p_impl->base, id, Repo::Type::AVAILABLE); return add_item_with_return(std::move(repo)); } @@ -95,14 +113,14 @@ RepoWeakPtr RepoSack::create_repo_from_libsolv_testcase(const std::string & id, RepoWeakPtr RepoSack::get_cmdline_repo() { - if (!cmdline_repo) { - std::unique_ptr repo(new Repo(base, CMDLINE_REPO_NAME, Repo::Type::COMMANDLINE)); + if (!p_impl->cmdline_repo) { + std::unique_ptr repo(new Repo(p_impl->base, CMDLINE_REPO_NAME, Repo::Type::COMMANDLINE)); repo->get_config().get_build_cache_option().set(libdnf5::Option::Priority::RUNTIME, false); - cmdline_repo = repo.get(); + p_impl->cmdline_repo = repo.get(); add_item(std::move(repo)); } - return cmdline_repo->get_weak_ptr(); + return p_impl->cmdline_repo->get_weak_ptr(); } @@ -163,9 +181,9 @@ std::map RepoSack::add_cmdline_packages( std::map path_to_package; if (!url_to_path.empty()) { - auto & logger = *base->get_logger(); + auto & logger = *p_impl->base->get_logger(); // download remote URLs - libdnf5::repo::FileDownloader downloader(base); + libdnf5::repo::FileDownloader downloader(p_impl->base); for (auto & [url, dest_path] : url_to_path) { logger.debug("Downloading package \"{}\" to file \"{}\"", url, dest_path.string()); // TODO(mblaha): temporarily used the dummy DownloadCallbacks instance @@ -187,7 +205,7 @@ std::map RepoSack::add_cmdline_packages( } if (!path_to_package.empty()) { - base->get_rpm_package_sack()->load_config_excludes_includes(); + p_impl->base->get_rpm_package_sack()->load_config_excludes_includes(); } return path_to_package; @@ -195,15 +213,15 @@ std::map RepoSack::add_cmdline_packages( RepoWeakPtr RepoSack::get_system_repo() { - if (!system_repo) { - std::unique_ptr repo(new Repo(base, SYSTEM_REPO_NAME, Repo::Type::SYSTEM)); + if (!p_impl->system_repo) { + std::unique_ptr repo(new Repo(p_impl->base, SYSTEM_REPO_NAME, Repo::Type::SYSTEM)); // TODO(mblaha): re-enable caching once we can reliably detect whether system repo is up-to-date repo->get_config().get_build_cache_option().set(libdnf5::Option::Priority::RUNTIME, false); - system_repo = repo.get(); + p_impl->system_repo = repo.get(); add_item(std::move(repo)); } - return system_repo->get_weak_ptr(); + return p_impl->system_repo->get_weak_ptr(); } /** @@ -213,7 +231,7 @@ RepoWeakPtr RepoSack::get_system_repo() { * @warning This function should not be used to load and update repositories. Instead, use `RepoSack::update_and_load_enabled_repos` */ void RepoSack::update_and_load_repos(libdnf5::repo::RepoQuery & repos, bool import_keys) { - auto logger = base->get_logger(); + auto logger = p_impl->base->get_logger(); std::atomic except_in_main_thread{false}; // set to true if an exception occurred in the main thread std::exception_ptr except_ptr; // for pass exception from thread_sack_loader to main thread, @@ -291,12 +309,12 @@ void RepoSack::update_and_load_repos(libdnf5::repo::RepoQuery & repos, bool impo finish_sack_loader(); throw; } - base->get_logger()->warning( + p_impl->base->get_logger()->warning( "Error loading repo \"{}\" (skipping due to \"skip_if_unavailable=true\"):", repo->get_id()); const auto & error_lines = utils::string::split(format(e, FormatDetailLevel::Plain), "\n"); for (const auto & line : error_lines) { if (!line.empty()) { - base->get_logger()->warning(' ' + line); + p_impl->base->get_logger()->warning(' ' + line); } } return false; @@ -355,7 +373,7 @@ void RepoSack::update_and_load_repos(libdnf5::repo::RepoQuery & repos, bool impo if (!remote_keys_files.empty()) { // download remote keys files to local temporary files - FileDownloader downloader(base); + FileDownloader downloader(p_impl->base); downloader.set_fail_fast(false); downloader.set_resume(false); @@ -439,7 +457,8 @@ void RepoSack::update_and_load_repos(libdnf5::repo::RepoQuery & repos, bool impo // the expired metadata still reflect the origin utimes( repo->get_downloader().get_metadata_path(RepoDownloader::MD_FILENAME_PRIMARY).c_str(), nullptr); - RepoCache(base, repo->get_config().get_cachedir()).remove_attribute(RepoCache::ATTRIBUTE_EXPIRED); + RepoCache(p_impl->base, repo->get_config().get_cachedir()) + .remove_attribute(RepoCache::ATTRIBUTE_EXPIRED); repo->mark_fresh(); repos_for_processing.erase(repos_for_processing.begin() + static_cast(idx)); @@ -471,7 +490,7 @@ void RepoSack::update_and_load_repos(libdnf5::repo::RepoQuery & repos, bool impo logger->debug("Downloading metadata for repo \"{}\"", repo->get_config().get_id()); auto cache_dir = repo->get_config().get_cachedir(); repo->download_metadata(cache_dir); - RepoCache(base, cache_dir).remove_attribute(RepoCache::ATTRIBUTE_EXPIRED); + RepoCache(p_impl->base, cache_dir).remove_attribute(RepoCache::ATTRIBUTE_EXPIRED); repo->mark_fresh(); repo->read_metadata_cache(); @@ -495,19 +514,20 @@ void RepoSack::update_and_load_repos(libdnf5::repo::RepoQuery & repos, bool impo fix_group_missing_xml(); - base->get_rpm_package_sack()->load_config_excludes_includes(); + p_impl->base->get_rpm_package_sack()->load_config_excludes_includes(); } void RepoSack::update_and_load_enabled_repos(bool load_system) { - libdnf_user_assert(!repos_updated_and_loaded, "RepoSack::updated_and_load_enabled_repos has already been called."); + libdnf_user_assert( + !p_impl->repos_updated_and_loaded, "RepoSack::updated_and_load_enabled_repos has already been called."); if (load_system) { // create the system repository if it does not exist - base->get_repo_sack()->get_system_repo(); + p_impl->base->get_repo_sack()->get_system_repo(); } - libdnf5::repo::RepoQuery repos(base); + libdnf5::repo::RepoQuery repos(p_impl->base); repos.filter_enabled(true); if (!load_system) { @@ -517,26 +537,26 @@ void RepoSack::update_and_load_enabled_repos(bool load_system) { update_and_load_repos(repos); // TODO(jmracek) Replace by call that will resolve active modules and apply modular filtering - base->get_module_sack()->p_impl->module_filtering(); + p_impl->base->get_module_sack()->p_impl->module_filtering(); - repos_updated_and_loaded = true; + p_impl->repos_updated_and_loaded = true; } void RepoSack::dump_debugdata(const std::string & dir) { - libdnf5::solv::Solver solver{get_rpm_pool(base)}; + libdnf5::solv::Solver solver{get_rpm_pool(p_impl->base)}; solver.write_debugdata(dir, false); } void RepoSack::dump_comps_debugdata(const std::string & dir) { - libdnf5::solv::Solver solver{get_comps_pool(base)}; + libdnf5::solv::Solver solver{get_comps_pool(p_impl->base)}; solver.write_debugdata(dir, false); } void RepoSack::create_repos_from_file(const std::string & path) { - auto & logger = *base->get_logger(); + auto & logger = *p_impl->base->get_logger(); ConfigParser parser; parser.read(path); const auto & cfg_parser_data = parser.get_data(); @@ -545,7 +565,7 @@ void RepoSack::create_repos_from_file(const std::string & path) { if (section == "main") { continue; } - auto repo_id = base->get_vars()->substitute(section); + auto repo_id = p_impl->base->get_vars()->substitute(section); logger.debug("Creating repo \"{}\" from config file \"{}\" section \"{}\"", repo_id, path, section); @@ -558,7 +578,7 @@ void RepoSack::create_repos_from_file(const std::string & path) { } repo->set_repo_file_path(path); auto & repo_cfg = repo->get_config(); - repo_cfg.load_from_parser(parser, section, *base->get_vars(), *base->get_logger()); + repo_cfg.load_from_parser(parser, section, *p_impl->base->get_vars(), *p_impl->base->get_logger()); if (repo_cfg.get_name_option().get_priority() == Option::Priority::DEFAULT) { logger.debug("Repo \"{}\" is missing name in configuration file \"{}\", using id.", repo_id, path); @@ -568,7 +588,7 @@ void RepoSack::create_repos_from_file(const std::string & path) { } void RepoSack::create_repos_from_config_file() { - base->with_config_file_path( + p_impl->base->with_config_file_path( std::function{[this](const std::string & path) { create_repos_from_file(path); }}); } @@ -576,7 +596,8 @@ void RepoSack::create_repos_from_dir(const std::string & dir_path) { std::error_code ec; std::filesystem::directory_iterator di(dir_path, ec); if (ec) { - base->get_logger()->warning("Cannot read repositories from directory \"{}\": {}", dir_path, ec.message()); + p_impl->base->get_logger()->warning( + "Cannot read repositories from directory \"{}\": {}", dir_path, ec.message()); return; } std::vector paths; @@ -593,7 +614,7 @@ void RepoSack::create_repos_from_dir(const std::string & dir_path) { } void RepoSack::create_repos_from_reposdir() { - for (auto & dir : base->get_config().get_reposdir_option().get_value()) { + for (auto & dir : p_impl->base->get_config().get_reposdir_option().get_value()) { if (std::filesystem::exists(dir)) { create_repos_from_dir(dir); } @@ -617,12 +638,12 @@ void RepoSack::create_repos_from_system_configuration() { } void RepoSack::load_repos_configuration_overrides() { - auto loger = base->get_logger(); + auto loger = p_impl->base->get_logger(); fs::path repos_override_dir_path{REPOS_OVERRIDE_DIR}; fs::path repos_distrib_override_dir_path{LIBDNF5_REPOS_DISTRIBUTION_OVERRIDE_DIR}; - const auto & config = base->get_config(); + const auto & config = p_impl->base->get_config(); const bool use_installroot_config{!config.get_use_host_config_option().get_value()}; if (use_installroot_config) { const fs::path installroot_path{config.get_installroot_option().get_value()}; @@ -639,26 +660,27 @@ void RepoSack::load_repos_configuration_overrides() { const auto & cfg_parser_data = parser.get_data(); for (const auto & cfg_parser_data_iter : cfg_parser_data) { const auto & section = cfg_parser_data_iter.first; - const auto repo_id_pattern = base->get_vars()->substitute(section); + const auto repo_id_pattern = p_impl->base->get_vars()->substitute(section); - RepoQuery repo_query(base); + RepoQuery repo_query(p_impl->base); repo_query.filter_id(repo_id_pattern, sack::QueryCmp::GLOB); for (auto & repo : repo_query) { - repo->get_config().load_from_parser(parser, section, *base->get_vars(), *base->get_logger()); + repo->get_config().load_from_parser( + parser, section, *p_impl->base->get_vars(), *p_impl->base->get_logger()); } } } } BaseWeakPtr RepoSack::get_base() const { - return base; + return p_impl->base; } void RepoSack::enable_source_repos() { - RepoQuery enabled_repos(base); + RepoQuery enabled_repos(p_impl->base); enabled_repos.filter_enabled(true); for (const auto & repo : enabled_repos) { - RepoQuery source_query(base); + RepoQuery source_query(p_impl->base); // There is no way how to find source (or debuginfo) repository for // given repo. This is only guessing according to the current practice: auto repoid = repo->get_id(); @@ -677,30 +699,30 @@ void RepoSack::enable_source_repos() { } void RepoSack::internalize_repos() { - auto rq = RepoQuery(base); + auto rq = RepoQuery(p_impl->base); for (auto & repo : rq.get_data()) { repo->internalize(); } - if (system_repo) { - system_repo->internalize(); + if (p_impl->system_repo) { + p_impl->system_repo->internalize(); } - if (cmdline_repo) { - cmdline_repo->internalize(); + if (p_impl->cmdline_repo) { + p_impl->cmdline_repo->internalize(); } } void RepoSack::fix_group_missing_xml() { if (has_system_repo()) { - auto & solv_repo = system_repo->get_solv_repo(); + auto & solv_repo = p_impl->system_repo->get_solv_repo(); auto & group_missing_xml = solv_repo.get_groups_missing_xml(); auto & environments_missing_xml = solv_repo.get_environments_missing_xml(); if (group_missing_xml.empty() && environments_missing_xml.empty()) { return; } - auto & logger = *base->get_logger(); - auto & system_state = base->p_impl->get_system_state(); + auto & logger = *p_impl->base->get_logger(); + auto & system_state = p_impl->base->p_impl->get_system_state(); auto comps_xml_dir = system_state.get_group_xml_dir(); bool directory_exists = true; std::error_code ec; @@ -710,7 +732,7 @@ void RepoSack::fix_group_missing_xml() { directory_exists = false; } if (!group_missing_xml.empty()) { - libdnf5::comps::GroupQuery available_groups(base); + libdnf5::comps::GroupQuery available_groups(p_impl->base); available_groups.filter_installed(false); for (const auto & group_id : group_missing_xml) { bool xml_saved = false; @@ -746,7 +768,7 @@ void RepoSack::fix_group_missing_xml() { } } if (!environments_missing_xml.empty()) { - libdnf5::comps::EnvironmentQuery available_environments(base); + libdnf5::comps::EnvironmentQuery available_environments(p_impl->base); available_environments.filter_installed(false); for (const auto & environment_id : environments_missing_xml) { bool xml_saved = false; @@ -786,4 +808,16 @@ void RepoSack::fix_group_missing_xml() { } } +bool RepoSack::has_system_repo() const noexcept { + return p_impl->system_repo != nullptr; +} + +bool RepoSack::has_cmdline_repo() const noexcept { + return p_impl->cmdline_repo != nullptr; +} + +RepoSackWeakPtr RepoSack::get_weak_ptr() { + return {this, &p_impl->sack_guard}; +} + } // namespace libdnf5::repo From 7aecff64d41374223c9e4ec00729a3d89bc68f30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Thu, 18 Jan 2024 08:48:14 +0100 Subject: [PATCH 6/8] Add pImpl to `libdnf5::rpm::KeyInfo` In order to allow access to `user_ids` add utility method: `add_user_id()` as protected so that it can be used from `Key`. --- include/libdnf5/rpm/rpm_signature.hpp | 38 ++++++----- libdnf5/repo/repo_pgp.cpp | 3 +- libdnf5/rpm/rpm_signature.cpp | 93 ++++++++++++++++++++++++--- 3 files changed, 107 insertions(+), 27 deletions(-) diff --git a/include/libdnf5/rpm/rpm_signature.hpp b/include/libdnf5/rpm/rpm_signature.hpp index ca4b5eb94..15e1de8d5 100644 --- a/include/libdnf5/rpm/rpm_signature.hpp +++ b/include/libdnf5/rpm/rpm_signature.hpp @@ -47,17 +47,15 @@ using RpmKeyPktPtr = std::unique_ptr class KeyInfo { public: - const std::string & get_key_id() const noexcept { return key_id; } + const std::string & get_key_id() const noexcept; std::string get_short_key_id() const; - const std::vector & get_user_ids() const noexcept { return user_ids; } - const std::string & get_fingerprint() const noexcept { return fingerprint; } - const std::string & get_url() const noexcept { return key_url; } - const std::string & get_path() const noexcept { return key_path; } - const std::string & get_raw_key() const noexcept { return raw_key; } - const long int & get_timestamp() const noexcept { return timestamp; } + const std::vector & get_user_ids() const noexcept; + const std::string & get_fingerprint() const noexcept; + const std::string & get_url() const noexcept; + const std::string & get_path() const noexcept; + const std::string & get_raw_key() const noexcept; + const long int & get_timestamp() const noexcept; -protected: - friend class RpmSignature; KeyInfo( const std::string & key_url, const std::string & key_path, @@ -66,13 +64,21 @@ class KeyInfo { const std::string & fingerprint, long int timestamp, const std::string & raw_key); - std::string key_url; - std::string key_path; - std::string key_id; - std::vector user_ids; - std::string fingerprint; - long int timestamp; - std::string raw_key; + + ~KeyInfo(); + + KeyInfo(const KeyInfo & src); + KeyInfo & operator=(const KeyInfo & src); + + KeyInfo(KeyInfo && src) noexcept; + KeyInfo & operator=(KeyInfo && src) noexcept; + +protected: + void add_user_id(const char * user_id); + +private: + class Impl; + std::unique_ptr p_impl; }; class RpmSignature { diff --git a/libdnf5/repo/repo_pgp.cpp b/libdnf5/repo/repo_pgp.cpp index fc0c8bcaf..702d47fb7 100644 --- a/libdnf5/repo/repo_pgp.cpp +++ b/libdnf5/repo/repo_pgp.cpp @@ -20,7 +20,6 @@ along with libdnf. If not, see . #include "repo_pgp.hpp" #include "libdnf5/base/base.hpp" -#include "libdnf5/logger/logger.hpp" #include "libdnf5/repo/repo_errors.hpp" #include "libdnf5/utils/bgettext/bgettext-mark-domain.h" #include "libdnf5/utils/fs/temp.hpp" @@ -39,7 +38,7 @@ Key::Key(const LrGpgKey * key, const LrGpgSubkey * subkey, const std::string & u lr_gpg_subkey_get_timestamp(subkey), lr_gpg_key_get_raw_key(key)) { for (auto * const * item = lr_gpg_key_get_userids(key); *item; ++item) { - user_ids.push_back(*item); + add_user_id(*item); } } diff --git a/libdnf5/rpm/rpm_signature.cpp b/libdnf5/rpm/rpm_signature.cpp index cb3efa572..cf1d598c8 100644 --- a/libdnf5/rpm/rpm_signature.cpp +++ b/libdnf5/rpm/rpm_signature.cpp @@ -82,6 +82,43 @@ static bool rpmdb_lookup(const RpmTransactionPtr & ts_ptr, const KeyInfo & key) return retval; } +class KeyInfo::Impl { +public: + Impl( + std::string key_url, + std::string key_path, + std::string key_id, + std::vector user_ids, + std::string fingerprint, + long int timestamp, + std::string raw_key); + +private: + friend KeyInfo; + std::string key_url; + std::string key_path; + std::string key_id; + std::vector user_ids; + std::string fingerprint; + long int timestamp; + std::string raw_key; +}; + +KeyInfo::Impl::Impl( + std::string key_url, + std::string key_path, + std::string key_id, + std::vector user_ids, + std::string fingerprint, + long int timestamp, + std::string raw_key) + : key_url(std::move(key_url)), + key_path(std::move(key_path)), + key_id(std::move(key_id)), + user_ids(std::move(user_ids)), + fingerprint(std::move(fingerprint)), + timestamp(timestamp), + raw_key(std::move(raw_key)) {} KeyInfo::KeyInfo( const std::string & key_url, @@ -91,16 +128,28 @@ KeyInfo::KeyInfo( const std::string & fingerprint, long int timestamp, const std::string & raw_key) - : key_url(key_url), - key_path(key_path), - key_id(key_id), - user_ids(user_ids), - fingerprint(fingerprint), - timestamp(timestamp), - raw_key(raw_key) {} + : p_impl(std::make_unique(key_url, key_path, key_id, user_ids, fingerprint, timestamp, raw_key)) {} + +KeyInfo::~KeyInfo() = default; + +KeyInfo::KeyInfo(const KeyInfo & src) : p_impl(new Impl(*src.p_impl)) {} +KeyInfo::KeyInfo(KeyInfo && src) noexcept = default; + +KeyInfo & KeyInfo::operator=(const KeyInfo & src) { + if (this != &src) { + if (p_impl) { + *p_impl = *src.p_impl; + } else { + p_impl = std::make_unique(*src.p_impl); + } + } + + return *this; +} +KeyInfo & KeyInfo::operator=(KeyInfo && src) noexcept = default; std::string KeyInfo::get_short_key_id() const { - auto short_key_id = key_id.size() > 8 ? key_id.substr(key_id.size() - 8) : key_id; + auto short_key_id = p_impl->key_id.size() > 8 ? p_impl->key_id.substr(p_impl->key_id.size() - 8) : p_impl->key_id; return short_key_id; } @@ -199,7 +248,7 @@ bool RpmSignature::import_key(const KeyInfo & key) const { if (!rpmdb_lookup(ts_ptr, key)) { uint8_t * pkt = nullptr; size_t pkt_len{0}; - if (pgpParsePkts(key.raw_key.c_str(), &pkt, &pkt_len) != PGPARMOR_PUBKEY) { + if (pgpParsePkts(key.get_raw_key().c_str(), &pkt, &pkt_len) != PGPARMOR_PUBKEY) { free(pkt); throw KeyImportError(M_("\"{}\": key is not an armored public key."), key.get_url()); } @@ -254,4 +303,30 @@ std::string RpmSignature::check_result_to_string(CheckResult result) { return {}; } +void KeyInfo::add_user_id(const char * user_id) { + p_impl->user_ids.emplace_back(user_id); +} + +const std::string & KeyInfo::get_key_id() const noexcept { + return p_impl->key_id; +} +const std::vector & KeyInfo::get_user_ids() const noexcept { + return p_impl->user_ids; +} +const std::string & KeyInfo::get_fingerprint() const noexcept { + return p_impl->fingerprint; +} +const std::string & KeyInfo::get_url() const noexcept { + return p_impl->key_url; +} +const std::string & KeyInfo::get_path() const noexcept { + return p_impl->key_path; +} +const std::string & KeyInfo::get_raw_key() const noexcept { + return p_impl->raw_key; +} +const long int & KeyInfo::get_timestamp() const noexcept { + return p_impl->timestamp; +} + } // namespace libdnf5::rpm From 7916725b3250b39d69b8db1c3367b140d30fb881 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Thu, 18 Jan 2024 12:48:30 +0100 Subject: [PATCH 7/8] Add pImpl to `libdnf5::transaction::Transaction` --- include/libdnf5/transaction/transaction.hpp | 71 ++++---- libdnf5/transaction/transaction.cpp | 170 ++++++++++++++++---- 2 files changed, 172 insertions(+), 69 deletions(-) diff --git a/include/libdnf5/transaction/transaction.hpp b/include/libdnf5/transaction/transaction.hpp index 78e24a6a6..b6b89ead7 100644 --- a/include/libdnf5/transaction/transaction.hpp +++ b/include/libdnf5/transaction/transaction.hpp @@ -80,7 +80,14 @@ class Transaction { public: using State = TransactionState; - virtual ~Transaction() = default; + virtual ~Transaction(); + + Transaction(const Transaction & src); + Transaction & operator=(const Transaction & src); + + Transaction(Transaction && src) noexcept; + Transaction & operator=(Transaction && src) noexcept; + bool operator==(const Transaction & other) const; bool operator<(const Transaction & other) const; @@ -90,52 +97,52 @@ class Transaction { /// Return 0 if the id wasn't set yet /// // @replaces libdnf:transaction/Transaction.hpp:method:Transaction.getId() - int64_t get_id() const noexcept { return id; } + int64_t get_id() const noexcept; /// Get date and time of the transaction start /// // @replaces libdnf:transaction/Transaction.hpp:method:Transaction.get_dt_begin() - int64_t get_dt_start() const noexcept { return dt_begin; } + int64_t get_dt_start() const noexcept; /// Get date and time of the transaction end /// // @replaces libdnf:transaction/Transaction.hpp:method:Transaction.get_dt_begin() - int64_t get_dt_end() const noexcept { return dt_end; } + int64_t get_dt_end() const noexcept; /// Get RPM database version before the transaction /// Format: ``:`` /// // @replaces libdnf:transaction/Transaction.hpp:method:Transaction.get_rpmdb_version_begin() - const std::string & get_rpmdb_version_begin() const noexcept { return rpmdb_version_begin; } + const std::string & get_rpmdb_version_begin() const noexcept; /// Get RPM database version after the transaction /// Format: ``:`` /// // @replaces libdnf:transaction/Transaction.hpp:method:Transaction.get_rpmdb_version_end() - const std::string & get_rpmdb_version_end() const noexcept { return rpmdb_version_end; } + const std::string & get_rpmdb_version_end() const noexcept; /// Get $releasever variable value that was used during the transaction /// // @replaces libdnf:transaction/Transaction.hpp:method:Transaction.get_releasever() - const std::string & get_releasever() const noexcept { return releasever; } + const std::string & get_releasever() const noexcept; /// Get UID of a user that started the transaction /// // @replaces libdnf:transaction/Transaction.hpp:method:Transaction.get_user_id() - uint32_t get_user_id() const noexcept { return user_id; } + uint32_t get_user_id() const noexcept; /// Get the description of the transaction (e.g. the CLI command that was executed) /// // @replaces libdnf:transaction/Transaction.hpp:method:Transaction.get_cmdline() - const std::string & get_description() const noexcept { return description; } + const std::string & get_description() const noexcept; /// Get a user-specified comment describing the transaction - const std::string & get_comment() const noexcept { return comment; } + const std::string & get_comment() const noexcept; /// Get transaction state /// // @replaces libdnf:transaction/Transaction.hpp:method:Transaction.getState() - State get_state() const noexcept { return state; } + State get_state() const noexcept; /// Return all comps environments associated with the transaction /// @@ -182,52 +189,52 @@ class Transaction { /// Set Transaction database id (primary key) /// // @replaces libdnf:transaction/private/Transaction.hpp:method:Transaction.setId(int64_t value) - void set_id(int64_t value) { id = value; } + void set_id(int64_t value); /// Set a user-specified comment describing the transaction - void set_comment(const std::string & value) { comment = value; } + void set_comment(const std::string & value); /// Set date and time of the transaction start /// // @replaces libdnf:transaction/private/Transaction.hpp:method:Transaction.setDtBegin(int64_t value) - void set_dt_start(int64_t value) { dt_begin = value; } + void set_dt_start(int64_t value); /// Set date and time of the transaction end /// // @replaces libdnf:transaction/private/Transaction.hpp:method:Transaction.setDtEnd(int64_t value) - void set_dt_end(int64_t value) { dt_end = value; } + void set_dt_end(int64_t value); /// Set the description of the transaction (e.g. the CLI command that was executed) /// // @replaces libdnf:transaction/private/Transaction.hpp:method:Transaction.setCmdline(const std::string & value) - void set_description(const std::string & value) { description = value; } + void set_description(const std::string & value); /// Set UID of a user that started the transaction /// // @replaces libdnf:transaction/private/Transaction.hpp:method:Transaction.setUserId(uint32_t value) - void set_user_id(uint32_t value) { user_id = value; } + void set_user_id(uint32_t value); /// Set $releasever variable value that was used during the transaction /// // @replaces libdnf:transaction/private/Transaction.hpp:method:Transaction.setReleasever(const std::string & value) - void set_releasever(const std::string & value) { releasever = value; } + void set_releasever(const std::string & value); /// Set RPM database version after the transaction /// Format: ``:`` /// // @replaces libdnf:transaction/private/Transaction.hpp:method:Transaction.setRpmdbVersionEnd(const std::string & value) - void set_rpmdb_version_end(const std::string & value) { rpmdb_version_end = value; } + void set_rpmdb_version_end(const std::string & value); /// Set RPM database version before the transaction /// Format: ``:`` /// // @replaces libdnf:transaction/private/Transaction.hpp:method:Transaction.setRpmdbVersionBegin(const std::string & value) - void set_rpmdb_version_begin(const std::string & value) { rpmdb_version_begin = value; } + void set_rpmdb_version_begin(const std::string & value); /// Set transaction state /// // @replaces libdnf:transaction/private/Transaction.hpp:method:Transaction.setState(libdnf::TransactionState value) - void set_state(State value) { state = value; } + void set_state(State value); /// Create a new rpm package in the transaction and return a reference to it. /// The package is owned by the transaction. @@ -274,26 +281,8 @@ class Transaction { // @replaces libdnf:transaction/private/Transaction.hpp:method:Transaction.finish(libdnf::TransactionState state) void finish(TransactionState state); - int64_t id{0}; - - int64_t dt_begin = 0; - int64_t dt_end = 0; - std::string rpmdb_version_begin; - std::string rpmdb_version_end; - // TODO(dmach): move to a new "vars" table? - std::string releasever; - uint32_t user_id = 0; - std::string description; - std::string comment; - State state = State::STARTED; - - std::optional>> console_output; - - std::optional> comps_environments; - std::optional> comps_groups; - std::optional> packages; - - BaseWeakPtr base; + class Impl; + std::unique_ptr p_impl; }; } // namespace libdnf5::transaction diff --git a/libdnf5/transaction/transaction.cpp b/libdnf5/transaction/transaction.cpp index 7d24b529a..3650328be 100644 --- a/libdnf5/transaction/transaction.cpp +++ b/libdnf5/transaction/transaction.cpp @@ -25,7 +25,6 @@ along with libdnf. If not, see . #include "db/db.hpp" #include "db/rpm.hpp" #include "db/trans.hpp" -#include "db/trans_item.hpp" #include "transaction/transaction_sr.hpp" #include "libdnf5/transaction/comps_environment.hpp" @@ -36,6 +35,37 @@ along with libdnf. If not, see . namespace libdnf5::transaction { +class Transaction::Impl { +public: + Impl(const BaseWeakPtr & base, int64_t id = 0); + +private: + friend Transaction; + + int64_t id{0}; + + int64_t dt_begin = 0; + int64_t dt_end = 0; + std::string rpmdb_version_begin; + std::string rpmdb_version_end; + // TODO(dmach): move to a new "vars" table? + std::string releasever; + uint32_t user_id = 0; + std::string description; + std::string comment; + TransactionState state = State::STARTED; + + std::optional>> console_output; + + std::optional> comps_environments; + std::optional> comps_groups; + std::optional> packages; + + BaseWeakPtr base; +}; + +Transaction::Impl::Impl(const BaseWeakPtr & base, int64_t id) : id(id), base(base) {} + std::string transaction_state_to_string(TransactionState state) { switch (state) { case TransactionState::STARTED: @@ -66,11 +96,28 @@ InvalidTransactionState::InvalidTransactionState(const std::string & state) : libdnf5::Error(M_("Invalid transaction state: {}"), state) {} -Transaction::Transaction(const BaseWeakPtr & base, int64_t id) : id(id), base(base) {} +Transaction::Transaction(const BaseWeakPtr & base, int64_t id) : p_impl(std::make_unique(base, id)) {} -Transaction::Transaction(const BaseWeakPtr & base) : base{base} {} +Transaction::Transaction(const BaseWeakPtr & base) : p_impl(std::make_unique(base)) {} +Transaction::~Transaction() = default; + +Transaction::Transaction(const Transaction & src) : p_impl(new Impl(*src.p_impl)) {} +Transaction::Transaction(Transaction && src) noexcept = default; + +Transaction & Transaction::operator=(const Transaction & src) { + if (this != &src) { + if (p_impl) { + *p_impl = *src.p_impl; + } else { + p_impl = std::make_unique(*src.p_impl); + } + } + + return *this; +} +Transaction & Transaction::operator=(Transaction && src) noexcept = default; bool Transaction::operator==(const Transaction & other) const { return get_id() == other.get_id(); @@ -88,63 +135,64 @@ bool Transaction::operator>(const Transaction & other) const { std::vector & Transaction::get_comps_environments() { - if (comps_environments) { - return *comps_environments; + if (p_impl->comps_environments) { + return *p_impl->comps_environments; } - comps_environments = - CompsEnvironmentDbUtils::get_transaction_comps_environments(*transaction_db_connect(*base), *this); - return *comps_environments; + p_impl->comps_environments = + CompsEnvironmentDbUtils::get_transaction_comps_environments(*transaction_db_connect(*p_impl->base), *this); + return *p_impl->comps_environments; } CompsEnvironment & Transaction::new_comps_environment() { - if (!comps_environments) { - comps_environments.emplace(); + if (!p_impl->comps_environments) { + p_impl->comps_environments.emplace(); } CompsEnvironment comps_env(*this); - return comps_environments->emplace_back(std::move(comps_env)); + return p_impl->comps_environments->emplace_back(std::move(comps_env)); } std::vector & Transaction::get_comps_groups() { - if (comps_groups) { - return *comps_groups; + if (p_impl->comps_groups) { + return *p_impl->comps_groups; } - comps_groups = CompsGroupDbUtils::get_transaction_comps_groups(*transaction_db_connect(*base), *this); - return *comps_groups; + p_impl->comps_groups = + CompsGroupDbUtils::get_transaction_comps_groups(*transaction_db_connect(*p_impl->base), *this); + return *p_impl->comps_groups; } CompsGroup & Transaction::new_comps_group() { - if (!comps_groups) { - comps_groups.emplace(); + if (!p_impl->comps_groups) { + p_impl->comps_groups.emplace(); } CompsGroup comps_group(*this); - return comps_groups->emplace_back(comps_group); + return p_impl->comps_groups->emplace_back(comps_group); } std::vector & Transaction::get_packages() { - if (packages) { - return *packages; + if (p_impl->packages) { + return *p_impl->packages; } - packages = RpmDbUtils::get_transaction_packages(*transaction_db_connect(*base), *this); - return *packages; + p_impl->packages = RpmDbUtils::get_transaction_packages(*transaction_db_connect(*p_impl->base), *this); + return *p_impl->packages; } Package & Transaction::new_package() { - if (!packages) { - packages.emplace(); + if (!p_impl->packages) { + p_impl->packages.emplace(); } Package const pkg(*this); - return packages->emplace_back(pkg); + return p_impl->packages->emplace_back(pkg); } @@ -230,11 +278,11 @@ void Transaction::fill_transaction_groups( } void Transaction::start() { - if (id != 0) { + if (p_impl->id != 0) { throw RuntimeError(M_("Transaction has already started!")); } - auto conn = transaction_db_connect(*base); + auto conn = transaction_db_connect(*p_impl->base); conn->exec("BEGIN"); try { auto query = TransactionDbUtils::trans_insert_new_query(*conn); @@ -267,7 +315,7 @@ void Transaction::finish(TransactionState state) { } */ - auto conn = transaction_db_connect(*base); + auto conn = transaction_db_connect(*p_impl->base); conn->exec("BEGIN"); try { set_state(state); @@ -321,4 +369,70 @@ std::string Transaction::serialize() { return json_serialize(transaction_replay); } + +// Getters +int64_t Transaction::get_id() const noexcept { + return p_impl->id; +} +int64_t Transaction::get_dt_start() const noexcept { + return p_impl->dt_begin; +} +int64_t Transaction::get_dt_end() const noexcept { + return p_impl->dt_end; +} +const std::string & Transaction::get_rpmdb_version_begin() const noexcept { + return p_impl->rpmdb_version_begin; +} +const std::string & Transaction::get_rpmdb_version_end() const noexcept { + return p_impl->rpmdb_version_end; +} +const std::string & Transaction::get_releasever() const noexcept { + return p_impl->releasever; +} +uint32_t Transaction::get_user_id() const noexcept { + return p_impl->user_id; +} +const std::string & Transaction::get_description() const noexcept { + return p_impl->description; +} +const std::string & Transaction::get_comment() const noexcept { + return p_impl->comment; +} +TransactionState Transaction::get_state() const noexcept { + return p_impl->state; +} + + +// Setters +void Transaction::set_id(int64_t value) { + p_impl->id = value; +} +void Transaction::set_comment(const std::string & value) { + p_impl->comment = value; +} +void Transaction::set_dt_start(int64_t value) { + p_impl->dt_begin = value; +} +void Transaction::set_dt_end(int64_t value) { + p_impl->dt_end = value; +} +void Transaction::set_description(const std::string & value) { + p_impl->description = value; +} +void Transaction::set_user_id(uint32_t value) { + p_impl->user_id = value; +} +void Transaction::set_releasever(const std::string & value) { + p_impl->releasever = value; +} +void Transaction::set_rpmdb_version_end(const std::string & value) { + p_impl->rpmdb_version_end = value; +} +void Transaction::set_rpmdb_version_begin(const std::string & value) { + p_impl->rpmdb_version_begin = value; +} +void Transaction::set_state(State value) { + p_impl->state = value; +} + } // namespace libdnf5::transaction From 8f3c9c24de0fc83cb1fb79694365f05185776ca3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Fri, 16 Feb 2024 14:13:27 +0100 Subject: [PATCH 8/8] Add pImpl to `libdnf5::rpm::RpmSignature` --- include/libdnf5/rpm/rpm_signature.hpp | 16 +++++++--- libdnf5/rpm/rpm_signature.cpp | 44 ++++++++++++++++++++++----- 2 files changed, 48 insertions(+), 12 deletions(-) diff --git a/include/libdnf5/rpm/rpm_signature.hpp b/include/libdnf5/rpm/rpm_signature.hpp index 15e1de8d5..9fa83987e 100644 --- a/include/libdnf5/rpm/rpm_signature.hpp +++ b/include/libdnf5/rpm/rpm_signature.hpp @@ -85,9 +85,14 @@ class RpmSignature { public: enum class CheckResult { OK, SKIPPED, FAILED_KEY_MISSING, FAILED_NOT_TRUSTED, FAILED_NOT_SIGNED, FAILED }; - explicit RpmSignature(const libdnf5::BaseWeakPtr & base) : base(base) {} - explicit RpmSignature(Base & base) : RpmSignature(base.get_weak_ptr()) {} - ~RpmSignature(){}; + explicit RpmSignature(const libdnf5::BaseWeakPtr & base); + explicit RpmSignature(Base & base); + ~RpmSignature(); + RpmSignature(const RpmSignature & src); + RpmSignature & operator=(const RpmSignature & src); + + RpmSignature(RpmSignature && src) noexcept; + RpmSignature & operator=(RpmSignature && src) noexcept; /// Check signature of the `package` using public keys stored in rpm database. /// @param package: package to check. @@ -97,7 +102,7 @@ class RpmSignature { /// CheckResult::FAILED_NOT_TRUSTED - signature is valid but the key is not trusted /// CheckResult::FAILED_NOT_SIGNED - package is not signed but signature is required /// CheckResult::FAILED - check failed for another reason - CheckResult check_package_signature(Package package) const; + CheckResult check_package_signature(const Package & pkg) const; /// Import public key into rpm database. /// @param key: GPG key to be imported into rpm database. @@ -114,7 +119,8 @@ class RpmSignature { static std::string check_result_to_string(CheckResult result); private: - BaseWeakPtr base; + class Impl; + std::unique_ptr p_impl; }; } // namespace libdnf5::rpm diff --git a/libdnf5/rpm/rpm_signature.cpp b/libdnf5/rpm/rpm_signature.cpp index cf1d598c8..cd0b4afc7 100644 --- a/libdnf5/rpm/rpm_signature.cpp +++ b/libdnf5/rpm/rpm_signature.cpp @@ -153,11 +153,41 @@ std::string KeyInfo::get_short_key_id() const { return short_key_id; } -RpmSignature::CheckResult RpmSignature::check_package_signature(rpm::Package pkg) const { +class RpmSignature::Impl { +public: + explicit Impl(const libdnf5::BaseWeakPtr & base) : base(base) {} + +private: + friend RpmSignature; + BaseWeakPtr base; +}; + +RpmSignature::RpmSignature(const libdnf5::BaseWeakPtr & base) : p_impl(std::make_unique(base)) {} +RpmSignature::RpmSignature(Base & base) : RpmSignature(base.get_weak_ptr()) {} +RpmSignature::~RpmSignature() = default; + +RpmSignature::RpmSignature(const RpmSignature & src) : p_impl(new Impl(*src.p_impl)) {} +RpmSignature::RpmSignature(RpmSignature && src) noexcept = default; + +RpmSignature & RpmSignature::operator=(const RpmSignature & src) { + if (this != &src) { + if (p_impl) { + *p_impl = *src.p_impl; + } else { + p_impl = std::make_unique(*src.p_impl); + } + } + + return *this; +} +RpmSignature & RpmSignature::operator=(RpmSignature && src) noexcept = default; + + +RpmSignature::CheckResult RpmSignature::check_package_signature(const rpm::Package & pkg) const { // is package gpg check even required? auto repo = pkg.get_repo(); if (repo->get_type() == libdnf5::repo::Repo::Type::COMMANDLINE) { - if (!base->get_config().get_localpkg_gpgcheck_option().get_value()) { + if (!p_impl->base->get_config().get_localpkg_gpgcheck_option().get_value()) { return CheckResult::SKIPPED; } } else { @@ -178,7 +208,7 @@ RpmSignature::CheckResult RpmSignature::check_package_signature(rpm::Package pkg // the vector of strings. libdnf5::rpm::RpmLogGuardStrings rpm_log_guard; - auto ts_ptr = create_transaction(base); + auto ts_ptr = create_transaction(p_impl->base); auto oldmask = rpmlogSetMask(RPMLOG_UPTO(RPMLOG_PRI(RPMLOG_INFO))); rpmtsSetVfyLevel(ts_ptr.get(), RPMSIG_SIGNATURE_TYPE); @@ -236,15 +266,15 @@ RpmSignature::CheckResult RpmSignature::check_package_signature(rpm::Package pkg } bool RpmSignature::key_present(const KeyInfo & key) const { - libdnf5::rpm::RpmLogGuard rpm_log_guard{base}; - auto ts_ptr = create_transaction(base); + libdnf5::rpm::RpmLogGuard rpm_log_guard{p_impl->base}; + auto ts_ptr = create_transaction(p_impl->base); return rpmdb_lookup(ts_ptr, key); } bool RpmSignature::import_key(const KeyInfo & key) const { libdnf5::rpm::RpmLogGuardStrings rpm_log_guard; - auto ts_ptr = create_transaction(base); + auto ts_ptr = create_transaction(p_impl->base); if (!rpmdb_lookup(ts_ptr, key)) { uint8_t * pkt = nullptr; size_t pkt_len{0}; @@ -271,7 +301,7 @@ std::vector RpmSignature::parse_key_file(const std::string & key_url) { } else { // download the remote key downloaded_key = std::make_unique("rpmkey"); - libdnf5::repo::FileDownloader downloader(base); + libdnf5::repo::FileDownloader downloader(p_impl->base); downloader.add(key_url, downloaded_key->get_path()); downloader.download(); key_path = downloaded_key->get_path();