Update package_info_sections not to use scols_table_print_range

`scols_table_print_range` is slow when the table is large (when the table contains info for all available packages). Since we need to print an empty line between the packages the only option is to print each package as a separate table. To achieve that and keep the class compatible with `package_list_sections` it needed to be changed. `package_list/info_sections` still take a `PackageSet` when adding a section, this could be updated to a more general vector of IPackage which could also be used from dnf5daemon but the sets would require converting and I am not certain it will be needed.
rpm-software-management · Mar 20, 2024 · 0f3a3d3 · 0f3a3d3
1 parent 5ffa43f
commit 0f3a3d3
Show file tree

Hide file tree

Showing 8 changed files with 123 additions and 221 deletions.
diff --git a/dnf5/commands/check-upgrade/check-upgrade.cpp b/dnf5/commands/check-upgrade/check-upgrade.cpp
@@ -99,7 +99,6 @@ void CheckUpgradeCommand::configure() {
 
 std::unique_ptr<libdnf5::cli::output::PackageListSections> CheckUpgradeCommand::create_output() {
     auto out = std::make_unique<libdnf5::cli::output::PackageListSections>();
-    out->setup_cols();
     return out;
 }
 
@@ -175,21 +174,21 @@ void CheckUpgradeCommand::run() {
         obsoletes.emplace(pkg.get_id(), obsoleted);
     }
 
-    auto colorizer = std::make_unique<libdnf5::cli::output::PkgColorizer>(
-        installed_query,
-        "",  //config.get_color_list_available_install_option().get_value(),
-        "",  //config.get_color_list_available_downgrade_option().get_value(),
-        config.get_color_list_available_reinstall_option().get_value(),
-        config.get_color_list_available_upgrade_option().get_value());
     auto sections = create_output();
 
     bool package_matched = false;
     package_matched |= sections->add_section("", upgrades_query);
-    package_matched |= sections->add_section("Obsoleting packages", obsoletes_query, colorizer, obsoletes);
+    package_matched |= sections->add_section("Obsoleting packages", obsoletes_query, obsoletes);
 
     if (package_matched) {
         // If any upgrades were found, print a table of them, and optionally print changelogs. Return exit code 100.
-        sections->print();
+        auto colorizer = std::make_unique<libdnf5::cli::output::PkgColorizer>(
+            installed_query,
+            "",  //config.get_color_list_available_install_option().get_value(),
+            "",  //config.get_color_list_available_downgrade_option().get_value(),
+            config.get_color_list_available_reinstall_option().get_value(),
+            config.get_color_list_available_upgrade_option().get_value());
+        sections->print(colorizer);
         if (changelogs->get_value()) {
             libdnf5::cli::output::print_changelogs(
                 upgrades_query, {libdnf5::cli::output::ChangelogFilterType::UPGRADES, full_package_query});

diff --git a/dnf5/commands/list/info.cpp b/dnf5/commands/list/info.cpp
@@ -27,7 +27,6 @@ using namespace libdnf5::cli;
 
 std::unique_ptr<libdnf5::cli::output::PackageListSections> InfoCommand::create_output() {
     auto out = std::make_unique<libdnf5::cli::output::PackageInfoSections>();
-    out->setup_cols();
     return out;
 }
 

diff --git a/dnf5/commands/list/list.cpp b/dnf5/commands/list/list.cpp
@@ -24,8 +24,6 @@ along with libdnf.  If not, see <https://www.gnu.org/licenses/>.
 #include <libdnf5/rpm/package_query.hpp>
 #include <libdnf5/utils/bgettext/bgettext-mark-domain.h>
 
-#include <iostream>
-
 namespace dnf5 {
 
 using namespace libdnf5::cli;
@@ -137,7 +135,6 @@ void ListCommand::configure() {
 
 std::unique_ptr<libdnf5::cli::output::PackageListSections> ListCommand::create_output() {
     auto out = std::make_unique<libdnf5::cli::output::PackageListSections>();
-    out->setup_cols();
     return out;
 }
 
@@ -194,12 +191,12 @@ void ListCommand::run() {
                 installed_latest.filter_latest_evr();
                 available.filter_nevra(installed_latest, libdnf5::sack::QueryCmp::NOT | libdnf5::sack::QueryCmp::LTE);
             }
-            package_matched |= sections->add_section("Installed packages", installed, colorizer);
-            package_matched |= sections->add_section("Available packages", available, colorizer);
+            package_matched |= sections->add_section("Installed packages", installed);
+            package_matched |= sections->add_section("Available packages", available);
             break;
         }
         case PkgNarrow::INSTALLED: {
-            package_matched |= sections->add_section("Installed packages", installed, colorizer);
+            package_matched |= sections->add_section("Installed packages", installed);
             break;
         }
         case PkgNarrow::AVAILABLE: {
@@ -208,15 +205,15 @@ void ListCommand::run() {
                 base_query.filter_priority();
                 base_query.filter_latest_evr();
             }
-            package_matched |= sections->add_section("Available packages", base_query, colorizer);
+            package_matched |= sections->add_section("Available packages", base_query);
             break;
         }
         case PkgNarrow::UPGRADES:
             base_query.filter_priority();
             base_query.filter_upgrades();
             base_query.filter_arch(std::vector<std::string>{"src", "nosrc"}, libdnf5::sack::QueryCmp::NEQ);
             base_query.filter_latest_evr();
-            package_matched |= sections->add_section("Available upgrades", base_query, colorizer);
+            package_matched |= sections->add_section("Available upgrades", base_query);
             break;
         case PkgNarrow::OBSOLETES: {
             base_query.filter_priority();
@@ -232,16 +229,16 @@ void ListCommand::run() {
                 }
                 obsoletes.emplace(pkg.get_id(), obsoleted);
             }
-            package_matched |= sections->add_section("Obsoleting packages", base_query, colorizer, obsoletes);
+            package_matched |= sections->add_section("Obsoleting packages", base_query, obsoletes);
             break;
         }
         case PkgNarrow::AUTOREMOVE:
             installed.filter_unneeded();
-            package_matched |= sections->add_section("Autoremove packages", installed, colorizer);
+            package_matched |= sections->add_section("Autoremove packages", installed);
             break;
         case PkgNarrow::EXTRAS:
             base_query.filter_extras();
-            package_matched |= sections->add_section("Extra packages", base_query, colorizer);
+            package_matched |= sections->add_section("Extra packages", base_query);
             break;
         case PkgNarrow::RECENT:
             base_query.filter_available();
@@ -252,14 +249,14 @@ void ListCommand::run() {
             auto recent_limit_days = config.get_recent_option().get_value();
             auto now = time(NULL);
             base_query.filter_recent(now - (recent_limit_days * 86400));
-            package_matched |= sections->add_section("Recently added packages", base_query, colorizer);
+            package_matched |= sections->add_section("Recently added packages", base_query);
             break;
     }
 
     if (!package_matched && !pkg_specs.empty()) {
         throw libdnf5::cli::CommandExitError(1, M_("No matching packages to list"));
     } else {
-        sections->print();
+        sections->print(colorizer);
     }
 }
 

diff --git a/include/libdnf5-cli/output/package_info_sections.hpp b/include/libdnf5-cli/output/package_info_sections.hpp
@@ -21,7 +21,6 @@ along with libdnf.  If not, see <https://www.gnu.org/licenses/>.
 #ifndef LIBDNF5_CLI_OUTPUT_PACKAGE_INFO_SECTIONS_HPP
 #define LIBDNF5_CLI_OUTPUT_PACKAGE_INFO_SECTIONS_HPP
 
-#include "interfaces/package.hpp"
 #include "package_list_sections.hpp"
 
 namespace libdnf5::cli::output {
@@ -31,19 +30,9 @@ class PackageInfoSections : public PackageListSections {
     PackageInfoSections();
     ~PackageInfoSections();
 
-    bool add_package(
-        IPackage & pkg,
-        const std::string & heading = "",
-        const std::unique_ptr<PkgColorizer> & colorizer = nullptr,
-        const std::vector<libdnf5::rpm::Package> & obsoletes = {});
-
-    bool add_section(
-        const std::string & heading,
-        const libdnf5::rpm::PackageSet & pkg_set,
-        const std::unique_ptr<PkgColorizer> & colorizer = nullptr,
-        const std::map<libdnf5::rpm::PackageId, std::vector<libdnf5::rpm::Package>> & obsoletes = {}) override;
-
-    void setup_cols() override;
+    /// Print the table
+    /// @param colorizer Optional class to select color for packages in output
+    void print(const std::unique_ptr<PkgColorizer> & colorizer = nullptr) override;
 };
 
 }  // namespace libdnf5::cli::output

diff --git a/include/libdnf5-cli/output/package_list_sections.hpp b/include/libdnf5-cli/output/package_list_sections.hpp
@@ -40,23 +40,19 @@ class PackageListSections {
     virtual ~PackageListSections();
 
     /// Print the table
-    void print();
+    /// @param colorizer Optional class to select color for packages in output
+    virtual void print(const std::unique_ptr<PkgColorizer> & colorizer = nullptr);
 
     /// Adds a new section to the smartcols table
     /// @param heading Header of the section
     /// @param pkg_set List of packages to be printed in this section
-    /// @param colorizer Optional class to select color for packages in output
     /// @param obsoletes Optional map of obsoleted packages by obsoleter
     /// @return Returns `true` in case at least one package was added, `false` otherwise
-    virtual bool add_section(
+    bool add_section(
         const std::string & heading,
         const libdnf5::rpm::PackageSet & pkg_set,
-        const std::unique_ptr<PkgColorizer> & colorizer = nullptr,
         const std::map<libdnf5::rpm::PackageId, std::vector<libdnf5::rpm::Package>> & obsoletes = {});
 
-    /// Setup table columns
-    virtual void setup_cols();
-
 protected:
     class Impl;
     std::unique_ptr<Impl> p_impl;

diff --git a/libdnf5-cli/output/package_info_sections.cpp b/libdnf5-cli/output/package_info_sections.cpp
@@ -21,129 +21,52 @@ along with libdnf.  If not, see <https://www.gnu.org/licenses/>.
 #include "libdnf5-cli/output/package_info_sections.hpp"
 
 #include "package_list_sections_impl.hpp"
-#include "utils/string.hpp"
 
 #include "libdnf5-cli/output/adapters/package.hpp"
-#include "libdnf5-cli/utils/units.hpp"
+#include "libdnf5-cli/output/packageinfo.hpp"
 
 #include <libdnf5/rpm/nevra.hpp>
 
 #include <algorithm>
+#include <iostream>
 
 namespace libdnf5::cli::output {
 
-namespace {
-
-enum { COL_KEY, COL_VALUE };
-
-struct libscols_line * add_line(struct libscols_table * table, const std::string & key, const ::std::string & value) {
-    struct libscols_line * ln = scols_table_new_line(table, NULL);
-    scols_line_set_data(ln, COL_KEY, key.c_str());
-    scols_line_set_data(ln, COL_VALUE, value.c_str());
-    return ln;
-}
-
-}  // namespace
-
-
 PackageInfoSections::PackageInfoSections() = default;
 
 PackageInfoSections::~PackageInfoSections() = default;
 
-
-bool PackageInfoSections::add_package(
-    IPackage & pkg,
-    const std::string & heading,
-    const std::unique_ptr<PkgColorizer> & colorizer,
-    const std::vector<libdnf5::rpm::Package> & obsoletes) {
-    auto * table = p_impl->table;
-    struct libscols_line * first_line = add_line(table, "Name", pkg.get_name());
-    if (colorizer) {
-        scols_line_set_color(first_line, colorizer->get_pkg_color(pkg).c_str());
-    }
-
-    add_line(table, "Epoch", pkg.get_epoch());
-    add_line(table, "Version", pkg.get_version());
-    add_line(table, "Release", pkg.get_release());
-    add_line(table, "Architecture", pkg.get_arch());
-
-    if (!obsoletes.empty()) {
-        auto iterator = obsoletes.begin();
-        add_line(table, "Obsoletes", iterator->get_full_nevra());
-        ++iterator;
-        for (; iterator != obsoletes.end(); ++iterator) {
-            add_line(table, "", iterator->get_full_nevra());
-        }
-    }
-
-    if (!pkg.is_installed()) {
-        add_line(
-            table, "Download size", utils::units::format_size_aligned(static_cast<int64_t>(pkg.get_download_size())));
-    }
-    add_line(table, "Installed size", utils::units::format_size_aligned(static_cast<int64_t>(pkg.get_install_size())));
-    if (pkg.get_arch() != "src") {
-        add_line(table, "Source", pkg.get_sourcerpm());
-    }
-    if (pkg.is_installed()) {
-        add_line(table, "From repository", pkg.get_from_repo_id());
-    } else {
-        add_line(table, "Repository", pkg.get_repo_id());
-    }
-    add_line(table, "Summary", pkg.get_summary());
-    add_line(table, "URL", pkg.get_url());
-    add_line(table, "License", pkg.get_license());
-
-    auto lines = libdnf5::utils::string::split(pkg.get_description(), "\n");
-    auto iterator = lines.begin();
-    add_line(table, "Description", *iterator);
-    ++iterator;
-    for (; iterator != lines.end(); ++iterator) {
-        add_line(table, "", *iterator);
-    }
-    struct libscols_line * last_line = add_line(table, "Vendor", pkg.get_vendor());
-
-    // for info output keep each package as a separate section, which means
-    // an empty line is printed between packages resulting in better readability
-    p_impl->sections.emplace_back(heading, first_line, last_line);
-    return true;
-}
-
-
-void PackageInfoSections::setup_cols() {
-    scols_table_new_column(p_impl->table, "key", 1, 0);
-    scols_table_new_column(p_impl->table, "value", 1, SCOLS_FL_WRAP);
-    scols_table_set_column_separator(p_impl->table, " : ");
-}
-
-
-bool PackageInfoSections::add_section(
-    const std::string & heading,
-    const libdnf5::rpm::PackageSet & pkg_set,
-    const std::unique_ptr<PkgColorizer> & colorizer,
-    const std::map<libdnf5::rpm::PackageId, std::vector<libdnf5::rpm::Package>> & obsoletes) {
-    if (!pkg_set.empty()) {
+void PackageInfoSections::print(const std::unique_ptr<PkgColorizer> & colorizer) {
+    bool separator_needed = false;
+    for (const auto & [heading, pkg_set, obsoletes] : p_impl->sections) {
         // sort the packages in section according to NEVRA
         std::vector<libdnf5::rpm::Package> packages;
         for (const auto & pkg : pkg_set) {
             packages.emplace_back(std::move(pkg));
         }
         std::sort(packages.begin(), packages.end(), libdnf5::rpm::cmp_nevra<libdnf5::rpm::Package>);
 
-        auto tmp_heading = heading;
-        for (const auto & pkg : packages) {
-            auto obsoletes_it = obsoletes.find(pkg.get_id());
-            PackageAdapter cli_pkg(pkg);
+        if (separator_needed) {
+            std::cout << std::endl;
+        }
+
+        if (!heading.empty()) {
+            std::cout << heading << std::endl;
+        }
+
+        for (auto package : packages) {
+            libdnf5::cli::output::PackageAdapter cli_pkg(package);
+            auto obsoletes_it = obsoletes.find(package.get_id());
             if (obsoletes_it != obsoletes.end()) {
-                add_package(cli_pkg, tmp_heading, colorizer, obsoletes_it->second);
+                libdnf5::cli::output::print_package_info(cli_pkg, colorizer, obsoletes_it->second);
             } else {
-                add_package(cli_pkg, tmp_heading, colorizer, {});
+                libdnf5::cli::output::print_package_info(cli_pkg, colorizer);
             }
-            tmp_heading = "";
         }
-        return true;
-    } else {
-        return false;
+
+        separator_needed = true;
     }
+    return;
 }
 
 }  // namespace libdnf5::cli::output