From f20c6f940d8d5dbb656668b1462cbe25d0c09cea Mon Sep 17 00:00:00 2001 From: Marek Blaha Date: Wed, 5 Jun 2024 15:31:12 +0200 Subject: [PATCH] offline: Add pImpl for OfflineState classes This change would allow us to do modification to these classes without breaking ABI. --- dnf5/commands/offline/offline.cpp | 50 +++---- dnf5/context.cpp | 21 +-- include/libdnf5/transaction/offline.hpp | 103 +++++++++----- libdnf5/transaction/offline.cpp | 173 ++++++++++++++++++++++-- 4 files changed, 263 insertions(+), 84 deletions(-) diff --git a/dnf5/commands/offline/offline.cpp b/dnf5/commands/offline/offline.cpp index e38bca018..31c3bdef3 100644 --- a/dnf5/commands/offline/offline.cpp +++ b/dnf5/commands/offline/offline.cpp @@ -293,8 +293,9 @@ void OfflineRebootCommand::run() { check_state(*state); - if (state->get_data().status != libdnf5::offline::STATUS_DOWNLOAD_COMPLETE && - state->get_data().status != libdnf5::offline::STATUS_READY) { + auto & offline_data = state->get_data(); + if (offline_data.get_status() != libdnf5::offline::STATUS_DOWNLOAD_COMPLETE && + offline_data.get_status() != libdnf5::offline::STATUS_READY) { throw libdnf5::cli::CommandExitError(1, M_("System is not ready for offline transaction.")); } if (!std::filesystem::is_directory(get_datadir())) { @@ -327,17 +328,17 @@ void OfflineRebootCommand::run() { } #endif - const auto & system_releasever = state->get_data().system_releasever; - const auto & target_releasever = state->get_data().target_releasever; + const auto & system_releasever = offline_data.get_system_releasever(); + const auto & target_releasever = offline_data.get_target_releasever(); - if (state->get_data().verb == "system-upgrade download") { + if (offline_data.get_verb() == "system-upgrade download") { std::cout << _("The system will now reboot to upgrade to release version ") << target_releasever << "." << std::endl; } else { std::cout << _("The system will now reboot to perform the offline transaction initiated by the following command:") << std::endl - << "\t" << state->get_data().cmd_line << std::endl; + << "\t" << offline_data.get_cmd_line() << std::endl; } if (!libdnf5::cli::utils::userconfirm::userconfirm(ctx.get_base().get_config())) { return; @@ -347,8 +348,8 @@ void OfflineRebootCommand::run() { std::filesystem::create_symlink(get_datadir(), libdnf5::offline::MAGIC_SYMLINK); } - state->get_data().status = libdnf5::offline::STATUS_READY; - state->get_data().poweroff_after = poweroff_after->get_value(); + offline_data.set_status(libdnf5::offline::STATUS_READY); + offline_data.set_poweroff_after(poweroff_after->get_value()); state->write(); dnf5::offline::log_status( @@ -407,19 +408,22 @@ void OfflineExecuteCommand::configure() { ctx.set_load_available_repos(Context::LoadAvailableRepos::NONE); // Get the cache from the cachedir specified in the state file - ctx.get_base().get_config().get_system_cachedir_option().set(state->get_data().cachedir); - ctx.get_base().get_config().get_cachedir_option().set(state->get_data().cachedir); + auto cachedir = state->get_data().get_cachedir(); + ctx.get_base().get_config().get_system_cachedir_option().set(cachedir); + ctx.get_base().get_config().get_cachedir_option().set(cachedir); - if (!state->get_data().module_platform_id.empty()) { - ctx.get_base().get_config().get_module_platform_id_option().set(state->get_data().module_platform_id); + auto module_platform_id = state->get_data().get_module_platform_id(); + if (!module_platform_id.empty()) { + ctx.get_base().get_config().get_module_platform_id_option().set(module_platform_id); } } void OfflineExecuteCommand::run() { auto & ctx = get_context(); + auto & offline_data = state->get_data(); - const auto & system_releasever = state->get_data().system_releasever; - const auto & target_releasever = state->get_data().target_releasever; + const auto & system_releasever = offline_data.get_system_releasever(); + const auto & target_releasever = offline_data.get_target_releasever(); dnf5::offline::log_status( ctx, @@ -435,11 +439,11 @@ void OfflineExecuteCommand::run() { std::filesystem::remove(libdnf5::offline::MAGIC_SYMLINK); - if (state->get_data().status != libdnf5::offline::STATUS_READY) { + if (offline_data.get_status() != libdnf5::offline::STATUS_READY) { throw libdnf5::cli::CommandExitError(1, M_("Use `dnf5 offline reboot` to begin the transaction.")); } - state->get_data().status = libdnf5::offline::STATUS_TRANSACTION_INCOMPLETE; + offline_data.set_status(libdnf5::offline::STATUS_TRANSACTION_INCOMPLETE); state->write(); const auto & installroot = ctx.get_base().get_config().get_installroot_option().get_value(); @@ -482,7 +486,7 @@ void OfflineExecuteCommand::run() { callbacks->get_multi_progress_bar()->set_total_num_of_bars(num_of_actions); transaction.set_callbacks(std::move(callbacks)); - transaction.set_description(state->get_data().cmd_line); + transaction.set_description(offline_data.get_cmd_line()); const auto result = transaction.run(); std::cout << std::endl; @@ -503,7 +507,7 @@ void OfflineExecuteCommand::run() { } std::string transaction_complete_message; - if (state->get_data().poweroff_after) { + if (offline_data.get_poweroff_after()) { transaction_complete_message = "Transaction complete! Cleaning up and powering off..."; } else { transaction_complete_message = "Transaction complete! Cleaning up and rebooting..."; @@ -516,7 +520,7 @@ void OfflineExecuteCommand::run() { // If the transaction succeeded, remove downloaded data clean_datadir(ctx, get_datadir()); - reboot(state->get_data().poweroff_after); + reboot(offline_data.get_poweroff_after()); } void OfflineCleanCommand::set_argument_parser() { @@ -670,20 +674,20 @@ void OfflineStatusCommand::run() { } check_state(*state); - const auto & status = state->get_data().status; + const auto & status = state->get_data().get_status(); if (status == libdnf5::offline::STATUS_DOWNLOAD_INCOMPLETE) { std::cout << no_transaction_message << std::endl; } else if (status == libdnf5::offline::STATUS_DOWNLOAD_COMPLETE || status == libdnf5::offline::STATUS_READY) { std::cout << _("An offline transaction was initiated by the following command:") << std::endl - << "\t" << state->get_data().cmd_line << std::endl + << "\t" << state->get_data().get_cmd_line() << std::endl << _("Run `dnf5 offline reboot` to reboot and perform the offline transaction.") << std::endl; } else if (status == libdnf5::offline::STATUS_TRANSACTION_INCOMPLETE) { std::cout << _("An offline transaction was started, but it did not finish. Run `dnf5 offline log` for more " "information. The command that initiated the transaction was:") << std::endl - << "\t" << state->get_data().cmd_line << std::endl; + << "\t" << state->get_data().get_cmd_line() << std::endl; } else { - std::cout << _("Unknown offline transaction status: ") << "`" << state->get_data().status << "`" << std::endl; + std::cout << _("Unknown offline transaction status: ") << "`" << status << "`" << std::endl; } } diff --git a/dnf5/context.cpp b/dnf5/context.cpp index 8c997b387..5357e1752 100644 --- a/dnf5/context.cpp +++ b/dnf5/context.cpp @@ -323,16 +323,17 @@ void Context::Impl::store_offline(libdnf5::base::Transaction & transaction) { const std::filesystem::path state_path{offline_datadir / libdnf5::offline::TRANSACTION_STATE_FILENAME}; libdnf5::offline::OfflineTransactionState state{state_path}; - if (state.get_data().status != libdnf5::offline::STATUS_DOWNLOAD_INCOMPLETE) { + auto & offline_data = state.get_data(); + if (offline_data.get_status() != libdnf5::offline::STATUS_DOWNLOAD_INCOMPLETE) { std::cout << "There is already an offline transaction queued, initiated by the following command:" << std::endl - << "\t" << state.get_data().cmd_line << std::endl + << "\t" << offline_data.get_cmd_line() << std::endl << "Continuing will cancel the old offline transaction and replace it with this one." << std::endl; if (!libdnf5::cli::utils::userconfirm::userconfirm(base.get_config())) { throw libdnf5::cli::AbortedByUserError(); } } - state.get_data().status = libdnf5::offline::STATUS_DOWNLOAD_INCOMPLETE; + offline_data.set_status(libdnf5::offline::STATUS_DOWNLOAD_INCOMPLETE); state.write(); // First, serialize the transaction @@ -372,8 +373,8 @@ void Context::Impl::store_offline(libdnf5::base::Transaction & transaction) { // Download and transaction test complete. Fill out entries in offline // transaction state file. - state.get_data().status = libdnf5::offline::STATUS_DOWNLOAD_COMPLETE; - state.get_data().cachedir = base.get_config().get_cachedir_option().get_value(); + offline_data.set_status(libdnf5::offline::STATUS_DOWNLOAD_COMPLETE); + offline_data.set_cachedir(base.get_config().get_cachedir_option().get_value()); std::vector command_vector; auto * current_command = owner.get_selected_command(); @@ -381,17 +382,17 @@ void Context::Impl::store_offline(libdnf5::base::Transaction & transaction) { command_vector.insert(command_vector.begin(), current_command->get_argument_parser_command()->get_id()); current_command = current_command->get_parent_command(); } - state.get_data().verb = libdnf5::utils::string::join(command_vector, " "); - state.get_data().cmd_line = get_cmdline(); + offline_data.set_verb(libdnf5::utils::string::join(command_vector, " ")); + offline_data.set_cmd_line(get_cmdline()); const auto & detected_releasever = libdnf5::Vars::detect_release(base.get_weak_ptr(), installroot); if (detected_releasever != nullptr) { - state.get_data().system_releasever = *detected_releasever; + offline_data.set_system_releasever(*detected_releasever); } - state.get_data().target_releasever = base.get_vars()->get_value("releasever"); + offline_data.set_target_releasever(base.get_vars()->get_value("releasever")); if (!base.get_config().get_module_platform_id_option().empty()) { - state.get_data().module_platform_id = base.get_config().get_module_platform_id_option().get_value(); + offline_data.set_module_platform_id(base.get_config().get_module_platform_id_option().get_value()); } state.write(); diff --git a/include/libdnf5/transaction/offline.hpp b/include/libdnf5/transaction/offline.hpp index 8faeb44a2..d8115c62d 100644 --- a/include/libdnf5/transaction/offline.hpp +++ b/include/libdnf5/transaction/offline.hpp @@ -21,9 +21,9 @@ along with libdnf. If not, see . #define LIBDNF5_TRANSACTION_OFFLINE_HPP #include -#include #include +#include namespace libdnf5::offline { @@ -45,42 +45,84 @@ const std::string STATUS_TRANSACTION_INCOMPLETE{"transaction-incomplete"}; // systemd.offline-updates(7). const std::filesystem::path MAGIC_SYMLINK{"/system-update"}; -const int STATE_VERSION = 1; -const std::string STATE_HEADER{"offline-transaction-state"}; - const std::filesystem::path DEFAULT_DATADIR{std::filesystem::path(libdnf5::SYSTEM_STATE_DIR) / "offline"}; const std::filesystem::path TRANSACTION_STATE_FILENAME{"offline-transaction-state.toml"}; +class OfflineTransactionState; + /// Data of the initiated offline transaction state, by default stored in the /// /usr/lib/sysimage/libdnf5/offline/offline-transaction-state.toml file. struct OfflineTransactionStateData { - /// Version of the state file. - int state_version = STATE_VERSION; - /// Current offline transaction status. One of download-incomplete, download-complete, ready, or transaction-incomplete. - std::string status = STATUS_DOWNLOAD_INCOMPLETE; - /// Cachedir to be used for the offline transaction. - std::string cachedir; - /// Target releasever for the offline transaction. - std::string target_releasever; - /// Detected current releasever when the offline transaction was initialized. - std::string system_releasever; - /// Dnf command used to initialize the offline transaction (e.g. "system-upgrade download"). - std::string verb; - /// Command line used to initialize the offline transaction. - std::string cmd_line; - /// Power off the system after the operation is complete? - bool poweroff_after = false; - /// module_platform_id for the offline transaction. - std::string module_platform_id; +public: + friend OfflineTransactionState; + + OfflineTransactionStateData(); + ~OfflineTransactionStateData(); + + OfflineTransactionStateData(const OfflineTransactionStateData & src); + OfflineTransactionStateData & operator=(const OfflineTransactionStateData & src); + + OfflineTransactionStateData(OfflineTransactionStateData && src) noexcept; + OfflineTransactionStateData & operator=(OfflineTransactionStateData && src) noexcept; + + /// Set the transaction state data file version + void set_state_version(int state_version); + int get_state_version() const; + + /// Set current offline transaction status. One of download-incomplete, + /// download-complete, ready, or transaction-incomplete. + void set_status(const std::string & status); + const std::string & get_status() const; + + /// Set the cachedir to be used for the offline transaction. + void set_cachedir(const std::string & cachedir); + const std::string & get_cachedir() const; + + /// Set the target releasever for the offline transaction. + void set_target_releasever(const std::string & target_releasever); + const std::string & get_target_releasever() const; + + /// Set the detected releasever in time the offline transaction was initialized. + void set_system_releasever(const std::string & system_releasever); + const std::string & get_system_releasever() const; + + /// Set the dnf command used to initialize the offline transaction (e.g. "system-upgrade download"). + void set_verb(const std::string & verb); + const std::string & get_verb() const; + + /// Set the command line used to initialize the offline transaction. + void set_cmd_line(const std::string & cmd_line); + const std::string & get_cmd_line() const; + + /// Set whether the system power off after the operation is complete is required + void set_poweroff_after(bool poweroff_after); + bool get_poweroff_after() const; + + /// Set module_platform_id for the offline transaction. + void set_module_platform_id(const std::string & module_platform_id); + const std::string & get_module_platform_id() const; + +private: + class Impl; + std::unique_ptr p_impl; }; + /// Class to handle offline transaction state. class OfflineTransactionState { public: + OfflineTransactionState() = delete; + ~OfflineTransactionState(); + /// Constructs a new OfflineTransactionState instance based on the state file location. /// @param path Path to the state file (default location is /usr/lib/sysimage/libdnf5/offline/offline-transaction-state.toml). OfflineTransactionState(std::filesystem::path path); + OfflineTransactionState(const OfflineTransactionState & src); + OfflineTransactionState & operator=(const OfflineTransactionState & src); + OfflineTransactionState(OfflineTransactionState && src) noexcept; + OfflineTransactionState & operator=(OfflineTransactionState && src) noexcept; + /// Returns offline transaction state data. OfflineTransactionStateData & get_data(); /// Write the current state to the file. @@ -91,25 +133,12 @@ class OfflineTransactionState { std::filesystem::path get_path() const; private: + class Impl; /// Read offline transaction state data from the file void read(); - std::exception_ptr read_exception; - std::filesystem::path path; - OfflineTransactionStateData data; + std::unique_ptr p_impl; }; } // namespace libdnf5::offline -TOML11_DEFINE_CONVERSION_NON_INTRUSIVE( - libdnf5::offline::OfflineTransactionStateData, - state_version, - status, - cachedir, - target_releasever, - system_releasever, - verb, - cmd_line, - poweroff_after, - module_platform_id) - #endif // LIBDNF5_TRANSACTION_OFFLINE_HPP diff --git a/libdnf5/transaction/offline.cpp b/libdnf5/transaction/offline.cpp index 5b852eabe..ee06c9daf 100644 --- a/libdnf5/transaction/offline.cpp +++ b/libdnf5/transaction/offline.cpp @@ -22,47 +22,192 @@ along with libdnf. If not, see . #include #include #include - +#include + + +const int STATE_VERSION = 1; +const std::string STATE_HEADER{"offline-transaction-state"}; + + +struct OfflineTransactionStateTomlData { + int state_version = STATE_VERSION; + std::string status = libdnf5::offline::STATUS_DOWNLOAD_INCOMPLETE; + std::string cachedir; + std::string target_releasever; + std::string system_releasever; + std::string verb; + std::string cmd_line; + bool poweroff_after = false; + std::string module_platform_id; +}; + +TOML11_DEFINE_CONVERSION_NON_INTRUSIVE( + OfflineTransactionStateTomlData, + state_version, + status, + cachedir, + target_releasever, + system_releasever, + verb, + cmd_line, + poweroff_after, + module_platform_id) namespace libdnf5::offline { -OfflineTransactionState::OfflineTransactionState(std::filesystem::path path) : path(std::move(path)) { +class OfflineTransactionStateData::Impl { +public: + friend OfflineTransactionStateData; + OfflineTransactionStateTomlData data; +}; + + +OfflineTransactionStateData::~OfflineTransactionStateData() = default; + +OfflineTransactionStateData::OfflineTransactionStateData() : p_impl(std::make_unique()){}; +OfflineTransactionStateData::OfflineTransactionStateData(const OfflineTransactionStateData & src) + : p_impl(new Impl(*src.p_impl)) {} +OfflineTransactionStateData::OfflineTransactionStateData(OfflineTransactionStateData && src) noexcept = default; + +OfflineTransactionStateData & OfflineTransactionStateData::operator=(const OfflineTransactionStateData & src) { + if (this != &src) { + if (p_impl) { + *p_impl = *src.p_impl; + } else { + p_impl = std::make_unique(*src.p_impl); + } + } + return *this; +} + +OfflineTransactionStateData & OfflineTransactionStateData::operator=(OfflineTransactionStateData && src) noexcept = + default; + +void OfflineTransactionStateData::set_state_version(int state_version) { + p_impl->data.state_version = state_version; +} +int OfflineTransactionStateData::get_state_version() const { + return p_impl->data.state_version; +} + +void OfflineTransactionStateData::set_status(const std::string & status) { + p_impl->data.status = status; +} +const std::string & OfflineTransactionStateData::get_status() const { + return p_impl->data.status; +} + +void OfflineTransactionStateData::set_cachedir(const std::string & cachedir) { + p_impl->data.cachedir = cachedir; +} +const std::string & OfflineTransactionStateData::get_cachedir() const { + return p_impl->data.cachedir; +} + +void OfflineTransactionStateData::set_target_releasever(const std::string & target_releasever) { + p_impl->data.target_releasever = target_releasever; +} +const std::string & OfflineTransactionStateData::get_target_releasever() const { + return p_impl->data.target_releasever; +} + +void OfflineTransactionStateData::set_system_releasever(const std::string & system_releasever) { + p_impl->data.system_releasever = system_releasever; +} +const std::string & OfflineTransactionStateData::get_system_releasever() const { + return p_impl->data.system_releasever; +} + +void OfflineTransactionStateData::set_verb(const std::string & verb) { + p_impl->data.verb = verb; +} +const std::string & OfflineTransactionStateData::get_verb() const { + return p_impl->data.verb; +} + +void OfflineTransactionStateData::set_cmd_line(const std::string & cmd_line) { + p_impl->data.cmd_line = cmd_line; +} +const std::string & OfflineTransactionStateData::get_cmd_line() const { + return p_impl->data.cmd_line; +} + +void OfflineTransactionStateData::set_poweroff_after(bool poweroff_after) { + p_impl->data.poweroff_after = poweroff_after; +} +bool OfflineTransactionStateData::get_poweroff_after() const { + return p_impl->data.poweroff_after; +} + +void OfflineTransactionStateData::set_module_platform_id(const std::string & module_platform_id) { + p_impl->data.module_platform_id = module_platform_id; +} +const std::string & OfflineTransactionStateData::get_module_platform_id() const { + return p_impl->data.module_platform_id; +} + + +class OfflineTransactionState::Impl { + friend OfflineTransactionState; + std::exception_ptr read_exception; + std::filesystem::path path; + OfflineTransactionStateData data; +}; + +OfflineTransactionState::~OfflineTransactionState() = default; + +OfflineTransactionState::OfflineTransactionState(std::filesystem::path path) : p_impl(std::make_unique()) { + p_impl->path = std::move(path); read(); } +OfflineTransactionState::OfflineTransactionState(const OfflineTransactionState & src) : p_impl(new Impl(*src.p_impl)) {} +OfflineTransactionState::OfflineTransactionState(OfflineTransactionState && src) noexcept = default; + +OfflineTransactionState & OfflineTransactionState::operator=(const OfflineTransactionState & src) { + if (this != &src) { + if (p_impl) { + *p_impl = *src.p_impl; + } else { + p_impl = std::make_unique(*src.p_impl); + } + } + return *this; +} +OfflineTransactionState & OfflineTransactionState::operator=(OfflineTransactionState && src) noexcept = default; OfflineTransactionStateData & OfflineTransactionState::get_data() { - return data; + return p_impl->data; } const std::exception_ptr & OfflineTransactionState::get_read_exception() const { - return read_exception; + return p_impl->read_exception; } std::filesystem::path OfflineTransactionState::get_path() const { - return path; + return p_impl->path; } void OfflineTransactionState::read() { try { - const std::ifstream file{path}; + const std::ifstream file{p_impl->path}; if (!file.good()) { - throw libdnf5::FileSystemError(errno, path, M_("error reading offline state file")); + throw libdnf5::FileSystemError(errno, p_impl->path, M_("error reading offline state file")); } - const auto & value = toml::parse(path); - data = toml::find(value, STATE_HEADER); - if (data.state_version != STATE_VERSION) { + const auto & value = toml::parse(p_impl->path); + p_impl->data.p_impl->data = toml::find(value, STATE_HEADER); + if (p_impl->data.get_state_version() != STATE_VERSION) { throw libdnf5::RuntimeError(M_("incompatible version of state data")); } } catch (const std::exception & ex) { - read_exception = std::current_exception(); - data = OfflineTransactionStateData{}; + p_impl->read_exception = std::current_exception(); + p_impl->data = OfflineTransactionStateData{}; } } void OfflineTransactionState::write() { - auto file = libdnf5::utils::fs::File(path, "w"); - file.write(toml::format(toml::value{{STATE_HEADER, data}})); + auto file = libdnf5::utils::fs::File(p_impl->path, "w"); + file.write(toml::format(toml::value{{STATE_HEADER, p_impl->data.p_impl->data}})); file.close(); }