From 98fc1bc2b03e6dcfb43a23f25ec27247a51f498a Mon Sep 17 00:00:00 2001 From: Evan Goode Date: Wed, 20 Mar 2024 00:04:39 +0000 Subject: [PATCH] system-upgrade: bugfixes, output changes Fix a couple issues that came up when writing the system-upgrade and offline tests: - Fix system-upgrade download --no-downgrade - Print more transaction tables, mainly so the tests can parse them - Exit cleanly when `dnf5 offline _execute` is run but there is no valid state file. This is important for when DNF 4 is running a system upgrade but DNF 5 is also installed, as dnf5-offline-transaction.service will otherwise interfere with the DNF 4 system-upgrade. - Clean command now deletes the /system-update magic symlink - system-upgrade download is now simplified to use more of main.cpp's general transaction flow, but it doesn't print the "Download complete!" message at the end or log a message marked with DOWNLOAD_FINISHED_ID. --- dnf5/commands/offline/offline.cpp | 27 ++++++++----- .../system-upgrade/system-upgrade.cpp | 38 ++++++------------- dnf5/include/dnf5/offline.cpp | 2 +- dnf5/include/dnf5/offline.hpp | 1 - doc/commands/system-upgrade.8.rst | 4 +- 5 files changed, 33 insertions(+), 39 deletions(-) diff --git a/dnf5/commands/offline/offline.cpp b/dnf5/commands/offline/offline.cpp index c4d018a331..5fc26bdbca 100644 --- a/dnf5/commands/offline/offline.cpp +++ b/dnf5/commands/offline/offline.cpp @@ -22,6 +22,7 @@ along with libdnf. If not, see . #include "dnf5/offline.hpp" #include "utils/string.hpp" +#include #include #include #include @@ -289,7 +290,12 @@ void OfflineRebootCommand::set_argument_parser() { void OfflineRebootCommand::run() { auto & ctx = get_context(); + if (!std::filesystem::exists(state->get_path())) { + throw libdnf5::cli::CommandExitError(1, M_("No offline transaction is stored.")); + } + check_state(*state); + if (state->get_data().status != dnf5::offline::STATUS_DOWNLOAD_COMPLETE && state->get_data().status != dnf5::offline::STATUS_READY) { throw libdnf5::cli::CommandExitError(1, M_("System is not ready for offline transaction.")); @@ -384,6 +390,14 @@ void OfflineExecuteCommand::configure() { OfflineSubcommand::configure(); auto & ctx = get_context(); + if (!std::filesystem::is_symlink(get_magic_symlink())) { + throw libdnf5::cli::CommandExitError(0, M_("Trigger file does not exist. Exiting.")); + } + + if (!std::filesystem::equivalent(get_magic_symlink(), get_datadir())) { + throw libdnf5::cli::CommandExitError(0, M_("Another offline transaction tool is running. Exiting.")); + } + check_state(*state); ctx.set_load_system_repo(true); @@ -421,14 +435,6 @@ void OfflineExecuteCommand::run() { "the user. To initiate the system upgrade/offline transaction, you should run `dnf5 offline reboot`.") << std::endl; - if (!std::filesystem::is_symlink(get_magic_symlink())) { - throw libdnf5::cli::CommandExitError(0, M_("Trigger file does not exist. Exiting.")); - } - - if (!std::filesystem::equivalent(get_magic_symlink(), get_datadir())) { - throw libdnf5::cli::CommandExitError(0, M_("Another offline transaction tool is running. Exiting.")); - } - std::filesystem::remove(get_magic_symlink()); if (state->get_data().status != dnf5::offline::STATUS_READY) { @@ -460,6 +466,8 @@ void OfflineExecuteCommand::run() { throw libdnf5::cli::GoalResolveError(transaction); } + libdnf5::cli::output::print_transaction_table(transaction); + PlymouthOutput plymouth; auto callbacks = std::make_unique(ctx, plymouth); /* callbacks->get_multi_progress_bar()->set_total_num_of_bars(num_of_actions); */ @@ -512,6 +520,7 @@ void OfflineCleanCommand::set_argument_parser() { void OfflineCleanCommand::run() { auto & ctx = get_context(); + std::filesystem::remove(get_magic_symlink()); clean_datadir(ctx, get_datadir()); } @@ -649,7 +658,7 @@ void OfflineStatusCommand::run() { const std::string no_transaction_message{"No offline transaction is stored."}; if (!std::filesystem::exists(state->get_path())) { - std::cerr << no_transaction_message << std::endl; + std::cout << no_transaction_message << std::endl; return; } check_state(*state); diff --git a/dnf5/commands/system-upgrade/system-upgrade.cpp b/dnf5/commands/system-upgrade/system-upgrade.cpp index a6297c768f..cd95129317 100644 --- a/dnf5/commands/system-upgrade/system-upgrade.cpp +++ b/dnf5/commands/system-upgrade/system-upgrade.cpp @@ -23,6 +23,7 @@ along with libdnf. If not, see . #include "libdnf5/utils/bgettext/bgettext-lib.h" +#include #include #include #include @@ -68,12 +69,13 @@ void SystemUpgradeDownloadCommand::set_argument_parser() { cmd.set_description(_("Download everything needed to upgrade to a new release")); no_downgrade = - dynamic_cast(parser.add_init_value(std::make_unique(true))); + dynamic_cast(parser.add_init_value(std::make_unique(false))); auto * no_downgrade_arg = parser.add_new_named_arg("no-downgrade"); no_downgrade_arg->set_long_name("no-downgrade"); no_downgrade_arg->set_description( _("Do not install packages from the new release if they are older than what is currently installed")); + no_downgrade_arg->set_const_value("true"); no_downgrade_arg->link_value(no_downgrade); cmd.register_named_arg(no_downgrade_arg); } @@ -83,16 +85,16 @@ void SystemUpgradeDownloadCommand::configure() { const std::filesystem::path installroot{ctx.base.get_config().get_installroot_option().get_value()}; - const auto & detected_releasever = libdnf5::Vars::detect_release(ctx.base.get_weak_ptr(), installroot); - if (detected_releasever == nullptr) { - throw libdnf5::cli::CommandExitError(1, M_("Couldn't detect the current release version of the system.")); - } - system_releasever = *detected_releasever; target_releasever = ctx.base.get_vars()->get_value("releasever"); - // Check --releasever - if (target_releasever == system_releasever) { - throw libdnf5::cli::CommandExitError(1, M_("Need a --releasever greater than the current system version.")); + const auto & detected_releasever = libdnf5::Vars::detect_release(ctx.base.get_weak_ptr(), installroot); + if (detected_releasever != nullptr) { + system_releasever = *detected_releasever; + + // Check --releasever + if (target_releasever == system_releasever) { + throw libdnf5::cli::CommandExitError(1, M_("Need a --releasever greater than the current system version.")); + } } ctx.set_load_system_repo(true); @@ -102,7 +104,7 @@ void SystemUpgradeDownloadCommand::configure() { void SystemUpgradeDownloadCommand::run() { auto & ctx = get_context(); - const auto & goal = std::make_unique(ctx.base); + const auto & goal = ctx.get_goal(); if (no_downgrade->get_value()) { goal->add_rpm_upgrade(); @@ -110,23 +112,7 @@ void SystemUpgradeDownloadCommand::run() { goal->add_rpm_distro_sync(); } - auto transaction = goal->resolve(); - if (transaction.get_problems() != libdnf5::GoalProblem::NO_PROBLEM) { - throw libdnf5::cli::GoalResolveError(transaction); - } - - if (transaction.get_transaction_packages_count() == 0) { - throw libdnf5::cli::CommandExitError( - 1, M_("The system-upgrade transaction is empty; your system is already up-to-date.")); - } - ctx.set_should_store_offline(true); - ctx.download_and_run(transaction); - - std::cout << _("Download complete!") << std::endl; - - dnf5::offline::log_status( - ctx, "Download finished.", dnf5::offline::DOWNLOAD_FINISHED_ID, system_releasever, target_releasever); } } // namespace dnf5 diff --git a/dnf5/include/dnf5/offline.cpp b/dnf5/include/dnf5/offline.cpp index d3deaf6f35..13734e95c5 100644 --- a/dnf5/include/dnf5/offline.cpp +++ b/dnf5/include/dnf5/offline.cpp @@ -36,7 +36,7 @@ void OfflineTransactionState::read() { try { const std::ifstream file{path}; if (!file.good()) { - throw libdnf5::FileSystemError(errno, path, M_("Error reading offline state file.")); + throw libdnf5::FileSystemError(errno, path, M_("error reading offline state file")); } const auto & value = toml::parse(path); data = toml::find(value, STATE_HEADER); diff --git a/dnf5/include/dnf5/offline.hpp b/dnf5/include/dnf5/offline.hpp index ba2345163d..752c8c1d4c 100644 --- a/dnf5/include/dnf5/offline.hpp +++ b/dnf5/include/dnf5/offline.hpp @@ -34,7 +34,6 @@ namespace dnf5::offline { // journald logs. These are the same as they are in `dnf4 system-upgrade`, so // `dnf5 offline log` will find offline transactions performed by DNF 4 and // vice-versa. -const std::string DOWNLOAD_FINISHED_ID{"9348174c5cc74001a71ef26bd79d302e"}; const std::string REBOOT_REQUESTED_ID{"9348174c5cc74001a71ef26bd79d302e"}; const std::string OFFLINE_STARTED_ID{"3e0a5636d16b4ca4bbe5321d06c6aa62"}; const std::string OFFLINE_FINISHED_ID{"8cec00a1566f4d3594f116450395f06c"}; diff --git a/doc/commands/system-upgrade.8.rst b/doc/commands/system-upgrade.8.rst index 278d592fb0..7a4a90714a 100644 --- a/doc/commands/system-upgrade.8.rst +++ b/doc/commands/system-upgrade.8.rst @@ -18,9 +18,9 @@ .. _system_upgrade_command_ref-label: -################ +####################### System-upgrade Command -################ +####################### Synopsis ========