From 144a5eb793c2b497b279c7a9ed0c8e394826e966 Mon Sep 17 00:00:00 2001 From: Marek Blaha Date: Fri, 15 Aug 2025 10:09:49 +0200 Subject: [PATCH 1/8] locker: Add methods to read/write lock file content Extend the Locker class to include the ability to read and write the contents of the lock file. This feature is intended to record information identifying the process that acquired the lock. --- include/libdnf5/utils/locker.hpp | 10 ++++++ libdnf5/utils/locker.cpp | 52 ++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/include/libdnf5/utils/locker.hpp b/include/libdnf5/utils/locker.hpp index adc34b6ac..5288264c0 100644 --- a/include/libdnf5/utils/locker.hpp +++ b/include/libdnf5/utils/locker.hpp @@ -44,6 +44,16 @@ class LIBDNF_API Locker { /// @throws libdnf5::SystemError if an unexpected error occurs when checking the lock state, like insufficient privileges bool write_lock(); + /// @brief Write content to the lock file + /// @param content The content to write to the lock file + /// @throws libdnf5::SystemError if the lock file is not opened or writing fails + void write_content(const std::string & content); + + /// @brief Read content from the lock file, requires acquiring a lock first. + /// @return The content of the lock file + /// @throws libdnf5::SystemError if reading fails (or if the file doesn't exist) + std::string read_content(); + /// @brief Unlock the existing lock and remove the underlying lock file /// @throws libdnf5::SystemError if an unexpected error occurs when unlocking void unlock(); diff --git a/libdnf5/utils/locker.cpp b/libdnf5/utils/locker.cpp index 1ab6fbdd9..912ade682 100644 --- a/libdnf5/utils/locker.cpp +++ b/libdnf5/utils/locker.cpp @@ -64,6 +64,58 @@ bool Locker::lock(short int type) { return true; } +void Locker::write_content(const std::string & content) { + if (lock_fd == -1) { + throw RuntimeError(M_("The lock file \"{}\" is not opened"), path); + } + + // Truncate the file first + if (ftruncate(lock_fd, 0) == -1) { + throw SystemError(errno, M_("Failed to truncate lock file \"{}\""), path); + } + if (lseek(lock_fd, 0, SEEK_SET) == -1) { + throw SystemError(errno, M_("Failed to seek lock file \"{}\""), path); + } + + // Write the content + ssize_t written = ::write(lock_fd, content.c_str(), content.size()); + if (written == -1) { + throw SystemError(errno, M_("Failed to write to lock file \"{}\""), path); + } + + // Ensure data is written to disk + if (fsync(lock_fd) == -1) { + throw SystemError(errno, M_("Failed to sync lock file \"{}\""), path); + } +} + +std::string Locker::read_content() { + if (lock_fd == -1) { + throw RuntimeError(M_("Cannot read content: no lock held on file \"{}\""), path); + } + + std::string content; + content.reserve(1024); + char buffer[1024]; + + if (lseek(lock_fd, 0, SEEK_SET) == -1) { + throw SystemError(errno, M_("Failed to seek lock file \"{}\""), path); + } + + while (true) { + auto read_bytes = ::read(lock_fd, buffer, sizeof(buffer)); + if (read_bytes == -1) { + throw SystemError(errno, M_("Failed to read from lock file \"{}\""), path); + } + if (read_bytes == 0) { + break; + } + content.append(buffer, static_cast(read_bytes)); + } + + return content; +} + void Locker::unlock() { if (lock_fd != -1) { if (close(lock_fd) == -1) { From 7d703c1660ce765806dc143b2cb2e6c6c0dc0b20 Mon Sep 17 00:00:00 2001 From: Marek Blaha Date: Mon, 18 Aug 2025 11:14:24 +0200 Subject: [PATCH 2/8] Add unit tests for the Locker class --- test/libdnf5/utils/test_locker.cpp | 131 +++++++++++++++++++++++++++++ test/libdnf5/utils/test_locker.hpp | 46 ++++++++++ 2 files changed, 177 insertions(+) create mode 100644 test/libdnf5/utils/test_locker.cpp create mode 100644 test/libdnf5/utils/test_locker.hpp diff --git a/test/libdnf5/utils/test_locker.cpp b/test/libdnf5/utils/test_locker.cpp new file mode 100644 index 000000000..3fb22e7e3 --- /dev/null +++ b/test/libdnf5/utils/test_locker.cpp @@ -0,0 +1,131 @@ +// Copyright Contributors to the DNF5 project +// SPDX-License-Identifier: GPL-2.0-or-later + + +#include "test_locker.hpp" + +#include "libdnf5/common/exception.hpp" +#include "libdnf5/utils/locker.hpp" + +#include + +#include +#include +#include + + +using namespace libdnf5::utils; + +CPPUNIT_TEST_SUITE_REGISTRATION(UtilsLockerTest); + + +void UtilsLockerTest::setUp() { + temp_dir = std::filesystem::temp_directory_path() / "libdnf5_locker_test"; + std::filesystem::create_directories(temp_dir); + lock_file_path = temp_dir + "/test.lock"; +} + + +void UtilsLockerTest::tearDown() { + std::filesystem::remove_all(temp_dir); +} + + +void UtilsLockerTest::test_constructor() { + Locker locker(lock_file_path); +} + + +void UtilsLockerTest::test_read_lock_success() { + Locker locker(lock_file_path); + CPPUNIT_ASSERT(locker.read_lock()); + CPPUNIT_ASSERT(std::filesystem::exists(lock_file_path)); +} + + +void UtilsLockerTest::test_write_lock_success() { + Locker locker(lock_file_path); + CPPUNIT_ASSERT(locker.write_lock()); + CPPUNIT_ASSERT(std::filesystem::exists(lock_file_path)); +} + + +void UtilsLockerTest::test_write_content() { + Locker locker(lock_file_path); + CPPUNIT_ASSERT(locker.write_lock()); + + const std::string test_content = "test content\nline 2\n"; + locker.write_content(test_content); + + std::ifstream file(lock_file_path); + std::string file_content((std::istreambuf_iterator(file)), std::istreambuf_iterator()); + CPPUNIT_ASSERT_EQUAL(test_content, file_content); +} + + +void UtilsLockerTest::test_read_content() { + { + std::ofstream file(lock_file_path); + file << "existing content\nwith multiple lines\n"; + } + + Locker locker(lock_file_path); + CPPUNIT_ASSERT(locker.read_lock()); + + std::string content = locker.read_content(); + CPPUNIT_ASSERT_EQUAL(std::string("existing content\nwith multiple lines\n"), content); +} + + +void UtilsLockerTest::test_read_write_content_cycle() { + Locker locker(lock_file_path); + CPPUNIT_ASSERT(locker.write_lock()); + + const std::string initial_content = "initial content"; + locker.write_content(initial_content); + + std::string read_content = locker.read_content(); + CPPUNIT_ASSERT_EQUAL(initial_content, read_content); + + const std::string updated_content = "updated content\nwith new line"; + locker.write_content(updated_content); + + read_content = locker.read_content(); + CPPUNIT_ASSERT_EQUAL(updated_content, read_content); +} + + +void UtilsLockerTest::test_unlock() { + Locker locker(lock_file_path); + CPPUNIT_ASSERT(locker.write_lock()); + CPPUNIT_ASSERT(std::filesystem::exists(lock_file_path)); + + locker.unlock(); + CPPUNIT_ASSERT(!std::filesystem::exists(lock_file_path)); + + Locker locker2(lock_file_path); + CPPUNIT_ASSERT(locker2.write_lock()); +} + + +void UtilsLockerTest::test_destructor_cleanup() { + { + Locker locker(lock_file_path); + CPPUNIT_ASSERT(locker.write_lock()); + CPPUNIT_ASSERT(std::filesystem::exists(lock_file_path)); + } + + CPPUNIT_ASSERT(!std::filesystem::exists(lock_file_path)); + + Locker locker2(lock_file_path); + CPPUNIT_ASSERT(locker2.write_lock()); +} + + +void UtilsLockerTest::test_nonexistent_directory() { + std::string invalid_path = "/nonexistent/directory/test.lock"; + Locker locker(invalid_path); + + CPPUNIT_ASSERT_THROW(locker.write_lock(), libdnf5::SystemError); + CPPUNIT_ASSERT_THROW(locker.read_lock(), libdnf5::SystemError); +} diff --git a/test/libdnf5/utils/test_locker.hpp b/test/libdnf5/utils/test_locker.hpp new file mode 100644 index 000000000..109155040 --- /dev/null +++ b/test/libdnf5/utils/test_locker.hpp @@ -0,0 +1,46 @@ +// Copyright Contributors to the DNF5 project +// SPDX-License-Identifier: GPL-2.0-or-later + + +#ifndef LIBDNF5_TEST_UTILS_LOCKER_HPP +#define LIBDNF5_TEST_UTILS_LOCKER_HPP + + +#include +#include + + +class UtilsLockerTest : public CppUnit::TestCase { + CPPUNIT_TEST_SUITE(UtilsLockerTest); + CPPUNIT_TEST(test_constructor); + CPPUNIT_TEST(test_read_lock_success); + CPPUNIT_TEST(test_write_lock_success); + CPPUNIT_TEST(test_write_content); + CPPUNIT_TEST(test_read_content); + CPPUNIT_TEST(test_read_write_content_cycle); + CPPUNIT_TEST(test_unlock); + CPPUNIT_TEST(test_destructor_cleanup); + CPPUNIT_TEST(test_nonexistent_directory); + CPPUNIT_TEST_SUITE_END(); + +public: + void setUp() override; + void tearDown() override; + + void test_constructor(); + void test_read_lock_success(); + void test_write_lock_success(); + void test_write_content(); + void test_read_content(); + void test_read_write_content_cycle(); + void test_unlock(); + void test_destructor_cleanup(); + void test_nonexistent_directory(); + +private: + std::string temp_dir; + std::string lock_file_path; +}; + + +#endif // LIBDNF5_TEST_UTILS_LOCKER_HPP From 2a870acf61cf38fbade64ff959a561f1d2d5db1a Mon Sep 17 00:00:00 2001 From: Marek Blaha Date: Fri, 15 Aug 2025 10:10:20 +0200 Subject: [PATCH 3/8] Add ActiveTransactionInfo class Introduce a new ActiveTransactionInfo class for storing transaction metadata. This metadata will be saved in the lock file, allowing other processes to provide the user with information about the currently running transaction. --- .../libdnf5/base/active_transaction_info.hpp | 67 +++++++++++++ libdnf5/base/active_transaction_info.cpp | 99 +++++++++++++++++++ 2 files changed, 166 insertions(+) create mode 100644 include/libdnf5/base/active_transaction_info.hpp create mode 100644 libdnf5/base/active_transaction_info.cpp diff --git a/include/libdnf5/base/active_transaction_info.hpp b/include/libdnf5/base/active_transaction_info.hpp new file mode 100644 index 000000000..9e38a7ddb --- /dev/null +++ b/include/libdnf5/base/active_transaction_info.hpp @@ -0,0 +1,67 @@ +// Copyright Contributors to the DNF5 project. +// SPDX-License-Identifier: LGPL-2.1-or-later + +#ifndef LIBDNF5_BASE_ACTIVE_TRANSACTION_INFO_HPP +#define LIBDNF5_BASE_ACTIVE_TRANSACTION_INFO_HPP + +#include "libdnf5/defs.h" + +#include + +#include +#include +#include + +namespace libdnf5::base { + +class LIBDNF_API ActiveTransactionInfo { +public: + ActiveTransactionInfo(); + ActiveTransactionInfo(const ActiveTransactionInfo &); + ActiveTransactionInfo(ActiveTransactionInfo &&) noexcept; + ActiveTransactionInfo & operator=(const ActiveTransactionInfo &); + ActiveTransactionInfo & operator=(ActiveTransactionInfo &&) noexcept; + ~ActiveTransactionInfo(); + + // Getters + /// @brief Get the transaction description + /// @return Description string + const std::string & get_description() const noexcept; + + /// @brief Get the transaction comment + /// @return Comment string + const std::string & get_comment() const noexcept; + + /// @brief Get the process ID of the transaction + /// @return Process ID, or -1 if not available + pid_t get_pid() const noexcept; + + /// @brief Get the transaction start time as Unix timestamp + /// @return Unix timestamp (seconds since epoch), or 0 if not available + time_t get_start_time() const noexcept; + + // Setters + void set_description(const std::string & description); + void set_comment(const std::string & comment); + void set_pid(pid_t pid); + void set_start_time(time_t start_time); + + // TOML conversion methods + /// @brief Convert ActiveTransactionInfo to TOML string format + /// @return TOML formatted string representation + /// @throws std::runtime_error if TOML formatting fails + std::string to_toml() const; + + /// @brief Create ActiveTransactionInfo from TOML string + /// @param toml_string The TOML string to parse + /// @return ActiveTransactionInfo object parsed from TOML, or empty object if parsing fails + static ActiveTransactionInfo from_toml(const std::string & toml_string); + +private: + class LIBDNF_LOCAL Impl; + std::unique_ptr p_impl; +}; + +} // namespace libdnf5::base + +#endif // LIBDNF5_BASE_ACTIVE_TRANSACTION_INFO_HPP diff --git a/libdnf5/base/active_transaction_info.cpp b/libdnf5/base/active_transaction_info.cpp new file mode 100644 index 000000000..5f548ce08 --- /dev/null +++ b/libdnf5/base/active_transaction_info.cpp @@ -0,0 +1,99 @@ +// Copyright Contributors to the DNF5 project. +// SPDX-License-Identifier: LGPL-2.1-or-later + +#include "libdnf5/base/active_transaction_info.hpp" + +#include + +#include + +namespace libdnf5::base { + +class ActiveTransactionInfo::Impl { +public: + pid_t pid = -1; + std::string description; + std::string comment; + time_t start_time = 0; +}; + +ActiveTransactionInfo::ActiveTransactionInfo() : p_impl(new Impl()) {} + +ActiveTransactionInfo::ActiveTransactionInfo(const ActiveTransactionInfo & src) : p_impl(new Impl(*src.p_impl)) {} +ActiveTransactionInfo & ActiveTransactionInfo::operator=(const ActiveTransactionInfo & src) { + if (this != &src) { + if (p_impl) { + *p_impl = *src.p_impl; + } else { + p_impl = std::make_unique(*src.p_impl); + } + } + + return *this; +} + +ActiveTransactionInfo::ActiveTransactionInfo(ActiveTransactionInfo && src) noexcept = default; +ActiveTransactionInfo & ActiveTransactionInfo::operator=(ActiveTransactionInfo && src) noexcept = default; + +ActiveTransactionInfo::~ActiveTransactionInfo() = default; + +const std::string & ActiveTransactionInfo::get_description() const noexcept { + return p_impl->description; +} + +const std::string & ActiveTransactionInfo::get_comment() const noexcept { + return p_impl->comment; +} + +pid_t ActiveTransactionInfo::get_pid() const noexcept { + return p_impl->pid; +} + +time_t ActiveTransactionInfo::get_start_time() const noexcept { + return p_impl->start_time; +} + +void ActiveTransactionInfo::set_description(const std::string & description) { + p_impl->description = description; +} + +void ActiveTransactionInfo::set_comment(const std::string & comment) { + p_impl->comment = comment; +} + +void ActiveTransactionInfo::set_pid(pid_t pid) { + p_impl->pid = pid; +} + +void ActiveTransactionInfo::set_start_time(time_t start_time) { + p_impl->start_time = start_time; +} + +std::string ActiveTransactionInfo::to_toml() const { + toml::value res; + res["description"] = get_description(); + res["comment"] = get_comment(); + res["pid"] = get_pid(); + res["start_time"] = get_start_time(); + return toml::format(res); +} + +ActiveTransactionInfo ActiveTransactionInfo::from_toml(const std::string & toml_string) { + ActiveTransactionInfo info; + + try { + auto data = toml::parse_str(toml_string); + info.set_description(toml::find_or(data, "description", "")); + info.set_comment(toml::find_or(data, "comment", "")); + info.set_pid(toml::find_or(data, "pid", -1)); + info.set_start_time(toml::find_or(data, "start_time", 0)); + } catch (const toml::syntax_error &) { + // Return default/empty ActiveTransactionInfo on TOML syntax errors + } catch (const toml::type_error &) { + // Return default/empty ActiveTransactionInfo on TOML type errors + } + + return info; +} + +} // namespace libdnf5::base From 2b0276b52d9ed66c705e04c39e6062952fd1da4a Mon Sep 17 00:00:00 2001 From: Marek Blaha Date: Fri, 15 Aug 2025 15:11:05 +0200 Subject: [PATCH 4/8] Specialized TransactionLocker class This class can write details about transaction into transaction lock file in TOML format. --- libdnf5/base/transaction.cpp | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/libdnf5/base/transaction.cpp b/libdnf5/base/transaction.cpp index 8cb210db7..c1af9547a 100644 --- a/libdnf5/base/transaction.cpp +++ b/libdnf5/base/transaction.cpp @@ -21,6 +21,8 @@ #include "rpm/transaction.hpp" #include "base_impl.hpp" + +#include "libdnf5/base/active_transaction_info.hpp" #ifdef WITH_MODULEMD #include "module/module_db.hpp" #include "module/module_sack_impl.hpp" @@ -51,6 +53,7 @@ #include #include +#include #include #include @@ -159,6 +162,28 @@ static std::vector>> get_remova return problem_output; } +class TransactionLocker : public libdnf5::utils::Locker { +public: + TransactionLocker(const std::filesystem::path & lock_path, const ActiveTransactionInfo & info); + + void write_transaction_metadata(); + +private: + ActiveTransactionInfo transaction_info; +}; + +TransactionLocker::TransactionLocker(const std::filesystem::path & path, const ActiveTransactionInfo & info) + : Locker(path.string()), + transaction_info(info) {} + +void TransactionLocker::write_transaction_metadata() { + try { + write_content(transaction_info.to_toml()); + } catch (const SystemError & e) { + //TODO(mblaha): log the error (would require base object) + } +} + } // namespace Transaction::Transaction(const BaseWeakPtr & base) : p_impl(new Impl(*this, base)) {} From ad6d5f5323f7daaa7633b6102f57d3d26cbde3a3 Mon Sep 17 00:00:00 2001 From: Marek Blaha Date: Tue, 16 Sep 2025 14:01:47 +0200 Subject: [PATCH 5/8] transaction: Use TransactionLocker instead of Locker --- libdnf5/base/transaction.cpp | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/libdnf5/base/transaction.cpp b/libdnf5/base/transaction.cpp index c1af9547a..e1f24978b 100644 --- a/libdnf5/base/transaction.cpp +++ b/libdnf5/base/transaction.cpp @@ -970,15 +970,24 @@ Transaction::TransactionRunResult Transaction::Impl::_run( auto & config = base->get_config(); + // prepare transaction info + ActiveTransactionInfo info; + info.set_description(description); + info.set_comment(comment); + info.set_pid(getpid()); + info.set_start_time( + std::chrono::duration_cast(std::chrono::system_clock::now().time_since_epoch()).count()); + // acquire the lock std::filesystem::path lock_file_path = config.get_installroot_option().get_value(); lock_file_path /= std::filesystem::path(libdnf5::TRANSACTION_LOCK_FILEPATH).relative_path(); std::filesystem::create_directories(lock_file_path.parent_path()); - libdnf5::utils::Locker locker(lock_file_path); - if (!locker.write_lock()) { + TransactionLocker transaction_locker(lock_file_path, info); + if (!transaction_locker.write_lock()) { return TransactionRunResult::ERROR_LOCK; } + transaction_locker.write_transaction_metadata(); // fill and check the rpm transaction libdnf5::rpm::Transaction rpm_transaction(base); @@ -1043,12 +1052,12 @@ Transaction::TransactionRunResult Transaction::Impl::_run( db_transaction.set_comment(comment); db_transaction.set_description(description); - if (user_id) { db_transaction.set_user_id(*user_id); } else { db_transaction.set_user_id(get_login_uid()); } + // // TODO(jrohel): nevra of running dnf5? //db_transaction.add_runtime_package("dnf5"); From 1ccd2774b9d151ba0a584cf651f9ad7d1757710a Mon Sep 17 00:00:00 2001 From: Marek Blaha Date: Tue, 16 Sep 2025 13:57:59 +0200 Subject: [PATCH 6/8] transaction: Store info about concurrently running transaction If a transaction run fails because another transaction is running concurrently, record information about that transaction so it can be used later to generate a clearer error message. --- include/libdnf5/base/transaction.hpp | 4 ++++ libdnf5/base/transaction.cpp | 9 +++++++++ libdnf5/base/transaction_impl.hpp | 5 +++++ 3 files changed, 18 insertions(+) diff --git a/include/libdnf5/base/transaction.hpp b/include/libdnf5/base/transaction.hpp index eaec8cecf..a834155ec 100644 --- a/include/libdnf5/base/transaction.hpp +++ b/include/libdnf5/base/transaction.hpp @@ -23,6 +23,7 @@ #include "transaction_errors.hpp" +#include "libdnf5/base/active_transaction_info.hpp" #include "libdnf5/base/base_weak.hpp" #include "libdnf5/base/goal_elements.hpp" #include "libdnf5/base/log_event.hpp" @@ -197,6 +198,9 @@ class LIBDNF_API Transaction { /// Retrieve captured RPM log messages std::vector get_rpm_messages(); + /// Retrieve metadat of concurrently running transaction + const ActiveTransactionInfo * get_concurrent_transaction() const noexcept; + private: friend class TransactionEnvironment; friend class TransactionGroup; diff --git a/libdnf5/base/transaction.cpp b/libdnf5/base/transaction.cpp index e1f24978b..0b563a583 100644 --- a/libdnf5/base/transaction.cpp +++ b/libdnf5/base/transaction.cpp @@ -38,6 +38,7 @@ #include "transaction_package_impl.hpp" #include "utils/string.hpp" +#include "libdnf5/base/active_transaction_info.hpp" #include "libdnf5/base/base.hpp" #include "libdnf5/common/exception.hpp" #include "libdnf5/common/sack/exclude_flags.hpp" @@ -954,6 +955,8 @@ Transaction::TransactionRunResult Transaction::Impl::_run( const std::optional user_id, const std::string & comment, const bool test_only) { + concurrent_transaction.reset(); + // do not allow running a transaction multiple times if (history_db_id > 0) { return TransactionRunResult::ERROR_RERUN; @@ -985,6 +988,8 @@ Transaction::TransactionRunResult Transaction::Impl::_run( TransactionLocker transaction_locker(lock_file_path, info); if (!transaction_locker.write_lock()) { + auto content = transaction_locker.read_content(); + concurrent_transaction = std::make_unique(ActiveTransactionInfo::from_toml(content)); return TransactionRunResult::ERROR_LOCK; } transaction_locker.write_transaction_metadata(); @@ -1605,4 +1610,8 @@ void Transaction::set_rpm_messages(std::vector && rpm_messages) { p_impl->set_rpm_messages(std::move(rpm_messages)); } +const ActiveTransactionInfo * Transaction::get_concurrent_transaction() const noexcept { + return p_impl->concurrent_transaction.get(); +} + } // namespace libdnf5::base diff --git a/libdnf5/base/transaction_impl.hpp b/libdnf5/base/transaction_impl.hpp index 2af723037..8aa53e1bb 100644 --- a/libdnf5/base/transaction_impl.hpp +++ b/libdnf5/base/transaction_impl.hpp @@ -26,6 +26,7 @@ #endif #include "rpm/solv/goal_private.hpp" +#include "libdnf5/base/active_transaction_info.hpp" #include "libdnf5/base/transaction.hpp" #include "libdnf5/base/transaction_environment.hpp" #include "libdnf5/base/transaction_group.hpp" @@ -158,6 +159,10 @@ class Transaction::Impl { std::unordered_map rpm_reason_overrides; // Used during transaction replay to verify no extra packages were pulled into the transaction std::vector, GoalJobSettings>> rpm_replays_nevra_cache; + + // If a transaction run fails due to a concurrent transaction, + // store information about that concurrent transaction. + std::unique_ptr concurrent_transaction{nullptr}; }; From 6b96e140c63d333224967cf25165b3251cf9172f Mon Sep 17 00:00:00 2001 From: Marek Blaha Date: Wed, 10 Sep 2025 12:42:36 +0200 Subject: [PATCH 7/8] dnf5: Print details about concurrent running transaction --- dnf5/context.cpp | 66 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/dnf5/context.cpp b/dnf5/context.cpp index da710297f..666a05b73 100644 --- a/dnf5/context.cpp +++ b/dnf5/context.cpp @@ -42,6 +42,7 @@ #include #include #include +#include #include #include @@ -60,6 +61,47 @@ namespace { constexpr const char * STORED_REPO_PREFIX = "@stored_transaction"; +// Helper function to format active transaction information for error messages +std::string format_active_transaction_info(const libdnf5::base::ActiveTransactionInfo & info) { + std::string result; + + if (!info.get_description().empty()) { + result += libdnf5::utils::sformat(_("Command: {}"), info.get_description()); + } + + if (info.get_pid() > 0) { + if (!result.empty()) { + result += "\n"; + } + if (kill(info.get_pid(), 0) == 0) { + result += libdnf5::utils::sformat(_("Process ID: {} (still running)"), info.get_pid()); + } else { + result += libdnf5::utils::sformat(_("Process ID: {}"), info.get_pid()); + } + } + + if (info.get_start_time() > 0) { + if (!result.empty()) { + result += "\n"; + } + result += + libdnf5::utils::sformat(_("Started: {}"), libdnf5::utils::string::format_epoch(info.get_start_time())); + } + + if (!info.get_comment().empty()) { + if (!result.empty()) { + result += "\n"; + } + result += libdnf5::utils::sformat(_("Comment: {}"), info.get_comment()); + } + + if (!result.empty()) { + result = _("Details about the currently running transaction:\n") + result; + } + + return result; +} + // The `KeyImportRepoCB` class implements callback only for importing repository key. class KeyImportRepoCB : public libdnf5::repo::RepoCallbacks2_1 { public: @@ -379,6 +421,18 @@ void Context::Impl::store_offline(libdnf5::base::Transaction & transaction) { if (result != libdnf5::base::Transaction::TransactionRunResult::SUCCESS) { print_error(libdnf5::utils::sformat( _("Transaction failed: {}"), libdnf5::base::Transaction::transaction_result_to_string(result))); + + // For transaction lock errors, provide detailed information about the running transaction + if (result == libdnf5::base::Transaction::TransactionRunResult::ERROR_LOCK) { + auto * active_info = transaction.get_concurrent_transaction(); + if (active_info) { + auto info_str = format_active_transaction_info(*active_info); + if (!info_str.empty()) { + print_error(info_str); + } + } + } + for (auto const & entry : transaction.get_gpg_signature_problems()) { print_error(entry); } @@ -532,6 +586,18 @@ void Context::Impl::download_and_run(libdnf5::base::Transaction & transaction) { if (result != libdnf5::base::Transaction::TransactionRunResult::SUCCESS) { print_error(libdnf5::utils::sformat( _("Transaction failed: {}"), libdnf5::base::Transaction::transaction_result_to_string(result))); + + // For transaction lock errors, provide detailed information about the running transaction + if (result == libdnf5::base::Transaction::TransactionRunResult::ERROR_LOCK) { + auto * active_info = transaction.get_concurrent_transaction(); + if (active_info) { + auto info_str = format_active_transaction_info(*active_info); + if (!info_str.empty()) { + print_error(info_str); + } + } + } + for (auto const & entry : transaction.get_gpg_signature_problems()) { print_error(entry); } From d4bf88e7519f1e4080fa1320e9a01eb57c6291d7 Mon Sep 17 00:00:00 2001 From: Marek Blaha Date: Wed, 24 Sep 2025 14:41:13 +0200 Subject: [PATCH 8/8] Handle TOML parsing errors for ActiveTransactionInfo Errors are no longer silently ignored, a new ActiveTransactionInfoParseError is thrown instead. The error is caught and logged during the transaction run. --- include/libdnf5/base/active_transaction_info.hpp | 11 ++++++++++- libdnf5/base/active_transaction_info.cpp | 10 ++++++---- libdnf5/base/transaction.cpp | 10 ++++++---- 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/include/libdnf5/base/active_transaction_info.hpp b/include/libdnf5/base/active_transaction_info.hpp index 9e38a7ddb..c70459a22 100644 --- a/include/libdnf5/base/active_transaction_info.hpp +++ b/include/libdnf5/base/active_transaction_info.hpp @@ -4,6 +4,7 @@ #ifndef LIBDNF5_BASE_ACTIVE_TRANSACTION_INFO_HPP #define LIBDNF5_BASE_ACTIVE_TRANSACTION_INFO_HPP +#include "libdnf5/common/exception.hpp" #include "libdnf5/defs.h" #include @@ -14,6 +15,13 @@ namespace libdnf5::base { +class LIBDNF_API ActiveTransactionInfoParseError : public libdnf5::Error { +public: + using libdnf5::Error::Error; + const char * get_domain_name() const noexcept override { return "libdnf5::base"; } + const char * get_name() const noexcept override { return "ActiveTransactionInfoParseError"; } +}; + class LIBDNF_API ActiveTransactionInfo { public: ActiveTransactionInfo(); @@ -54,7 +62,8 @@ class LIBDNF_API ActiveTransactionInfo { /// @brief Create ActiveTransactionInfo from TOML string /// @param toml_string The TOML string to parse - /// @return ActiveTransactionInfo object parsed from TOML, or empty object if parsing fails + /// @return ActiveTransactionInfo object parsed from TOML + /// @throws ActiveTransactionInfoParseError if TOML parsing fails static ActiveTransactionInfo from_toml(const std::string & toml_string); private: diff --git a/libdnf5/base/active_transaction_info.cpp b/libdnf5/base/active_transaction_info.cpp index 5f548ce08..e1512723d 100644 --- a/libdnf5/base/active_transaction_info.cpp +++ b/libdnf5/base/active_transaction_info.cpp @@ -3,6 +3,8 @@ #include "libdnf5/base/active_transaction_info.hpp" +#include "libdnf5/utils/bgettext/bgettext-mark-domain.h" + #include #include @@ -87,10 +89,10 @@ ActiveTransactionInfo ActiveTransactionInfo::from_toml(const std::string & toml_ info.set_comment(toml::find_or(data, "comment", "")); info.set_pid(toml::find_or(data, "pid", -1)); info.set_start_time(toml::find_or(data, "start_time", 0)); - } catch (const toml::syntax_error &) { - // Return default/empty ActiveTransactionInfo on TOML syntax errors - } catch (const toml::type_error &) { - // Return default/empty ActiveTransactionInfo on TOML type errors + } catch (const toml::syntax_error & ex) { + throw ActiveTransactionInfoParseError(M_("Error parsing transaction info: {}"), std::string(ex.what())); + } catch (const toml::type_error & ex) { + throw ActiveTransactionInfoParseError(M_("Error parsing transaction info: {}"), std::string(ex.what())); } return info; diff --git a/libdnf5/base/transaction.cpp b/libdnf5/base/transaction.cpp index 0b563a583..9b1be8072 100644 --- a/libdnf5/base/transaction.cpp +++ b/libdnf5/base/transaction.cpp @@ -986,10 +986,15 @@ Transaction::TransactionRunResult Transaction::Impl::_run( lock_file_path /= std::filesystem::path(libdnf5::TRANSACTION_LOCK_FILEPATH).relative_path(); std::filesystem::create_directories(lock_file_path.parent_path()); + auto logger = base->get_logger().get(); TransactionLocker transaction_locker(lock_file_path, info); if (!transaction_locker.write_lock()) { auto content = transaction_locker.read_content(); - concurrent_transaction = std::make_unique(ActiveTransactionInfo::from_toml(content)); + try { + concurrent_transaction = std::make_unique(ActiveTransactionInfo::from_toml(content)); + } catch (const ActiveTransactionInfoParseError & ex) { + logger->warning(ex.what()); + } return TransactionRunResult::ERROR_LOCK; } transaction_locker.write_transaction_metadata(); @@ -1105,9 +1110,6 @@ Transaction::TransactionRunResult Transaction::Impl::_run( db_transaction.set_dt_start(std::chrono::duration_cast(time).count()); db_transaction.start(); - - auto logger = base->get_logger().get(); - int pipe_out_from_scriptlets[2]; if (pipe2(pipe_out_from_scriptlets, O_CLOEXEC) == -1) { logger->error("Transaction::Run: Cannot create pipe: {}", std::strerror(errno));