Show the output of failed scriptlets to the user

dnf5/context.cpp

@@ -843,6 +843,27 @@ void RpmTransCB::cpio_error(const libdnf5::rpm::TransactionItem & item) {
  multi_progress_bar.print();
 }
 
+void RpmTransCB::script_output(
+ [[maybe_unused]] const libdnf5::rpm::TransactionItem * item,
+ [[maybe_unused]] libdnf5::rpm::Nevra nevra,
+ [[maybe_unused]] libdnf5::rpm::TransactionCallbacks::ScriptType type,
+ uint64_t return_code,
+ const std::string output) {
+ if (!output.empty() && return_code != RPMRC_OK) {
+ libdnf5::cli::progressbar::MessageType msg_type;
+ if (return_code == RPMRC_NOTFOUND) {
+ msg_type = libdnf5::cli::progressbar::MessageType::WARNING;
+ } else {
+ msg_type = libdnf5::cli::progressbar::MessageType::ERROR;
+ }
+ active_progress_bar->add_message(msg_type, "Scriptlet output:");
+ for (auto & line : libdnf5::utils::string::split(output, "\n")) {
+ active_progress_bar->add_message(msg_type, line);
+ }
+ multi_progress_bar.print();
+ }
+}
+
 void RpmTransCB::script_error(
  [[maybe_unused]] const libdnf5::rpm::TransactionItem * item,
  libdnf5::rpm::Nevra nevra,
@@ -883,7 +904,10 @@ 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)));
  [[fallthrough]];
  default:
  active_progress_bar->add_message(

dnf5/include/dnf5/context.hpp

@@ -224,6 +224,13 @@ class DNF_API RpmTransCB : public libdnf5::rpm::TransactionCallbacks {
 
  void cpio_error(const libdnf5::rpm::TransactionItem & item) override;
 
+ void script_output(
+ [[maybe_unused]] const libdnf5::rpm::TransactionItem * item,
+ libdnf5::rpm::Nevra nevra,
+ libdnf5::rpm::TransactionCallbacks::ScriptType type,
+ uint64_t return_code,
+ const std::string output) override;
+
  void script_error(
  [[maybe_unused]] const libdnf5::rpm::TransactionItem * item,
  libdnf5::rpm::Nevra nevra,

include/libdnf5/rpm/transaction_callbacks.hpp

@@ -92,6 +92,14 @@ class LIBDNF_API TransactionCallbacks {
  virtual void script_error(const TransactionItem * item, Nevra nevra, ScriptType type, uint64_t return_code);
  virtual void script_start(const TransactionItem * item, Nevra nevra, ScriptType type);
  virtual void script_stop(const TransactionItem * item, Nevra nevra, ScriptType type, uint64_t return_code);
+ /// Called after the scriptlet finished
+ /// @param item Transaction item
+ /// @param nevra NEVRA of the package the scriptlet belongs to
+ /// @param type Type of the scriptlet
+ /// @param return_code Return code of the scriptlet: OK, non-critical error, error
+ /// @param output Output produced by the scriptlet
+ virtual void script_output(
+ const TransactionItem * item, Nevra nevra, ScriptType type, uint64_t return_code, const std::string output);
  virtual void elem_progress(const TransactionItem & item, uint64_t amount, uint64_t total);
  virtual void verify_progress(uint64_t amount, uint64_t total);
  virtual void verify_start(uint64_t total);

libdnf5/base/transaction.cpp

@@ -49,13 +49,10 @@ along with libdnf. If not, see <https://www.gnu.org/licenses/>.
 #include "libdnf5/utils/locker.hpp"
 
 #include <fmt/format.h>
-#include <unistd.h>
 
 #include <filesystem>
 #include <iostream>
 #include <ranges>
-#include <string_view>
-#include <thread>
 
 
 namespace libdnf5::base {
@@ -869,37 +866,6 @@ TransactionPackage Transaction::Impl::make_transaction_package(
  return tspkg;
 }
 
-// Reads the output of scriptlets from the file descriptor and processes them.
-static void process_scriptlets_output(int fd, Logger * logger) {
- 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));
- std::string_view::size_type start = 0;
- do {
- auto end = str.find('\n', start);
- logger->info("[scriptlet] {}", str.substr(start, end - start));
- if (end == std::string_view::npos) {
- break;
- }
- start = end + 1;
- } while (start < str.size());
- } else {
- if (len == -1) {
- logger->error("Transaction::Run: Cannot read scriptlet output from pipe: {}", std::strerror(errno));
- }
- break;
- }
- } while (true);
- } catch (const std::exception & ex) {
- // The thread must not throw exceptions.
- logger->error("Transaction::Run: Exception while processing scriptlet output: {}", ex.what());
- }
- close(fd);
-}
-
 static bool contains_any_inbound_package(std::vector<TransactionPackage> & packages) {
  for (const auto & package : packages) {
  if (transaction_item_action_is_inbound(package.get_action())) {
@@ -1078,31 +1044,12 @@ Transaction::TransactionRunResult Transaction::Impl::_run(
  }
 #endif
 
- int pipe_out_from_scriptlets[2];
- if (pipe(pipe_out_from_scriptlets) == -1) {
- logger->error("Transaction::Run: Cannot create pipe: {}", std::strerror(errno));
- return TransactionRunResult::ERROR_RPM_RUN;
- }
-
- // This thread processes the output of RPM scriptlets.
- std::thread thread_processes_scriptlets_output(process_scriptlets_output, pipe_out_from_scriptlets[0], logger);
-
- // 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_callbacks(std::move(callbacks));
  rpm_transaction.set_flags(rpm_transaction_flags);
 
  // execute rpm transaction
  ret = rpm_transaction.run();
 
- // Reset/close file descriptor for output of RPM scriptlets. Required to end thread_processes_scriptlets_output.
- rpm_transaction.set_script_out_fd(-1);
-
- thread_processes_scriptlets_output.join();
-
  // TODO(mblaha): Handle ret == -1 and ret > 0, fill problems list
 
  if (ret == 0) {

libdnf5/rpm/transaction.cpp

@@ -29,16 +29,22 @@ along with libdnf. If not, see <https://www.gnu.org/licenses/>.
 
 #include <fcntl.h>
 #include <fmt/format.h>
+#include <libdnf5/logger/log_router.hpp>
 #include <rpm/rpmbuild.h>
 #include <rpm/rpmdb.h>
 #include <rpm/rpmlib.h>
 #include <rpm/rpmpgp.h>
 #include <rpm/rpmtag.h>
 #include <sys/stat.h>
 #include <sys/types.h>
+#include <unistd.h>
 
+#include <cstring>
+#include <exception>
 #include <map>
-
+#include <mutex>
+#include <string_view>
+#include <thread>
 
 namespace libdnf5::rpm {
 
@@ -198,6 +204,41 @@ void Transaction::fill(const base::Transaction & transaction) {
  };
 }
 
+void Transaction::process_scriptlets_output(int fd, Transaction * transaction) {
+ auto logger = transaction->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));
+ {
+ std::lock_guard<std::mutex> lock(transaction->last_script_output_mutex);
+ transaction->last_script_output.append(str);
+ }
+ std::string_view::size_type start = 0;
+ do {
+ auto end = str.find('\n', start);
+ logger->info("[scriptlet] {}", str.substr(start, end - start));
+ if (end == std::string_view::npos) {
+ break;
+ }
+ start = end + 1;
+ } while (start < str.size());
+ } else {
+ if (len == -1) {
+ logger->error("Transaction::Run: Cannot read scriptlet output from pipe: {}", std::strerror(errno));
+ }
+ break;
+ }
+ } while (true);
+ } catch (const std::exception & ex) {
+ // The thread must not throw exceptions.
+ logger->error("Transaction::Run: Exception while processing scriptlet output: {}", ex.what());
+ }
+ close(fd);
+}
+
 int Transaction::run() {
  rpmprobFilterFlags ignore_set = RPMPROB_FILTER_NONE;
  if (downgrade_requested) {
@@ -206,6 +247,25 @@ int Transaction::run() {
  if (base->get_config().get_ignorearch_option().get_value()) {
  ignore_set |= RPMPROB_FILTER_IGNOREARCH;
  }
+ // do not process the scriptlets output for test transactions
+ bool catch_scriptlet_output = (get_flags() & RPMTRANS_FLAG_TEST) != RPMTRANS_FLAG_TEST;
+ auto logger = base->get_logger().get();
+ int pipe_out_from_scriptlets[2];
+ std::thread thread_processes_scriptlets_output;
+ if (catch_scriptlet_output) {
+ if (pipe(pipe_out_from_scriptlets) == -1) {
+ logger->error("Transaction::Run: Cannot create pipe: {}", std::strerror(errno));
+ return -1;
+ }
+
+ // This thread processes the output of RPM scriptlets.
+ thread_processes_scriptlets_output = std::thread(process_scriptlets_output, pipe_out_from_scriptlets[0], this);
+
+ // Set file descriptor for output of scriptlets in 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]);
+ }
  rpmtsSetNotifyStyle(ts, 1);
  rpmtsSetNotifyCallback(ts, ts_callback, &callbacks_holder);
  auto * const callbacks = callbacks_holder.callbacks.get();
@@ -220,6 +280,12 @@ int Transaction::run() {
  }
  rpmtsSetNotifyCallback(ts, nullptr, nullptr);
 
+ if (catch_scriptlet_output) {
+ // Reset/close file descriptor for output of RPM scriptlets. Required to end thread_processes_scriptlets_output.
+ set_script_out_fd(-1);
+ thread_processes_scriptlets_output.join();
+ }
+
  return rc;
 }
 
@@ -606,6 +672,8 @@ void * Transaction::ts_callback(
  to_full_nevra_string(nevra),
  total);
  if (callbacks) {
+ std::lock_guard<std::mutex> lock(transaction.last_script_output_mutex);
+ callbacks->script_output(item, nevra, script_type, return_code, transaction.last_script_output);
  callbacks->script_error(item, nevra, script_type, return_code);
  }
  } else {
@@ -627,6 +695,8 @@ void * Transaction::ts_callback(
  if (callbacks) {
  callbacks->script_start(item, nevra, script_type);
  }
+ std::lock_guard<std::mutex> lock(transaction.last_script_output_mutex);
+ transaction.last_script_output = "";
  break;
  }
  case RPMCALLBACK_SCRIPT_STOP: {
@@ -640,6 +710,7 @@ void * Transaction::ts_callback(
  to_full_nevra_string(nevra),
  total);
  if (callbacks) {
+ callbacks->script_output(item, nevra, script_type, total, transaction.last_script_output);
  callbacks->script_stop(item, nevra, script_type, total);
  }
  break;

libdnf5/rpm/transaction.hpp

@@ -342,6 +342,10 @@ class Transaction {
  bool downgrade_requested{false};
  std::vector<TransactionItem> transaction_items;
 
+ /// The outputs of the last executed scriptlet is stored here
+ std::string last_script_output;
+ std::mutex last_script_output_mutex;
+
  RpmLogGuard rpm_log_guard;
 
 
@@ -426,6 +430,9 @@ class Transaction {
  const rpm_loff_t total,
  [[maybe_unused]] const void * pkg_key,
  rpmCallbackData data);
+
+ /// Reads the output of scriptlets from the file descriptor and processes them.
+ static void process_scriptlets_output(int fd, Transaction * transaction);
 };
 
 } // namespace libdnf5::rpm

libdnf5/rpm/transaction_callbacks.cpp

@@ -85,6 +85,8 @@ void TransactionCallbacks::script_start(const TransactionItem *, Nevra, ScriptTy
 
 void TransactionCallbacks::script_stop(const TransactionItem *, Nevra, ScriptType, uint64_t) {}
 
+void TransactionCallbacks::script_output(const TransactionItem *, Nevra, ScriptType, uint64_t, const std::string) {}
+
 void TransactionCallbacks::elem_progress(const TransactionItem &, uint64_t, uint64_t) {}
 
 void TransactionCallbacks::verify_progress(uint64_t, uint64_t) {}