From 27abada6fb7aeb7ed9a7f5936764237b0a62b36d Mon Sep 17 00:00:00 2001 From: Ivar Scholten Date: Thu, 27 Nov 2025 16:12:37 +0100 Subject: [PATCH 1/6] manager: fix a few gcc/clang-tidy/spellchecker lints Signed-off-by: Ivar Scholten --- src/manager.cpp | 10 ++++------ src/system_unix.cpp | 9 ++++++--- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/manager.cpp b/src/manager.cpp index 9cfd2e1c..8c00bbb1 100644 --- a/src/manager.cpp +++ b/src/manager.cpp @@ -4,10 +4,8 @@ #include #include #include -#include #include #include -#include #include #include @@ -342,7 +340,7 @@ std::map start_modules(ManagerConfig& config, MQTTAbstractio continue; } - // FIXME (aw): implicitely adding ModuleReadyInfo and setting its ready member + // FIXME (aw): implicitly adding ModuleReadyInfo and setting its ready member auto module_it = modules_ready.emplace(module_id, ModuleReadyInfo{false, nullptr, nullptr}).first; std::vector capabilities; @@ -352,7 +350,7 @@ std::map start_modules(ManagerConfig& config, MQTTAbstractio } if (not capabilities.empty()) { - EVLOG_info << fmt::format("Module {} wants to aquire the following capabilities: {}", module_name, + EVLOG_info << fmt::format("Module {} wants to acquire the following capabilities: {}", module_name, fmt::join(capabilities.begin(), capabilities.end(), " ")); } @@ -808,7 +806,7 @@ int boot(const po::variables_map& vm) { const auto module_iter = module_handles.find(pid); if (module_iter == module_handles.end()) { - throw std::runtime_error(fmt::format("Unkown child width pid ({}) died.", pid)); + throw std::runtime_error(fmt::format("Unknown child width pid ({}) died.", pid)); } const auto module_name = module_iter->second; @@ -859,7 +857,7 @@ int boot(const po::variables_map& vm) { } } else { // unknown payload - EVLOG_error << fmt::format("Received unkown command via controller ipc:\n{}\n... ignoring", + EVLOG_error << fmt::format("Received unknown command via controller ipc:\n{}\n... ignoring", payload.dump(DUMP_INDENT)); } } else if (msg.status == controller_ipc::MESSAGE_RETURN_STATUS::ERROR) { diff --git a/src/system_unix.cpp b/src/system_unix.cpp index 7335b824..2c984881 100644 --- a/src/system_unix.cpp +++ b/src/system_unix.cpp @@ -61,8 +61,10 @@ GetPasswdEntryResult get_passwd_entry(const std::string& user_name) { return GetPasswdEntryResult("Could not get supplementary groups for user name: " + user_name); } - return GetPasswdEntryResult(entry->pw_uid, entry->pw_gid, - std::vector(groups.begin(), groups.begin() + ngroups)); + // Clang-tidy recommends using `std::span` here instead, which isn't available in C++17. + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic) + const std::vector user_groups{groups.begin(), groups.begin() + ngroups}; + return GetPasswdEntryResult(entry->pw_uid, entry->pw_gid, user_groups); } } // namespace @@ -137,7 +139,8 @@ std::string set_real_user(const std::string& user_name) { void SubProcess::send_error_and_exit(const std::string& message) { assert(pid == 0); - write(fd, message.c_str(), std::min(message.size(), MAX_PIPE_MESSAGE_SIZE - 1)); + // There isn't much we can do if writing the error message fails, just exit + [[maybe_unused]] auto _write = write(fd, message.c_str(), std::min(message.size(), MAX_PIPE_MESSAGE_SIZE - 1)); close(fd); _exit(EXIT_FAILURE); } From 339b3b47a30efa8a026a5778300a3ff1f574de34 Mon Sep 17 00:00:00 2001 From: Ivar Scholten Date: Thu, 27 Nov 2025 16:28:58 +0100 Subject: [PATCH 2/6] manager: simplify module execution Signed-off-by: Ivar Scholten --- src/manager.cpp | 52 ++++++++++++++----------------------------------- 1 file changed, 15 insertions(+), 37 deletions(-) diff --git a/src/manager.cpp b/src/manager.cpp index 8c00bbb1..d7c3e0ba 100644 --- a/src/manager.cpp +++ b/src/manager.cpp @@ -116,19 +116,23 @@ void setup_environment(const ModuleStartInfo& module_info, const RuntimeSettings } } -std::vector arguments_to_exec_argv(std::vector& arguments) { +static void exec_module(const std::string& bin, std::vector& arguments, system::SubProcess& proc_handle) { + // Convert the argument list to the format required by `execv()`. std::vector argv_list(arguments.size() + 1); - std::transform(arguments.begin(), arguments.end(), argv_list.begin(), - [](std::string& value) { return value.data(); }); + std::transform(arguments.begin(), arguments.end(), argv_list.begin(), [](auto& value) { return value.data(); }); + argv_list.back() = nullptr; // Add a null terminator - // add NULL for exec - argv_list.back() = nullptr; - return argv_list; + // Execute the module binary, replacing the current process. + execv(bin.c_str(), argv_list.data()); + + // `execv()` failed, notify the parent process and exit. + const auto msg = fmt::format("Syscall to execv() with \"{} {}\" failed ({})", bin, + fmt::join(arguments.begin() + 1, arguments.end(), " "), strerror(errno)); + proc_handle.send_error_and_exit(msg); } void exec_cpp_module(system::SubProcess& proc_handle, const ModuleStartInfo& module_info, const RuntimeSettings& rs, const MQTTSettings& mqtt_settings) { - const auto exec_binary = module_info.path.c_str(); std::vector arguments = { module_info.printable_name, "--prefix", @@ -149,13 +153,7 @@ void exec_cpp_module(system::SubProcess& proc_handle, const ModuleStartInfo& mod std::to_string(mqtt_settings.broker_port)}); } - const auto argv_list = arguments_to_exec_argv(arguments); - execv(exec_binary, argv_list.data()); - - // exec failed - proc_handle.send_error_and_exit(fmt::format("Syscall to execv() with \"{} {}\" failed ({})", exec_binary, - fmt::join(arguments.begin() + 1, arguments.end(), " "), - strerror(errno))); + exec_module(module_info.path.string(), arguments, proc_handle); } void exec_javascript_module(system::SubProcess& proc_handle, const ModuleStartInfo& module_info, @@ -165,24 +163,15 @@ void exec_javascript_module(system::SubProcess& proc_handle, const ModuleStartIn // FIXME (aw): everest directory layout const auto node_modules_path = rs.prefix / defaults::LIB_DIR / defaults::NAMESPACE / "node_modules"; setenv("NODE_PATH", node_modules_path.c_str(), 0); - setup_environment(module_info, rs, mqtt_settings); - const auto node_binary = "node"; - std::vector arguments = { "node", "--unhandled-rejections=strict", module_info.path.string(), }; - const auto argv_list = arguments_to_exec_argv(arguments); - execvp(node_binary, argv_list.data()); - - // exec failed - proc_handle.send_error_and_exit(fmt::format("Syscall to execv() with \"{} {}\" failed ({})", node_binary, - fmt::join(arguments.begin() + 1, arguments.end(), " "), - strerror(errno))); + exec_module("node", arguments, proc_handle); } void exec_python_module(system::SubProcess& proc_handle, const ModuleStartInfo& module_info, const RuntimeSettings& rs, @@ -190,22 +179,11 @@ void exec_python_module(system::SubProcess& proc_handle, const ModuleStartInfo& // instead of using setenv, using execvpe might be a better way for a controlled environment! const auto pythonpath = rs.prefix / defaults::LIB_DIR / defaults::NAMESPACE / "everestpy"; - setenv("PYTHONPATH", pythonpath.c_str(), 0); - setup_environment(module_info, rs, mqtt_settings); - const auto python_binary = "python3"; - - std::vector arguments = {python_binary, module_info.path.c_str()}; - - const auto argv_list = arguments_to_exec_argv(arguments); - execvp(python_binary, argv_list.data()); - - // exec failed - proc_handle.send_error_and_exit(fmt::format("Syscall to execv() with \"{} {}\" failed ({})", python_binary, - fmt::join(arguments.begin() + 1, arguments.end(), " "), - strerror(errno))); + std::vector arguments = {"python3", module_info.path.c_str()}; + exec_module("python3", arguments, proc_handle); } void exec_module(const RuntimeSettings& rs, const MQTTSettings& mqtt_settings, const ModuleStartInfo& module, From f3ee6e8900e3442be8f9ed1b6720d6d5080195ed Mon Sep 17 00:00:00 2001 From: Ivar Scholten Date: Thu, 27 Nov 2025 16:44:17 +0100 Subject: [PATCH 3/6] manager: append to PYTHONPATH when spawning Python modules When spawning Python modules the manager previously only set PYTHONPATH if it isn't already present in the environment, which can result in modules not being able to locate the `everestpy` package. This commit fixes that by prepending the `everestpy` path to PYTHONPATH if already set. Signed-off-by: Ivar Scholten --- src/manager.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/manager.cpp b/src/manager.cpp index d7c3e0ba..b0c958b5 100644 --- a/src/manager.cpp +++ b/src/manager.cpp @@ -178,10 +178,17 @@ void exec_python_module(system::SubProcess& proc_handle, const ModuleStartInfo& const MQTTSettings& mqtt_settings) { // instead of using setenv, using execvpe might be a better way for a controlled environment! - const auto pythonpath = rs.prefix / defaults::LIB_DIR / defaults::NAMESPACE / "everestpy"; - setenv("PYTHONPATH", pythonpath.c_str(), 0); setup_environment(module_info, rs, mqtt_settings); + // Prepend the everestpy path to $PYTHONPATH. This ensures modules can always find everestpy. + const auto everestpy_path = rs.prefix / defaults::LIB_DIR / defaults::NAMESPACE / "everestpy"; + if (const auto prev_pythonpath = std::getenv("PYTHONPATH")) { + const auto pythonpath = fmt::format("{}:{}", everestpy_path.string(), prev_pythonpath); + setenv("PYTHONPATH", pythonpath.c_str(), 1); + } else { + setenv("PYTHONPATH", everestpy_path.c_str(), 1); + } + std::vector arguments = {"python3", module_info.path.c_str()}; exec_module("python3", arguments, proc_handle); } From 29a114f320500aad87c5a3b51d09e51afe81d944 Mon Sep 17 00:00:00 2001 From: Ivar Scholten Date: Thu, 27 Nov 2025 17:52:48 +0100 Subject: [PATCH 4/6] python: activate virtual environments if present We noticed the lack of Python dependency isolation with `pydantic`, `PyEvJosev` requires a rather old version which conflicts with our own modules. While we could upgrade Josev to temporarily fix the problem, this doesn't scale well: having modules be tightly coupled to one other's internals makes it hard to develop them in isolation. This commit makes the first step towards fixing that by having the manager process activate each module's virtual environment before starting it (if present). It does not automatically generate or install the module's dependencies in the virtual environment. Ideally CMake would generate it for us at build time, but this turns out to be a bit trickier than expected: * Virtual environments are not portable, they contain absolute paths to themselves in various generated scripts. * While `everest-cmake` is already able to generate a wheel for each module (containing the dependency manifest and the module itself), it isn't able to cross-compile them. This matters because a lot of Python libraries link to native code that is fetched/compiled at install time, so forcibly installing wheels generated by CMake would break EVerest's cross-compilation support. There sadly doesn't appear to be a standardized way to indicate a cross-compilation context, though we might be able to utilize projects like crossenv[1] or cibuildwheel[2]. This needs more investigation. * If cross-compilation doesn't turn out to be viable we could instead install dependencies at runtime, from the manager process. This completely resolves any cross-compilation concerns, but in turn has the downside of slowing down module startup. Assuming we're fine with requiring `pyproject.toml` (PEP 621) for dependency management, the installation procedure could look something like this: - Install the `pyproject.toml` to a standardized location from CMake. - At runtime, use the build[3] module to generate a virtual environment from the `pyproject.toml`. It looks like we can install only the `build-frontend` and dependencies with a bit of scripting, so that we don't redundantly include the module itself like the CLI does by default. This is useful for incrementality, directly copying the module source when making a change is a lot faster than going through the build frontend. In any case, after creating a virtual environment we would need to enter it from the manager process, which this commit does. I'm not sure what the best way to proceed forward is, I'd love some feedback. [1]: https://pypi.org/project/crossenv [2]: https://github.com/pypa/cibuildwheel [3]: https://pypi.org/project/build Signed-off-by: Ivar Scholten --- src/manager.cpp | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/src/manager.cpp b/src/manager.cpp index b0c958b5..e3a82b5c 100644 --- a/src/manager.cpp +++ b/src/manager.cpp @@ -189,8 +189,31 @@ void exec_python_module(system::SubProcess& proc_handle, const ModuleStartInfo& setenv("PYTHONPATH", everestpy_path.c_str(), 1); } - std::vector arguments = {"python3", module_info.path.c_str()}; - exec_module("python3", arguments, proc_handle); + std::string python_binary = "python3"; + + // Check if a virtual environment exists in the module directory, and if so use its python runtime. + const auto venv_dir = module_info.path.parent_path() / ".venv"; + if (fs::exists(venv_dir)) { + const auto venv_bin_dir = venv_dir / "bin"; + const auto venv_python = venv_bin_dir / "python3"; + if (fs::exists(venv_python)) { + // Activate the virtual environment. This approximates the behaviour of the `.venv/bin/activate` script. + python_binary = venv_python.string(); + setenv("VIRTUAL_ENV", venv_dir.c_str(), 1); + setenv("VIRTUAL_ENV_PROMPT", "venv", 1); + unsetenv("PYTHONHOME"); + + if (const auto prev_path = std::getenv("PATH")) { + const auto path = fmt::format("{}:{}", venv_bin_dir.string(), prev_path); + setenv("PATH", path.c_str(), 1); + } else { + setenv("PATH", venv_bin_dir.c_str(), 1); + } + } + } + + std::vector arguments = {python_binary, module_info.path.c_str()}; + exec_module(python_binary, arguments, proc_handle); } void exec_module(const RuntimeSettings& rs, const MQTTSettings& mqtt_settings, const ModuleStartInfo& module, From b24e809e1b14e37241619aab401e7d520fc99c64 Mon Sep 17 00:00:00 2001 From: Andreas Heinrich Date: Mon, 1 Dec 2025 19:38:30 +0100 Subject: [PATCH 5/6] Use execvp instead of execv, since in case of 'python3' or 'node' PATH lookup is needed Signed-off-by: Andreas Heinrich --- src/manager.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/manager.cpp b/src/manager.cpp index dd6e0d16..719ceb03 100644 --- a/src/manager.cpp +++ b/src/manager.cpp @@ -117,13 +117,13 @@ void setup_environment(const ModuleStartInfo& module_info, const RuntimeSettings } static void exec_module(const std::string& bin, std::vector& arguments, system::SubProcess& proc_handle) { - // Convert the argument list to the format required by `execv()`. + // Convert the argument list to the format required by `execv*()`. std::vector argv_list(arguments.size() + 1); std::transform(arguments.begin(), arguments.end(), argv_list.begin(), [](auto& value) { return value.data(); }); argv_list.back() = nullptr; // Add a null terminator // Execute the module binary, replacing the current process. - execv(bin.c_str(), argv_list.data()); + execvp(bin.c_str(), argv_list.data()); // `execv()` failed, notify the parent process and exit. const auto msg = fmt::format("Syscall to execv() with \"{} {}\" failed ({})", bin, From fef9a359d30f5d64b0840bc61316a205d3d73e8d Mon Sep 17 00:00:00 2001 From: Andreas Heinrich Date: Mon, 1 Dec 2025 19:40:22 +0100 Subject: [PATCH 6/6] Revert "manager: fix a few gcc/clang-tidy/spellchecker lints" This reverts commit 27abada6fb7aeb7ed9a7f5936764237b0a62b36d. Signed-off-by: Andreas Heinrich --- src/manager.cpp | 10 ++++++---- src/system_unix.cpp | 9 +++------ 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/manager.cpp b/src/manager.cpp index 719ceb03..466d7974 100644 --- a/src/manager.cpp +++ b/src/manager.cpp @@ -4,8 +4,10 @@ #include #include #include +#include #include #include +#include #include #include @@ -374,7 +376,7 @@ std::map start_modules(ManagerConfig& config, MQTTAbstractio continue; } - // FIXME (aw): implicitly adding ModuleReadyInfo and setting its ready member + // FIXME (aw): implicitely adding ModuleReadyInfo and setting its ready member auto module_it = modules_ready.emplace(module_id, ModuleReadyInfo{false, nullptr, nullptr}).first; std::vector capabilities; @@ -384,7 +386,7 @@ std::map start_modules(ManagerConfig& config, MQTTAbstractio } if (not capabilities.empty()) { - EVLOG_info << fmt::format("Module {} wants to acquire the following capabilities: {}", module_name, + EVLOG_info << fmt::format("Module {} wants to aquire the following capabilities: {}", module_name, fmt::join(capabilities.begin(), capabilities.end(), " ")); } @@ -843,7 +845,7 @@ int boot(const po::variables_map& vm) { const auto module_iter = module_handles.find(pid); if (module_iter == module_handles.end()) { - throw std::runtime_error(fmt::format("Unknown child width pid ({}) died.", pid)); + throw std::runtime_error(fmt::format("Unkown child width pid ({}) died.", pid)); } const auto module_name = module_iter->second; @@ -894,7 +896,7 @@ int boot(const po::variables_map& vm) { } } else { // unknown payload - EVLOG_error << fmt::format("Received unknown command via controller ipc:\n{}\n... ignoring", + EVLOG_error << fmt::format("Received unkown command via controller ipc:\n{}\n... ignoring", payload.dump(DUMP_INDENT)); } } else if (msg.status == controller_ipc::MESSAGE_RETURN_STATUS::ERROR) { diff --git a/src/system_unix.cpp b/src/system_unix.cpp index 2c984881..7335b824 100644 --- a/src/system_unix.cpp +++ b/src/system_unix.cpp @@ -61,10 +61,8 @@ GetPasswdEntryResult get_passwd_entry(const std::string& user_name) { return GetPasswdEntryResult("Could not get supplementary groups for user name: " + user_name); } - // Clang-tidy recommends using `std::span` here instead, which isn't available in C++17. - // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic) - const std::vector user_groups{groups.begin(), groups.begin() + ngroups}; - return GetPasswdEntryResult(entry->pw_uid, entry->pw_gid, user_groups); + return GetPasswdEntryResult(entry->pw_uid, entry->pw_gid, + std::vector(groups.begin(), groups.begin() + ngroups)); } } // namespace @@ -139,8 +137,7 @@ std::string set_real_user(const std::string& user_name) { void SubProcess::send_error_and_exit(const std::string& message) { assert(pid == 0); - // There isn't much we can do if writing the error message fails, just exit - [[maybe_unused]] auto _write = write(fd, message.c_str(), std::min(message.size(), MAX_PIPE_MESSAGE_SIZE - 1)); + write(fd, message.c_str(), std::min(message.size(), MAX_PIPE_MESSAGE_SIZE - 1)); close(fd); _exit(EXIT_FAILURE); }