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

@@ -22,6 +22,7 @@ along with libdnf.  If not, see <https://www.gnu.org/licenses/>.
 #include "dnf5/offline.hpp"
 #include "utils/string.hpp"
 
+#include <libdnf5-cli/output/transaction_table.hpp>
 #include <libdnf5-cli/utils/userconfirm.hpp>
 #include <libdnf5/base/goal.hpp>
 #include <libdnf5/conf/const.hpp>
@@ -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<PlymouthTransCB>(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);

dnf5/commands/system-upgrade/system-upgrade.cpp

@@ -23,6 +23,7 @@ along with libdnf.  If not, see <https://www.gnu.org/licenses/>.
 
 #include "libdnf5/utils/bgettext/bgettext-lib.h"
 
+#include <libdnf5-cli/output/transaction_table.hpp>
 #include <libdnf5-cli/utils/userconfirm.hpp>
 #include <libdnf5/base/goal.hpp>
 #include <libdnf5/conf/const.hpp>
@@ -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<libdnf5::OptionBool *>(parser.add_init_value(std::make_unique<libdnf5::OptionBool>(true)));
+        dynamic_cast<libdnf5::OptionBool *>(parser.add_init_value(std::make_unique<libdnf5::OptionBool>(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,31 +104,15 @@ void SystemUpgradeDownloadCommand::configure() {
 void SystemUpgradeDownloadCommand::run() {
     auto & ctx = get_context();
 
-    const auto & goal = std::make_unique<libdnf5::Goal>(ctx.base);
+    const auto & goal = ctx.get_goal();
 
     if (no_downgrade->get_value()) {
         goal->add_rpm_upgrade();
     } else {
         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

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<OfflineTransactionStateData>(value, STATE_HEADER);

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"};

doc/commands/system-upgrade.8.rst

@@ -18,9 +18,9 @@
 
 .. _system_upgrade_command_ref-label:
 
-################
+#######################
  System-upgrade Command
-################
+#######################
 
 Synopsis
 ========