From 5fd8cf4fb8db1d65b6a20d2777bad22647e7f5e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Kundr=C3=A1t?= Date: Thu, 23 Oct 2025 16:04:10 +0200 Subject: [PATCH 7/9] Port to libyang v4 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Organization: Wires I'm rather disappointed with the upstream changes that implement that shared libyang "printed context" mapped to a fixed address, I think it's effectively a step backwards. Upstream is happy with their solution, so here we are. Change-Id: I6b89422ec66bc694dc2c6ecd0d40aa1e7f76a741 Depends-on: https://gerrit.cesnet.cz/c/CzechLight/libyang-cpp/+/8984 Signed-off-by: Mattias Walström --- .zuul.yaml | 8 +++--- CMakeLists.txt | 6 ++--- ci/build.sh | 4 +++ include/sysrepo-cpp/Enum.hpp | 18 ++++++++++--- include/sysrepo-cpp/utils/utils.hpp | 5 ++++ src/Connection.cpp | 3 +-- src/Session.cpp | 2 +- src/utils/enum.hpp | 18 +++++++------ src/utils/utils.cpp | 18 +++++++++++++ tests/session.cpp | 39 +++++++++++++++++++++-------- 10 files changed, 90 insertions(+), 31 deletions(-) diff --git a/.zuul.yaml b/.zuul.yaml index cef174f..7ada6f7 100644 --- a/.zuul.yaml +++ b/.zuul.yaml @@ -4,9 +4,9 @@ - f38-gcc-cover: required-projects: - name: github/CESNET/libyang - override-checkout: cesnet/2025-07-08 + override-checkout: devel - name: github/sysrepo/sysrepo - override-checkout: cesnet/2025-06-02 + override-checkout: devel - name: github/doctest/doctest override-checkout: v2.4.8 - name: github/rollbear/trompeloeil @@ -15,9 +15,9 @@ - f38-clang-asan-ubsan: required-projects: &projects - name: github/CESNET/libyang - override-checkout: cesnet/2025-07-08 + override-checkout: devel - name: github/sysrepo/sysrepo - override-checkout: cesnet/2025-06-02 + override-checkout: devel - name: github/doctest/doctest override-checkout: v2.4.11 - name: github/rollbear/trompeloeil diff --git a/CMakeLists.txt b/CMakeLists.txt index f2a9b09..49338dd 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -19,7 +19,7 @@ add_custom_target(sysrepo-cpp-version-cmake cmake/ProjectGitVersionRunner.cmake ) include(cmake/ProjectGitVersion.cmake) -set(SYSREPO_CPP_PKG_VERSION "6") +set(SYSREPO_CPP_PKG_VERSION "7") prepare_git_version(SYSREPO_CPP_VERSION ${SYSREPO_CPP_PKG_VERSION}) find_package(Doxygen) @@ -27,8 +27,8 @@ option(WITH_DOCS "Create and install internal documentation (needs Doxygen)" ${D option(WITH_EXAMPLES "Build examples" ON) find_package(PkgConfig) -pkg_check_modules(LIBYANG_CPP REQUIRED libyang-cpp>=3 IMPORTED_TARGET) -pkg_check_modules(SYSREPO REQUIRED sysrepo>=3.7.4 IMPORTED_TARGET) +pkg_check_modules(LIBYANG_CPP REQUIRED libyang-cpp>=5 IMPORTED_TARGET) +pkg_check_modules(SYSREPO REQUIRED sysrepo>=4.2.4 IMPORTED_TARGET) include_directories(${CMAKE_CURRENT_SOURCE_DIR}/include) diff --git a/ci/build.sh b/ci/build.sh index 785a811..67bc7d3 100755 --- a/ci/build.sh +++ b/ci/build.sh @@ -47,6 +47,10 @@ if [[ $ZUUL_JOB_NAME =~ .*-tsan ]]; then # Our TSAN does not have interceptors for a variety of "less common" functions such as pthread_mutex_clocklock. # Disable all functions which are optional in sysrepo/libnetconf2/Netopeer2. CMAKE_OPTIONS="${CMAKE_OPTIONS} -DHAVE_PTHREAD_MUTEX_TIMEDLOCK=OFF -DHAVE_PTHREAD_MUTEX_CLOCKLOCK=OFF -DHAVE_PTHREAD_RWLOCK_CLOCKRDLOCK=OFF -DHAVE_PTHREAD_RWLOCK_CLOCKWRLOCK=OFF -DHAVE_PTHREAD_COND_CLOCKWAIT=OFF" + + # This is currently broken on TSAN ("Failed to map the printed context (Operation not permitted)."). + # Even if it wasn't broken, I want at least one build in the matrix with this (mis)feature disabled. + CMAKE_OPTIONS="${CMAKE_OPTIONS} -DPRINTED_CONTEXT_ADDRESS=0" fi if [[ $ZUUL_JOB_NAME =~ .*-cover.* ]]; then diff --git a/include/sysrepo-cpp/Enum.hpp b/include/sysrepo-cpp/Enum.hpp index 48467c6..505797a 100644 --- a/include/sysrepo-cpp/Enum.hpp +++ b/include/sysrepo-cpp/Enum.hpp @@ -176,12 +176,24 @@ enum class NotificationType : uint32_t { }; /** - * Wraps `sr_conn_flag_e`. + * Wraps `sr_context_flag_t`. + */ +enum class ContextFlags : uint32_t { + Default = 0x00, /**< SR_CTX_DEFAULT */ + NoPrinted = 0x01, /**< SR_CTX_NO_PRINTED */ + LibYangPrivParsed = 0x02, /**< SR_CTX_SET_PRIV_PARSED */ +}; + +constexpr ContextFlags operator|(const ContextFlags a, const ContextFlags b) +{ + return implEnumBitOr(a, b); +} + +/** + * Wraps `sr_conn_flag_t`. */ enum class ConnectionFlags : uint32_t { Default = 0x00, /**< SR_CONN_DEFAULT */ - CacheRunning = 0x01, /**< SR_CONN_CACHE_RUNNING */ - LibYangPrivParsed = 0x02, /**< SR_CONN_CTX_SET_PRIV_PARSED */ }; constexpr ConnectionFlags operator|(const ConnectionFlags a, const ConnectionFlags b) diff --git a/include/sysrepo-cpp/utils/utils.hpp b/include/sysrepo-cpp/utils/utils.hpp index 64b1c2e..6f2446f 100644 --- a/include/sysrepo-cpp/utils/utils.hpp +++ b/include/sysrepo-cpp/utils/utils.hpp @@ -12,6 +12,11 @@ namespace sysrepo { Session wrapUnmanagedSession(sr_session_ctx_s* session); void setLogLevelStderr(const LogLevel); +enum class GlobalContextEffect { + Lazy, /**< Set these flags the next time the shared context is applied */ + Immediate, /**< Apply immediately */ +}; +ContextFlags setGlobalContextOptions(const ContextFlags flags, const GlobalContextEffect when); std::optional findMatchingDiscard(libyang::DataNode root, const std::string& xpath); std::vector findMatchingDiscardPrefixes(libyang::DataNode root, const std::string& xpathPrefix); void unlinkFromForest(std::optional& root, libyang::DataNode node); diff --git a/src/Connection.cpp b/src/Connection.cpp index 1717b9a..96e340c 100644 --- a/src/Connection.cpp +++ b/src/Connection.cpp @@ -26,7 +26,7 @@ Connection::Connection(const ConnectionFlags options) : ctx(nullptr) { sr_conn_ctx_t* ctx; - auto res = sr_connect(static_cast(options), &ctx); + auto res = sr_connect(static_cast(options), &ctx); throwIfError(res, "Couldn't connect to sysrepo"); this->ctx = std::shared_ptr(ctx, sr_disconnect); @@ -96,5 +96,4 @@ uint32_t Connection::getId() const { return sr_get_cid(ctx.get()); } - } diff --git a/src/Session.cpp b/src/Session.cpp index 994ce87..95b8f24 100644 --- a/src/Session.cpp +++ b/src/Session.cpp @@ -241,7 +241,7 @@ std::optional Session::operationalChanges(const std::optional void Session::discardOperationalChanges(const std::optional& moduleName, std::chrono::milliseconds timeout) { SYSREPO_CPP_SESSION_MTX; - auto res = sr_discard_oper_changes(nullptr, m_sess.get(), moduleName ? nullptr : moduleName->c_str(), timeout.count()); + auto res = sr_discard_oper_changes(m_sess.get(), moduleName ? nullptr : moduleName->c_str(), timeout.count()); throwIfError(res, "Session::discardOoperationalChanges: Couldn't discard "s + (moduleName ? "for module \"" + *moduleName + "\"" : "globally"s), m_sess.get()); } diff --git a/src/utils/enum.hpp b/src/utils/enum.hpp index 2d3a298..e917b21 100644 --- a/src/utils/enum.hpp +++ b/src/utils/enum.hpp @@ -151,13 +151,16 @@ static_assert(toNotificationType(SR_EV_NOTIF_MODIFIED) == NotificationType::Modi static_assert(toNotificationType(SR_EV_NOTIF_SUSPENDED) == NotificationType::Suspended); static_assert(toNotificationType(SR_EV_NOTIF_RESUMED) == NotificationType::Resumed); -static_assert(std::is_same_v>); +static_assert(std::is_same_v, std::underlying_type_t>); static_assert(static_cast(SR_CONN_DEFAULT) == ConnectionFlags::Default); -static_assert(static_cast(SR_CONN_CACHE_RUNNING) == ConnectionFlags::CacheRunning); -static_assert(static_cast(SR_CONN_CTX_SET_PRIV_PARSED) == ConnectionFlags::LibYangPrivParsed); -static_assert(static_cast(SR_CONN_CACHE_RUNNING | SR_CONN_CTX_SET_PRIV_PARSED) == (ConnectionFlags::CacheRunning | ConnectionFlags::LibYangPrivParsed)); -static_assert(std::is_same_v>); +static_assert(std::is_same_v, std::underlying_type_t>); +static_assert(static_cast(SR_CTX_DEFAULT) == ContextFlags::Default); +static_assert(static_cast(SR_CTX_SET_PRIV_PARSED) == ContextFlags::LibYangPrivParsed); +static_assert(static_cast(SR_CTX_NO_PRINTED) == ContextFlags::NoPrinted); +static_assert(static_cast(SR_CTX_SET_PRIV_PARSED | SR_CTX_NO_PRINTED) == (ContextFlags::LibYangPrivParsed | ContextFlags::NoPrinted)); + +// upstream project provides no dedicated type for these static_assert(static_cast(SR_OPER_DEFAULT) == GetOptions::Default); static_assert(static_cast(SR_OPER_NO_STATE) == GetOptions::OperNoState); static_assert(static_cast(SR_OPER_NO_CONFIG) == GetOptions::OperNoConfig); @@ -168,9 +171,10 @@ static_assert(static_cast(SR_OPER_NO_POLL_CACHED) == GetOptions::Ope static_assert(static_cast(SR_OPER_NO_RUN_CACHED) == GetOptions::OperNoRunningCached); static_assert(static_cast(SR_GET_NO_FILTER) == GetOptions::NoFilter); -constexpr sr_get_options_t toGetOptions(const GetOptions opts) +// upstream project provides no dedicated type for these +constexpr uint32_t toGetOptions(const GetOptions opts) { - return static_cast(opts); + return static_cast(opts); } static_assert(static_cast(SRSN_YP_CHANGE_CREATE) == YangPushChange::Create); diff --git a/src/utils/utils.cpp b/src/utils/utils.cpp index 61d0c7f..dbbc5ce 100644 --- a/src/utils/utils.cpp +++ b/src/utils/utils.cpp @@ -13,6 +13,7 @@ extern "C" { #include } #include "enum.hpp" +#include "exception.hpp" #include "misc.hpp" namespace sysrepo { @@ -37,6 +38,23 @@ void setLogLevelStderr(const LogLevel level) sr_log_stderr(toLogLevel(level)); } +/** + * @brief Set global sysrepo-level context options + * + * Be advised of consequences of manipulating a shared global state, especially when using multiple connections. + * + * Wraps `sr_context_options`. + */ +ContextFlags setGlobalContextOptions(const ContextFlags flags, const GlobalContextEffect when) +{ + uint32_t old; + throwIfError( + sr_context_options(static_cast(flags), when == GlobalContextEffect::Immediate, &old), + "sr_context_options failed", + nullptr); + return static_cast(old); +} + std::timespec toTimespec(std::chrono::time_point tp) { // https://embeddedartistry.com/blog/2019/01/31/converting-between-timespec-stdchrono#-std-chrono-time_point-to-timespec- diff --git a/tests/session.cpp b/tests/session.cpp index 47b5631..a709a09 100644 --- a/tests/session.cpp +++ b/tests/session.cpp @@ -627,17 +627,34 @@ TEST_CASE("session") DOCTEST_SUBCASE("libyang context flags") { - sess.setItem("/test_module:popelnice/s", "666"); - REQUIRE(sess.getOneNode("/test_module:popelnice/s").asTerm().valueStr() == "666"); - // Parsed type info is not preserved by libyang unless its context is constructed with a flag, - // and that flag is not used by sysrepo by default... - REQUIRE_THROWS_AS(sess.getOneNode("/test_module:popelnice/s").schema().asLeaf().valueType().asString().length(), libyang::ParsedInfoUnavailable); - - // ...unless we pass that flag explicitly as a parameter to the connection. - auto sess2 = sysrepo::Connection{sysrepo::ConnectionFlags::LibYangPrivParsed}.sessionStart(); - sess2.setItem("/test_module:popelnice/s", "333"); - REQUIRE(sess2.getOneNode("/test_module:popelnice/s").asTerm().valueStr() == "333"); - REQUIRE(sess2.getOneNode("/test_module:popelnice/s").schema().asLeaf().valueType().asString().length().parts[0].max == 10); + DOCTEST_SUBCASE("default") + { + sess.setItem("/test_module:popelnice/s", "666"); + REQUIRE(sess.getOneNode("/test_module:popelnice/s").asTerm().valueStr() == "666"); + // Parsed type info is not preserved by libyang unless its context is constructed with a flag, + // and that flag is not used by sysrepo by default. + REQUIRE_THROWS_AS(sess.getOneNode("/test_module:popelnice/s").schema().asLeaf().valueType().asString().length(), libyang::ParsedInfoUnavailable); + + // pending changes hold the context lock, so we cannot change the global context options + REQUIRE_THROWS_WITH_AS( + sysrepo::setGlobalContextOptions( + sysrepo::ContextFlags::LibYangPrivParsed | sysrepo::ContextFlags::NoPrinted, + sysrepo::GlobalContextEffect::Immediate), + "sr_context_options failed: SR_ERR_TIME_OUT", + sysrepo::ErrorWithCode); + } + + // Since this is actually a shared global state, changing it is "tricky". The best bet is to actually + // ensure that there are no other connections (which we cannot do here, we already have a connection opened + // from the common code at the very top of this test case), and especially no internal sysrepo-level locks + // on the shared libyang context. For example, no uncommited changes -- hence a new subcase. + DOCTEST_SUBCASE("Connection::setGlobalContextOptions LibYangPrivParsed") + { + sysrepo::setGlobalContextOptions(sysrepo::ContextFlags::LibYangPrivParsed | sysrepo::ContextFlags::NoPrinted, sysrepo::GlobalContextEffect::Immediate); + sess.setItem("/test_module:popelnice/s", "333"); + REQUIRE(sess.getOneNode("/test_module:popelnice/s").asTerm().valueStr() == "333"); + REQUIRE(sess.getOneNode("/test_module:popelnice/s").schema().asLeaf().valueType().asString().length().parts[0].max == 10); + } } DOCTEST_SUBCASE("replay support") -- 2.43.0