Show the output of failed scriptlets to the user

dnf5/context.cpp

@@ -843,6 +843,17 @@ void RpmTransCB::cpio_error(const libdnf5::rpm::TransactionItem & item) {
     multi_progress_bar.print();
 }
 
+void RpmTransCB::script_output_to_progress(const libdnf5::cli::progressbar::MessageType message_type) {
+    auto transaction = context.get_transaction();
+    auto output = transaction->get_last_script_output();
+    if (!output.empty()) {
+        active_progress_bar->add_message(message_type, "Scriptlet output:");
+        for (auto & line : libdnf5::utils::string::split(output, "\n")) {
+            active_progress_bar->add_message(message_type, line);
+        }
+    }
+}
+
 void RpmTransCB::script_error(
     [[maybe_unused]] const libdnf5::rpm::TransactionItem * item,
     libdnf5::rpm::Nevra nevra,
@@ -855,6 +866,7 @@ void RpmTransCB::script_error(
             script_type_to_string(type),
             to_full_nevra_string(nevra),
             return_code));
+    script_output_to_progress(libdnf5::cli::progressbar::MessageType::ERROR);
     multi_progress_bar.print();
 }
 
@@ -883,7 +895,11 @@ void RpmTransCB::script_stop(
             // Print the error message here.
             active_progress_bar->add_message(
                 libdnf5::cli::progressbar::MessageType::WARNING,
-                fmt::format("Error in {} scriptlet: {}", script_type_to_string(type), to_full_nevra_string(nevra)));
+                fmt::format(
+                    "Non-critical error in {} scriptlet: {}",
+                    script_type_to_string(type),
+                    to_full_nevra_string(nevra)));
+            script_output_to_progress(libdnf5::cli::progressbar::MessageType::WARNING);
             [[fallthrough]];
         default:
             active_progress_bar->add_message(

dnf5/include/dnf5/context.hpp

@@ -259,6 +259,8 @@ class DNF_API RpmTransCB : public libdnf5::rpm::TransactionCallbacks {
 
     DNF_LOCAL static std::chrono::time_point<std::chrono::steady_clock> prev_print_time;
 
+    DNF_LOCAL void script_output_to_progress(libdnf5::cli::progressbar::MessageType message_type);
+
     libdnf5::cli::progressbar::MultiProgressBar multi_progress_bar;
     libdnf5::cli::progressbar::DownloadProgressBar * active_progress_bar{nullptr};
     Context & context;

include/libdnf5/base/transaction.hpp

@@ -193,12 +193,17 @@ class LIBDNF_API Transaction {
     void set_download_local_pkgs(bool value);
     bool get_download_local_pkgs() const noexcept;
 
+    /// Retrieve output of the last executed rpm scriptlet.
+    std::string get_last_script_output();
+
 private:
     friend class TransactionEnvironment;
     friend class TransactionGroup;
     friend class TransactionModule;
     friend class TransactionPackage;
     friend class libdnf5::Goal;
+    friend class libdnf5::rpm::
+        Transaction;  // to access clear_last_script_output() from the rpm transaction SCRIPT_START callback
 
     LIBDNF_LOCAL Transaction(const libdnf5::BaseWeakPtr & base);
 
@@ -209,6 +214,9 @@ class LIBDNF_API Transaction {
     std::optional<uint32_t> user_id;
     std::string comment;
     std::string description;
+
+    /// Clear the recorded last scriptlet output
+    LIBDNF_LOCAL void clear_last_script_output();
 };
 
 }  // namespace libdnf5::base

libdnf5/base/transaction.cpp

@@ -870,13 +870,15 @@ TransactionPackage Transaction::Impl::make_transaction_package(
 }
 
 // Reads the output of scriptlets from the file descriptor and processes them.
-static void process_scriptlets_output(int fd, Logger * logger) {
+void Transaction::Impl::process_scriptlets_output(int fd) {
+    auto logger = base->get_logger().get();
     try {
         char buf[512];
         do {
             auto len = read(fd, buf, sizeof(buf));
             if (len > 0) {
                 std::string_view str(buf, static_cast<size_t>(len));
+                append_last_script_output(str);
                 std::string_view::size_type start = 0;
                 do {
                     auto end = str.find('\n', start);
@@ -1085,13 +1087,15 @@ Transaction::TransactionRunResult Transaction::Impl::_run(
     }
 
     // This thread processes the output of RPM scriptlets.
-    std::thread thread_processes_scriptlets_output(process_scriptlets_output, pipe_out_from_scriptlets[0], logger);
+    std::thread thread_processes_scriptlets_output(
+        &Transaction::Impl::process_scriptlets_output, this, pipe_out_from_scriptlets[0]);
 
     // Set file descriptor for output of scriptlets in transaction.
     rpm_transaction.set_script_out_fd(pipe_out_from_scriptlets[1]);
     // set_script_out_fd() copies the file descriptor using dup(). Closing the original fd.
     close(pipe_out_from_scriptlets[1]);
 
+    rpm_transaction.set_base_transaction(transaction);
     rpm_transaction.set_callbacks(std::move(callbacks));
     rpm_transaction.set_flags(rpm_transaction_flags);
 
@@ -1427,6 +1431,21 @@ bool Transaction::Impl::check_gpg_signatures() {
     return result;
 }
 
+std::string Transaction::Impl::get_last_script_output() {
+    std::lock_guard<std::mutex> lock(last_script_output_mutex);
+    return last_script_output;
+}
+
+void Transaction::Impl::clear_last_script_output() {
+    std::lock_guard<std::mutex> lock(last_script_output_mutex);
+    last_script_output.clear();
+}
+
+void Transaction::Impl::append_last_script_output(std::string_view output) {
+    std::lock_guard<std::mutex> lock(last_script_output_mutex);
+    last_script_output.append(output);
+}
+
 std::string Transaction::serialize(
     const std::filesystem::path & packages_path, const std::filesystem::path & comps_path) const {
     transaction::TransactionReplay transaction_replay;
@@ -1515,4 +1534,12 @@ void Transaction::set_download_local_pkgs(bool value) {
     p_impl->download_local_pkgs = value;
 }
 
+std::string Transaction::get_last_script_output() {
+    return p_impl->get_last_script_output();
+}
+
+void Transaction::clear_last_script_output() {
+    p_impl->clear_last_script_output();
+}
+
 }  // namespace libdnf5::base

libdnf5/base/transaction_impl.hpp

@@ -37,6 +37,8 @@ along with libdnf.  If not, see <https://www.gnu.org/licenses/>.
 
 #include <solv/transaction.h>
 
+#include <mutex>
+
 
 namespace libdnf5::base {
 
@@ -91,6 +93,12 @@ class Transaction::Impl {
         const std::optional<uint32_t> user_id,
         const std::string & comment);
 
+    /// Handling of rpm scriptlets output
+    std::string get_last_script_output();
+    void clear_last_script_output();
+    void append_last_script_output(std::string_view output);
+    void process_scriptlets_output(int fd);
+
 private:
     friend Transaction;
     friend class libdnf5::Goal;
@@ -125,6 +133,9 @@ class Transaction::Impl {
     // whether also the command line repo packages should be downloaded to the destination
     bool download_local_pkgs{false};
 
+    std::string last_script_output{};
+    std::mutex last_script_output_mutex;
+
     TransactionRunResult _run(
         std::unique_ptr<libdnf5::rpm::TransactionCallbacks> && callbacks,
         const std::string & description,

libdnf5/rpm/transaction.cpp

@@ -618,6 +618,10 @@ void * Transaction::ts_callback(
         }
         case RPMCALLBACK_SCRIPT_START: {
             // amount is script tag
+            if (callbacks_holder.base_transaction) {
+                // a new scriptlet is running, clear the stored output
+                callbacks_holder.base_transaction->clear_last_script_output();
+            }
             auto script_type = rpm_tag_to_script_type(static_cast<rpmTag_e>(amount));
             auto nevra = trans_element_to_nevra(trans_element);
             logger.info(

libdnf5/rpm/transaction.hpp

@@ -281,6 +281,12 @@ class Transaction {
         callbacks_holder.callbacks = std::move(callbacks);
     }
 
+    /// Set pointer to the running libdnf5::Base::Transaction.
+    /// @param base_transaction a pointer to the transaction
+    void set_base_transaction(libdnf5::base::Transaction * base_transaction) {
+        callbacks_holder.base_transaction = base_transaction;
+    }
+
     /// Fill the RPM transaction from base::Transaction.
     /// @param transcation The base::Transaction object.
     void fill(const base::Transaction & transaction);
@@ -327,12 +333,13 @@ class Transaction {
     struct CallbacksHolder {
         std::unique_ptr<TransactionCallbacks> callbacks;
         Transaction * transaction;
+        libdnf5::base::Transaction * base_transaction;
     };
 
     BaseWeakPtr base;
     rpmts ts;
     FD_t script_fd{nullptr};
-    CallbacksHolder callbacks_holder{nullptr, this};
+    CallbacksHolder callbacks_holder{nullptr, this, nullptr};
     FD_t fd_in_cb{nullptr};  // file descriptor used by transaction in callback (install/reinstall package)
 
     TransactionItem * last_added_item{nullptr};  // item added by last install/reinstall/erase/...