From b3ee78ee00a13d62ea06b2e947b26b458e100123 Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Sat, 4 Apr 2026 23:40:20 +0200 Subject: [PATCH 01/15] expected: Handle void function result in Expected::andThen --- include/ppplugin/expected.h | 36 ++++++++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/include/ppplugin/expected.h b/include/ppplugin/expected.h index 36a4ca4..eabf041 100644 --- a/include/ppplugin/expected.h +++ b/include/ppplugin/expected.h @@ -459,17 +459,33 @@ template constexpr auto Expected::andThen(F&& func) const& { if constexpr (std::is_invocable_v) { - using ReturnType = AndThenReturnType>; + using InvokeResultType = std::invoke_result_t; + using ReturnType = AndThenReturnType; if (!hasValue()) { return static_cast(uncheckedError()); } - return static_cast(std::forward(func)(uncheckedValue())); + if constexpr (std::is_void_v) { + // cannot cast void function return type to result type; + // thus, handle separately here + std::forward(func)(uncheckedValue()); + return ReturnType {}; + } else { + return static_cast(std::forward(func)(uncheckedValue())); + } } else if constexpr (std::is_invocable_v) { - using ReturnType = AndThenReturnType>; + using InvokeResultType = std::invoke_result_t; + using ReturnType = AndThenReturnType; if (!hasValue()) { return static_cast(uncheckedError()); } - return static_cast(std::forward(func)()); + if constexpr (std::is_void_v) { + // cannot cast void function return type to result type; + // thus, handle separately here + std::forward(func)(); + return ReturnType {}; + } else { + return static_cast(std::forward(func)()); + } } else { static_assert(std::is_invocable_v, "Given function has to be invocable!"); @@ -658,10 +674,18 @@ template template constexpr auto Expected::andThen(F&& func) && { + using FunctionReturnType = std::invoke_result_t; + using ReturnType = Expected; + if (error_) { - return std::move(*error_); + return ReturnType { std::move(*error_) }; + } + if constexpr (std::is_void_v) { + func(); + return ReturnType {}; + } else { + return ReturnType { std::forward(func)() }; } - return std::forward(func)(); } template constexpr void Expected::operator*() const From a49474c7d6a22137e3af63441dd2c4a0c33cfc72 Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Sun, 5 Apr 2026 00:49:38 +0200 Subject: [PATCH 02/15] clang-format: Fix SpaceInEmptyBraces for clang-format >= 22 --- .clang-format | 1 + 1 file changed, 1 insertion(+) diff --git a/.clang-format b/.clang-format index e0958e5..6214c59 100644 --- a/.clang-format +++ b/.clang-format @@ -10,3 +10,4 @@ QualifierOrder: [ 'const', 'type', ] +SpaceInEmptyBraces: Block From 731916b934ebed9419867041e7ecb6fa10ca869a Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Sun, 5 Apr 2026 00:51:25 +0200 Subject: [PATCH 03/15] shell: First draft --- .github/workflows/build-and-test.yml | 4 +- CMakeLists.txt | 2 +- cmake/ppplugin-config.cmake.in | 2 +- include/ppplugin/detail/string_utils.h | 30 +++++ include/ppplugin/shell/plugin.h | 77 +++++++++++ include/ppplugin/shell/shell_session.h | 180 +++++++++++++++++++++++++ src/CMakeLists.txt | 13 +- src/shell_plugin.cpp | 17 +++ src/shell_session.cpp | 1 + test/CMakeLists.txt | 1 + test/shell_tests/CMakeLists.txt | 9 ++ test/shell_tests/shell_tests.cpp | 57 ++++++++ test/shell_tests/test.sh | 27 ++++ 13 files changed, 411 insertions(+), 9 deletions(-) create mode 100644 include/ppplugin/detail/string_utils.h create mode 100644 include/ppplugin/shell/plugin.h create mode 100644 include/ppplugin/shell/shell_session.h create mode 100644 src/shell_plugin.cpp create mode 100644 src/shell_session.cpp create mode 100644 test/shell_tests/CMakeLists.txt create mode 100644 test/shell_tests/shell_tests.cpp create mode 100644 test/shell_tests/test.sh diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index af40f50..d806a96 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -92,7 +92,7 @@ jobs: retention-days: 5 unittest: - name: "unittest" + name: "unittests" needs: build runs-on: ubuntu-latest strategy: @@ -151,7 +151,7 @@ jobs: retention-days: 3 ctest: - name: "ctest" + name: "cmake tests" needs: build runs-on: ubuntu-latest steps: diff --git a/CMakeLists.txt b/CMakeLists.txt index 7b49739..5a66e06 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -24,7 +24,7 @@ option(PPPLUGIN_ENABLE_UNDEFINED_SANITIZE option(PPPLUGIN_ENABLE_UNREACHABLE_SANITIZE "Enable compilation with unreachable sanitize flags" OFF) -find_package(Boost 1.70.0 REQUIRED CONFIG COMPONENTS headers filesystem) +find_package(Boost 1.70.0 REQUIRED CONFIG COMPONENTS headers filesystem process) find_package(Python 3.0 REQUIRED COMPONENTS Development) find_package(Lua 5.2 REQUIRED) diff --git a/cmake/ppplugin-config.cmake.in b/cmake/ppplugin-config.cmake.in index 9d24d6b..996c55f 100644 --- a/cmake/ppplugin-config.cmake.in +++ b/cmake/ppplugin-config.cmake.in @@ -2,7 +2,7 @@ include(CMakeFindDependencyMacro) find_dependency(Boost @Boost_VERSION_MAJOR@ COMPONENTS headers filesystem - python) + python process) find_dependency(Python @Python_VERSION_MAJOR@ COMPONENTS Development) find_dependency(Lua @LUA_VERSION_MAJOR@) diff --git a/include/ppplugin/detail/string_utils.h b/include/ppplugin/detail/string_utils.h new file mode 100644 index 0000000..83644d9 --- /dev/null +++ b/include/ppplugin/detail/string_utils.h @@ -0,0 +1,30 @@ +#ifndef PPPLUGIN_DETAIL_STRING_UTILS_H +#define PPPLUGIN_DETAIL_STRING_UTILS_H + +#include +#include +#include + +namespace ppplugin { +[[nodiscard]] inline bool endsWith(std::string_view string, std::string_view end) +{ + if (string.size() < end.size()) { + return false; + } + auto result = string.compare(string.size() - end.size(), end.size(), end); + return result == 0; +} + +template +[[nodiscard]] inline std::optional toInteger(std::string_view string) +{ + int value {}; + auto result = std::from_chars(string.begin(), string.end(), value); + if (result.ec == std::errc {} && result.ptr == string.end()) { + return value; + } + return std::nullopt; +} +} // namespace ppplugin + +#endif // PPPLUGIN_DETAIL_STRING_UTILS_H diff --git a/include/ppplugin/shell/plugin.h b/include/ppplugin/shell/plugin.h new file mode 100644 index 0000000..584d10a --- /dev/null +++ b/include/ppplugin/shell/plugin.h @@ -0,0 +1,77 @@ +#ifndef PPPLUGIN_SHELL_PLUGIN_H +#define PPPLUGIN_SHELL_PLUGIN_H + +#include "ppplugin/errors.h" +#include "shell_session.h" + +#include +#include + +namespace ppplugin { +class ShellPlugin { +public: + [[nodiscard]] static Expected load( + const std::filesystem::path& plugin_library_path); + + ~ShellPlugin() = default; + ShellPlugin(const ShellPlugin&) = delete; + ShellPlugin(ShellPlugin&&) = default; + ShellPlugin& operator=(const ShellPlugin&) = delete; + ShellPlugin& operator=(ShellPlugin&&) = default; + + // NOLINTNEXTLINE(google-explicit-constructor,hicpp-explicit-conversions) + operator bool() const { return true; } + + /** + * Accepted types are: + * - void + * - bool + * - int + * - double + * - std::string + * - const char* + * - std::tuple + */ + template + [[nodiscard]] CallResult call(const std::string& function_name, Args&&... args); + + template + [[nodiscard]] CallResult global(const std::string& variable_name); + template + [[nodiscard]] CallResult global(const std::string& variable_name, VariableType&& new_value); + +private: + explicit ShellPlugin() + // : shell_ { "/tmp/a.out" } + : shell_ { "/bin/sh" } + { + } + +private: + ShellSession shell_; +}; + +template +CallResult ShellPlugin::call(const std::string& function_name, Args&&... args) +{ + if constexpr (std::is_void_v) { + return shell_.callWithoutResult(function_name, { args... }); + } else { + return shell_.callWithResult(function_name, { args... }); + } +} + +template +CallResult ShellPlugin::global(const std::string& variable_name) +{ + return shell_.environmentVariable(variable_name); +} + +template +CallResult ShellPlugin::global(const std::string& variable_name, VariableType&& new_value) +{ + return shell_.environmentVariable(variable_name, std::forward(new_value)); +} +} // namespace ppplugin + +#endif // PPPLUGIN_SHELL_PLUGIN_H diff --git a/include/ppplugin/shell/shell_session.h b/include/ppplugin/shell/shell_session.h new file mode 100644 index 0000000..ab52bd7 --- /dev/null +++ b/include/ppplugin/shell/shell_session.h @@ -0,0 +1,180 @@ +#ifndef PPPLUGIN_SHELL_SESSION_H +#define PPPLUGIN_SHELL_SESSION_H + +#include "ppplugin/detail/string_utils.h" +#include "ppplugin/errors.h" + +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include + +namespace ppplugin { +class ShellSession { +public: + explicit ShellSession(const std::string& shell_binary) + : context_ { std::make_unique() } + , stdin_pipe_ { *context_ } + , stdout_pipe_ { *context_ } + , stderr_pipe_ { *context_ } + , io_ { stdin_pipe_, stdout_pipe_, stderr_pipe_ } + , shell_process_ { *context_, shell_binary, {}, io_ } + , wait_for_result_mutex_ { std::make_unique() } + , wait_for_result_ { std::make_unique() } + { + thread_ = std::thread { [context = context_.get(), wait_for_result = wait_for_result_.get()]() { assert(context); runContextLoop(*context); wait_for_result->notify_all(); } }; + } + ~ShellSession() + { + if (shell_process_.running()) { + shell_process_.interrupt(); + std::this_thread::sleep_for(SHELL_SHUTDOWN_TIMEOUT); + shell_process_.terminate(); + } + if (context_) { + context_->stop(); + } + if (thread_.joinable()) { + thread_.join(); + } + } + ShellSession(const ShellSession&) = delete; + ShellSession(ShellSession&&) = default; + ShellSession& operator=(const ShellSession&) = delete; + ShellSession& operator=(ShellSession&&) = default; + + CallResult callWithoutResult(const std::string& function_name, const std::vector& arguments) + { + auto result = callWithResult(function_name, arguments); + return result.andThen([]() { }); + } + CallResult callWithResult(const std::string& function_name, const std::vector& arguments) + { + if (!shell_process_.running()) { + return CallError { CallError::Code::unknown, "Shell is not running!" }; + } + auto command_line = '\'' + function_name + '\''; + for (auto&& argument : arguments) { + // TODO: escape quotes in arguments + command_line += " '"; + auto escaped_argument = argument; + boost::replace_all(escaped_argument, "'", "\\'"); + command_line += argument; + command_line += "'"; + } + auto end_marker = boost::uuids::to_string(boost::uuids::random_generator()()) + '\n'; + command_line += " ; echo :$?" + end_marker; + std::cout << "WRITING: " << command_line; + boost::asio::write(stdin_pipe_, boost::asio::buffer(command_line)); + + std::string result; + constexpr auto BUFFER_SIZE = 1024; + // for the abort condition to work, the end marker must fully fit into the buffer + assert(end_marker.size() < BUFFER_SIZE); + std::array buffer {}; + + std::function read_handler = [this, &result, &buffer, &end_marker, &read_handler](const boost::system::error_code& error, std::size_t bytes_transferred) { + if (error.failed()) { + // TODO? report error to user (error.what())? + std::cout << "ERROR while reading: " << error.what() << '\n'; + return; + } + result.append(buffer.data(), bytes_transferred); + std::cout << "PROGRESS... (" << result << ")\n"; + if (endsWith(result, end_marker)) { + std::lock_guard lock { *wait_for_result_mutex_ }; + is_result_available_ = true; + wait_for_result_->notify_all(); + } else { + stdout_pipe_.async_read_some(boost::asio::buffer(buffer), read_handler); + } + }; + is_result_available_ = false; + stdout_pipe_.async_read_some(boost::asio::buffer(buffer), read_handler); + std::unique_lock lock { *wait_for_result_mutex_ }; + while (!wait_for_result_->wait_for(lock, std::chrono::milliseconds { 50 }, [this]() { return is_result_available_ || !shell_process_.running(); })) { } + if (!shell_process_.running()) { + return CallError { CallError::Code::unknown, std::to_string(shell_process_.exit_code()) }; + } + assert(result.size() > end_marker.size()); + result.resize(result.size() - end_marker.size()); + + auto exit_code_pos = result.find_last_of(':'); + assert(exit_code_pos != std::string::npos); + auto exit_code = toInteger(result.substr(exit_code_pos + 1)).value_or(std::numeric_limits::max()); + std::cout << "EXIT_CODE: " << exit_code << '\n'; + if (exit_code != 0) { + return CallError { CallError::Code::unknown, std::to_string(exit_code) }; + } + result.resize(exit_code_pos); + + std::cout << "DONE: " << command_line << '\n'; + std::cout << "RESULT: " << result << '\n'; + + return result; + } + + CallResult environmentVariable(const std::string& variable_name) + { + // auto it = environment_variables_.find(variable_name); + // if (it != environment_variables_.end()) { + // return it->second; + // } + // return CallError { CallError::Code::symbolNotFound }; + return callWithResult("printenv", { variable_name }).andThen([](auto&& result) { + assert(!result.empty()); + assert(result.back() == '\n'); + result.pop_back(); + return result; + }); + } + + CallResult environmentVariable(const std::string& variable_name, const std::string& new_value) + { + std::string variable_assignment = variable_name + "='" + new_value + '\''; + return callWithoutResult("export", { variable_assignment }).andThen([]() { }); + } + +private: + static void runContextLoop(boost::asio::io_context& context) + { + while (true) { + try { + auto work = boost::asio::make_work_guard(context); + context.run(); + break; + } catch (const std::exception& exception) { + } catch (...) { + } + } + } + +private: + static constexpr std::chrono::milliseconds SHELL_SHUTDOWN_TIMEOUT { 10 }; + +private: + std::thread thread_; + std::unique_ptr context_; + boost::asio::writable_pipe stdin_pipe_; + boost::asio::readable_pipe stdout_pipe_; + boost::asio::readable_pipe stderr_pipe_; + boost::process::process_stdio io_; + boost::process::process shell_process_; + + std::unique_ptr wait_for_result_mutex_; + std::unique_ptr wait_for_result_; + bool is_result_available_ { false }; + + // TODO: explore possibility to import/export variables and functions for each call +}; +} // namespace ppplugin + +#endif // PPPLUGIN_SHELL_SESSION_H diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index eccba01..7a7bac7 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -12,19 +12,22 @@ set(LIBRARY_SOURCES "python_exception.cpp" "python_tuple.cpp" "python_object.cpp" - "python_guard.cpp") + "python_guard.cpp" + "shell_plugin.cpp" + "shell_session.cpp") -add_library(${LIBRARY_TARGET} ${LIBRARY_SOURCES}) +file(GLOB_RECURSE LIBRARY_HEADERS ${CMAKE_SOURCE_DIR}/include/**/*.h) + +add_library(${LIBRARY_TARGET} ${LIBRARY_SOURCES} ${LIBRARY_HEADERS}) target_link_libraries( ${LIBRARY_TARGET} PRIVATE ${LUA_LIBRARIES} - PUBLIC Boost::filesystem Python::Python + PUBLIC Boost::filesystem Boost::process Python::Python $<$:fmt::fmt>) target_include_directories( ${LIBRARY_TARGET} PRIVATE ${LUA_INCLUDE_DIR} - PUBLIC $ - $) + PUBLIC $) target_compile_definitions( ${LIBRARY_TARGET} PUBLIC diff --git a/src/shell_plugin.cpp b/src/shell_plugin.cpp new file mode 100644 index 0000000..a14ffe3 --- /dev/null +++ b/src/shell_plugin.cpp @@ -0,0 +1,17 @@ +#include "ppplugin/shell/plugin.h" + +#include "ppplugin/errors.h" +#include "ppplugin/expected.h" + +#include + +namespace ppplugin { +Expected ShellPlugin::load( + const std::filesystem::path& plugin_library_path) +{ + ShellPlugin plugin; + // TODO: handle error + std::ignore = plugin.call(".", plugin_library_path.string()); + return plugin; +} +} // namespace ppplugin diff --git a/src/shell_session.cpp b/src/shell_session.cpp new file mode 100644 index 0000000..16b30c1 --- /dev/null +++ b/src/shell_session.cpp @@ -0,0 +1 @@ +#include "ppplugin/shell/shell_session.h" diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 5f7aae4..fd1281b 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -9,6 +9,7 @@ target_link_libraries(${TESTS_NAME} PRIVATE GTest::GTest GTest::Main gmock Threads::Threads ${LIBRARY_TARGET}) add_subdirectory(lua_tests) +add_subdirectory(shell_tests) add_subdirectory(python_tests) gtest_discover_tests(${TESTS_NAME}) diff --git a/test/shell_tests/CMakeLists.txt b/test/shell_tests/CMakeLists.txt new file mode 100644 index 0000000..e43c5f7 --- /dev/null +++ b/test/shell_tests/CMakeLists.txt @@ -0,0 +1,9 @@ +target_sources(${TESTS_NAME} PRIVATE shell_tests.cpp) + +add_custom_target(shell_tests ALL COMMENT "Shell test files") +add_custom_command( + TARGET shell_tests + POST_BUILD + COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_SOURCE_DIR}/test.sh + ${CMAKE_CURRENT_BINARY_DIR}/test.sh + COMMENT "Copying POSIX shell plugins to output directory...") diff --git a/test/shell_tests/shell_tests.cpp b/test/shell_tests/shell_tests.cpp new file mode 100644 index 0000000..21f640c --- /dev/null +++ b/test/shell_tests/shell_tests.cpp @@ -0,0 +1,57 @@ +#include "test_helper.h" + +#include +#include + +#include + +class ShellTest : public testing::Test { +protected: + void SetUp() override + { + auto load_result = ppplugin::ShellPlugin::load("./shell_tests/test.sh"); + ASSERT_TRUE(load_result.hasValue()); + + plugin = std::make_unique(std::move(load_result.valueRef()->get())); + } + +protected: + std::unique_ptr plugin; +}; + +TEST_F(ShellTest, getEnvironmentVariable) +{ + auto result = plugin->global("env_var"); + + ASSERT_TRUE(result.hasValue()) << ppplugin::test::errorOutput(result); + EXPECT_EQ(result.valueOr(""), "abc"); +} + +TEST_F(ShellTest, setEnvironmentVariable) +{ + auto original_result = plugin->global("env_var"); + auto set_result = plugin->global("env_var", "xyz"); + auto changed_result = plugin->global("env_var"); + + ASSERT_TRUE(original_result.hasValue()) << ppplugin::test::errorOutput(original_result); + ASSERT_TRUE(set_result.hasValue()) << ppplugin::test::errorOutput(set_result); + ASSERT_TRUE(changed_result.hasValue()) << ppplugin::test::errorOutput(changed_result); + + EXPECT_NE(original_result.valueOr(""), changed_result.valueOr("")); + EXPECT_EQ(changed_result.valueOr(""), "xyz"); +} + +TEST_F(ShellTest, callFunction) +{ + auto result = plugin->call("print_first", "qwertz"); + + EXPECT_TRUE(result.hasValue()) << ppplugin::test::errorOutput(result); +} + +TEST_F(ShellTest, callFunctionWithStringResult) +{ + auto result = plugin->call("print_first", "qwerty"); + + EXPECT_TRUE(result.hasValue()) << ppplugin::test::errorOutput(result); + EXPECT_EQ(result.value().value(), "qwerty\n"); +} diff --git a/test/shell_tests/test.sh b/test/shell_tests/test.sh new file mode 100644 index 0000000..6e85bbf --- /dev/null +++ b/test/shell_tests/test.sh @@ -0,0 +1,27 @@ +#!/bin/sh + +export env_var="abc" + +echo "source" + +print_first() +{ + echo a > /tmp/a + echo "${1}" +} + +print_env_var() +{ + var_name="${1}" + eval echo "\${${var_name}}" +} + +change_environment() +{ + env_var="xyz" +} + +exit_shell() +{ + exit +} From 45d56cda78485353adfc2505cbbdb783c8a65307 Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Mon, 6 Apr 2026 01:08:07 +0200 Subject: [PATCH 04/15] shell: Second draft --- include/ppplugin/shell/plugin.h | 2 +- include/ppplugin/shell/shell_session.h | 100 ++++++++++++------------- src/shell_plugin.cpp | 8 +- test/shell_tests/shell_tests.cpp | 21 +++++- 4 files changed, 75 insertions(+), 56 deletions(-) diff --git a/include/ppplugin/shell/plugin.h b/include/ppplugin/shell/plugin.h index 584d10a..fd5df83 100644 --- a/include/ppplugin/shell/plugin.h +++ b/include/ppplugin/shell/plugin.h @@ -20,7 +20,7 @@ class ShellPlugin { ShellPlugin& operator=(ShellPlugin&&) = default; // NOLINTNEXTLINE(google-explicit-constructor,hicpp-explicit-conversions) - operator bool() const { return true; } + operator bool() { return shell_.isRunning(); } /** * Accepted types are: diff --git a/include/ppplugin/shell/shell_session.h b/include/ppplugin/shell/shell_session.h index ab52bd7..6c76a4d 100644 --- a/include/ppplugin/shell/shell_session.h +++ b/include/ppplugin/shell/shell_session.h @@ -2,11 +2,10 @@ #define PPPLUGIN_SHELL_SESSION_H #include "ppplugin/detail/string_utils.h" +#include "ppplugin/detail/thread_safe.h" #include "ppplugin/errors.h" #include -#include -#include #include #include #include @@ -23,20 +22,27 @@ class ShellSession { explicit ShellSession(const std::string& shell_binary) : context_ { std::make_unique() } , stdin_pipe_ { *context_ } - , stdout_pipe_ { *context_ } + , stdout_pipe_ { std::make_unique(boost::asio::readable_pipe { *context_ }) } , stderr_pipe_ { *context_ } - , io_ { stdin_pipe_, stdout_pipe_, stderr_pipe_ } + , io_ { stdin_pipe_, *stdout_pipe_, stderr_pipe_ } , shell_process_ { *context_, shell_binary, {}, io_ } - , wait_for_result_mutex_ { std::make_unique() } - , wait_for_result_ { std::make_unique() } + , is_running_ { std::make_unique(true) } { - thread_ = std::thread { [context = context_.get(), wait_for_result = wait_for_result_.get()]() { assert(context); runContextLoop(*context); wait_for_result->notify_all(); } }; + thread_ = std::thread { [context = context_.get()]() { assert(context); runContextLoop(*context); } }; + + // TODO: make movable across threads + shell_process_.async_wait([stdout_pipe = stdout_pipe_.get(), is_running = is_running_.get()](const boost::system::error_code& ec, int exit_code) { + std::cout << "PROCESS EXIT: " << ec.what() << " (" << exit_code << ")\n"; + assert(stdout_pipe); + stdout_pipe->cancel(); + assert(is_running); + is_running->store(false); + }); } ~ShellSession() { if (shell_process_.running()) { - shell_process_.interrupt(); - std::this_thread::sleep_for(SHELL_SHUTDOWN_TIMEOUT); + // TODO? use shell_process_.request_exit() first? shell_process_.terminate(); } if (context_) { @@ -51,12 +57,12 @@ class ShellSession { ShellSession& operator=(const ShellSession&) = delete; ShellSession& operator=(ShellSession&&) = default; - CallResult callWithoutResult(const std::string& function_name, const std::vector& arguments) + [[nodiscard]] CallResult callWithoutResult(const std::string& function_name, const std::vector& arguments) { auto result = callWithResult(function_name, arguments); return result.andThen([]() { }); } - CallResult callWithResult(const std::string& function_name, const std::vector& arguments) + [[nodiscard]] CallResult callWithResult(const std::string& function_name, const std::vector& arguments) { if (!shell_process_.running()) { return CallError { CallError::Code::unknown, "Shell is not running!" }; @@ -76,35 +82,22 @@ class ShellSession { boost::asio::write(stdin_pipe_, boost::asio::buffer(command_line)); std::string result; - constexpr auto BUFFER_SIZE = 1024; - // for the abort condition to work, the end marker must fully fit into the buffer - assert(end_marker.size() < BUFFER_SIZE); - std::array buffer {}; - - std::function read_handler = [this, &result, &buffer, &end_marker, &read_handler](const boost::system::error_code& error, std::size_t bytes_transferred) { - if (error.failed()) { - // TODO? report error to user (error.what())? - std::cout << "ERROR while reading: " << error.what() << '\n'; - return; - } - result.append(buffer.data(), bytes_transferred); - std::cout << "PROGRESS... (" << result << ")\n"; - if (endsWith(result, end_marker)) { - std::lock_guard lock { *wait_for_result_mutex_ }; - is_result_available_ = true; - wait_for_result_->notify_all(); - } else { - stdout_pipe_.async_read_some(boost::asio::buffer(buffer), read_handler); - } - }; - is_result_available_ = false; - stdout_pipe_.async_read_some(boost::asio::buffer(buffer), read_handler); - std::unique_lock lock { *wait_for_result_mutex_ }; - while (!wait_for_result_->wait_for(lock, std::chrono::milliseconds { 50 }, [this]() { return is_result_available_ || !shell_process_.running(); })) { } + // use future with empty handler to avoid blocking read() syscall; + // this allows cancellation of pipe in case shell process dies + boost::asio::async_read_until(*stdout_pipe_, + boost::asio::dynamic_buffer(result), + end_marker, boost::asio::use_future([](auto&&...) { })) + .wait(); + if (!shell_process_.running()) { - return CallError { CallError::Code::unknown, std::to_string(shell_process_.exit_code()) }; + is_running_->store(false); + return CallError { CallError::Code::unknown, "shell ended prematurely" }; } - assert(result.size() > end_marker.size()); + + if (!endsWith(result, end_marker)) { + return CallError { CallError::Code::unknown, "failed to get command output" }; + } + // remove end marker result.resize(result.size() - end_marker.size()); auto exit_code_pos = result.find_last_of(':'); @@ -122,13 +115,8 @@ class ShellSession { return result; } - CallResult environmentVariable(const std::string& variable_name) + [[nodiscard]] CallResult environmentVariable(const std::string& variable_name) { - // auto it = environment_variables_.find(variable_name); - // if (it != environment_variables_.end()) { - // return it->second; - // } - // return CallError { CallError::Code::symbolNotFound }; return callWithResult("printenv", { variable_name }).andThen([](auto&& result) { assert(!result.empty()); assert(result.back() == '\n'); @@ -137,10 +125,20 @@ class ShellSession { }); } - CallResult environmentVariable(const std::string& variable_name, const std::string& new_value) + [[nodiscard]] CallResult environmentVariable(const std::string& variable_name, const std::string& new_value) { std::string variable_assignment = variable_name + "='" + new_value + '\''; - return callWithoutResult("export", { variable_assignment }).andThen([]() { }); + return callWithoutResult("export", { variable_assignment }); + } + + [[nodiscard]] bool isRunning() + { + return shell_process_.running(); + } + + [[nodiscard]] bool isRunning() const + { + return is_running_->load(); } private: @@ -152,7 +150,9 @@ class ShellSession { context.run(); break; } catch (const std::exception& exception) { + assert(false); } catch (...) { + assert(false); } } } @@ -161,17 +161,15 @@ class ShellSession { static constexpr std::chrono::milliseconds SHELL_SHUTDOWN_TIMEOUT { 10 }; private: + // thread for running io_context std::thread thread_; std::unique_ptr context_; boost::asio::writable_pipe stdin_pipe_; - boost::asio::readable_pipe stdout_pipe_; + std::unique_ptr stdout_pipe_; boost::asio::readable_pipe stderr_pipe_; boost::process::process_stdio io_; boost::process::process shell_process_; - - std::unique_ptr wait_for_result_mutex_; - std::unique_ptr wait_for_result_; - bool is_result_available_ { false }; + std::unique_ptr> is_running_; // TODO: explore possibility to import/export variables and functions for each call }; diff --git a/src/shell_plugin.cpp b/src/shell_plugin.cpp index a14ffe3..e048ad7 100644 --- a/src/shell_plugin.cpp +++ b/src/shell_plugin.cpp @@ -10,8 +10,10 @@ Expected ShellPlugin::load( const std::filesystem::path& plugin_library_path) { ShellPlugin plugin; - // TODO: handle error - std::ignore = plugin.call(".", plugin_library_path.string()); - return plugin; + auto result = plugin.call(".", plugin_library_path.string()); + if (result.hasValue()) { + return plugin; + } + return LoadError::unknown; } } // namespace ppplugin diff --git a/test/shell_tests/shell_tests.cpp b/test/shell_tests/shell_tests.cpp index 21f640c..dcbb59f 100644 --- a/test/shell_tests/shell_tests.cpp +++ b/test/shell_tests/shell_tests.cpp @@ -19,6 +19,12 @@ class ShellTest : public testing::Test { std::unique_ptr plugin; }; +TEST(ShellTestStandalone, failToLoadNonexistentFile) +{ + auto plugin = ppplugin::ShellPlugin::load("./path/that/does/not/exist.sh"); + EXPECT_FALSE(plugin); +} + TEST_F(ShellTest, getEnvironmentVariable) { auto result = plugin->global("env_var"); @@ -52,6 +58,19 @@ TEST_F(ShellTest, callFunctionWithStringResult) { auto result = plugin->call("print_first", "qwerty"); - EXPECT_TRUE(result.hasValue()) << ppplugin::test::errorOutput(result); + ASSERT_TRUE(result.hasValue()) << ppplugin::test::errorOutput(result); EXPECT_EQ(result.value().value(), "qwerty\n"); } + +TEST_F(ShellTest, moveToOtherThread) +{ + auto result = plugin->global("some_var", "some_value"); + EXPECT_TRUE(result) << ppplugin::test::errorOutput(result); + + std::thread thread { [moved_plugin = std::move(*plugin)]() mutable { + auto result = moved_plugin.global("some_var"); + EXPECT_TRUE(result) << ppplugin::test::errorOutput(result); + EXPECT_EQ(result.value().value(), "some_value"); + } }; + thread.join(); +} From 913713b76c03ed38925302b075bccd6b36dd5dcd Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Mon, 6 Apr 2026 13:16:45 +0200 Subject: [PATCH 05/15] cmake: Fix library header installation --- src/CMakeLists.txt | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 7a7bac7..6598c50 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -1,5 +1,3 @@ -set(LIBRARY_SOURCES ppplugin.cpp) - set(LIBRARY_SOURCES "boost_dll_loader.cpp" "c_plugin.cpp" @@ -27,7 +25,8 @@ target_link_libraries( target_include_directories( ${LIBRARY_TARGET} PRIVATE ${LUA_INCLUDE_DIR} - PUBLIC $) + PUBLIC $ + PUBLIC $) target_compile_definitions( ${LIBRARY_TARGET} PUBLIC @@ -42,7 +41,7 @@ set_target_properties( set(TARGETS_EXPORT_NAME "${PROJECT_NAME}-targets") install(DIRECTORY "${CMAKE_SOURCE_DIR}/include/" - DESTINATION "include/${LIBRARY_NAME}") + DESTINATION "include/") install( TARGETS ${LIBRARY_TARGET} EXPORT "${TARGETS_EXPORT_NAME}" From a09f3e8bcfc8ed1649e24ea96f5620fa533a18d2 Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Mon, 6 Apr 2026 15:50:05 +0200 Subject: [PATCH 06/15] src|cmake: Replicate include directory structure in src --- src/CMakeLists.txt | 26 ++++++++++---------- src/{c_plugin.cpp => c/plugin.cpp} | 0 src/{cpp_plugin.cpp => cpp/plugin.cpp} | 0 src/{ => lua}/lua_script.cpp | 0 src/{ => lua}/lua_state.cpp | 0 src/{lua_plugin.cpp => lua/plugin.cpp} | 0 src/{python_plugin.cpp => python/plugin.cpp} | 0 src/{ => python}/python_exception.cpp | 0 src/{ => python}/python_guard.cpp | 0 src/{ => python}/python_interpreter.cpp | 0 src/python/python_list.cpp | 25 +++++++++++++++++++ src/{ => python}/python_object.cpp | 0 src/{ => python}/python_tuple.cpp | 0 src/{shell_plugin.cpp => shell/plugin.cpp} | 0 src/{ => shell}/shell_session.cpp | 0 15 files changed, 38 insertions(+), 13 deletions(-) rename src/{c_plugin.cpp => c/plugin.cpp} (100%) rename src/{cpp_plugin.cpp => cpp/plugin.cpp} (100%) rename src/{ => lua}/lua_script.cpp (100%) rename src/{ => lua}/lua_state.cpp (100%) rename src/{lua_plugin.cpp => lua/plugin.cpp} (100%) rename src/{python_plugin.cpp => python/plugin.cpp} (100%) rename src/{ => python}/python_exception.cpp (100%) rename src/{ => python}/python_guard.cpp (100%) rename src/{ => python}/python_interpreter.cpp (100%) create mode 100644 src/python/python_list.cpp rename src/{ => python}/python_object.cpp (100%) rename src/{ => python}/python_tuple.cpp (100%) rename src/{shell_plugin.cpp => shell/plugin.cpp} (100%) rename src/{ => shell}/shell_session.cpp (100%) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 6598c50..bcdf740 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -1,18 +1,18 @@ set(LIBRARY_SOURCES "boost_dll_loader.cpp" - "c_plugin.cpp" - "cpp_plugin.cpp" - "lua_plugin.cpp" - "lua_state.cpp" - "lua_script.cpp" - "python_plugin.cpp" - "python_interpreter.cpp" - "python_exception.cpp" - "python_tuple.cpp" - "python_object.cpp" - "python_guard.cpp" - "shell_plugin.cpp" - "shell_session.cpp") + "c/plugin.cpp" + "cpp/plugin.cpp" + "lua/plugin.cpp" + "lua/lua_state.cpp" + "lua/lua_script.cpp" + "python/plugin.cpp" + "python/python_interpreter.cpp" + "python/python_exception.cpp" + "python/python_tuple.cpp" + "python/python_object.cpp" + "python/python_guard.cpp" + "shell/plugin.cpp" + "shell/shell_session.cpp") file(GLOB_RECURSE LIBRARY_HEADERS ${CMAKE_SOURCE_DIR}/include/**/*.h) diff --git a/src/c_plugin.cpp b/src/c/plugin.cpp similarity index 100% rename from src/c_plugin.cpp rename to src/c/plugin.cpp diff --git a/src/cpp_plugin.cpp b/src/cpp/plugin.cpp similarity index 100% rename from src/cpp_plugin.cpp rename to src/cpp/plugin.cpp diff --git a/src/lua_script.cpp b/src/lua/lua_script.cpp similarity index 100% rename from src/lua_script.cpp rename to src/lua/lua_script.cpp diff --git a/src/lua_state.cpp b/src/lua/lua_state.cpp similarity index 100% rename from src/lua_state.cpp rename to src/lua/lua_state.cpp diff --git a/src/lua_plugin.cpp b/src/lua/plugin.cpp similarity index 100% rename from src/lua_plugin.cpp rename to src/lua/plugin.cpp diff --git a/src/python_plugin.cpp b/src/python/plugin.cpp similarity index 100% rename from src/python_plugin.cpp rename to src/python/plugin.cpp diff --git a/src/python_exception.cpp b/src/python/python_exception.cpp similarity index 100% rename from src/python_exception.cpp rename to src/python/python_exception.cpp diff --git a/src/python_guard.cpp b/src/python/python_guard.cpp similarity index 100% rename from src/python_guard.cpp rename to src/python/python_guard.cpp diff --git a/src/python_interpreter.cpp b/src/python/python_interpreter.cpp similarity index 100% rename from src/python_interpreter.cpp rename to src/python/python_interpreter.cpp diff --git a/src/python/python_list.cpp b/src/python/python_list.cpp new file mode 100644 index 0000000..845c82f --- /dev/null +++ b/src/python/python_list.cpp @@ -0,0 +1,25 @@ +#include "ppplugin/python/python_list.h" + +#include + +#define PY_SSIZE_T_CLEAN +#include // NOLINT(misc-include-cleaner) + +namespace ppplugin { +PyObject* PythonList::initList(int size) +{ + if (size < 0) { + return nullptr; + } + auto* new_list = PyList_New(size); + assert(new_list); + return new_list; +} + +void PythonList::setListItem(int index, PyObject* value) +{ + // PyList_SetItem will steal this reference, so claim ownership first + assert(value); + assert(PyList_SetItem(object(), index, value) == 0); +} +} // namespace ppplugin diff --git a/src/python_object.cpp b/src/python/python_object.cpp similarity index 100% rename from src/python_object.cpp rename to src/python/python_object.cpp diff --git a/src/python_tuple.cpp b/src/python/python_tuple.cpp similarity index 100% rename from src/python_tuple.cpp rename to src/python/python_tuple.cpp diff --git a/src/shell_plugin.cpp b/src/shell/plugin.cpp similarity index 100% rename from src/shell_plugin.cpp rename to src/shell/plugin.cpp diff --git a/src/shell_session.cpp b/src/shell/shell_session.cpp similarity index 100% rename from src/shell_session.cpp rename to src/shell/shell_session.cpp From a082e97d6de9d34af021a8dfc5fc1ecfe2128b0b Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Mon, 6 Apr 2026 15:50:54 +0200 Subject: [PATCH 07/15] ppplugin.cpp|cmake: Create source file including all headers --- include/ppplugin.cpp | 39 +++++++++++++++++++++++++++++++++++++++ src/CMakeLists.txt | 1 + 2 files changed, 40 insertions(+) create mode 100644 include/ppplugin.cpp diff --git a/include/ppplugin.cpp b/include/ppplugin.cpp new file mode 100644 index 0000000..8460ca8 --- /dev/null +++ b/include/ppplugin.cpp @@ -0,0 +1,39 @@ +// this source file includes all header files of the library; +// it allows the language server to infer the correct compilation flags for header +// files that do not have a corresponding source file; additionally, this will +// cause the compiler to compile check them in the compilation process, even if +// they are not used elsewhere in the library (external interface) + +#include "ppplugin/errors.h" +#include "ppplugin/expected.h" +#include "ppplugin/noop_plugin.h" +#include "ppplugin/plugin.h" +#include "ppplugin/plugin_manager.h" + +#include "ppplugin/c/plugin.h" + +#include "ppplugin/cpp/plugin.h" + +#include "ppplugin/lua/lua_helpers.h" +#include "ppplugin/lua/lua_script.h" +#include "ppplugin/lua/lua_state.h" +#include "ppplugin/lua/plugin.h" + +#include "ppplugin/shell/plugin.h" +#include "ppplugin/shell/shell_session.h" + +#include "ppplugin/python/plugin.h" +#include "ppplugin/python/python_exception.h" +#include "ppplugin/python/python_forward_defs.h" +#include "ppplugin/python/python_guard.h" +#include "ppplugin/python/python_interpreter.h" +#include "ppplugin/python/python_object.h" +#include "ppplugin/python/python_tuple.h" + +#include "ppplugin/detail/boost_dll_loader.h" +#include "ppplugin/detail/compatibility_utils.h" +#include "ppplugin/detail/compiler_info.h" +#include "ppplugin/detail/function_details.h" +#include "ppplugin/detail/scope_guard.h" +#include "ppplugin/detail/string_utils.h" +#include "ppplugin/detail/template_helpers.h" diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index bcdf740..2f59fd5 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -1,4 +1,5 @@ set(LIBRARY_SOURCES + "${CMAKE_SOURCE_DIR}/include/ppplugin.cpp" "boost_dll_loader.cpp" "c/plugin.cpp" "cpp/plugin.cpp" From c5631e52b1ba648b51f1f1b4b780fcdc53f27005 Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Mon, 6 Apr 2026 20:38:45 +0200 Subject: [PATCH 08/15] clang-tidy|include: Fix linting issues for clang-tidy 22.1.2 --- .clang-tidy | 2 +- include/ppplugin/c/plugin.h | 20 +++++++++++-------- include/ppplugin/cpp/plugin.h | 20 +++++++++++-------- include/ppplugin/detail/scope_guard.h | 5 ++--- include/ppplugin/errors.h | 4 ++-- include/ppplugin/python/python_forward_defs.h | 7 +++++-- include/ppplugin/python/python_interpreter.h | 6 +++--- include/ppplugin/python/python_object.h | 8 ++++---- include/ppplugin/shell/plugin.h | 4 ++-- include/ppplugin/shell/shell_session.h | 20 +++++++++---------- 10 files changed, 53 insertions(+), 43 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 143d361..336ba04 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -2,7 +2,7 @@ Checks: > -*, clang-diagnostic-*, clang-analyzer-optin.*, - llvm-*,-llvm-header-guard, + llvm-*,-llvm-header-guard,-llvm-prefer-static-over-anonymous-namespace, google-*,-google-readability-todo,-google-readability-braces-around-statements, hicpp-*,-hicpp-braces-around-statements,-hicpp-use-emplace,-hicpp-named-parameter, -hicpp-no-array-decay,-hicpp-uppercase-literal-suffix, diff --git a/include/ppplugin/c/plugin.h b/include/ppplugin/c/plugin.h index 94f8ac9..9cd0c7c 100644 --- a/include/ppplugin/c/plugin.h +++ b/include/ppplugin/c/plugin.h @@ -44,22 +44,26 @@ CallResult CPlugin::call(const std::string& function_name, Args&&.. template CallResult CPlugin::global(const std::string& variable_name) { - auto p = detail::boost_dll::getSymbol(plugin_, variable_name); - if (p.hasValue()) { - return *reinterpret_cast(p.value().value()); + auto result_pointer = detail::boost_dll::getSymbol(plugin_, variable_name); + if (result_pointer.hasValue()) { + // raw type casting necessary due to lack of type information in shared library + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) + return *reinterpret_cast(result_pointer.value().value()); } - return { p.error().value() }; + return { result_pointer.error().value() }; } template CallResult CPlugin::global(const std::string& variable_name, VariableType&& new_value) { - auto p = detail::boost_dll::getSymbol(plugin_, variable_name); - if (p.hasValue()) { - *reinterpret_cast(p.value().value()) = std::forward(new_value); + auto result_pointer = detail::boost_dll::getSymbol(plugin_, variable_name); + if (result_pointer.hasValue()) { + // raw type casting necessary due to lack of type information in shared library + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) + *reinterpret_cast(result_pointer.value().value()) = std::forward(new_value); return {}; } - return { p.error().value() }; + return { result_pointer.error().value() }; } } // namespace ppplugin diff --git a/include/ppplugin/cpp/plugin.h b/include/ppplugin/cpp/plugin.h index 867b3db..723c2d7 100644 --- a/include/ppplugin/cpp/plugin.h +++ b/include/ppplugin/cpp/plugin.h @@ -48,22 +48,26 @@ CallResult CppPlugin::call(const std::string& function_name, Args&& template CallResult CppPlugin::global(const std::string& variable_name) { - auto p = detail::boost_dll::getSymbol(plugin_, variable_name); - if (p.hasValue()) { - return *reinterpret_cast(p.value().value()); + auto result_pointer = detail::boost_dll::getSymbol(plugin_, variable_name); + if (result_pointer.hasValue()) { + // raw type casting necessary due to lack of type information in shared library + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) + return *reinterpret_cast(result_pointer.value().value()); } - return { p.error().value() }; + return { result_pointer.error().value() }; } template CallResult CppPlugin::global(const std::string& variable_name, VariableType&& new_value) { - auto p = detail::boost_dll::getSymbol(plugin_, variable_name); - if (p.hasValue()) { - *reinterpret_cast(p.value().value()) = std::forward(new_value); + auto result_pointer = detail::boost_dll::getSymbol(plugin_, variable_name); + if (result_pointer.hasValue()) { + // raw type casting necessary due to lack of type information in shared library + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) + *reinterpret_cast(result_pointer.value().value()) = std::forward(new_value); return {}; } - return { p.error().value() }; + return { result_pointer.error().value() }; } } // namespace ppplugin diff --git a/include/ppplugin/detail/scope_guard.h b/include/ppplugin/detail/scope_guard.h index c2cd92d..d33b52f 100644 --- a/include/ppplugin/detail/scope_guard.h +++ b/include/ppplugin/detail/scope_guard.h @@ -8,7 +8,7 @@ class ScopeGuard final { template >> explicit ScopeGuard(Func&& func) - : function_ { func } + : function_ { std::forward(func) } { } ~ScopeGuard() { call(); } @@ -40,8 +40,7 @@ class ScopeGuard final { void cancel() { function_ = {}; } private: - std::function - function_; + std::function function_; }; } // namespace ppplugin::detail diff --git a/include/ppplugin/errors.h b/include/ppplugin/errors.h index 8a85b5c..a1e48da 100644 --- a/include/ppplugin/errors.h +++ b/include/ppplugin/errors.h @@ -13,7 +13,7 @@ namespace ppplugin { class CallError { public: - enum class Code { + enum class Code : std::uint8_t { unknown, notLoaded, symbolNotFound, @@ -92,7 +92,7 @@ class CallError { } } -enum class LoadError { +enum class LoadError : std::uint8_t { unknown, fileNotFound, fileInvalid, diff --git a/include/ppplugin/python/python_forward_defs.h b/include/ppplugin/python/python_forward_defs.h index 54663e7..944e213 100644 --- a/include/ppplugin/python/python_forward_defs.h +++ b/include/ppplugin/python/python_forward_defs.h @@ -4,9 +4,12 @@ // TODO: these types is not a part of the official API; // consider alternatives like storing void* and casting to actual // type in source file (via helper function) -struct _ts; // NOLINT(bugprone-reserved-identifier); Python defines this type name + +// NOLINTNEXTLINE(bugprone-reserved-identifier,readability-identifier-naming) +struct _ts; // Python defines this type name using PyThreadState = _ts; -struct _object; // NOLINT(bugprone-reserved-identifier); Python defines this type name +// NOLINTNEXTLINE(bugprone-reserved-identifier,readability-identifier-naming) +struct _object; // Python defines this type name using PyObject = _object; #endif // PPPLUGIN_PYTHON_FORWARD_DEFS_H diff --git a/include/ppplugin/python/python_interpreter.h b/include/ppplugin/python/python_interpreter.h index 264e7b8..6d451bc 100644 --- a/include/ppplugin/python/python_interpreter.h +++ b/include/ppplugin/python/python_interpreter.h @@ -63,7 +63,7 @@ class PythonInterpreter { template CallResult PythonInterpreter::call(const std::string& function_name, Args&&... args) { - PythonGuard python_guard { state() }; + const PythonGuard python_guard { state() }; PythonTuple args_tuple { std::forward(args)... }; return internalCall(function_name, args_tuple.pyObject()).andThen([](PythonObject&& result) -> CallResult { @@ -81,7 +81,7 @@ CallResult PythonInterpreter::call(const std::string& function_name template CallResult PythonInterpreter::global(const std::string& variable_name) { - PythonGuard python_guard { state() }; + const PythonGuard python_guard { state() }; return internalGlobal(variable_name).andThen([](PythonObject&& object) -> CallResult { if (auto result = std::move(object).template as()) { return *result; @@ -93,7 +93,7 @@ CallResult PythonInterpreter::global(const std::string& variable_n template CallResult PythonInterpreter::global(const std::string& variable_name, VariableType&& new_value) { - PythonGuard python_guard { state() }; + const PythonGuard python_guard { state() }; return internalGlobal(variable_name, PythonObject::from(std::forward(new_value))); } } // namespace ppplugin diff --git a/include/ppplugin/python/python_object.h b/include/ppplugin/python/python_object.h index be777a4..f63243b 100644 --- a/include/ppplugin/python/python_object.h +++ b/include/ppplugin/python/python_object.h @@ -109,12 +109,12 @@ class PythonObject { }; template -PythonObject PythonObject::from(const std::map& dict) +PythonObject PythonObject::from(const std::map& value) { PythonObject object { initDict() }; - for (auto&& [key, value] : dict) { - auto py_key = PythonObject::from(key); - auto py_value = PythonObject::from(value); + for (auto&& [item_key, item_value] : value) { + auto py_key = PythonObject::from(item_key); + auto py_value = PythonObject::from(item_value); object.setDictItem(py_key.pyObject(), py_value.pyObject()); } return object; diff --git a/include/ppplugin/shell/plugin.h b/include/ppplugin/shell/plugin.h index fd5df83..3392430 100644 --- a/include/ppplugin/shell/plugin.h +++ b/include/ppplugin/shell/plugin.h @@ -55,9 +55,9 @@ template CallResult ShellPlugin::call(const std::string& function_name, Args&&... args) { if constexpr (std::is_void_v) { - return shell_.callWithoutResult(function_name, { args... }); + return shell_.callWithoutResult(function_name, { std::forward(args)... }); } else { - return shell_.callWithResult(function_name, { args... }); + return shell_.callWithResult(function_name, { std::forward(args)... }); } } diff --git a/include/ppplugin/shell/shell_session.h b/include/ppplugin/shell/shell_session.h index 6c76a4d..08eb5ae 100644 --- a/include/ppplugin/shell/shell_session.h +++ b/include/ppplugin/shell/shell_session.h @@ -31,13 +31,13 @@ class ShellSession { thread_ = std::thread { [context = context_.get()]() { assert(context); runContextLoop(*context); } }; // TODO: make movable across threads - shell_process_.async_wait([stdout_pipe = stdout_pipe_.get(), is_running = is_running_.get()](const boost::system::error_code& ec, int exit_code) { - std::cout << "PROCESS EXIT: " << ec.what() << " (" << exit_code << ")\n"; - assert(stdout_pipe); - stdout_pipe->cancel(); - assert(is_running); - is_running->store(false); - }); + shell_process_.async_wait( + [stdout_pipe = stdout_pipe_.get(), is_running = is_running_.get()](const boost::system::error_code& /*exit_status*/, int /*exit_code*/) { + assert(stdout_pipe); + stdout_pipe->cancel(); + assert(is_running); + is_running->store(false); + }); } ~ShellSession() { @@ -53,9 +53,9 @@ class ShellSession { } } ShellSession(const ShellSession&) = delete; - ShellSession(ShellSession&&) = default; + ShellSession(ShellSession&&) noexcept = default; ShellSession& operator=(const ShellSession&) = delete; - ShellSession& operator=(ShellSession&&) = default; + ShellSession& operator=(ShellSession&&) noexcept = default; [[nodiscard]] CallResult callWithoutResult(const std::string& function_name, const std::vector& arguments) { @@ -127,7 +127,7 @@ class ShellSession { [[nodiscard]] CallResult environmentVariable(const std::string& variable_name, const std::string& new_value) { - std::string variable_assignment = variable_name + "='" + new_value + '\''; + const std::string variable_assignment = variable_name + "='" + new_value + '\''; return callWithoutResult("export", { variable_assignment }); } From 158ac9702e021872f7e5900a8199270958e1ff11 Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Tue, 7 Apr 2026 01:57:07 +0200 Subject: [PATCH 09/15] ci|cmake|shell: Fix Boost::process dependency for Boost version < 1.85 --- .github/workflows/build-and-test.yml | 10 +++++++++- .github/workflows/cpp-lint.yml | 4 ++++ CMakeLists.txt | 11 ++++++++++- cmake/ppplugin-config.cmake.in | 10 ++++++++-- include/ppplugin/shell/shell_session.h | 14 ++++++++++---- src/CMakeLists.txt | 2 +- 6 files changed, 42 insertions(+), 9 deletions(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index d806a96..fcee63c 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -61,8 +61,12 @@ jobs: gtest-dev - name: Fixup environment run: | + # add missing final symlink for Lua library ln -s liblua-5.2.so.0 /usr/lib/liblua-5.2.so + # use ninja-build as ninja binary (alternative: install samurai instead) ln -s /usr/lib/ninja-build/bin/ninja /usr/bin/ninja + # patch Boost.process to not use ::close_range() since Alpine's musl implementation does not provide it + sed -i 's/#define BOOST_PROCESS_V2_HAS_CLOSE_RANGE 1/\/\/#define BOOST_PROCESS_V2_HAS_CLOSE_RANGE 1\n#include /g' /usr/include/boost/process/v2/posix/detail/close_handles.ipp shell: alpine.sh --root {0} - name: Prepare @@ -179,8 +183,12 @@ jobs: gtest-dev - name: Fixup environment run: | - ln -s /usr/lib/ninja-build/bin/ninja /usr/bin/ninja + # add missing final symlink for Lua library ln -s liblua-5.2.so.0 /usr/lib/liblua-5.2.so + # use ninja-build as ninja binary (alternative: install samurai instead) + ln -s /usr/lib/ninja-build/bin/ninja /usr/bin/ninja + # patch Boost.process to not use ::close_range() since Alpine's musl implementation does not provide it + sed -i 's/#define BOOST_PROCESS_V2_HAS_CLOSE_RANGE 1/\/\/#define BOOST_PROCESS_V2_HAS_CLOSE_RANGE 1\n#include /g' /usr/include/boost/process/v2/posix/detail/close_handles.ipp shell: alpine.sh --root {0} - name: Run cmake tests diff --git a/.github/workflows/cpp-lint.yml b/.github/workflows/cpp-lint.yml index b0f2164..b5d96ec 100644 --- a/.github/workflows/cpp-lint.yml +++ b/.github/workflows/cpp-lint.yml @@ -47,8 +47,12 @@ jobs: clang19-extra-tools - name: Fixup environment run: | + # add missing final symlink for Lua library ln -s liblua-5.2.so.0 /usr/lib/liblua-5.2.so + # use ninja-build as ninja binary (alternative: install samurai instead) ln -s /usr/lib/ninja-build/bin/ninja /usr/bin/ninja + # patch Boost.process to not use ::close_range() since Alpine's musl implementation does not provide it + sed -i 's/#define BOOST_PROCESS_V2_HAS_CLOSE_RANGE 1/\/\/#define BOOST_PROCESS_V2_HAS_CLOSE_RANGE 1\n#include /g' /usr/include/boost/process/v2/posix/detail/close_handles.ipp shell: alpine.sh --root {0} - name: Generate compile_commands.json diff --git a/CMakeLists.txt b/CMakeLists.txt index 5a66e06..509c807 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -24,7 +24,16 @@ option(PPPLUGIN_ENABLE_UNDEFINED_SANITIZE option(PPPLUGIN_ENABLE_UNREACHABLE_SANITIZE "Enable compilation with unreachable sanitize flags" OFF) -find_package(Boost 1.70.0 REQUIRED CONFIG COMPONENTS headers filesystem process) +# required Boost header-only components: dll, algorithm, asio, process, uuid; +# Boost.filesystem is only required because of Boost.dll +find_package(Boost 1.70.0 REQUIRED CONFIG COMPONENTS headers filesystem) +if(${Boost_VERSION} VERSION_GREATER_EQUAL 1.85.0) + # Boost.process was header-only until 1.84; requires linking in >= 1.85 + find_package(Boost REQUIRED CONFIG COMPONENTS process) +else() + # add dummy target so that this check is only necessary once + add_library(Boost::process ALIAS Boost::headers) +endif() find_package(Python 3.0 REQUIRED COMPONENTS Development) find_package(Lua 5.2 REQUIRED) diff --git a/cmake/ppplugin-config.cmake.in b/cmake/ppplugin-config.cmake.in index 996c55f..dade355 100644 --- a/cmake/ppplugin-config.cmake.in +++ b/cmake/ppplugin-config.cmake.in @@ -1,8 +1,14 @@ @PACKAGE_INIT@ include(CMakeFindDependencyMacro) -find_dependency(Boost @Boost_VERSION_MAJOR@ COMPONENTS headers filesystem - python process) +find_dependency(Boost @Boost_VERSION_MAJOR@ CONFIG COMPONENTS headers filesystem) +if(@Boost_VERSION@ VERSION_GREATER_EQUAL 1.85.0) + # Boost.process was header-only until 1.84; requires linking in >= 1.85 + find_dependency(Boost CONFIG COMPONENTS process) +else() + # add dummy target so that this check is only necessary once + add_library(Boost::process ALIAS Boost::headers) +endif() find_dependency(Python @Python_VERSION_MAJOR@ COMPONENTS Development) find_dependency(Lua @LUA_VERSION_MAJOR@) diff --git a/include/ppplugin/shell/shell_session.h b/include/ppplugin/shell/shell_session.h index 08eb5ae..830a9f0 100644 --- a/include/ppplugin/shell/shell_session.h +++ b/include/ppplugin/shell/shell_session.h @@ -12,9 +12,15 @@ #include #include +#include +#if BOOST_VERSION >= 108800 #include -#include -#include +#else +#include +#endif // BOOST_VERSION +#include +#include +#include namespace ppplugin { class ShellSession { @@ -167,8 +173,8 @@ class ShellSession { boost::asio::writable_pipe stdin_pipe_; std::unique_ptr stdout_pipe_; boost::asio::readable_pipe stderr_pipe_; - boost::process::process_stdio io_; - boost::process::process shell_process_; + boost::process::v2::process_stdio io_; + boost::process::v2::process shell_process_; std::unique_ptr> is_running_; // TODO: explore possibility to import/export variables and functions for each call diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 2f59fd5..e4c35df 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -21,7 +21,7 @@ add_library(${LIBRARY_TARGET} ${LIBRARY_SOURCES} ${LIBRARY_HEADERS}) target_link_libraries( ${LIBRARY_TARGET} PRIVATE ${LUA_LIBRARIES} - PUBLIC Boost::filesystem Boost::process Python::Python + PUBLIC Boost::headers Boost::filesystem Boost::process Python::Python $<$:fmt::fmt>) target_include_directories( ${LIBRARY_TARGET} From c8376b879cbff7c5a6c949ceb29b2e9709a6b949 Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Tue, 7 Apr 2026 01:57:47 +0200 Subject: [PATCH 10/15] shell: Minor cleanup --- include/ppplugin/shell/plugin.h | 3 +++ include/ppplugin/shell/shell_session.h | 11 +++++------ src/shell/plugin.cpp | 2 -- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/include/ppplugin/shell/plugin.h b/include/ppplugin/shell/plugin.h index 3392430..7ae0e01 100644 --- a/include/ppplugin/shell/plugin.h +++ b/include/ppplugin/shell/plugin.h @@ -21,6 +21,8 @@ class ShellPlugin { // NOLINTNEXTLINE(google-explicit-constructor,hicpp-explicit-conversions) operator bool() { return shell_.isRunning(); } + // NOLINTNEXTLINE(google-explicit-constructor,hicpp-explicit-conversions) + operator bool() const { return shell_.isRunning(); } /** * Accepted types are: @@ -57,6 +59,7 @@ CallResult ShellPlugin::call(const std::string& function_name, Args if constexpr (std::is_void_v) { return shell_.callWithoutResult(function_name, { std::forward(args)... }); } else { + // TODO: handle results other than std::string return shell_.callWithResult(function_name, { std::forward(args)... }); } } diff --git a/include/ppplugin/shell/shell_session.h b/include/ppplugin/shell/shell_session.h index 830a9f0..c69ce22 100644 --- a/include/ppplugin/shell/shell_session.h +++ b/include/ppplugin/shell/shell_session.h @@ -2,7 +2,6 @@ #define PPPLUGIN_SHELL_SESSION_H #include "ppplugin/detail/string_utils.h" -#include "ppplugin/detail/thread_safe.h" #include "ppplugin/errors.h" #include @@ -84,7 +83,7 @@ class ShellSession { } auto end_marker = boost::uuids::to_string(boost::uuids::random_generator()()) + '\n'; command_line += " ; echo :$?" + end_marker; - std::cout << "WRITING: " << command_line; + boost::asio::write(stdin_pipe_, boost::asio::buffer(command_line)); std::string result; @@ -109,15 +108,12 @@ class ShellSession { auto exit_code_pos = result.find_last_of(':'); assert(exit_code_pos != std::string::npos); auto exit_code = toInteger(result.substr(exit_code_pos + 1)).value_or(std::numeric_limits::max()); - std::cout << "EXIT_CODE: " << exit_code << '\n'; if (exit_code != 0) { return CallError { CallError::Code::unknown, std::to_string(exit_code) }; } + // remove colon and exit code result.resize(exit_code_pos); - std::cout << "DONE: " << command_line << '\n'; - std::cout << "RESULT: " << result << '\n'; - return result; } @@ -139,11 +135,14 @@ class ShellSession { [[nodiscard]] bool isRunning() { + // if a non-const object is available, return actual status return shell_process_.running(); } [[nodiscard]] bool isRunning() const { + // return cached state because boost::process::v2::process::running() + // is not const since it modifies its internal state return is_running_->load(); } diff --git a/src/shell/plugin.cpp b/src/shell/plugin.cpp index e048ad7..1627ce2 100644 --- a/src/shell/plugin.cpp +++ b/src/shell/plugin.cpp @@ -3,8 +3,6 @@ #include "ppplugin/errors.h" #include "ppplugin/expected.h" -#include - namespace ppplugin { Expected ShellPlugin::load( const std::filesystem::path& plugin_library_path) From 7fbc12657f08d1ad2e483f102013b9e6e63a0634 Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Tue, 7 Apr 2026 02:00:25 +0200 Subject: [PATCH 11/15] python: Fix include order due to source file renaming --- src/python/plugin.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/python/plugin.cpp b/src/python/plugin.cpp index 3c11c85..0e7c99c 100644 --- a/src/python/plugin.cpp +++ b/src/python/plugin.cpp @@ -1,6 +1,6 @@ +#include "ppplugin/python/plugin.h" #include "ppplugin/errors.h" #include "ppplugin/expected.h" -#include "ppplugin/python/plugin.h" #include From 4f2a153778df1ac8a6bedf56e7778cb9d75df51e Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Tue, 7 Apr 2026 02:07:39 +0200 Subject: [PATCH 12/15] cmake|errors: Fix minor CI issues --- include/ppplugin/errors.h | 1 + src/CMakeLists.txt | 5 ++--- test/shell_tests/CMakeLists.txt | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/ppplugin/errors.h b/include/ppplugin/errors.h index a1e48da..00cbdb0 100644 --- a/include/ppplugin/errors.h +++ b/include/ppplugin/errors.h @@ -4,6 +4,7 @@ #include "ppplugin/detail/compatibility_utils.h" #include "ppplugin/expected.h" +#include #ifndef PPPLUGIN_CPP17_COMPATIBILITY #include #endif // PPPLUGIN_CPP17_COMPATIBILITY diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index e4c35df..6713cbf 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -27,7 +27,7 @@ target_include_directories( ${LIBRARY_TARGET} PRIVATE ${LUA_INCLUDE_DIR} PUBLIC $ - PUBLIC $) + $) target_compile_definitions( ${LIBRARY_TARGET} PUBLIC @@ -41,8 +41,7 @@ set_target_properties( SOVERSION ${ppplugin_VERSION_MAJOR}) set(TARGETS_EXPORT_NAME "${PROJECT_NAME}-targets") -install(DIRECTORY "${CMAKE_SOURCE_DIR}/include/" - DESTINATION "include/") +install(DIRECTORY "${CMAKE_SOURCE_DIR}/include/" DESTINATION "include/") install( TARGETS ${LIBRARY_TARGET} EXPORT "${TARGETS_EXPORT_NAME}" diff --git a/test/shell_tests/CMakeLists.txt b/test/shell_tests/CMakeLists.txt index e43c5f7..eeef6e7 100644 --- a/test/shell_tests/CMakeLists.txt +++ b/test/shell_tests/CMakeLists.txt @@ -6,4 +6,4 @@ add_custom_command( POST_BUILD COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_SOURCE_DIR}/test.sh ${CMAKE_CURRENT_BINARY_DIR}/test.sh - COMMENT "Copying POSIX shell plugins to output directory...") + COMMENT "Copying POSIX shell plugins to output directory...") From b36e59e5b70ae0975393b4c68e513d91091565d5 Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Tue, 7 Apr 2026 13:43:44 +0200 Subject: [PATCH 13/15] cmake: Use compile-time configuration of cpp17 mode for installed CMake config --- cmake/ppplugin-config.cmake.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/ppplugin-config.cmake.in b/cmake/ppplugin-config.cmake.in index dade355..5e89629 100644 --- a/cmake/ppplugin-config.cmake.in +++ b/cmake/ppplugin-config.cmake.in @@ -12,7 +12,7 @@ endif() find_dependency(Python @Python_VERSION_MAJOR@ COMPONENTS Development) find_dependency(Lua @LUA_VERSION_MAJOR@) -if(${PPPLUGIN_ENABLE_CPP17_COMPATIBILITY}) +if(@PPPLUGIN_ENABLE_CPP17_COMPATIBILITY@) find_dependency(fmt @fmt_VERSION_MAJOR@) endif() From 00d6f4332438e18c91025cbd64609c97368a8de1 Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Wed, 8 Apr 2026 21:23:55 +0200 Subject: [PATCH 14/15] templates|strings: Add IsAnyOfV and IsStringlikeV helpers --- include/ppplugin/detail/string_utils.h | 10 ++++++++-- include/ppplugin/detail/template_helpers.h | 10 ++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/include/ppplugin/detail/string_utils.h b/include/ppplugin/detail/string_utils.h index 83644d9..caf41af 100644 --- a/include/ppplugin/detail/string_utils.h +++ b/include/ppplugin/detail/string_utils.h @@ -1,11 +1,17 @@ #ifndef PPPLUGIN_DETAIL_STRING_UTILS_H #define PPPLUGIN_DETAIL_STRING_UTILS_H +#include "template_helpers.h" + #include #include #include -namespace ppplugin { +namespace ppplugin::detail { +template +constexpr auto IsStringlikeV = // NOLINT(readability-identifier-naming) + templates::IsAnyOfV; + [[nodiscard]] inline bool endsWith(std::string_view string, std::string_view end) { if (string.size() < end.size()) { @@ -25,6 +31,6 @@ template } return std::nullopt; } -} // namespace ppplugin +} // namespace ppplugin::detail #endif // PPPLUGIN_DETAIL_STRING_UTILS_H diff --git a/include/ppplugin/detail/template_helpers.h b/include/ppplugin/detail/template_helpers.h index b09f224..90d619f 100644 --- a/include/ppplugin/detail/template_helpers.h +++ b/include/ppplugin/detail/template_helpers.h @@ -185,6 +185,9 @@ struct IsSpecialization : std::false_type { }; template