From 395e9622473d922b7227af41a53a609cb2219300 Mon Sep 17 00:00:00 2001 From: Rafal Sapala Date: Tue, 31 Mar 2026 15:26:45 +0200 Subject: [PATCH 01/22] Compiling --- src/BUILD | 4 +- src/python/BUILD | 110 +++++++++++++++++++++++++++++++++- src/python/python_backend.hpp | 22 ++++--- 3 files changed, 126 insertions(+), 10 deletions(-) diff --git a/src/BUILD b/src/BUILD index ea624f5e59..11fad6eb33 100644 --- a/src/BUILD +++ b/src/BUILD @@ -568,7 +568,7 @@ ovms_cc_library( }), deps = select({ "//:not_disable_python": [ - "//src/python:libovmspythonmodule", + "//src/python:libovmspython", ], "//:disable_python": [] }) + select({ @@ -1131,7 +1131,7 @@ ovms_cc_library( # TODO split dependencies "//src/kfserving_api:kfserving_api_cpp", ] + select({ "//:not_disable_python": [ - "//src/python:libovmspythonmodule", + "//src/python:libovmspython", ], "//:disable_python": [] }), diff --git a/src/python/BUILD b/src/python/BUILD index f4fd4c571e..4bc3f12597 100644 --- a/src/python/BUILD +++ b/src/python/BUILD @@ -16,7 +16,62 @@ load("@pybind11_bazel//:build_defs.bzl", "pybind_extension") load("@mediapipe//mediapipe/framework/port:build_config.bzl", "mediapipe_cc_proto_library", "mediapipe_proto_library") -load("//:common_settings.bzl", "PYBIND_DEPS", "ovms_cc_library") +load("//:common_settings.bzl", + "COMMON_STATIC_LIBS_LINKOPTS", + "COMMON_FUZZER_COPTS", "COMMON_FUZZER_LINKOPTS", + "COMMON_LOCAL_DEFINES", "PYBIND_DEPS", "ovms_cc_library") + +# Copts for the python shared library. Same hardening flags as +# LINUX_COMMON_STATIC_LIBS_COPTS but WITHOUT -fvisibility=hidden. +# Omitting -fvisibility=hidden lets entry-point symbols +# (PythonInterpreterModule, etc.) remain visible in the ELF dynamic +# symbol table, and avoids a -Werror=attributes conflict that arises +# when OvmsPyTensor (default visibility) has a pybind11::object field +# (hidden visibility). +_SHARED_LIB_COPTS_LINUX = [ + "-Wall", + "-Wno-unknown-pragmas", + "-Wno-sign-compare", + # -fvisibility=hidden intentionally omitted + "-Werror", + "-Wno-deprecated-declarations", + "-Wimplicit-fallthrough", + "-fcf-protection=full", + "-Wformat", + "-Wformat-security", + "-Werror=format-security", + "-Wl,-z,noexecstack", + "-fPIC", + "-Wl,-z,relro", + "-Wl,-z,relro,-z,now", + "-Wl,-z,nodlopen", + "-fstack-protector-strong", + # pybind11 declares its entire namespace with + # __attribute__((visibility("hidden"))), so any struct that contains a + # pybind11 type (e.g. py::object) will trigger -Wattributes when the + # containing struct has default visibility. This is expected and safe + # for code compiled into a single DSO — suppress the diagnostic here. + "-Wno-attributes", +] + +COPTS_SO = select({ + "//conditions:default": _SHARED_LIB_COPTS_LINUX, + "//src:windows": [], # Windows builds do not produce this .so +}) + select({ + "//conditions:default": ["-DPYTHON_DISABLE=1"], + "//:not_disable_python": ["-DPYTHON_DISABLE=0"], +}) + select({ + "//conditions:default": ["-DMEDIAPIPE_DISABLE=1"], + "//:not_disable_mediapipe": ["-DMEDIAPIPE_DISABLE=0"], +}) + select({ + "//conditions:default": [], + "//:fuzzer_build": COMMON_FUZZER_COPTS, +}) + +LINKOPTS_SO = COMMON_STATIC_LIBS_LINKOPTS + select({ + "//conditions:default": [], + "//:fuzzer_build": COMMON_FUZZER_LINKOPTS, +}) mediapipe_proto_library( name = "pythonexecutorcalculator_proto", # pythonexecutorcalculator_cc_proto - just mediapipe stuff with mediapipe_proto_library adding nonvisible target @@ -142,3 +197,56 @@ ovms_cc_library( alwayslink = 1, data = ["//src/python/binding:pyovms.so"], ) + +# Shared library built from all Python binding sources. +# Deps whose symbols are compiled with -fvisibility=hidden (the project default) +# are linked into the .so with hidden visibility; only the python module +# interface symbols compiled via srcs/COPTS_SO are exported. +cc_binary( + name = "libovmspython.so", + linkshared = True, + srcs = [ + "ovms_py_tensor.cpp", + "ovms_py_tensor.hpp", + "python_backend.cpp", + "python_backend.hpp", + "pythonnoderesources.cpp", + "pythonnoderesources.hpp", + "pytensor_ovtensor_converter_calculator.cc", + "python_executor_calculator.cc", + "pythoninterpretermodule.cpp", + "pythoninterpretermodule.hpp", + "utils.hpp", + ], + deps = PYBIND_DEPS + [ + ":pythonexecutorcalculator_cc_proto", + ":pytensorovtensorconvertercalculator_cc_proto", + "@mediapipe//mediapipe/framework:calculator_framework", + "//third_party:openvino", + "//src:libovmslogging", + "//src:libovmsstatus", + "//src:libovms_module", + "//src:libovmsmediapipe_utils", + "//src:libovmsprecision", + "//src:cpp_headers", + ], + copts = COPTS_SO, + linkopts = LINKOPTS_SO, + local_defines = COMMON_LOCAL_DEFINES, + visibility = ["//visibility:public"], +) + +# cc_import wrapper so that other targets can depend on the shared library +# the same way they previously depended on :libovmspythonmodule. +cc_import( + name = "libovmspython", + shared_library = ":libovmspython.so", + hdrs = [ + "ovms_py_tensor.hpp", + "python_backend.hpp", + "pythonnoderesources.hpp", + "pythoninterpretermodule.hpp", + "utils.hpp", + ], + visibility = ["//visibility:public"], +) diff --git a/src/python/python_backend.hpp b/src/python/python_backend.hpp index 058fd815ab..9389b0231f 100644 --- a/src/python/python_backend.hpp +++ b/src/python/python_backend.hpp @@ -32,24 +32,32 @@ using namespace py::literals; namespace ovms { +#if defined(_WIN32) +#define PYTHON_BACKEND_EXPORT __declspec(dllexport) +#else +#define PYTHON_BACKEND_EXPORT __attribute__((visibility("default"))) +#endif + class PythonBackend { std::unique_ptr pyovmsModule; std::unique_ptr tensorClass; public: - PythonBackend(); - ~PythonBackend(); - static bool createPythonBackend(std::unique_ptr& pythonBackend); + PYTHON_BACKEND_EXPORT PythonBackend(); + PYTHON_BACKEND_EXPORT ~PythonBackend(); + PYTHON_BACKEND_EXPORT static bool createPythonBackend(std::unique_ptr& pythonBackend); - bool createOvmsPyTensor(const std::string& name, void* ptr, const std::vector& shape, const std::string& datatype, + PYTHON_BACKEND_EXPORT bool createOvmsPyTensor(const std::string& name, void* ptr, const std::vector& shape, const std::string& datatype, py::ssize_t size, std::unique_ptr>& outTensor, bool copy = false); - bool createEmptyOvmsPyTensor(const std::string& name, const std::vector& shape, const std::string& datatype, + PYTHON_BACKEND_EXPORT bool createEmptyOvmsPyTensor(const std::string& name, const std::vector& shape, const std::string& datatype, py::ssize_t size, std::unique_ptr>& outTensor); // Checks if object is tensorClass instance. Throws UnexpectedPythonObjectError if it's not. - void validateOvmsPyTensor(const py::object& object) const; + PYTHON_BACKEND_EXPORT void validateOvmsPyTensor(const py::object& object) const; - bool getOvmsPyTensorData(std::unique_ptr>& outTensor, void** data); + PYTHON_BACKEND_EXPORT bool getOvmsPyTensorData(std::unique_ptr>& outTensor, void** data); }; + +#undef PYTHON_BACKEND_EXPORT } // namespace ovms From dd13d4d19589143fa72c07326b4b6eb12b5bacf2 Mon Sep 17 00:00:00 2001 From: Rafal Sapala Date: Tue, 31 Mar 2026 15:56:39 +0200 Subject: [PATCH 02/22] Double register fix --- src/BUILD | 6 ++++++ src/python/BUILD | 17 +++-------------- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/src/BUILD b/src/BUILD index 11fad6eb33..f7e68f7ae6 100644 --- a/src/BUILD +++ b/src/BUILD @@ -569,6 +569,9 @@ ovms_cc_library( deps = select({ "//:not_disable_python": [ "//src/python:libovmspython", + "//src/python:pythonnoderesources", + "//src/python:pythonexecutorcalculator", + "//src/python:pytensorovtensorconvertercalculator", ], "//:disable_python": [] }) + select({ @@ -1132,6 +1135,9 @@ ovms_cc_library( # TODO split dependencies ] + select({ "//:not_disable_python": [ "//src/python:libovmspython", + "//src/python:pythonnoderesources", + "//src/python:pythonexecutorcalculator", + "//src/python:pytensorovtensorconvertercalculator", ], "//:disable_python": [] }), diff --git a/src/python/BUILD b/src/python/BUILD index 4bc3f12597..c2ee22fd5f 100644 --- a/src/python/BUILD +++ b/src/python/BUILD @@ -130,7 +130,7 @@ ovms_cc_library( "pythonexecutorcalculator_cc_proto", "utils", ], - visibility = ["//visibility:private"], + visibility = ["//src:__pkg__"], alwayslink = 1, data = ["//src/python/binding:pyovms.so"], ) @@ -146,7 +146,7 @@ ovms_cc_library( "pytensorovtensorconvertercalculator_cc_proto", "pythonbackend", ], - visibility = ["//visibility:private"], + visibility = ["//src:__pkg__"], alwayslink = 1, data = ["//src/python/binding:pyovms.so"], ) @@ -162,7 +162,7 @@ ovms_cc_library( "pythonbackend", "pythonnoderesources", ], - visibility = ["//visibility:private"], + visibility = ["//src:__pkg__"], alwayslink = 1, data = ["//src/python/binding:pyovms.so"], ) @@ -210,24 +210,14 @@ cc_binary( "ovms_py_tensor.hpp", "python_backend.cpp", "python_backend.hpp", - "pythonnoderesources.cpp", - "pythonnoderesources.hpp", - "pytensor_ovtensor_converter_calculator.cc", - "python_executor_calculator.cc", "pythoninterpretermodule.cpp", "pythoninterpretermodule.hpp", "utils.hpp", ], deps = PYBIND_DEPS + [ - ":pythonexecutorcalculator_cc_proto", - ":pytensorovtensorconvertercalculator_cc_proto", - "@mediapipe//mediapipe/framework:calculator_framework", - "//third_party:openvino", "//src:libovmslogging", "//src:libovmsstatus", "//src:libovms_module", - "//src:libovmsmediapipe_utils", - "//src:libovmsprecision", "//src:cpp_headers", ], copts = COPTS_SO, @@ -244,7 +234,6 @@ cc_import( hdrs = [ "ovms_py_tensor.hpp", "python_backend.hpp", - "pythonnoderesources.hpp", "pythoninterpretermodule.hpp", "utils.hpp", ], From c5a9b8f60f1b3b870f9f9dbc73d0bda5d6ff9b1a Mon Sep 17 00:00:00 2001 From: Rafal Sapala Date: Tue, 31 Mar 2026 16:21:43 +0200 Subject: [PATCH 03/22] Windows and packaging --- create_package.sh | 7 ++++ src/python/BUILD | 68 ++++++++++++++++++++++++++++++++++++-- windows_create_package.bat | 9 +++++ 3 files changed, 82 insertions(+), 2 deletions(-) diff --git a/create_package.sh b/create_package.sh index 68a89f0f36..29de805fc7 100755 --- a/create_package.sh +++ b/create_package.sh @@ -65,6 +65,13 @@ if [ -f /ovms_release/lib/libsrc_Slibovms_Ushared.so ] ; then \ fi # Add Python bindings for pyovms, openvino, openvino_tokenizers and openvino_genai, so they are all available for OVMS Python servables +if ! [[ $debug_bazel_flags == *"_py_off"* ]]; then + if [ ! -f /ovms_release/lib/libovmspython.so ]; then + echo "Missing libovmspython.so in package staging. Ensure //src/python:libovmspython is built." + exit 1 + fi +fi + if ! [[ $debug_bazel_flags == *"_py_off"* ]]; then cp -r /opt/intel/openvino/python /ovms_release/lib/python ; fi if ! [[ $debug_bazel_flags == *"_py_off"* ]] && [ "$FUZZER_BUILD" == "0" ]; then mv /ovms_release/lib/pyovms.so /ovms_release/lib/python ; fi if ! [[ $debug_bazel_flags == *"_py_off"* ]]; then mv /ovms_release/lib/python/bin/convert_tokenizer /ovms_release/bin/convert_tokenizer ; \ diff --git a/src/python/BUILD b/src/python/BUILD index c2ee22fd5f..558510a47d 100644 --- a/src/python/BUILD +++ b/src/python/BUILD @@ -16,6 +16,7 @@ load("@pybind11_bazel//:build_defs.bzl", "pybind_extension") load("@mediapipe//mediapipe/framework/port:build_config.bzl", "mediapipe_cc_proto_library", "mediapipe_proto_library") +load("@aspect_bazel_lib//:e2e/copy_action/copy.bzl", "simple_copy_file") load("//:common_settings.bzl", "COMMON_STATIC_LIBS_LINKOPTS", "COMMON_FUZZER_COPTS", "COMMON_FUZZER_LINKOPTS", @@ -54,9 +55,43 @@ _SHARED_LIB_COPTS_LINUX = [ "-Wno-attributes", ] +_SHARED_LIB_COPTS_WINDOWS = [ + "/guard:cf", + "/W4", + "/WX", + "/external:anglebrackets", + "/external:W0", + "/sdl", + "/analyze", + "/Gy", + "/GS", + "/DYNAMICBASE", + "/Qspectre", + "/wd4305", + "/wd4324", + "/wd4068", + "/wd4458", + "/wd4100", + "/wd4389", + "/wd4127", + "/wd4673", + "/wd4670", + "/wd4244", + "/wd4297", + "/wd4702", + "/wd4267", + "/wd4996", + "/wd6240", + "/wd6326", + "/wd6385", + "/wd6294", + "/guard:cf", + "/utf-8", +] + COPTS_SO = select({ "//conditions:default": _SHARED_LIB_COPTS_LINUX, - "//src:windows": [], # Windows builds do not produce this .so + "//src:windows": _SHARED_LIB_COPTS_WINDOWS, }) + select({ "//conditions:default": ["-DPYTHON_DISABLE=1"], "//:not_disable_python": ["-DPYTHON_DISABLE=0"], @@ -226,10 +261,17 @@ cc_binary( visibility = ["//visibility:public"], ) +simple_copy_file( + name = "copy_libovmspython", + src = "libovmspython.so", + out = "libovmspython.dll", + visibility = ["//visibility:public"], +) + # cc_import wrapper so that other targets can depend on the shared library # the same way they previously depended on :libovmspythonmodule. cc_import( - name = "libovmspython", + name = "libovmspython_unix", shared_library = ":libovmspython.so", hdrs = [ "ovms_py_tensor.hpp", @@ -237,5 +279,27 @@ cc_import( "pythoninterpretermodule.hpp", "utils.hpp", ], + visibility = ["//visibility:private"], +) + +cc_import( + name = "libovmspython_windows", + interface_library = ":libovmspython.so.if.lib", + shared_library = ":copy_libovmspython", + hdrs = [ + "ovms_py_tensor.hpp", + "python_backend.hpp", + "pythoninterpretermodule.hpp", + "utils.hpp", + ], + visibility = ["//visibility:private"], +) + +alias( + name = "libovmspython", + actual = select({ + "//src:windows": ":libovmspython_windows", + "//conditions:default": ":libovmspython_unix", + }), visibility = ["//visibility:public"], ) diff --git a/windows_create_package.bat b/windows_create_package.bat index 13402df4ce..959fedef5c 100644 --- a/windows_create_package.bat +++ b/windows_create_package.bat @@ -53,11 +53,20 @@ if !errorlevel! neq 0 exit /b !errorlevel! set "dest_dir=C:\opt" if /i "%with_python%"=="true" ( + if not exist %cd%\bazel-out\x64_windows-opt\bin\src\python\libovmspython.dll ( + echo Missing libovmspython.dll in bazel output. Ensure //src/python:libovmspython is built. + exit /b 1 + ) + :: Copy pyovms module md dist\windows\ovms\python copy %cd%\bazel-out\x64_windows-opt\bin\src\python\binding\pyovms.pyd dist\windows\ovms\python if !errorlevel! neq 0 exit /b !errorlevel! + :: Copy shared OVMS python runtime library required by ovms.exe when Python is enabled. + copy %cd%\bazel-out\x64_windows-opt\bin\src\python\libovmspython.dll dist\windows\ovms + if !errorlevel! neq 0 exit /b !errorlevel! + :: Prepare self-contained python set "python_version=3.12.10" From e842ce0785a4674b54aa44396e70bd86ffe443b7 Mon Sep 17 00:00:00 2001 From: Rafal Sapala Date: Wed, 1 Apr 2026 17:20:27 +0200 Subject: [PATCH 04/22] Runtime load and windows fix --- src/BUILD | 27 ++++- src/module.hpp | 8 ++ src/python/BUILD | 18 ++++ src/python/python_runtime_entry.cpp | 24 +++++ src/python/pythoninterpretermodule.hpp | 6 +- src/servablemanagermodule.cpp | 5 +- src/server.cpp | 131 ++++++++++++++++++++++--- src/test/pythonnode_test.cpp | 12 ++- 8 files changed, 206 insertions(+), 25 deletions(-) create mode 100644 src/python/python_runtime_entry.cpp diff --git a/src/BUILD b/src/BUILD index f7e68f7ae6..0295caf510 100644 --- a/src/BUILD +++ b/src/BUILD @@ -568,7 +568,6 @@ ovms_cc_library( }), deps = select({ "//:not_disable_python": [ - "//src/python:libovmspython", "//src/python:pythonnoderesources", "//src/python:pythonexecutorcalculator", "//src/python:pytensorovtensorconvertercalculator", @@ -719,9 +718,12 @@ ovms_cc_library( "//conditions:default": ["-lOpenCL"], # TODO make as direct dependency "//src:windows" : ["/DEFAULTLIB:Rpcrt4.lib"],}), data = select({ - "//:not_disable_python": [ + ("//:not_disable_python", "@platforms//os:linux"): [ "//src/python/binding:pyovms.so", ], + ("//:not_disable_python", "@platforms//os:windows"): [ + "//src/python/binding:pyovms.dll", + ], "//:disable_python": [] }) + select({ "//:is_windows_and_python_is_enabled": [ @@ -1134,7 +1136,6 @@ ovms_cc_library( # TODO split dependencies "//src/kfserving_api:kfserving_api_cpp", ] + select({ "//:not_disable_python": [ - "//src/python:libovmspython", "//src/python:pythonnoderesources", "//src/python:pythonexecutorcalculator", "//src/python:pytensorovtensorconvertercalculator", @@ -2455,9 +2456,12 @@ cc_binary( "//:disable_mediapipe" : [], }), data = select({ - "//:not_disable_python": [ + ("//:not_disable_python", "@platforms//os:linux"): [ "//src/python/binding:pyovms.so", ], + ("//:not_disable_python", "@platforms//os:windows"): [ + "//src/python/binding:pyovms.dll", + ], "//:disable_python": [] }), # linkstatic = False, # Use for dynamic linking when necessary @@ -2712,7 +2716,15 @@ cc_test( "//src:libcustom_node_image_transformation.so", "//src:libcustom_node_add_one.so", "//src:libcustom_node_horizontal_ocr.so", - ], + ] + select({ + ("//:not_disable_python", "@platforms//os:linux"): [ + "//src/python:libovmspython.so", + ], + ("//:not_disable_python", "@platforms//os:windows"): [ + "//src/python:libovmspython.dll", + ], + "//:disable_python": [], + }), deps = [ "optimum-cli", "//src:ovms_lib", @@ -2766,6 +2778,11 @@ cc_test( [ "serialization_common", ], + }) + select({ + "//:not_disable_python": [ + "//src/python:pythoninterpretermodule_runtime", + ], + "//:disable_python": [], }), copts = COPTS_TESTS, local_defines = COMMON_LOCAL_DEFINES, diff --git a/src/module.hpp b/src/module.hpp index c9abcadd67..1816e7803d 100644 --- a/src/module.hpp +++ b/src/module.hpp @@ -18,6 +18,7 @@ namespace ovms { class Config; class Status; +class PythonBackend; enum class ModuleState { NOT_INITIALIZED, STARTED_INITIALIZE, @@ -34,6 +35,13 @@ class Module { public: virtual Status start(const ovms::Config& config) = 0; virtual void shutdown() = 0; + virtual PythonBackend* getPythonBackend() const { + return nullptr; + } + virtual bool ownsPythonInterpreter() const { + return false; + } + virtual void releaseGILFromThisThread() const {} virtual ~Module() = default; ModuleState getState() const; }; diff --git a/src/python/BUILD b/src/python/BUILD index 558510a47d..4af7608f8b 100644 --- a/src/python/BUILD +++ b/src/python/BUILD @@ -233,6 +233,23 @@ ovms_cc_library( data = ["//src/python/binding:pyovms.so"], ) +# Lightweight target for tests that need direct PythonInterpreterModule usage +# without pulling MediaPipe calculator registration code. +ovms_cc_library( + name = "pythoninterpretermodule_runtime", + hdrs = ["pythoninterpretermodule.hpp",], + srcs = ["pythoninterpretermodule.cpp",], + deps = PYBIND_DEPS + [ + "//src:cpp_headers", + "//src:libovmslogging", + "//src:libovms_module", + "pythonbackend", + ], + visibility = ["//src:__pkg__"], + alwayslink = 1, + data = ["//src/python/binding:pyovms.so"], +) + # Shared library built from all Python binding sources. # Deps whose symbols are compiled with -fvisibility=hidden (the project default) # are linked into the .so with hidden visibility; only the python module @@ -247,6 +264,7 @@ cc_binary( "python_backend.hpp", "pythoninterpretermodule.cpp", "pythoninterpretermodule.hpp", + "python_runtime_entry.cpp", "utils.hpp", ], deps = PYBIND_DEPS + [ diff --git a/src/python/python_runtime_entry.cpp b/src/python/python_runtime_entry.cpp new file mode 100644 index 0000000000..d05c84498e --- /dev/null +++ b/src/python/python_runtime_entry.cpp @@ -0,0 +1,24 @@ +//***************************************************************************** +// Copyright 2026 Intel Corporation +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +//***************************************************************************** + +#include "../module.hpp" +#include "pythoninterpretermodule.hpp" + +namespace ovms { +extern "C" Module* OVMS_createPythonInterpreterModule() { + return new PythonInterpreterModule(); +} +} // namespace ovms diff --git a/src/python/pythoninterpretermodule.hpp b/src/python/pythoninterpretermodule.hpp index e87a3cc28f..1a9e5751ed 100644 --- a/src/python/pythoninterpretermodule.hpp +++ b/src/python/pythoninterpretermodule.hpp @@ -39,9 +39,9 @@ class PythonInterpreterModule : public Module { ~PythonInterpreterModule(); Status start(const ovms::Config& config) override; void shutdown() override; - PythonBackend* getPythonBackend() const; - void releaseGILFromThisThread() const; + PythonBackend* getPythonBackend() const override; + void releaseGILFromThisThread() const override; void reacquireGILForThisThread() const; - bool ownsPythonInterpreter() const; + bool ownsPythonInterpreter() const override; }; } // namespace ovms diff --git a/src/servablemanagermodule.cpp b/src/servablemanagermodule.cpp index 247b3bf0da..d8f1ee0c8b 100644 --- a/src/servablemanagermodule.cpp +++ b/src/servablemanagermodule.cpp @@ -23,9 +23,6 @@ #include "metric_module.hpp" #include "modelmanager.hpp" #include "server.hpp" -#if (PYTHON_DISABLE == 0) -#include "python/pythoninterpretermodule.hpp" -#endif namespace ovms { class PythonBackend; @@ -33,7 +30,7 @@ class PythonBackend; ServableManagerModule::ServableManagerModule(ovms::Server& ovmsServer) { PythonBackend* pythonBackend = nullptr; #if (PYTHON_DISABLE == 0) - auto pythonModule = dynamic_cast(ovmsServer.getModule(PYTHON_INTERPRETER_MODULE_NAME)); + auto pythonModule = ovmsServer.getModule(PYTHON_INTERPRETER_MODULE_NAME); if (pythonModule != nullptr) pythonBackend = pythonModule->getPythonBackend(); #endif diff --git a/src/server.cpp b/src/server.cpp index ec0a7e4b10..cbb1afa951 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -32,11 +32,13 @@ #include #ifdef __linux__ +#include #include #include #include #elif _WIN32 #include +#include #include #include @@ -69,13 +71,105 @@ #include "stringutils.hpp" #include "version.hpp" +using grpc::ServerBuilder; + +namespace ovms { + #if (PYTHON_DISABLE == 0) -#include "python/pythoninterpretermodule.hpp" +namespace { +#ifdef __linux__ +using PythonLibraryHandle = void*; +#elif _WIN32 +using PythonLibraryHandle = HMODULE; #endif +using CreatePythonInterpreterModuleFn = Module* (*)(); -using grpc::ServerBuilder; +PythonLibraryHandle pythonRuntimeHandle = nullptr; +CreatePythonInterpreterModuleFn createPythonInterpreterModuleFn = nullptr; -namespace ovms { +bool ensurePythonRuntimeLoaded() { + if (createPythonInterpreterModuleFn != nullptr) { + return true; + } + +#ifdef __linux__ + std::vector candidates{ + "libovmspython.so", + "./libovmspython.so", + "src/python/libovmspython.so", + "./src/python/libovmspython.so", + "bazel-bin/src/python/libovmspython.so", + "./bazel-bin/src/python/libovmspython.so" + }; + + for (const auto& candidate : candidates) { + pythonRuntimeHandle = dlopen(candidate.c_str(), RTLD_NOW | RTLD_LOCAL); + if (pythonRuntimeHandle != nullptr) { + break; + } + } + + if (pythonRuntimeHandle == nullptr) { + SPDLOG_WARN("Python runtime library libovmspython.so failed to load: {}", dlerror()); + return false; + } + createPythonInterpreterModuleFn = reinterpret_cast(dlsym(pythonRuntimeHandle, "OVMS_createPythonInterpreterModule")); + if (createPythonInterpreterModuleFn == nullptr) { + SPDLOG_WARN("Python runtime library libovmspython.so missing symbol OVMS_createPythonInterpreterModule: {}", dlerror()); + dlclose(pythonRuntimeHandle); + pythonRuntimeHandle = nullptr; + return false; + } +#elif _WIN32 + std::vector candidates{ + "libovmspython.dll", + ".\\libovmspython.dll", + "src\\python\\libovmspython.dll", + ".\\src\\python\\libovmspython.dll", + "bazel-bin\\src\\python\\libovmspython.dll", + ".\\bazel-bin\\src\\python\\libovmspython.dll" + }; + + for (const auto& candidate : candidates) { + pythonRuntimeHandle = LoadLibraryA(candidate.c_str()); + if (pythonRuntimeHandle != nullptr) { + break; + } + } + + if (pythonRuntimeHandle == nullptr) { + DWORD error = GetLastError(); + SPDLOG_WARN("Python runtime library libovmspython.dll failed to load: {} ({})", error, std::system_category().message(error)); + return false; + } + createPythonInterpreterModuleFn = reinterpret_cast(GetProcAddress(pythonRuntimeHandle, "OVMS_createPythonInterpreterModule")); + if (createPythonInterpreterModuleFn == nullptr) { + DWORD error = GetLastError(); + SPDLOG_WARN("Python runtime library libovmspython.dll missing symbol OVMS_createPythonInterpreterModule: {} ({})", error, std::system_category().message(error)); + FreeLibrary(pythonRuntimeHandle); + pythonRuntimeHandle = nullptr; + return false; + } +#endif + + SPDLOG_INFO("Python runtime library loaded successfully"); + return true; +} + +void unloadPythonRuntime() { + createPythonInterpreterModuleFn = nullptr; + if (pythonRuntimeHandle == nullptr) { + return; + } +#ifdef __linux__ + dlclose(pythonRuntimeHandle); +#elif _WIN32 + FreeLibrary(pythonRuntimeHandle); +#endif + pythonRuntimeHandle = nullptr; +} +} // namespace +#endif Server& Server::instance() { static Server global; @@ -302,8 +396,12 @@ std::unique_ptr Server::createModule(const std::string& name) { if (name == SERVABLE_MANAGER_MODULE_NAME) return std::make_unique(*this); #if (PYTHON_DISABLE == 0) - if (name == PYTHON_INTERPRETER_MODULE_NAME) - return std::make_unique(); + if (name == PYTHON_INTERPRETER_MODULE_NAME) { + if (!ensurePythonRuntimeLoaded()) { + return nullptr; + } + return std::unique_ptr(createPythonInterpreterModuleFn()); + } #endif if (name == METRICS_MODULE_NAME) return std::make_unique(); @@ -379,8 +477,16 @@ Status Server::startModules(ovms::Config& config) { #if (PYTHON_DISABLE == 0) if (config.getServerSettings().withPython) { - INSERT_MODULE(PYTHON_INTERPRETER_MODULE_NAME, it); - START_MODULE(it); + auto pythonModule = this->createModule(PYTHON_INTERPRETER_MODULE_NAME); + if (pythonModule == nullptr) { + SPDLOG_WARN("Python requested in configuration, but runtime library could not be loaded. Continuing with Python features disabled."); + } else { + std::unique_lock lock(modulesMtx); + std::tie(it, inserted) = this->modules.emplace(PYTHON_INTERPRETER_MODULE_NAME, std::move(pythonModule)); + if (!inserted) + return Status(StatusCode::MODULE_ALREADY_INSERTED, PYTHON_INTERPRETER_MODULE_NAME); + START_MODULE(it); + } } #endif #if MTR_ENABLED @@ -407,12 +513,12 @@ Status Server::startModules(ovms::Config& config) { START_MODULE(it); #if (PYTHON_DISABLE == 0) if (config.getServerSettings().withPython) { - GET_MODULE(PYTHON_INTERPRETER_MODULE_NAME, it); - auto pythonModule = dynamic_cast(it->second.get()); - if (pythonModule->ownsPythonInterpreter()) { + std::shared_lock lock(modulesMtx); + auto pythonModuleIt = modules.find(PYTHON_INTERPRETER_MODULE_NAME); + if (pythonModuleIt != modules.end() && pythonModuleIt->second != nullptr && pythonModuleIt->second->ownsPythonInterpreter()) { // Natively GIL is held by the thread that initialized interpreter, so we only need to release it, if we own the interpreter. // If it was initialized externally, then the external thread shall release the GIL before launching that module. - pythonModule->releaseGILFromThisThread(); + pythonModuleIt->second->releaseGILFromThisThread(); } } #endif @@ -472,6 +578,9 @@ void Server::shutdownModules() { // this is because the OS can have a delay between freeing up port before it can be requested and used again std::shared_lock lock(modulesMtx); modules.clear(); +#if (PYTHON_DISABLE == 0) + unloadPythonRuntime(); +#endif } static int statusToExitCode(const Status& status) { diff --git a/src/test/pythonnode_test.cpp b/src/test/pythonnode_test.cpp index 54c9acbfa1..3434302081 100644 --- a/src/test/pythonnode_test.cpp +++ b/src/test/pythonnode_test.cpp @@ -39,8 +39,8 @@ #include "../metric_config.hpp" #include "../metric_module.hpp" #include "../model_service.hpp" +#include "../module.hpp" #include "../precision.hpp" -#include "../python/pythoninterpretermodule.hpp" #include "../python/pythonnoderesources.hpp" #include "../servablemanagermodule.hpp" #include "../server.hpp" @@ -112,7 +112,15 @@ class PythonFlowTest : public ::testing::Test { }; static PythonBackend* getPythonBackend() { - return dynamic_cast(ovms::Server::instance().getModule(PYTHON_INTERPRETER_MODULE_NAME))->getPythonBackend(); + auto* pythonModule = ovms::Server::instance().getModule(PYTHON_INTERPRETER_MODULE_NAME); + if (pythonModule == nullptr) { + throw std::runtime_error("Python interpreter module is not available"); + } + auto* pythonBackend = pythonModule->getPythonBackend(); + if (pythonBackend == nullptr) { + throw std::runtime_error("Python backend is not available"); + } + return pythonBackend; } // --------------------------------------- OVMS initializing Python nodes tests From 9ea30b04004c622e62a3d103560fa311b4664bec Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 2 Apr 2026 09:27:27 +0000 Subject: [PATCH 05/22] disable Resume test with SKIP_AND_EXIT_IF_NOT_RUNNING_UNSTABLE Agent-Logs-Url: https://github.com/openvinotoolkit/model_server/sessions/354f91e5-e091-443d-87c3-9941dd0bb134 Co-authored-by: rasapala <58549742+rasapala@users.noreply.github.com> --- src/test/pull_hf_model_test.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/pull_hf_model_test.cpp b/src/test/pull_hf_model_test.cpp index 1fbd0798f6..ebf98f2615 100644 --- a/src/test/pull_hf_model_test.cpp +++ b/src/test/pull_hf_model_test.cpp @@ -272,6 +272,7 @@ class TestHfDownloader : public ovms::HfDownloader { }; TEST_F(HfDownloaderPullHfModel, Resume) { + SKIP_AND_EXIT_IF_NOT_RUNNING_UNSTABLE(); // SSL proxy blocked workaround std::string modelName = "OpenVINO/Phi-3-mini-FastDraft-50M-int8-ov"; std::string downloadPath = ovms::FileSystem::joinPath({this->directoryPath, "repository"}); std::string task = "text_generation"; From 58039477777540114f277b05bad49d6d24693179 Mon Sep 17 00:00:00 2001 From: Rafal Sapala Date: Thu, 2 Apr 2026 11:43:44 +0200 Subject: [PATCH 06/22] Style --- src/server.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/server.cpp b/src/server.cpp index cbb1afa951..016e3dc410 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -99,8 +99,7 @@ bool ensurePythonRuntimeLoaded() { "src/python/libovmspython.so", "./src/python/libovmspython.so", "bazel-bin/src/python/libovmspython.so", - "./bazel-bin/src/python/libovmspython.so" - }; + "./bazel-bin/src/python/libovmspython.so"}; for (const auto& candidate : candidates) { pythonRuntimeHandle = dlopen(candidate.c_str(), RTLD_NOW | RTLD_LOCAL); @@ -127,8 +126,7 @@ bool ensurePythonRuntimeLoaded() { "src\\python\\libovmspython.dll", ".\\src\\python\\libovmspython.dll", "bazel-bin\\src\\python\\libovmspython.dll", - ".\\bazel-bin\\src\\python\\libovmspython.dll" - }; + ".\\bazel-bin\\src\\python\\libovmspython.dll"}; for (const auto& candidate : candidates) { pythonRuntimeHandle = LoadLibraryA(candidate.c_str()); From 577b968cf0de4cb7766b235f7ed4a69c87304a5b Mon Sep 17 00:00:00 2001 From: Rafal Sapala Date: Thu, 2 Apr 2026 11:54:39 +0200 Subject: [PATCH 07/22] Fix bazel --- src/BUILD | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/BUILD b/src/BUILD index 0295caf510..031486e119 100644 --- a/src/BUILD +++ b/src/BUILD @@ -718,13 +718,13 @@ ovms_cc_library( "//conditions:default": ["-lOpenCL"], # TODO make as direct dependency "//src:windows" : ["/DEFAULTLIB:Rpcrt4.lib"],}), data = select({ - ("//:not_disable_python", "@platforms//os:linux"): [ - "//src/python/binding:pyovms.so", - ], - ("//:not_disable_python", "@platforms//os:windows"): [ + "//:is_windows_and_python_is_enabled": [ "//src/python/binding:pyovms.dll", ], - "//:disable_python": [] + "//:disable_python": [], + "//conditions:default": [ + "//src/python/binding:pyovms.so", + ], }) + select({ "//:is_windows_and_python_is_enabled": [ "//src/python/binding:copy_pyovms", @@ -2456,13 +2456,13 @@ cc_binary( "//:disable_mediapipe" : [], }), data = select({ - ("//:not_disable_python", "@platforms//os:linux"): [ - "//src/python/binding:pyovms.so", - ], - ("//:not_disable_python", "@platforms//os:windows"): [ + "//:is_windows_and_python_is_enabled": [ "//src/python/binding:pyovms.dll", ], - "//:disable_python": [] + "//:disable_python": [], + "//conditions:default": [ + "//src/python/binding:pyovms.so", + ], }), # linkstatic = False, # Use for dynamic linking when necessary ) @@ -2717,13 +2717,13 @@ cc_test( "//src:libcustom_node_add_one.so", "//src:libcustom_node_horizontal_ocr.so", ] + select({ - ("//:not_disable_python", "@platforms//os:linux"): [ - "//src/python:libovmspython.so", - ], - ("//:not_disable_python", "@platforms//os:windows"): [ + "//:is_windows_and_python_is_enabled": [ "//src/python:libovmspython.dll", ], "//:disable_python": [], + "//conditions:default": [ + "//src/python:libovmspython.so", + ], }), deps = [ "optimum-cli", From 9fd9122ffb07543f58f6fe586020a39656937393 Mon Sep 17 00:00:00 2001 From: Rafal Sapala Date: Thu, 2 Apr 2026 12:48:50 +0200 Subject: [PATCH 08/22] Adding null checks --- src/BUILD | 4 ++-- src/kfs_frontend/kfs_graph_executor_impl.cpp | 5 +++++ src/python/python_executor_calculator.cc | 3 +++ 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/BUILD b/src/BUILD index 031486e119..250181708a 100644 --- a/src/BUILD +++ b/src/BUILD @@ -719,7 +719,7 @@ ovms_cc_library( "//src:windows" : ["/DEFAULTLIB:Rpcrt4.lib"],}), data = select({ "//:is_windows_and_python_is_enabled": [ - "//src/python/binding:pyovms.dll", + "//src/python/binding:pyovms.pyd", ], "//:disable_python": [], "//conditions:default": [ @@ -2457,7 +2457,7 @@ cc_binary( }), data = select({ "//:is_windows_and_python_is_enabled": [ - "//src/python/binding:pyovms.dll", + "//src/python/binding:pyovms.pyd", ], "//:disable_python": [], "//conditions:default": [ diff --git a/src/kfs_frontend/kfs_graph_executor_impl.cpp b/src/kfs_frontend/kfs_graph_executor_impl.cpp index 034f6f0907..2554720a1e 100644 --- a/src/kfs_frontend/kfs_graph_executor_impl.cpp +++ b/src/kfs_frontend/kfs_graph_executor_impl.cpp @@ -748,6 +748,11 @@ static Status deserializeTensor(const std::string& requestedName, const KFSReque #if (PYTHON_DISABLE == 0) static Status deserializeTensor(const std::string& requestedName, const KFSRequest& request, std::unique_ptr>& outTensor, PythonBackend* pythonBackend) { + if (pythonBackend == nullptr) { + const std::string details = "Python backend is not available. Ensure libovmspython runtime library is accessible when using Python tensor inputs."; + SPDLOG_DEBUG("[servable name: {} version: {}] {}", request.model_name(), request.model_version(), details); + return Status(StatusCode::MEDIAPIPE_PYTHON_EXECUTION_ERROR, details); + } auto requestInputItr = request.inputs().begin(); auto status = getRequestInput(requestInputItr, requestedName, request); if (!status.ok()) { diff --git a/src/python/python_executor_calculator.cc b/src/python/python_executor_calculator.cc index 4e36cda652..3e713ab0ab 100644 --- a/src/python/python_executor_calculator.cc +++ b/src/python/python_executor_calculator.cc @@ -200,6 +200,9 @@ class PythonExecutorCalculator : public CalculatorBase { } nodeResources = it->second; + if (nodeResources == nullptr || nodeResources->pythonBackend == nullptr) { + return absl::Status(absl::StatusCode::kFailedPrecondition, "Python backend is not available for PythonExecutorCalculator"); + } outputTimestamp = mediapipe::Timestamp(mediapipe::Timestamp::Unset()); LOG(INFO) << "PythonExecutorCalculator [Node: " << cc->NodeName() << "] Open end"; return absl::OkStatus(); From 15b31ab8baf0d957b727309edf8490ba8aa72171 Mon Sep 17 00:00:00 2001 From: Rafal Sapala Date: Thu, 2 Apr 2026 12:54:45 +0200 Subject: [PATCH 09/22] Fix compile --- src/kfs_frontend/kfs_graph_executor_impl.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/kfs_frontend/kfs_graph_executor_impl.cpp b/src/kfs_frontend/kfs_graph_executor_impl.cpp index 2554720a1e..3952e4bc56 100644 --- a/src/kfs_frontend/kfs_graph_executor_impl.cpp +++ b/src/kfs_frontend/kfs_graph_executor_impl.cpp @@ -751,7 +751,7 @@ static Status deserializeTensor(const std::string& requestedName, const KFSReque if (pythonBackend == nullptr) { const std::string details = "Python backend is not available. Ensure libovmspython runtime library is accessible when using Python tensor inputs."; SPDLOG_DEBUG("[servable name: {} version: {}] {}", request.model_name(), request.model_version(), details); - return Status(StatusCode::MEDIAPIPE_PYTHON_EXECUTION_ERROR, details); + return Status(StatusCode::MEDIAPIPE_EXECUTION_ERROR, details); } auto requestInputItr = request.inputs().begin(); auto status = getRequestInput(requestInputItr, requestedName, request); From 3a96f21e7731454dcccfb97142316cba52312099 Mon Sep 17 00:00:00 2001 From: Rafal Sapala Date: Thu, 2 Apr 2026 16:43:04 +0200 Subject: [PATCH 10/22] Fix windows load --- src/server.cpp | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/server.cpp b/src/server.cpp index 016e3dc410..11e17bc181 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -128,6 +128,33 @@ bool ensurePythonRuntimeLoaded() { "bazel-bin\\src\\python\\libovmspython.dll", ".\\bazel-bin\\src\\python\\libovmspython.dll"}; + char executablePath[MAX_PATH] = {0}; + DWORD executablePathLength = GetModuleFileNameA(nullptr, executablePath, MAX_PATH); + if (executablePathLength > 0 && executablePathLength < MAX_PATH) { + std::string exePath(executablePath, executablePathLength); + std::string exeDir = "."; + size_t separatorPos = exePath.find_last_of("\\/"); + if (separatorPos != std::string::npos) { + exeDir = exePath.substr(0, separatorPos); + } + + std::vector executableRelativeCandidates{ + exeDir + "\\libovmspython.dll", + exeDir + "\\src\\python\\libovmspython.dll", + exeDir + "\\..\\src\\python\\libovmspython.dll", + }; + + std::string runfilesRoot = exePath + ".runfiles"; + std::vector runfilesCandidates{ + runfilesRoot + "\\src\\python\\libovmspython.dll", + runfilesRoot + "\\_main\\src\\python\\libovmspython.dll", + runfilesRoot + "\\model_server\\src\\python\\libovmspython.dll", + }; + + candidates.insert(candidates.end(), executableRelativeCandidates.begin(), executableRelativeCandidates.end()); + candidates.insert(candidates.end(), runfilesCandidates.begin(), runfilesCandidates.end()); + } + for (const auto& candidate : candidates) { pythonRuntimeHandle = LoadLibraryA(candidate.c_str()); if (pythonRuntimeHandle != nullptr) { From 5d935f680a51d5466363bec84698825e0567f265 Mon Sep 17 00:00:00 2001 From: Rafal Sapala Date: Thu, 2 Apr 2026 16:50:48 +0200 Subject: [PATCH 11/22] Add tests fixuture --- src/test/pythonnode_test.cpp | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/src/test/pythonnode_test.cpp b/src/test/pythonnode_test.cpp index 3434302081..88c2850585 100644 --- a/src/test/pythonnode_test.cpp +++ b/src/test/pythonnode_test.cpp @@ -41,6 +41,7 @@ #include "../model_service.hpp" #include "../module.hpp" #include "../precision.hpp" +#include "../python/pythoninterpretermodule.hpp" #include "../python/pythonnoderesources.hpp" #include "../servablemanagermodule.hpp" #include "../server.hpp" @@ -73,6 +74,21 @@ We do this because we don't want to restart interpreter in the tests. It's launching along with the server and even though most tests will not use the server, the interpreter remains initialized. */ std::unique_ptr serverThread; +std::unique_ptr standalonePythonModule; + +static void ensureStandalonePythonModuleInitialized() { + if (standalonePythonModule != nullptr) { + return; + } + standalonePythonModule = std::make_unique(); + auto status = standalonePythonModule->start(ovms::Config::instance()); + if (!status.ok()) { + throw std::runtime_error("Standalone python interpreter module failed to start"); + } + if (standalonePythonModule->ownsPythonInterpreter()) { + standalonePythonModule->releaseGILFromThisThread(); + } +} class PythonFlowTest : public ::testing::Test { protected: @@ -106,6 +122,11 @@ class PythonFlowTest : public ::testing::Test { ovms::Server::instance().setShutdownRequest(1); serverThread->join(); ovms::Server::instance().setShutdownRequest(0); + if (standalonePythonModule != nullptr) { + standalonePythonModule->reacquireGILForThisThread(); + standalonePythonModule->shutdown(); + standalonePythonModule.reset(); + } std::string path = getGenericFullPathForTmp("/tmp/pythonNodeTestRemoveFile.txt"); ASSERT_TRUE(!std::filesystem::exists(path)); } @@ -113,10 +134,15 @@ class PythonFlowTest : public ::testing::Test { static PythonBackend* getPythonBackend() { auto* pythonModule = ovms::Server::instance().getModule(PYTHON_INTERPRETER_MODULE_NAME); - if (pythonModule == nullptr) { - throw std::runtime_error("Python interpreter module is not available"); + if (pythonModule != nullptr) { + auto* pythonBackend = pythonModule->getPythonBackend(); + if (pythonBackend != nullptr) { + return pythonBackend; + } } - auto* pythonBackend = pythonModule->getPythonBackend(); + + ensureStandalonePythonModuleInitialized(); + auto* pythonBackend = standalonePythonModule->getPythonBackend(); if (pythonBackend == nullptr) { throw std::runtime_error("Python backend is not available"); } From d8c9a816837ea0522000eab03dac1692445e3776 Mon Sep 17 00:00:00 2001 From: Rafal Sapala Date: Tue, 7 Apr 2026 13:40:20 +0200 Subject: [PATCH 12/22] Add module to test env --- src/BUILD | 3 ++ src/test/python_environment.cpp | 63 ++++++++++++++++++++++++++++----- src/test/python_environment.hpp | 18 +++++----- src/test/pythonnode_test.cpp | 25 ++----------- 4 files changed, 70 insertions(+), 39 deletions(-) diff --git a/src/BUILD b/src/BUILD index 250181708a..9d3ab0b193 100644 --- a/src/BUILD +++ b/src/BUILD @@ -2962,6 +2962,9 @@ cc_library( srcs = ["test/python_environment.cpp",], linkopts = [], deps = PYBIND_DEPS + [ + "//src/python:pythoninterpretermodule_runtime", + "//src:cpp_headers", + "libovmsstatus", "@com_google_googletest//:gtest", ], local_defines = COMMON_LOCAL_DEFINES, diff --git a/src/test/python_environment.cpp b/src/test/python_environment.cpp index 72f6425d4c..f3d8ea5ea1 100644 --- a/src/test/python_environment.cpp +++ b/src/test/python_environment.cpp @@ -16,29 +16,76 @@ #include "python_environment.hpp" #include +#include + +#include "../config.hpp" +#include "../status.hpp" + +namespace { +PythonEnvironment* g_pythonEnvironment = nullptr; +} void PythonEnvironment::SetUp() { #if (PYTHON_DISABLE == 0) - py::initialize_interpreter(); - releaseGILFromThisThread(); + pythonModule = std::make_unique(); + auto status = pythonModule->start(ovms::Config::instance()); + if (!status.ok()) { + throw std::runtime_error("Global python interpreter module failed to start"); + } + if (pythonModule->ownsPythonInterpreter()) { + pythonModule->releaseGILFromThisThread(); + } + g_pythonEnvironment = this; #endif } void PythonEnvironment::TearDown() { #if (PYTHON_DISABLE == 0) - reacquireGILForThisThread(); - py::finalize_interpreter(); + g_pythonEnvironment = nullptr; + if (pythonModule != nullptr) { + if (pythonModule->ownsPythonInterpreter()) { + pythonModule->reacquireGILForThisThread(); + } + pythonModule->shutdown(); + pythonModule.reset(); + } #endif } -void PythonEnvironment::releaseGILFromThisThread() const { +ovms::PythonBackend* PythonEnvironment::getPythonBackend() const { #if (PYTHON_DISABLE == 0) - this->GILScopedRelease = std::make_unique(); + if (pythonModule == nullptr) { + return nullptr; + } + return pythonModule->getPythonBackend(); +#else + return nullptr; #endif } -void PythonEnvironment::reacquireGILForThisThread() const { +ovms::PythonInterpreterModule* PythonEnvironment::getPythonInterpreterModule() const { +#if (PYTHON_DISABLE == 0) + return pythonModule.get(); +#else + return nullptr; +#endif +} + +ovms::PythonBackend* getGlobalPythonBackend() { + auto* pythonInterpreterModule = getGlobalPythonInterpreterModule(); + if (pythonInterpreterModule == nullptr) { + return nullptr; + } + return pythonInterpreterModule->getPythonBackend(); +} + +ovms::PythonInterpreterModule* getGlobalPythonInterpreterModule() { #if (PYTHON_DISABLE == 0) - this->GILScopedRelease.reset(); + if (g_pythonEnvironment == nullptr) { + return nullptr; + } + return g_pythonEnvironment->getPythonInterpreterModule(); +#else + return nullptr; #endif } diff --git a/src/test/python_environment.hpp b/src/test/python_environment.hpp index 26fe5f38b0..51963ca7ea 100644 --- a/src/test/python_environment.hpp +++ b/src/test/python_environment.hpp @@ -19,19 +19,21 @@ #include #include -#pragma warning(push) -#pragma warning(disable : 6326 28182 6011 28020) -#include // everything needed for embedding -#pragma warning(pop) +#include "../python/pythoninterpretermodule.hpp" -namespace py = pybind11; +namespace ovms { +class PythonBackend; +} class PythonEnvironment : public testing::Environment { - mutable std::unique_ptr GILScopedRelease; + std::unique_ptr pythonModule; public: void SetUp() override; void TearDown() override; - void releaseGILFromThisThread() const; - void reacquireGILForThisThread() const; + ovms::PythonInterpreterModule* getPythonInterpreterModule() const; + ovms::PythonBackend* getPythonBackend() const; }; + +ovms::PythonBackend* getGlobalPythonBackend(); +ovms::PythonInterpreterModule* getGlobalPythonInterpreterModule(); diff --git a/src/test/pythonnode_test.cpp b/src/test/pythonnode_test.cpp index 88c2850585..2f919b9cb5 100644 --- a/src/test/pythonnode_test.cpp +++ b/src/test/pythonnode_test.cpp @@ -41,7 +41,6 @@ #include "../model_service.hpp" #include "../module.hpp" #include "../precision.hpp" -#include "../python/pythoninterpretermodule.hpp" #include "../python/pythonnoderesources.hpp" #include "../servablemanagermodule.hpp" #include "../server.hpp" @@ -59,6 +58,7 @@ #include "c_api_test_utils.hpp" #include "constructor_enabled_model_manager.hpp" #include "platform_utils.hpp" +#include "python_environment.hpp" #include "test_utils.hpp" namespace py = pybind11; @@ -74,21 +74,6 @@ We do this because we don't want to restart interpreter in the tests. It's launching along with the server and even though most tests will not use the server, the interpreter remains initialized. */ std::unique_ptr serverThread; -std::unique_ptr standalonePythonModule; - -static void ensureStandalonePythonModuleInitialized() { - if (standalonePythonModule != nullptr) { - return; - } - standalonePythonModule = std::make_unique(); - auto status = standalonePythonModule->start(ovms::Config::instance()); - if (!status.ok()) { - throw std::runtime_error("Standalone python interpreter module failed to start"); - } - if (standalonePythonModule->ownsPythonInterpreter()) { - standalonePythonModule->releaseGILFromThisThread(); - } -} class PythonFlowTest : public ::testing::Test { protected: @@ -122,11 +107,6 @@ class PythonFlowTest : public ::testing::Test { ovms::Server::instance().setShutdownRequest(1); serverThread->join(); ovms::Server::instance().setShutdownRequest(0); - if (standalonePythonModule != nullptr) { - standalonePythonModule->reacquireGILForThisThread(); - standalonePythonModule->shutdown(); - standalonePythonModule.reset(); - } std::string path = getGenericFullPathForTmp("/tmp/pythonNodeTestRemoveFile.txt"); ASSERT_TRUE(!std::filesystem::exists(path)); } @@ -141,8 +121,7 @@ static PythonBackend* getPythonBackend() { } } - ensureStandalonePythonModuleInitialized(); - auto* pythonBackend = standalonePythonModule->getPythonBackend(); + auto* pythonBackend = getGlobalPythonBackend(); if (pythonBackend == nullptr) { throw std::runtime_error("Python backend is not available"); } From c6d74999c71f44d24280e84d49a15a1a2a34e499 Mon Sep 17 00:00:00 2001 From: Rafal Sapala Date: Tue, 7 Apr 2026 14:58:46 +0200 Subject: [PATCH 13/22] Fix windows --- src/python/python_runtime_entry.cpp | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/python/python_runtime_entry.cpp b/src/python/python_runtime_entry.cpp index d05c84498e..2394ff870a 100644 --- a/src/python/python_runtime_entry.cpp +++ b/src/python/python_runtime_entry.cpp @@ -17,8 +17,14 @@ #include "../module.hpp" #include "pythoninterpretermodule.hpp" -namespace ovms { -extern "C" Module* OVMS_createPythonInterpreterModule() { - return new PythonInterpreterModule(); +#if defined(_WIN32) +#define PYTHON_RUNTIME_EXPORT __declspec(dllexport) +#else +#define PYTHON_RUNTIME_EXPORT __attribute__((visibility("default"))) +#endif + +extern "C" PYTHON_RUNTIME_EXPORT ovms::Module* OVMS_createPythonInterpreterModule() { + return new ovms::PythonInterpreterModule(); } -} // namespace ovms + +#undef PYTHON_RUNTIME_EXPORT From 2e3274f48d9db02e38ca4e77a7b8b7ee346d1c8c Mon Sep 17 00:00:00 2001 From: Rafal Sapala Date: Thu, 30 Apr 2026 13:25:19 +0200 Subject: [PATCH 14/22] Fix link --- src/BUILD | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/BUILD b/src/BUILD index 7f75b7a69f..61d99cad1c 100644 --- a/src/BUILD +++ b/src/BUILD @@ -2585,7 +2585,7 @@ cc_test( ], }) + select({ "//:not_disable_python": [ - "//src/python:pythoninterpretermodule_runtime", + "//src/python:libovmspythonmodule", ], "//:disable_python": [], }), @@ -2767,7 +2767,7 @@ cc_library( srcs = ["test/python_environment.cpp",], linkopts = [], deps = PYBIND_DEPS + [ - "//src/python:pythoninterpretermodule_runtime", + "//src/python:libovmspythonmodule", "//src:cpp_headers", "libovmsstatus", "@com_google_googletest//:gtest", From 59f050c3112319238c70bacc0ba70a2b232e0dd5 Mon Sep 17 00:00:00 2001 From: Rafal Sapala Date: Thu, 30 Apr 2026 14:35:43 +0200 Subject: [PATCH 15/22] Runtime python check and tests linux --- create_package.sh | 2 +- run_unit_tests.sh | 7 +- src/BUILD | 1 + src/python/python_runtime_entry.cpp | 47 +++++ src/server.cpp | 37 +++- src/test/python_runtime_library_test.cpp | 252 +++++++++++++++++++++++ src/test/unit_tests.cpp | 7 +- 7 files changed, 347 insertions(+), 6 deletions(-) create mode 100644 src/test/python_runtime_library_test.cpp diff --git a/create_package.sh b/create_package.sh index 49ba6eb04f..e216f7aba9 100755 --- a/create_package.sh +++ b/create_package.sh @@ -113,7 +113,7 @@ ls -lahR /ovms_release/ # removing 29MB of cpython packages for unsupported python versions rls_python=cpython-"$(python3 --version 2>&1 | awk '{gsub(/\./, "", $2); print $2}' | cut -c1-3)" -find /ovms_release/ovms/lib/python/openvino -name *cpython* | grep -vZ $rls_python | xargs rm -rf -- +find /ovms_release/lib/python/openvino -name *cpython* | grep -vZ $rls_python | xargs rm -rf -- mkdir -p /ovms_pkg/${BASE_OS} cd /ovms_pkg/${BASE_OS} diff --git a/run_unit_tests.sh b/run_unit_tests.sh index 9c054291c8..b5c8891e18 100755 --- a/run_unit_tests.sh +++ b/run_unit_tests.sh @@ -25,6 +25,7 @@ FAIL_LOG=${FAIL_LOG:-"fail.log"} if [ -f /etc/redhat-release ] ; then dist="--//:distro=redhat" ; fi debug_bazel_flags=${debug_bazel_flags:-"--config=mp_on_py_on $dist"} TEST_FILTER="--test_filter=*" +UNIT_TEST_TARGETS="//src:ovms_test" SHARED_OPTIONS=" \ --jobs=$JOBS \ ${debug_bazel_flags} \ @@ -93,7 +94,7 @@ if [ "$RUN_TESTS" == "1" ] ; then if [ "$CHECK_COVERAGE" == "1" ] ; then if bazel coverage --instrumentation_filter="-src/test" --combined_report=lcov \ ${SHARED_OPTIONS} ${TEST_FILTER} \ - //src:ovms_test ${SHARED_OPTIONS} > ${TEST_LOG} 2>&1 ; then + ${UNIT_TEST_TARGETS} ${SHARED_OPTIONS} > ${TEST_LOG} 2>&1 ; then if ! generate_coverage_report ; then compress_logs exit 1 @@ -104,12 +105,12 @@ if [ "$RUN_TESTS" == "1" ] ; then fi fi bazel test ${SHARED_OPTIONS} "${TEST_FILTER}" //src/python/binding:test_python_binding || exit 1 - bazel build ${SHARED_OPTIONS} //src:ovms_test || exit 1 + bazel build ${SHARED_OPTIONS} ${UNIT_TEST_TARGETS} || exit 1 echo "Executing unit tests" failed=0 # For RH UBI and Ubuntu - if ! bazel test --jobs=$JOBS ${debug_bazel_flags} ${SHARED_OPTIONS} --test_summary=detailed --test_output=streamed --test_filter="*" //src:ovms_test > ${TEST_LOG} 2>&1 ; then + if ! bazel test --jobs=$JOBS ${debug_bazel_flags} ${SHARED_OPTIONS} --test_summary=detailed --test_output=streamed --test_filter="*" ${UNIT_TEST_TARGETS} > ${TEST_LOG} 2>&1 ; then failed=1 fi cat ${TEST_LOG} | tail -500 diff --git a/src/BUILD b/src/BUILD index 61d99cad1c..954f88d602 100644 --- a/src/BUILD +++ b/src/BUILD @@ -2383,6 +2383,7 @@ cc_test( "//:not_disable_python": [ # OvmsPyTensor is currently not used in OVMS core and is just a base for the binding. # "test/python/ovms_py_tensor_test.cpp", + "test/python_runtime_library_test.cpp", "test/pythonnode_test.cpp", # LLM logic uses Python for processing Jinja templates when built with Python enabled "test/llm/llmtemplate_test.cpp", diff --git a/src/python/python_runtime_entry.cpp b/src/python/python_runtime_entry.cpp index 2394ff870a..ee6a749f23 100644 --- a/src/python/python_runtime_entry.cpp +++ b/src/python/python_runtime_entry.cpp @@ -17,6 +17,15 @@ #include "../module.hpp" #include "pythoninterpretermodule.hpp" +#include + +#pragma warning(push) +#pragma warning(disable : 6326 28182 6011 28020) +#include +#pragma warning(pop) + +namespace py = pybind11; + #if defined(_WIN32) #define PYTHON_RUNTIME_EXPORT __declspec(dllexport) #else @@ -27,4 +36,42 @@ extern "C" PYTHON_RUNTIME_EXPORT ovms::Module* OVMS_createPythonInterpreterModul return new ovms::PythonInterpreterModule(); } +extern "C" PYTHON_RUNTIME_EXPORT bool OVMS_validatePythonEnvironment(const char** errorMessage) { + static thread_local std::string lastError; + if (errorMessage != nullptr) { + *errorMessage = nullptr; + } + + bool ownsInterpreter = false; + try { + if (!Py_IsInitialized()) { + py::initialize_interpreter(); + ownsInterpreter = true; + } + { + py::gil_scoped_acquire acquire; + // Validate that OVMS Python bindings are importable and executable. + py::module_::import("pyovms"); + } + if (ownsInterpreter) { + py::finalize_interpreter(); + } + return true; + } catch (const py::error_already_set& e) { + lastError = e.what(); + } catch (const std::exception& e) { + lastError = e.what(); + } catch (...) { + lastError = "Unknown python runtime validation error"; + } + + if (ownsInterpreter && Py_IsInitialized()) { + py::finalize_interpreter(); + } + if (errorMessage != nullptr) { + *errorMessage = lastError.c_str(); + } + return false; +} + #undef PYTHON_RUNTIME_EXPORT diff --git a/src/server.cpp b/src/server.cpp index 20215a1b37..3f1ca69f3d 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -85,12 +85,14 @@ using PythonLibraryHandle = void*; using PythonLibraryHandle = HMODULE; #endif using CreatePythonInterpreterModuleFn = Module* (*)(); +using ValidatePythonEnvironmentFn = bool (*)(const char** errorMessage); PythonLibraryHandle pythonRuntimeHandle = nullptr; CreatePythonInterpreterModuleFn createPythonInterpreterModuleFn = nullptr; +ValidatePythonEnvironmentFn validatePythonEnvironmentFn = nullptr; bool ensurePythonRuntimeLoaded() { - if (createPythonInterpreterModuleFn != nullptr) { + if (createPythonInterpreterModuleFn != nullptr && validatePythonEnvironmentFn != nullptr) { return true; } @@ -121,6 +123,14 @@ bool ensurePythonRuntimeLoaded() { pythonRuntimeHandle = nullptr; return false; } + validatePythonEnvironmentFn = reinterpret_cast(dlsym(pythonRuntimeHandle, "OVMS_validatePythonEnvironment")); + if (validatePythonEnvironmentFn == nullptr) { + SPDLOG_WARN("Python runtime library libovmspython.so missing symbol OVMS_validatePythonEnvironment: {}", dlerror()); + createPythonInterpreterModuleFn = nullptr; + dlclose(pythonRuntimeHandle); + pythonRuntimeHandle = nullptr; + return false; + } #elif _WIN32 std::vector candidates{ "libovmspython.dll", @@ -177,14 +187,39 @@ bool ensurePythonRuntimeLoaded() { pythonRuntimeHandle = nullptr; return false; } + validatePythonEnvironmentFn = reinterpret_cast(GetProcAddress(pythonRuntimeHandle, "OVMS_validatePythonEnvironment")); + if (validatePythonEnvironmentFn == nullptr) { + DWORD error = GetLastError(); + SPDLOG_WARN("Python runtime library libovmspython.dll missing symbol OVMS_validatePythonEnvironment: {} ({})", error, std::system_category().message(error)); + createPythonInterpreterModuleFn = nullptr; + FreeLibrary(pythonRuntimeHandle); + pythonRuntimeHandle = nullptr; + return false; + } #endif + const char* pythonRuntimeValidationError = nullptr; + if (!validatePythonEnvironmentFn(&pythonRuntimeValidationError)) { + SPDLOG_WARN("Python runtime environment validation failed. Ensure Python dependencies and PYTHONPATH are configured. Details: {}", + pythonRuntimeValidationError != nullptr ? pythonRuntimeValidationError : "Unknown error"); + createPythonInterpreterModuleFn = nullptr; + validatePythonEnvironmentFn = nullptr; +#ifdef __linux__ + dlclose(pythonRuntimeHandle); +#elif _WIN32 + FreeLibrary(pythonRuntimeHandle); +#endif + pythonRuntimeHandle = nullptr; + return false; + } + SPDLOG_INFO("Python runtime library loaded successfully"); return true; } void unloadPythonRuntime() { createPythonInterpreterModuleFn = nullptr; + validatePythonEnvironmentFn = nullptr; if (pythonRuntimeHandle == nullptr) { return; } diff --git a/src/test/python_runtime_library_test.cpp b/src/test/python_runtime_library_test.cpp new file mode 100644 index 0000000000..0a3dc1c8dc --- /dev/null +++ b/src/test/python_runtime_library_test.cpp @@ -0,0 +1,252 @@ +//***************************************************************************** +// Copyright 2026 Intel Corporation +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +//***************************************************************************** + +#include +#include +#include +#include +#include +#include + +#include +#include + +#ifdef __linux__ +#include +#endif + +using testing::HasSubstr; + +namespace { + +std::filesystem::path getExecutablePath() { +#ifdef __linux__ + return std::filesystem::canonical("/proc/self/exe"); +#elif _WIN32 + return std::filesystem::current_path(); +#endif +} + +std::vector getRunfilesRoots() { + std::vector roots; + if (const char* testSrcDir = std::getenv("TEST_SRCDIR"); testSrcDir != nullptr && testSrcDir[0] != '\0') { + roots.emplace_back(testSrcDir); + } + const auto executablePath = getExecutablePath(); + roots.emplace_back(executablePath.string() + ".runfiles"); + return roots; +} + +std::vector getWorkspacePrefixes() { + std::vector prefixes; + if (const char* workspace = std::getenv("TEST_WORKSPACE"); workspace != nullptr && workspace[0] != '\0') { + prefixes.emplace_back(workspace); + } + prefixes.emplace_back("_main"); + prefixes.emplace_back("model_server"); + return prefixes; +} + +std::filesystem::path findRunfile(const std::filesystem::path& relativePath) { + for (const auto& root : getRunfilesRoots()) { + for (const auto& prefix : getWorkspacePrefixes()) { + const auto candidate = root / prefix / relativePath; + if (std::filesystem::exists(candidate)) { + return candidate; + } + } + const auto directCandidate = root / relativePath; + if (std::filesystem::exists(directCandidate)) { + return directCandidate; + } + } + return {}; +} + +class ScopedEnvironmentVariable { + std::string name; + bool hadValue; + std::string previousValue; + +public: + ScopedEnvironmentVariable(const std::string& name, const std::string& value) : + name(name), + hadValue(false) { + if (const char* currentValue = std::getenv(name.c_str()); currentValue != nullptr) { + hadValue = true; + previousValue = currentValue; + } +#ifdef __linux__ + setenv(name.c_str(), value.c_str(), 1); +#elif _WIN32 + _putenv_s(name.c_str(), value.c_str()); +#endif + } + + ~ScopedEnvironmentVariable() { + if (hadValue) { +#ifdef __linux__ + setenv(name.c_str(), previousValue.c_str(), 1); +#elif _WIN32 + _putenv_s(name.c_str(), previousValue.c_str()); +#endif + } else { +#ifdef __linux__ + unsetenv(name.c_str()); +#elif _WIN32 + _putenv_s(name.c_str(), ""); +#endif + } + } +}; + +#ifdef __linux__ +using ValidatePythonEnvironmentFn = bool (*)(const char** errorMessage); + +class ScopedSharedLibrary { + void* handle; + +public: + explicit ScopedSharedLibrary(const std::filesystem::path& path) : + handle(dlopen(path.c_str(), RTLD_NOW | RTLD_LOCAL)) { + } + + ~ScopedSharedLibrary() { + if (handle != nullptr) { + dlclose(handle); + } + } + + void* get() const { + return handle; + } +}; + +int runIsolatedOvmsTest(const std::string& gtestFilter, const std::string& pythonPath = "") { + const auto executablePath = getExecutablePath(); + const std::string filterArgument = "--gtest_filter=" + gtestFilter; + pid_t pid = fork(); + if (pid == 0) { + setenv("OVMS_SKIP_GLOBAL_PYTHON_ENVIRONMENT", "1", 1); + unsetenv("TEST_PREMATURE_EXIT_FILE"); + if (!pythonPath.empty()) { + setenv("PYTHONPATH", pythonPath.c_str(), 1); + } + execl(executablePath.c_str(), executablePath.c_str(), filterArgument.c_str(), static_cast(nullptr)); + _exit(127); + } + + int status = 0; + if (pid < 0 || waitpid(pid, &status, 0) < 0) { + return -1; + } + if (!WIFEXITED(status)) { + return -1; + } + return WEXITSTATUS(status); +} +#endif + +} // namespace + +TEST(PythonRuntimeLibrary, MissingLibraryPathFailsToLoad) { +#ifndef __linux__ + GTEST_SKIP() << "Linux-only libovmspython.so test"; +#else + const auto missingLibraryPath = std::filesystem::temp_directory_path() / "missing_libovmspython.so"; + ASSERT_FALSE(std::filesystem::exists(missingLibraryPath)); + + ScopedSharedLibrary library(missingLibraryPath); + + EXPECT_EQ(library.get(), nullptr); +#endif +} + +TEST(PythonRuntimeLibrary, ExistingLibraryExportsRequiredSymbols) { +#ifndef __linux__ + GTEST_SKIP() << "Linux-only libovmspython.so test"; +#else + const auto libraryPath = findRunfile("src/python/libovmspython.so"); + ASSERT_FALSE(libraryPath.empty()); + + ScopedSharedLibrary library(libraryPath); + ASSERT_NE(library.get(), nullptr) << dlerror(); + + EXPECT_NE(dlsym(library.get(), "OVMS_createPythonInterpreterModule"), nullptr); + EXPECT_NE(dlsym(library.get(), "OVMS_validatePythonEnvironment"), nullptr); +#endif +} + +TEST(PythonRuntimeLibrary, ValidationFailsWithoutBindingOnPythonPath) { +#ifndef __linux__ + GTEST_SKIP() << "Linux-only libovmspython.so test"; +#else + const auto emptyPythonPath = std::filesystem::temp_directory_path() / "ovms_empty_pythonpath"; + std::filesystem::create_directories(emptyPythonPath); + EXPECT_EQ(runIsolatedOvmsTest("PythonRuntimeLibraryIsolated.ValidationFailsWithoutBindingOnPythonPath", emptyPythonPath.string()), 0); +#endif +} + +TEST(PythonRuntimeLibrary, ValidationSucceedsWithBindingOnPythonPath) { +#ifndef __linux__ + GTEST_SKIP() << "Linux-only libovmspython.so test"; +#else + const auto bindingPath = findRunfile("src/python/binding/pyovms.so"); + ASSERT_FALSE(bindingPath.empty()); + EXPECT_EQ(runIsolatedOvmsTest("PythonRuntimeLibraryIsolated.ValidationSucceedsWithBindingOnPythonPath", bindingPath.parent_path().string()), 0); +#endif +} + +TEST(PythonRuntimeLibraryIsolated, ValidationFailsWithoutBindingOnPythonPath) { +#ifndef __linux__ + GTEST_SKIP() << "Linux-only libovmspython.so test"; +#else + const auto libraryPath = findRunfile("src/python/libovmspython.so"); + ASSERT_FALSE(libraryPath.empty()); + ASSERT_EQ(std::getenv("OVMS_SKIP_GLOBAL_PYTHON_ENVIRONMENT"), std::string("1")); + + ScopedSharedLibrary library(libraryPath); + ASSERT_NE(library.get(), nullptr) << dlerror(); + + auto validate = reinterpret_cast(dlsym(library.get(), "OVMS_validatePythonEnvironment")); + ASSERT_NE(validate, nullptr); + + const char* errorMessage = nullptr; + EXPECT_FALSE(validate(&errorMessage)); + ASSERT_NE(errorMessage, nullptr); + EXPECT_THAT(std::string(errorMessage), HasSubstr("pyovms")); +#endif +} + +TEST(PythonRuntimeLibraryIsolated, ValidationSucceedsWithBindingOnPythonPath) { +#ifndef __linux__ + GTEST_SKIP() << "Linux-only libovmspython.so test"; +#else + const auto libraryPath = findRunfile("src/python/libovmspython.so"); + ASSERT_FALSE(libraryPath.empty()); + ASSERT_EQ(std::getenv("OVMS_SKIP_GLOBAL_PYTHON_ENVIRONMENT"), std::string("1")); + + ScopedSharedLibrary library(libraryPath); + ASSERT_NE(library.get(), nullptr) << dlerror(); + + auto validate = reinterpret_cast(dlsym(library.get(), "OVMS_validatePythonEnvironment")); + ASSERT_NE(validate, nullptr); + + const char* errorMessage = nullptr; + EXPECT_TRUE(validate(&errorMessage)); + EXPECT_EQ(errorMessage, nullptr); +#endif +} \ No newline at end of file diff --git a/src/test/unit_tests.cpp b/src/test/unit_tests.cpp index 4a841c6171..64f92b9d6e 100644 --- a/src/test/unit_tests.cpp +++ b/src/test/unit_tests.cpp @@ -14,6 +14,8 @@ // limitations under the License. //***************************************************************************** +#include + #include "environment.hpp" #include "gpuenvironment.hpp" #include "gguf_environment.hpp" @@ -25,7 +27,10 @@ int main(int argc, char** argv) { ::testing::AddGlobalTestEnvironment(new Environment); ::testing::AddGlobalTestEnvironment(new GPUEnvironment); ::testing::AddGlobalTestEnvironment(new GGUFEnvironment); - ::testing::AddGlobalTestEnvironment(new PythonEnvironment); + if (const char* skipPythonEnvironment = std::getenv("OVMS_SKIP_GLOBAL_PYTHON_ENVIRONMENT"); + skipPythonEnvironment == nullptr || std::string(skipPythonEnvironment) != "1") { + ::testing::AddGlobalTestEnvironment(new PythonEnvironment); + } ::testing::FLAGS_gtest_death_test_style = "threadsafe"; return RUN_ALL_TESTS(); } From 225d52865dd413114a4e97f7524972f63faa63ee Mon Sep 17 00:00:00 2001 From: Rafal Sapala Date: Thu, 30 Apr 2026 15:27:58 +0200 Subject: [PATCH 16/22] Fix tests --- Dockerfile.redhat | 2 +- Dockerfile.ubuntu | 2 +- run_unit_tests.sh | 2 +- src/BUILD | 29 +++- src/python/BUILD | 1 + src/test/python_runtime_library_test.cpp | 198 ++++++++++------------- src/test/unit_tests.cpp | 5 +- 7 files changed, 121 insertions(+), 118 deletions(-) diff --git a/Dockerfile.redhat b/Dockerfile.redhat index 3a59009f21..aabc451d32 100644 --- a/Dockerfile.redhat +++ b/Dockerfile.redhat @@ -318,7 +318,7 @@ RUN rm -f /usr/lib64/cmake/OpenSSL/OpenSSLConfig.cmake # Builds unit tests together with ovms server in one step # It speeds up CI when tests are executed outside of the image building # hadolint ignore=SC2046 -RUN bazel build --jobs=$JOBS ${debug_bazel_flags} ${minitrace_flags} //src:ovms $(if [ "$OPTIMIZE_BUILDING_TESTS" == "1" ] ; then echo -n //src:ovms_test; fi) +RUN bazel build --jobs=$JOBS ${debug_bazel_flags} ${minitrace_flags} //src:ovms $(if [ "$OPTIMIZE_BUILDING_TESTS" == "1" ] ; then echo -n "//src:ovms_test //src:python_runtime_library_test"; fi) # Tests execution COPY ci/check_coverage.bat /ovms/ diff --git a/Dockerfile.ubuntu b/Dockerfile.ubuntu index f7e57e380c..cc317cd7b5 100644 --- a/Dockerfile.ubuntu +++ b/Dockerfile.ubuntu @@ -326,7 +326,7 @@ ARG OPTIMIZE_BUILDING_TESTS=0 # Builds unit tests together with ovms server in one step # It speeds up CI when tests are executed outside of the image building # hadolint ignore=SC2046 -RUN if [ "$FUZZER_BUILD" == "0" ]; then bazel build --jobs=$JOBS ${debug_bazel_flags} ${minitrace_flags} //src:ovms $(if [ "${OPTIMIZE_BUILDING_TESTS}" == "1" ] ; then echo -n //src:ovms_test; fi); fi; +RUN if [ "$FUZZER_BUILD" == "0" ]; then bazel build --jobs=$JOBS ${debug_bazel_flags} ${minitrace_flags} //src:ovms $(if [ "${OPTIMIZE_BUILDING_TESTS}" == "1" ] ; then echo -n "//src:ovms_test //src:python_runtime_library_test"; fi); fi; ARG RUN_TESTS=0 RUN if [ "$RUN_TESTS" == "1" ] ; then mkdir -p demos/common/export_models/ && mv export_model.py demos/common/export_models/ && ./prepare_llm_models.sh /ovms/src/test/llm_testing docker && ./run_unit_tests.sh ; fi diff --git a/run_unit_tests.sh b/run_unit_tests.sh index b5c8891e18..07bf369ffc 100755 --- a/run_unit_tests.sh +++ b/run_unit_tests.sh @@ -25,7 +25,7 @@ FAIL_LOG=${FAIL_LOG:-"fail.log"} if [ -f /etc/redhat-release ] ; then dist="--//:distro=redhat" ; fi debug_bazel_flags=${debug_bazel_flags:-"--config=mp_on_py_on $dist"} TEST_FILTER="--test_filter=*" -UNIT_TEST_TARGETS="//src:ovms_test" +UNIT_TEST_TARGETS="//src:ovms_test //src:python_runtime_library_test" SHARED_OPTIONS=" \ --jobs=$JOBS \ ${debug_bazel_flags} \ diff --git a/src/BUILD b/src/BUILD index 954f88d602..b2daa8805f 100644 --- a/src/BUILD +++ b/src/BUILD @@ -2383,7 +2383,6 @@ cc_test( "//:not_disable_python": [ # OvmsPyTensor is currently not used in OVMS core and is just a base for the binding. # "test/python/ovms_py_tensor_test.cpp", - "test/python_runtime_library_test.cpp", "test/pythonnode_test.cpp", # LLM logic uses Python for processing Jinja templates when built with Python enabled "test/llm/llmtemplate_test.cpp", @@ -2609,6 +2608,34 @@ cc_library( copts = COPTS_TESTS, linkopts = COMMON_STATIC_LIBS_LINKOPTS, ) + +cc_test( + name = "python_runtime_library_test", + srcs = [ + "test/python_runtime_library_test.cpp", + ], + copts = ["-Wno-format-security"], + data = select({ + "//src:windows": [ + "//src/python:libovmspython.dll", + "//src/python/binding:pyovms.pyd", + ], + "//:disable_python": [], + "//conditions:default": [ + "//src/python:libovmspython.so", + "//src/python/binding:pyovms.so", + ], + }), + linkopts = select({ + "//src:windows": [], + "//conditions:default": ["-ldl"], + }), + visibility = ["//visibility:public"], + deps = [ + "@com_google_googletest//:gtest_main", + ], +) + cc_library( name = "test_test_models", hdrs = ["test/test_models.hpp",], diff --git a/src/python/BUILD b/src/python/BUILD index d5342ba245..c5d289d725 100644 --- a/src/python/BUILD +++ b/src/python/BUILD @@ -324,3 +324,4 @@ alias( }), visibility = ["//visibility:public"], ) + diff --git a/src/test/python_runtime_library_test.cpp b/src/test/python_runtime_library_test.cpp index 0a3dc1c8dc..cc29bc0870 100644 --- a/src/test/python_runtime_library_test.cpp +++ b/src/test/python_runtime_library_test.cpp @@ -17,8 +17,6 @@ #include #include #include -#include -#include #include #include @@ -32,49 +30,27 @@ using testing::HasSubstr; namespace { -std::filesystem::path getExecutablePath() { #ifdef __linux__ - return std::filesystem::canonical("/proc/self/exe"); -#elif _WIN32 - return std::filesystem::current_path(); -#endif -} +using ValidatePythonEnvironmentFn = bool (*)(const char** errorMessage); -std::vector getRunfilesRoots() { - std::vector roots; - if (const char* testSrcDir = std::getenv("TEST_SRCDIR"); testSrcDir != nullptr && testSrcDir[0] != '\0') { - roots.emplace_back(testSrcDir); - } - const auto executablePath = getExecutablePath(); - roots.emplace_back(executablePath.string() + ".runfiles"); - return roots; -} +class ScopedSharedLibrary { + void* handle; -std::vector getWorkspacePrefixes() { - std::vector prefixes; - if (const char* workspace = std::getenv("TEST_WORKSPACE"); workspace != nullptr && workspace[0] != '\0') { - prefixes.emplace_back(workspace); +public: + explicit ScopedSharedLibrary(const std::filesystem::path& path) : + handle(dlopen(path.c_str(), RTLD_NOW | RTLD_LOCAL)) { } - prefixes.emplace_back("_main"); - prefixes.emplace_back("model_server"); - return prefixes; -} -std::filesystem::path findRunfile(const std::filesystem::path& relativePath) { - for (const auto& root : getRunfilesRoots()) { - for (const auto& prefix : getWorkspacePrefixes()) { - const auto candidate = root / prefix / relativePath; - if (std::filesystem::exists(candidate)) { - return candidate; - } - } - const auto directCandidate = root / relativePath; - if (std::filesystem::exists(directCandidate)) { - return directCandidate; + ~ScopedSharedLibrary() { + if (handle != nullptr) { + dlclose(handle); } } - return {}; -} + + void* get() const { + return handle; + } +}; class ScopedEnvironmentVariable { std::string name; @@ -89,74 +65,90 @@ class ScopedEnvironmentVariable { hadValue = true; previousValue = currentValue; } -#ifdef __linux__ setenv(name.c_str(), value.c_str(), 1); -#elif _WIN32 - _putenv_s(name.c_str(), value.c_str()); -#endif } ~ScopedEnvironmentVariable() { if (hadValue) { -#ifdef __linux__ setenv(name.c_str(), previousValue.c_str(), 1); -#elif _WIN32 - _putenv_s(name.c_str(), previousValue.c_str()); -#endif } else { -#ifdef __linux__ unsetenv(name.c_str()); -#elif _WIN32 - _putenv_s(name.c_str(), ""); -#endif } } }; -#ifdef __linux__ -using ValidatePythonEnvironmentFn = bool (*)(const char** errorMessage); +std::filesystem::path findLibrary(const std::string& libName) { + std::vector searchPaths; -class ScopedSharedLibrary { - void* handle; + if (const char* testSrcDir = std::getenv("TEST_SRCDIR"); testSrcDir != nullptr && testSrcDir[0] != '\0') { + std::filesystem::path srcDir(testSrcDir); + if (const char* workspace = std::getenv("TEST_WORKSPACE"); workspace != nullptr && workspace[0] != '\0') { + searchPaths.emplace_back(srcDir / workspace / "src/python" / libName); + searchPaths.emplace_back(srcDir / workspace / "bazel-bin" / "src/python" / libName); + } + searchPaths.emplace_back(srcDir / "_main" / "src/python" / libName); + searchPaths.emplace_back(srcDir / "_main" / "bazel-bin" / "src/python" / libName); + searchPaths.emplace_back(srcDir / "model_server" / "src/python" / libName); + searchPaths.emplace_back(srcDir / "model_server" / "bazel-bin" / "src/python" / libName); + } -public: - explicit ScopedSharedLibrary(const std::filesystem::path& path) : - handle(dlopen(path.c_str(), RTLD_NOW | RTLD_LOCAL)) { + try { + const auto testBinaryPath = std::filesystem::canonical("/proc/self/exe"); + const auto runfilesDir = testBinaryPath.string() + ".runfiles"; + searchPaths.emplace_back(std::filesystem::path(runfilesDir) / "src/python" / libName); + searchPaths.emplace_back(std::filesystem::path(runfilesDir) / "_main" / "src/python" / libName); + searchPaths.emplace_back(std::filesystem::path(runfilesDir) / "model_server" / "src/python" / libName); + } catch (...) { } - ~ScopedSharedLibrary() { - if (handle != nullptr) { - dlclose(handle); + searchPaths.emplace_back(std::filesystem::path("bazel-bin/src/python") / libName); + searchPaths.emplace_back(std::filesystem::path("src/python") / libName); + searchPaths.emplace_back(libName); + + for (const auto& path : searchPaths) { + if (std::filesystem::exists(path)) { + return path; } } - void* get() const { - return handle; - } -}; + return {}; +} + +std::filesystem::path findPyovmsBinding() { + std::vector searchPaths; -int runIsolatedOvmsTest(const std::string& gtestFilter, const std::string& pythonPath = "") { - const auto executablePath = getExecutablePath(); - const std::string filterArgument = "--gtest_filter=" + gtestFilter; - pid_t pid = fork(); - if (pid == 0) { - setenv("OVMS_SKIP_GLOBAL_PYTHON_ENVIRONMENT", "1", 1); - unsetenv("TEST_PREMATURE_EXIT_FILE"); - if (!pythonPath.empty()) { - setenv("PYTHONPATH", pythonPath.c_str(), 1); + if (const char* testSrcDir = std::getenv("TEST_SRCDIR"); testSrcDir != nullptr && testSrcDir[0] != '\0') { + std::filesystem::path srcDir(testSrcDir); + if (const char* workspace = std::getenv("TEST_WORKSPACE"); workspace != nullptr && workspace[0] != '\0') { + searchPaths.emplace_back(srcDir / workspace / "src/python/binding" / "pyovms.so"); + searchPaths.emplace_back(srcDir / workspace / "bazel-bin" / "src/python/binding" / "pyovms.so"); } - execl(executablePath.c_str(), executablePath.c_str(), filterArgument.c_str(), static_cast(nullptr)); - _exit(127); + searchPaths.emplace_back(srcDir / "_main" / "src/python/binding" / "pyovms.so"); + searchPaths.emplace_back(srcDir / "_main" / "bazel-bin" / "src/python/binding" / "pyovms.so"); + searchPaths.emplace_back(srcDir / "model_server" / "src/python/binding" / "pyovms.so"); + searchPaths.emplace_back(srcDir / "model_server" / "bazel-bin" / "src/python/binding" / "pyovms.so"); } - int status = 0; - if (pid < 0 || waitpid(pid, &status, 0) < 0) { - return -1; + try { + const auto testBinaryPath = std::filesystem::canonical("/proc/self/exe"); + const auto runfilesDir = testBinaryPath.string() + ".runfiles"; + searchPaths.emplace_back(std::filesystem::path(runfilesDir) / "src/python/binding" / "pyovms.so"); + searchPaths.emplace_back(std::filesystem::path(runfilesDir) / "_main" / "src/python/binding" / "pyovms.so"); + searchPaths.emplace_back(std::filesystem::path(runfilesDir) / "model_server" / "src/python/binding" / "pyovms.so"); + } catch (...) { } - if (!WIFEXITED(status)) { - return -1; + + searchPaths.emplace_back(std::filesystem::path("bazel-bin/src/python/binding/pyovms.so")); + searchPaths.emplace_back(std::filesystem::path("src/python/binding/pyovms.so")); + searchPaths.emplace_back("pyovms.so"); + + for (const auto& path : searchPaths) { + if (std::filesystem::exists(path)) { + return path; + } } - return WEXITSTATUS(status); + + return {}; } #endif @@ -179,8 +171,8 @@ TEST(PythonRuntimeLibrary, ExistingLibraryExportsRequiredSymbols) { #ifndef __linux__ GTEST_SKIP() << "Linux-only libovmspython.so test"; #else - const auto libraryPath = findRunfile("src/python/libovmspython.so"); - ASSERT_FALSE(libraryPath.empty()); + const auto libraryPath = findLibrary("libovmspython.so"); + ASSERT_FALSE(libraryPath.empty()) << "Could not find libovmspython.so"; ScopedSharedLibrary library(libraryPath); ASSERT_NE(library.get(), nullptr) << dlerror(); @@ -194,33 +186,16 @@ TEST(PythonRuntimeLibrary, ValidationFailsWithoutBindingOnPythonPath) { #ifndef __linux__ GTEST_SKIP() << "Linux-only libovmspython.so test"; #else - const auto emptyPythonPath = std::filesystem::temp_directory_path() / "ovms_empty_pythonpath"; - std::filesystem::create_directories(emptyPythonPath); - EXPECT_EQ(runIsolatedOvmsTest("PythonRuntimeLibraryIsolated.ValidationFailsWithoutBindingOnPythonPath", emptyPythonPath.string()), 0); -#endif -} - -TEST(PythonRuntimeLibrary, ValidationSucceedsWithBindingOnPythonPath) { -#ifndef __linux__ - GTEST_SKIP() << "Linux-only libovmspython.so test"; -#else - const auto bindingPath = findRunfile("src/python/binding/pyovms.so"); - ASSERT_FALSE(bindingPath.empty()); - EXPECT_EQ(runIsolatedOvmsTest("PythonRuntimeLibraryIsolated.ValidationSucceedsWithBindingOnPythonPath", bindingPath.parent_path().string()), 0); -#endif -} - -TEST(PythonRuntimeLibraryIsolated, ValidationFailsWithoutBindingOnPythonPath) { -#ifndef __linux__ - GTEST_SKIP() << "Linux-only libovmspython.so test"; -#else - const auto libraryPath = findRunfile("src/python/libovmspython.so"); - ASSERT_FALSE(libraryPath.empty()); - ASSERT_EQ(std::getenv("OVMS_SKIP_GLOBAL_PYTHON_ENVIRONMENT"), std::string("1")); + const auto libraryPath = findLibrary("libovmspython.so"); + ASSERT_FALSE(libraryPath.empty()) << "Could not find libovmspython.so"; ScopedSharedLibrary library(libraryPath); ASSERT_NE(library.get(), nullptr) << dlerror(); + const auto emptyPythonPath = std::filesystem::temp_directory_path() / "ovms_empty_pythonpath"; + std::filesystem::create_directories(emptyPythonPath); + ScopedEnvironmentVariable pythonPathEnv("PYTHONPATH", emptyPythonPath.string()); + auto validate = reinterpret_cast(dlsym(library.get(), "OVMS_validatePythonEnvironment")); ASSERT_NE(validate, nullptr); @@ -231,17 +206,20 @@ TEST(PythonRuntimeLibraryIsolated, ValidationFailsWithoutBindingOnPythonPath) { #endif } -TEST(PythonRuntimeLibraryIsolated, ValidationSucceedsWithBindingOnPythonPath) { +TEST(PythonRuntimeLibrary, ValidationSucceedsWithBindingOnPythonPath) { #ifndef __linux__ GTEST_SKIP() << "Linux-only libovmspython.so test"; #else - const auto libraryPath = findRunfile("src/python/libovmspython.so"); - ASSERT_FALSE(libraryPath.empty()); - ASSERT_EQ(std::getenv("OVMS_SKIP_GLOBAL_PYTHON_ENVIRONMENT"), std::string("1")); + const auto libraryPath = findLibrary("libovmspython.so"); + ASSERT_FALSE(libraryPath.empty()) << "Could not find libovmspython.so"; + const auto bindingPath = findPyovmsBinding(); + ASSERT_FALSE(bindingPath.empty()) << "Could not find pyovms.so"; ScopedSharedLibrary library(libraryPath); ASSERT_NE(library.get(), nullptr) << dlerror(); + ScopedEnvironmentVariable pythonPathEnv("PYTHONPATH", bindingPath.parent_path().string()); + auto validate = reinterpret_cast(dlsym(library.get(), "OVMS_validatePythonEnvironment")); ASSERT_NE(validate, nullptr); diff --git a/src/test/unit_tests.cpp b/src/test/unit_tests.cpp index 64f92b9d6e..0b26f90f5b 100644 --- a/src/test/unit_tests.cpp +++ b/src/test/unit_tests.cpp @@ -27,10 +27,7 @@ int main(int argc, char** argv) { ::testing::AddGlobalTestEnvironment(new Environment); ::testing::AddGlobalTestEnvironment(new GPUEnvironment); ::testing::AddGlobalTestEnvironment(new GGUFEnvironment); - if (const char* skipPythonEnvironment = std::getenv("OVMS_SKIP_GLOBAL_PYTHON_ENVIRONMENT"); - skipPythonEnvironment == nullptr || std::string(skipPythonEnvironment) != "1") { - ::testing::AddGlobalTestEnvironment(new PythonEnvironment); - } + ::testing::AddGlobalTestEnvironment(new PythonEnvironment); ::testing::FLAGS_gtest_death_test_style = "threadsafe"; return RUN_ALL_TESTS(); } From 3ee85ff1e647752c18284772e6a86d584ff1f6fa Mon Sep 17 00:00:00 2001 From: Rafal Sapala Date: Thu, 30 Apr 2026 15:31:15 +0200 Subject: [PATCH 17/22] Style --- src/test/python_runtime_library_test.cpp | 151 ++++++++++++++++------- 1 file changed, 103 insertions(+), 48 deletions(-) diff --git a/src/test/python_runtime_library_test.cpp b/src/test/python_runtime_library_test.cpp index cc29bc0870..79f704f0a1 100644 --- a/src/test/python_runtime_library_test.cpp +++ b/src/test/python_runtime_library_test.cpp @@ -26,32 +26,70 @@ #include #endif +#ifdef _WIN32 +#include +#endif + using testing::HasSubstr; namespace { -#ifdef __linux__ using ValidatePythonEnvironmentFn = bool (*)(const char** errorMessage); class ScopedSharedLibrary { - void* handle; +public: +#ifdef _WIN32 + using HandleType = HMODULE; +#else + using HandleType = void*; +#endif + +private: + HandleType handle; public: explicit ScopedSharedLibrary(const std::filesystem::path& path) : - handle(dlopen(path.c_str(), RTLD_NOW | RTLD_LOCAL)) { + handle( +#ifdef _WIN32 + LoadLibraryA(path.string().c_str()) +#else + dlopen(path.c_str(), RTLD_NOW | RTLD_LOCAL) +#endif + ) { } ~ScopedSharedLibrary() { if (handle != nullptr) { +#ifdef _WIN32 + FreeLibrary(handle); +#else dlclose(handle); +#endif } } - void* get() const { + HandleType get() const { return handle; } }; +const char* getLibraryLoadError() { +#ifdef _WIN32 + return "LoadLibrary failed"; +#else + const char* error = dlerror(); + return error == nullptr ? "dlopen failed" : error; +#endif +} + +void* findSymbol(ScopedSharedLibrary::HandleType handle, const char* symbolName) { +#ifdef _WIN32 + return reinterpret_cast(GetProcAddress(handle, symbolName)); +#else + return dlsym(handle, symbolName); +#endif +} + class ScopedEnvironmentVariable { std::string name; bool hadValue; @@ -65,18 +103,46 @@ class ScopedEnvironmentVariable { hadValue = true; previousValue = currentValue; } +#ifdef _WIN32 + _putenv_s(name.c_str(), value.c_str()); +#else setenv(name.c_str(), value.c_str(), 1); +#endif } ~ScopedEnvironmentVariable() { if (hadValue) { +#ifdef _WIN32 + _putenv_s(name.c_str(), previousValue.c_str()); +#else setenv(name.c_str(), previousValue.c_str(), 1); +#endif } else { +#ifdef _WIN32 + _putenv_s(name.c_str(), ""); +#else unsetenv(name.c_str()); +#endif } } }; +std::string getRuntimeLibraryFilename() { +#ifdef _WIN32 + return "libovmspython.dll"; +#else + return "libovmspython.so"; +#endif +} + +std::string getBindingFilename() { +#ifdef _WIN32 + return "pyovms.pyd"; +#else + return "pyovms.so"; +#endif +} + std::filesystem::path findLibrary(const std::string& libName) { std::vector searchPaths; @@ -119,28 +185,31 @@ std::filesystem::path findPyovmsBinding() { if (const char* testSrcDir = std::getenv("TEST_SRCDIR"); testSrcDir != nullptr && testSrcDir[0] != '\0') { std::filesystem::path srcDir(testSrcDir); + const std::string bindingFilename = getBindingFilename(); if (const char* workspace = std::getenv("TEST_WORKSPACE"); workspace != nullptr && workspace[0] != '\0') { - searchPaths.emplace_back(srcDir / workspace / "src/python/binding" / "pyovms.so"); - searchPaths.emplace_back(srcDir / workspace / "bazel-bin" / "src/python/binding" / "pyovms.so"); + searchPaths.emplace_back(srcDir / workspace / "src/python/binding" / bindingFilename); + searchPaths.emplace_back(srcDir / workspace / "bazel-bin" / "src/python/binding" / bindingFilename); } - searchPaths.emplace_back(srcDir / "_main" / "src/python/binding" / "pyovms.so"); - searchPaths.emplace_back(srcDir / "_main" / "bazel-bin" / "src/python/binding" / "pyovms.so"); - searchPaths.emplace_back(srcDir / "model_server" / "src/python/binding" / "pyovms.so"); - searchPaths.emplace_back(srcDir / "model_server" / "bazel-bin" / "src/python/binding" / "pyovms.so"); + searchPaths.emplace_back(srcDir / "_main" / "src/python/binding" / bindingFilename); + searchPaths.emplace_back(srcDir / "_main" / "bazel-bin" / "src/python/binding" / bindingFilename); + searchPaths.emplace_back(srcDir / "model_server" / "src/python/binding" / bindingFilename); + searchPaths.emplace_back(srcDir / "model_server" / "bazel-bin" / "src/python/binding" / bindingFilename); } try { + const std::string bindingFilename = getBindingFilename(); const auto testBinaryPath = std::filesystem::canonical("/proc/self/exe"); const auto runfilesDir = testBinaryPath.string() + ".runfiles"; - searchPaths.emplace_back(std::filesystem::path(runfilesDir) / "src/python/binding" / "pyovms.so"); - searchPaths.emplace_back(std::filesystem::path(runfilesDir) / "_main" / "src/python/binding" / "pyovms.so"); - searchPaths.emplace_back(std::filesystem::path(runfilesDir) / "model_server" / "src/python/binding" / "pyovms.so"); + searchPaths.emplace_back(std::filesystem::path(runfilesDir) / "src/python/binding" / bindingFilename); + searchPaths.emplace_back(std::filesystem::path(runfilesDir) / "_main" / "src/python/binding" / bindingFilename); + searchPaths.emplace_back(std::filesystem::path(runfilesDir) / "model_server" / "src/python/binding" / bindingFilename); } catch (...) { } - searchPaths.emplace_back(std::filesystem::path("bazel-bin/src/python/binding/pyovms.so")); - searchPaths.emplace_back(std::filesystem::path("src/python/binding/pyovms.so")); - searchPaths.emplace_back("pyovms.so"); + const std::string bindingFilename = getBindingFilename(); + searchPaths.emplace_back(std::filesystem::path("bazel-bin/src/python/binding") / bindingFilename); + searchPaths.emplace_back(std::filesystem::path("src/python/binding") / bindingFilename); + searchPaths.emplace_back(bindingFilename); for (const auto& path : searchPaths) { if (std::filesystem::exists(path)) { @@ -150,81 +219,67 @@ std::filesystem::path findPyovmsBinding() { return {}; } -#endif } // namespace TEST(PythonRuntimeLibrary, MissingLibraryPathFailsToLoad) { -#ifndef __linux__ - GTEST_SKIP() << "Linux-only libovmspython.so test"; -#else - const auto missingLibraryPath = std::filesystem::temp_directory_path() / "missing_libovmspython.so"; + const auto missingLibraryPath = std::filesystem::temp_directory_path() / ("missing_" + getRuntimeLibraryFilename()); ASSERT_FALSE(std::filesystem::exists(missingLibraryPath)); ScopedSharedLibrary library(missingLibraryPath); EXPECT_EQ(library.get(), nullptr); -#endif } TEST(PythonRuntimeLibrary, ExistingLibraryExportsRequiredSymbols) { -#ifndef __linux__ - GTEST_SKIP() << "Linux-only libovmspython.so test"; -#else - const auto libraryPath = findLibrary("libovmspython.so"); - ASSERT_FALSE(libraryPath.empty()) << "Could not find libovmspython.so"; + const auto runtimeLibraryFilename = getRuntimeLibraryFilename(); + const auto libraryPath = findLibrary(runtimeLibraryFilename); + ASSERT_FALSE(libraryPath.empty()) << "Could not find " << runtimeLibraryFilename; ScopedSharedLibrary library(libraryPath); - ASSERT_NE(library.get(), nullptr) << dlerror(); + ASSERT_NE(library.get(), nullptr) << getLibraryLoadError(); - EXPECT_NE(dlsym(library.get(), "OVMS_createPythonInterpreterModule"), nullptr); - EXPECT_NE(dlsym(library.get(), "OVMS_validatePythonEnvironment"), nullptr); -#endif + EXPECT_NE(findSymbol(library.get(), "OVMS_createPythonInterpreterModule"), nullptr); + EXPECT_NE(findSymbol(library.get(), "OVMS_validatePythonEnvironment"), nullptr); } TEST(PythonRuntimeLibrary, ValidationFailsWithoutBindingOnPythonPath) { -#ifndef __linux__ - GTEST_SKIP() << "Linux-only libovmspython.so test"; -#else - const auto libraryPath = findLibrary("libovmspython.so"); - ASSERT_FALSE(libraryPath.empty()) << "Could not find libovmspython.so"; + const auto runtimeLibraryFilename = getRuntimeLibraryFilename(); + const auto libraryPath = findLibrary(runtimeLibraryFilename); + ASSERT_FALSE(libraryPath.empty()) << "Could not find " << runtimeLibraryFilename; ScopedSharedLibrary library(libraryPath); - ASSERT_NE(library.get(), nullptr) << dlerror(); + ASSERT_NE(library.get(), nullptr) << getLibraryLoadError(); const auto emptyPythonPath = std::filesystem::temp_directory_path() / "ovms_empty_pythonpath"; std::filesystem::create_directories(emptyPythonPath); ScopedEnvironmentVariable pythonPathEnv("PYTHONPATH", emptyPythonPath.string()); - auto validate = reinterpret_cast(dlsym(library.get(), "OVMS_validatePythonEnvironment")); + auto validate = reinterpret_cast(findSymbol(library.get(), "OVMS_validatePythonEnvironment")); ASSERT_NE(validate, nullptr); const char* errorMessage = nullptr; EXPECT_FALSE(validate(&errorMessage)); ASSERT_NE(errorMessage, nullptr); EXPECT_THAT(std::string(errorMessage), HasSubstr("pyovms")); -#endif } TEST(PythonRuntimeLibrary, ValidationSucceedsWithBindingOnPythonPath) { -#ifndef __linux__ - GTEST_SKIP() << "Linux-only libovmspython.so test"; -#else - const auto libraryPath = findLibrary("libovmspython.so"); - ASSERT_FALSE(libraryPath.empty()) << "Could not find libovmspython.so"; + const auto runtimeLibraryFilename = getRuntimeLibraryFilename(); + const auto libraryPath = findLibrary(runtimeLibraryFilename); + ASSERT_FALSE(libraryPath.empty()) << "Could not find " << runtimeLibraryFilename; const auto bindingPath = findPyovmsBinding(); - ASSERT_FALSE(bindingPath.empty()) << "Could not find pyovms.so"; + ASSERT_FALSE(bindingPath.empty()) << "Could not find " << getBindingFilename(); ScopedSharedLibrary library(libraryPath); - ASSERT_NE(library.get(), nullptr) << dlerror(); + ASSERT_NE(library.get(), nullptr) << getLibraryLoadError(); ScopedEnvironmentVariable pythonPathEnv("PYTHONPATH", bindingPath.parent_path().string()); - auto validate = reinterpret_cast(dlsym(library.get(), "OVMS_validatePythonEnvironment")); + auto validate = reinterpret_cast(findSymbol(library.get(), "OVMS_validatePythonEnvironment")); ASSERT_NE(validate, nullptr); const char* errorMessage = nullptr; EXPECT_TRUE(validate(&errorMessage)); EXPECT_EQ(errorMessage, nullptr); -#endif } \ No newline at end of file From 94d1e61df97f90cf8b39fa62833d578ed04c93c6 Mon Sep 17 00:00:00 2001 From: Rafal Sapala Date: Thu, 30 Apr 2026 15:42:40 +0200 Subject: [PATCH 18/22] Windows tests --- windows_test.bat | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/windows_test.bat b/windows_test.bat index ebd8a8147e..a1ad1ffae2 100644 --- a/windows_test.bat +++ b/windows_test.bat @@ -31,8 +31,12 @@ set "OVMS_MEDIA_URL_ALLOW_REDIRECTS=1" IF "%~2"=="--with_python" ( set "bazelBuildArgs=--config=win_mp_on_py_on --action_env OpenVINO_DIR=%openvino_dir%" + set "testTargets=//src:ovms_test //src:python_runtime_library_test" + set "runPythonRuntimeTest=%cd%\bazel-bin\src\python_runtime_library_test.exe --gtest_filter=!gtestFilter! >> win_full_test.log 2>&1" ) ELSE ( set "bazelBuildArgs=--config=win_mp_on_py_off --action_env OpenVINO_DIR=%openvino_dir%" + set "testTargets=//src:ovms_test" + set "runPythonRuntimeTest=" ) IF "%~3"=="" ( @@ -41,7 +45,7 @@ IF "%~3"=="" ( set "gtestFilter=%3" ) -set "buildTestCommand=bazel %bazelStartupCmd% build %bazelBuildArgs% --jobs=%NUMBER_OF_PROCESSORS% --verbose_failures //src:ovms_test" +set "buildTestCommand=bazel %bazelStartupCmd% build %bazelBuildArgs% --jobs=%NUMBER_OF_PROCESSORS% --verbose_failures %testTargets%" set "changeConfigsCmd=python windows_change_test_configs.py" set "runTest=%cd%\bazel-bin\src\ovms_test.exe --gtest_filter=!gtestFilter! > win_full_test.log 2>&1" @@ -99,6 +103,13 @@ if !errorlevel! neq 0 exit /b !errorlevel! :: Start unit test echo Running: %runTest% %runTest% +if !errorlevel! neq 0 goto :exit_build_error + +IF "%~2"=="--with_python" ( + echo Running: %runPythonRuntimeTest% + %runPythonRuntimeTest% + if !errorlevel! neq 0 goto :exit_build_error +) :: Cut tests log to results set regex="\[ .* ms" @@ -111,6 +122,6 @@ if !errorlevel! equ 0 goto :exit_build_error echo [INFO] Tests finished with no failures. Check the summary in win_test_summary.log. exit /b 0 :exit_build_error -echo [ERROR] Check tests summary in 'win_test_summary.log' and tests logs in 'win_full_test.log'. Rerun failed test with: windows_setupvars.bat and %cd%\bazel-bin\src\ovms_test.exe --gtest_filter='*.*' +echo [ERROR] Check tests summary in 'win_test_summary.log' and tests logs in 'win_full_test.log'. Rerun failed tests with: windows_setupvars.bat and %cd%\bazel-bin\src\ovms_test.exe --gtest_filter='*.*' and %cd%\bazel-bin\src\python_runtime_library_test.exe --gtest_filter='*.*' exit /b 1 endlocal From 31295036afe91acfeb9b05a5485c90f7bdb46522 Mon Sep 17 00:00:00 2001 From: Rafal Sapala Date: Thu, 30 Apr 2026 16:06:52 +0200 Subject: [PATCH 19/22] style --- src/test/python_runtime_library_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/python_runtime_library_test.cpp b/src/test/python_runtime_library_test.cpp index 79f704f0a1..38f07bb5be 100644 --- a/src/test/python_runtime_library_test.cpp +++ b/src/test/python_runtime_library_test.cpp @@ -282,4 +282,4 @@ TEST(PythonRuntimeLibrary, ValidationSucceedsWithBindingOnPythonPath) { const char* errorMessage = nullptr; EXPECT_TRUE(validate(&errorMessage)); EXPECT_EQ(errorMessage, nullptr); -} \ No newline at end of file +} From c616148425cb168e82156cebf2de9d1f674de343 Mon Sep 17 00:00:00 2001 From: rasapala Date: Mon, 4 May 2026 17:10:47 +0200 Subject: [PATCH 20/22] Add OVMS developer skills and follow-up fixes --- .github/skills/build-bazel-target/SKILL.md | 161 +++++++++++++++++ .github/skills/build-builder-image/SKILL.md | 191 ++++++++++++++++++++ .github/skills/run-make-style/SKILL.md | 120 ++++++++++++ .github/skills/run-single-gtest/SKILL.md | 159 ++++++++++++++++ 4 files changed, 631 insertions(+) create mode 100644 .github/skills/build-bazel-target/SKILL.md create mode 100644 .github/skills/build-builder-image/SKILL.md create mode 100644 .github/skills/run-make-style/SKILL.md create mode 100644 .github/skills/run-single-gtest/SKILL.md diff --git a/.github/skills/build-bazel-target/SKILL.md b/.github/skills/build-bazel-target/SKILL.md new file mode 100644 index 0000000000..30e235b6a5 --- /dev/null +++ b/.github/skills/build-bazel-target/SKILL.md @@ -0,0 +1,161 @@ +--- +name: build-bazel-target +description: "Use when building a specific Bazel target for OVMS inside the -build Docker container. Default targets are //src:ovms (server binary) and //src:ovms_test (C++ gtest binary); also handles //src:ovms_shared (C API libovms_shared.so) and arbitrary user-supplied targets. Covers locating/starting the build container, invoking bazel build, forwarding http_proxy/https_proxy/no_proxy, switching distro to redhat via --//:distro=redhat, and reading bazel build error output. Trigger phrases: 'build ovms', 'build ovms_test', 'rebuild server', 'bazel build', '//src:ovms', '//src:ovms_test', 'compile target', 'build in container'." +--- + +# Build a Bazel Target in the Build Container + + + +Use this skill any time the user asks to **build** an OVMS Bazel target (server, tests, C API library, or any other label). For *running* tests, use the `run-single-gtest` skill instead. + +## Apply user defaults (every run) + +Before prompting the user for anything, parse the **USER DEFAULTS** comment block at the top of this file: + +1. If `DEFAULT_CONTAINER` is set, use it verbatim and skip the "choose container" questions. +2. If `DEFAULT_TARGETS` is set and the user did not specify targets in the request, use that list as the Bazel target arguments. +3. If `DEFAULT_DISTRO` is `redhat`, add `--//:distro=redhat` to every Bazel command. +4. Otherwise (placeholder still ``), follow the normal interactive flow described in the sections below. + +Always tell the user which defaults were applied (e.g. "Using `DEFAULT_CONTAINER=ovms-build-fix_pull_all_dl_failed` and `DEFAULT_TARGETS=//src:ovms //src:ovms_test` from the skill file."). + +## When to use + +- User asks to build, rebuild, or compile a specific Bazel target. +- User mentions `bazel build`, `//src:ovms`, `//src:ovms_test`, `//src:ovms_shared`, or another `//...` label. +- User asks to verify that recent edits compile cleanly without running tests. + +## Default targets + +If the user does not name a target, build **both** of these (in this order — server first, tests second; many test deps overlap with the server, so this maximizes cache reuse): + +1. `//src:ovms` — main OVMS server binary +2. `//src:ovms_test` — C++ gtest binary + +Other commonly requested targets: + +| Target | Purpose | +|--------|---------| +| `//src:ovms_shared` | C API shared library (`libovms_shared.so`) | +| `//src:ovms` | Main server binary | +| `//src:ovms_test` | Unit test binary | + +## Do NOT use + +- For *running* tests — use the `run-single-gtest` skill. +- For Python functional tests (`make test_functional`). +- For full Docker image builds (`make docker_build`). + +## Pre-flight + +1. Confirm the workspace is the OVMS repo root (contains `WORKSPACE`, `src/`, `Makefile`). +2. Confirm the target label. If ambiguous, ask once or default to `//src:ovms` + `//src:ovms_test`. +3. For Red Hat builds, append `--//:distro=redhat` to **every** Bazel command. +4. **Resolve the image tag (`OVMS_CPP_IMAGE_TAG`) before creating a new container.** Do **not** silently fall back to `latest`. + ```bash + echo "${OVMS_CPP_IMAGE_TAG:-}" + ``` + - If `OVMS_CPP_IMAGE_TAG` is **already exported** in the user's shell, reuse that value verbatim and confirm with the user. + - Otherwise ask: "`OVMS_CPP_IMAGE_TAG` is not set. What tag should I use? (e.g. `latest-`, `latest-`)" + - Use the resolved tag when constructing the `docker run` image argument: `openvino/model_server-build:${OVMS_CPP_IMAGE_TAG}`. + +## Choose the `-build` container (ask unless `DEFAULT_CONTAINER` is set) + +Builds must run **inside** the `-build` Docker container with the repo mounted at `/ovms`. **Do not auto-discover or pick a running container.** If `DEFAULT_CONTAINER` is set, use it as described above. Otherwise, ask the user one of two questions and wait for an answer before invoking `docker exec` / `docker run`: + +> **Question A — reuse:** "Which existing `-build` container should I use? Please give me its name or ID (e.g. from `docker ps --format '{{.Names}}\t{{.Image}}'`)." +> +> **Question B — create:** "No container provided — should I create a new one named `ovms-build-` from `` with `-v $(pwd):/ovms`? (suggest a name that makes the purpose obvious in `docker ps`)." + +Only proceed once `DEFAULT_CONTAINER` is applied, the user supplies a container name/ID, or the user confirms a new container with a specific name. + +### Step A — reuse a user-specified container + +Use the **name or ID the user gave you** verbatim. Do not substitute another container even if you see one in `docker ps`. + +```bash +docker exec -e http_proxy="$http_proxy" \ + -e https_proxy="$https_proxy" \ + -e no_proxy="$no_proxy" \ + \ + bash -lc 'cd /ovms && bazel build //src:ovms //src:ovms_test' +``` + +Single target: +```bash +docker exec -e http_proxy="$http_proxy" \ + -e https_proxy="$https_proxy" \ + -e no_proxy="$no_proxy" \ + \ + bash -lc 'cd /ovms && bazel build //src:ovms_test' +``` + +### Step B — create a new, clearly named container (only after user confirms) + +Always: +- Mount the **current** working directory: `-v "$(pwd)":/ovms` (run from the OVMS repo root). +- Give the container a **meaningful name** with `--name` so the user can identify it in `docker ps` (e.g. `ovms-build-`, `ovms-build-`, `ovms-redhat-build`). Confirm the name with the user before running. +- Prefer `-d` (detached) + `docker exec` for repeatable build runs, instead of `--rm` (which discards the container after one command and forces a new one for every re-run). + +```bash +# Confirm with user: name="ovms-build-", image="openvino/model_server-build:${OVMS_CPP_IMAGE_TAG}" +docker run -d -it \ + --name \ + -v "$(pwd)":/ovms \ + -w /ovms \ + -e http_proxy="$http_proxy" -e https_proxy="$https_proxy" -e no_proxy="$no_proxy" \ + openvino/model_server-build:${OVMS_CPP_IMAGE_TAG} bash + +# Then build against that named container: +docker exec -e http_proxy="$http_proxy" \ + -e https_proxy="$https_proxy" \ + -e no_proxy="$no_proxy" \ + \ + bash -lc 'cd /ovms && bazel build //src:ovms //src:ovms_test' +``` + +User can verify which container is doing what at any time with: + +```bash +docker ps --format 'table {{.Names}}\t{{.Image}}\t{{.Status}}\t{{.Command}}' +``` + +## Red Hat (UBI9) builds + +Add `--//:distro=redhat` to **every** Bazel command: + +```bash +bazel build --//:distro=redhat //src:ovms //src:ovms_test +``` + +The `--//:distro=redhat` flag is **always required** in every Bazel command, regardless of whether you are running from the host or inside the container. While the `-build` container auto-detects the OS from `/etc/redhat-release` for environment setup, Bazel still requires the explicit flag to configure its build behavior. + +## After the build + +- **Success**: report the target(s) built and the elapsed wall time from Bazel's final `INFO: Elapsed time:` line. Built artifacts live under `bazel-bin/src/` (e.g. `bazel-bin/src/ovms`, `bazel-bin/src/ovms_test`). +- **Failure**: surface the first compiler error verbatim. If output is truncated, re-run with `--verbose_failures` against the **same user-supplied container** and tail the failing action log: + ```bash + docker exec -w /ovms \ + bash -lc 'bazel build --verbose_failures //src:ovms_test 2>&1 | tail -n 200' + ``` + +## Tips & pitfalls + +- **Do not run `bazel` on the host.** Always go through the `-build` container so the toolchain and OpenVINO paths match. +- **Forward proxies** (`http_proxy`/`https_proxy`/`no_proxy`) for the *first* build of a fresh checkout — Bazel fetches external repos over the network. Cached builds do not need them. +- **Build the server before the tests** when both are requested; this maximizes cache reuse and surfaces production-code errors first. +- **Avoid `bazel clean`** unless the user explicitly asks — full rebuilds are very slow. Prefer `bazel build` and let Bazel handle incremental work. +- **One in-progress build at a time per workspace**: if a previous async build is still running, wait for it or kill its terminal before starting another — two concurrent Bazel servers will block each other on `bazel-out`. +- **Linker / shared-lib changes**: when modifying `BUILD.bazel`, `WORKSPACE`, or `third_party/**`, a clean rebuild of the affected target may be required. diff --git a/.github/skills/build-builder-image/SKILL.md b/.github/skills/build-builder-image/SKILL.md new file mode 100644 index 0000000000..95f86fba9f --- /dev/null +++ b/.github/skills/build-builder-image/SKILL.md @@ -0,0 +1,191 @@ +--- +name: build-builder-image +description: "Use when building (or rebuilding) the OVMS -build Docker image on the host via the repository Makefile. Covers `make ovms_builder_image`, the BASE_OS variants (ubuntu24, ubuntu22, redhat), proxy forwarding, GPU / NPU / Mediapipe / Python / fuzzer / debug toggles, NO_DOCKER_CACHE / USE_BUILDX flags, and JOBS parallelism. Also covers the full pipeline target `make docker_build` (builder + targz_package + release images). Trigger phrases: 'build the build container', 'build builder image', 'rebuild -build image', 'make ovms_builder_image', 'make docker_build', 'new build image', 'BASE_OS=redhat', 'rebuild docker build image'." +--- + +# Build the OVMS `-build` Docker Image (Host Make) + +Use this skill when the user wants to **(re)build the `*-build` Docker image on the host** using the repository `Makefile`. This is the image other skills (`build-bazel-target`, `run-single-gtest`) execute inside. + +Building the `-build` image is **time-expensive** (10s of minutes to over an hour, depending on flags and cache). Always check first whether an existing image can be reused. Only build a new one when the user explicitly requests it, or when dependencies / Dockerfiles / build env changed. + +## When to use + +- User asks to build, rebuild, or refresh the `-build` Docker image. +- User mentions `make ovms_builder_image`, `make docker_build`, `Dockerfile.ubuntu`, `Dockerfile.redhat`, `BASE_OS`, `NO_DOCKER_CACHE`. +- A previous build failed inside the container with toolchain / dependency errors and a rebuild of the builder image is the documented fix. + +## Do NOT use + +- For building Bazel targets (`//src:ovms`, `//src:ovms_test`) — use `build-bazel-target`. +- For running tests — use `run-single-gtest`. +- For producing the final release `.tar.gz` only (use `make targz_package`) or release runtime images only (`make ovms_release_images`). + +## Pre-flight + +1. Confirm the workspace is the OVMS repo root (contains `Makefile`, `Dockerfile.ubuntu`, `Dockerfile.redhat`). +2. Check for an existing `-build` image first — building from scratch is expensive: + ```bash + docker images | grep -- -build + ``` + If a suitable image exists and the user has not asked for a rebuild, **stop here** and tell them to reuse it. +3. Confirm host prerequisites: `docker` available, sufficient free disk (typically ≥ 30 GB), and proxy env vars exported if the network is restricted. +4. **Resolve the image tag (`OVMS_CPP_IMAGE_TAG`) before invoking make.** Do **not** silently fall back to `latest` — it would overwrite the shared default image and make `docker ps` ambiguous about which build belongs to which feature. + ```bash + echo "${OVMS_CPP_IMAGE_TAG:-}" + ``` + - If `OVMS_CPP_IMAGE_TAG` is **already exported** in the user's shell, reuse that value verbatim and confirm with the user. + - If it is **unset**, ask the user one of: + > "`OVMS_CPP_IMAGE_TAG` is not set. What tag should I use? Suggested patterns: + > - feature/branch tag, e.g. `fix_pull_all_dl_failed`, `lfs-cancel`, `pr-4156` + > - your username, e.g. `$USER` (→ `${USER}`) + > Reply with the tag, or explicitly say \"use latest\" if you really want to overwrite `:latest`." + - Only use `latest` when the user **explicitly** asks for it. + - Pass the chosen tag on the make command line: `OVMS_CPP_IMAGE_TAG=`. Optionally also set `IMAGE_TAG_SUFFIX` if the user wants a suffix appended to an existing tag rather than a full override. + +## Default invocation + +The default builder-image target on Ubuntu 24.04 (after the Pre-flight step has resolved `OVMS_CPP_IMAGE_TAG`): + +```bash +make ovms_builder_image OVMS_CPP_IMAGE_TAG= +``` + +This produces image `openvino/model_server-build:` (tag built from `OVMS_CPP_DOCKER_IMAGE` × `OVMS_CPP_IMAGE_TAG`). Do **not** invoke `make ovms_builder_image` without the tag override unless the user explicitly chose `latest`. + +To run the **full** pipeline (builder image → packaged `.tar.gz` → release CPU/GPU images): + +```bash +make docker_build OVMS_CPP_IMAGE_TAG= +``` + +## Common Makefile flags + +All flags are passed on the `make` command line as `KEY=value` pairs (e.g. `make ovms_builder_image OVMS_CPP_IMAGE_TAG= BASE_OS=redhat NO_DOCKER_CACHE=true`). + +| Flag | Default | Purpose | +|------|---------|---------| +| `BASE_OS` | `ubuntu24` | Base distro for the builder image. Supported: `ubuntu24`, `ubuntu22`, `redhat`. | +| `BASE_OS_TAG_UBUNTU` | `24.04` | Override Ubuntu base tag. | +| `BASE_OS_TAG_REDHAT` | `9.7` | Override RHEL/UBI9 base tag. | +| `OV_USE_BINARY` | `1` (Ubuntu) / `0` (RedHat) | Use prebuilt OpenVINO archive vs. build from source. RHEL must use `0`. | +| `DLDT_PACKAGE_URL` | from `versions.mk` | URL of the OpenVINO binary tarball when `OV_USE_BINARY=1`. | +| `MEDIAPIPE_DISABLE` | `0` | Set `1` to disable MediaPipe (also forces `PYTHON_DISABLE=1`). | +| `PYTHON_DISABLE` | `0` | Set `1` to omit Python custom-node bindings. | +| `FUZZER_BUILD` | `0` | Set `1` for fuzzer build (incompatible with `RUN_TESTS=1`, `CHECK_COVERAGE=1`, `BASE_OS=redhat`). | +| `BAZEL_BUILD_TYPE` | `opt` | `dbg` for debug symbols + no strip. | +| `MINITRACE` | `OFF` | `ON` enables minitrace instrumentation. | +| `OV_TRACING_ENABLE` | `0` | `1` enables OpenVINO tracing macros. | +| `RUN_TESTS` | `0` | `1` runs C++ tests inside the build stage (lengthens build). | +| `BUILD_TESTS` | `0` | `1` only compiles tests inside the build stage. | +| `CHECK_COVERAGE` | `0` | `1` enables coverage instrumentation (requires `RUN_TESTS=1`). | +| `RUN_GPU_TESTS` | unset | Set to enable GPU tests during the build. | +| `GPU` / `NPU` | `0` / `0` | Toggle accelerator runtimes in release stage (used by `ovms_release_images`). | +| `JOBS` | `$(CORES_TOTAL)` | Parallelism passed to the build stage; default is derived from physical cores (`sockets × cores-per-socket` via `lscpu`). | +| `NO_DOCKER_CACHE` | unset | `true` adds `--no-cache` and re-pulls the base image. | +| `USE_BUILDX` | unset | `true` switches to `docker buildx build` (BuildKit). | +| `IMAGE_TAG_SUFFIX` | empty | Appended to the image tag (e.g. `-myfeature`). | +| `OVMS_CPP_DOCKER_IMAGE` | `openvino/model_server` | Override image repo. | +| `OVMS_CPP_IMAGE_TAG` | `latest` | Override image tag. **Always set explicitly** — see Pre-flight step 4. | +| `OVMS_METADATA_FILE` | unset | Path to a JSON file copied into the image as `.workspace/metadata.json`. | +| `BUILD_NGINX` | `0` | `1` enables nginx integration. | +| `OV_SOURCE_BRANCH` / `OV_SOURCE_ORG` | from `versions.mk` | Override OpenVINO sources when building OV from source. | +| `OV_GENAI_BRANCH` / `OV_GENAI_ORG` | from `versions.mk` | Override OpenVINO GenAI sources. | +| `OV_TOKENIZERS_BRANCH` / `OV_TOKENIZERS_ORG` | from `versions.mk` | Override OV tokenizers sources. | + +The `BUILD_ARGS` variable in the Makefile consolidates these into `--build-arg` flags passed to the underlying `docker build` invocation; you do not need to construct them manually. + +## Recipes + +### 1. Default Ubuntu 24 builder, reuse cache + +```bash +make ovms_builder_image OVMS_CPP_IMAGE_TAG= +``` + +### 2. Red Hat (UBI9) builder + +```bash +make ovms_builder_image OVMS_CPP_IMAGE_TAG= BASE_OS=redhat +``` + +The Makefile auto-selects `--//:distro=redhat` for Bazel inside the image and forces `OV_USE_BINARY=0`. + +### 3. Force a clean rebuild (no Docker cache) + +```bash +make ovms_builder_image OVMS_CPP_IMAGE_TAG= NO_DOCKER_CACHE=true +``` + +### 4. Debug build with extra parallelism + +```bash +make ovms_builder_image OVMS_CPP_IMAGE_TAG= BAZEL_BUILD_TYPE=dbg JOBS=32 +``` + +### 5. Tag a side-by-side image for a feature branch + +```bash +make ovms_builder_image OVMS_CPP_IMAGE_TAG= IMAGE_TAG_SUFFIX=-fix_pull_all_dl_failed +# produces openvino/model_server-build:-fix_pull_all_dl_failed +``` + +### 6. Slim build without Mediapipe / Python custom nodes + +```bash +make ovms_builder_image OVMS_CPP_IMAGE_TAG= MEDIAPIPE_DISABLE=1 PYTHON_DISABLE=1 +``` + +### 7. Full pipeline (builder → tar.gz → release images) + +```bash +make docker_build OVMS_CPP_IMAGE_TAG= BASE_OS=ubuntu24 +# or +make docker_build OVMS_CPP_IMAGE_TAG= BASE_OS=redhat +``` + +### 8. Use BuildKit / buildx + +```bash +make ovms_builder_image OVMS_CPP_IMAGE_TAG= USE_BUILDX=true +``` + +## Proxy forwarding + +The Makefile automatically forwards `http_proxy`, `https_proxy`, and `no_proxy` from the host environment into `docker build` via `--build-arg`. **Export them in your shell before running make** if your network requires a proxy: + +```bash +export http_proxy=http://proxy:port +export https_proxy=$http_proxy +export no_proxy=localhost,127.0.0.1,.intel.com +make ovms_builder_image OVMS_CPP_IMAGE_TAG= +``` + +## Validate after build + +```bash +docker images | grep openvino/model_server-build +``` + +Expect a fresh `openvino/model_server-build:` image. To smoke-test it, start a container and run a trivial Bazel query: + +```bash +docker run --rm \ + -v "$(pwd)":/ovms \ + -e http_proxy="$http_proxy" -e https_proxy="$https_proxy" -e no_proxy="$no_proxy" \ + openvino/model_server-build:${OVMS_CPP_IMAGE_TAG} \ + bash -lc 'cd /ovms && bazel info release' +``` + +## Tips & pitfalls + +- **Never silently use the default `latest` tag.** Resolve `OVMS_CPP_IMAGE_TAG` from the user's environment or ask explicitly (Pre-flight step 4). Overwriting `:latest` clobbers the shared default and makes `docker ps` / `docker images` ambiguous about which build belongs to which feature. +- **Reuse before rebuild.** Always `docker images | grep -- -build` first; recreate only when dependencies, `Dockerfile.*`, `WORKSPACE`, `third_party/**`, or `versions.mk` actually changed. +- **`NO_DOCKER_CACHE=true` is expensive.** Use it only when you suspect cache poisoning, base-image drift, or a broken layer. +- **Distro/`OV_USE_BINARY` constraints are enforced** by the Makefile. RedHat requires `OV_USE_BINARY=0`; Ubuntu 22 requires `OV_USE_BINARY=1`. +- **`MEDIAPIPE_DISABLE=1` requires `PYTHON_DISABLE=1`** — the Makefile errors out otherwise. +- **`FUZZER_BUILD=1` is incompatible** with `RUN_TESTS=1`, `CHECK_COVERAGE=1`, and `BASE_OS=redhat`. +- **Image tag collisions.** Without `IMAGE_TAG_SUFFIX`, repeated builds overwrite `:latest`. Use a suffix when keeping multiple branches' images side by side. +- **Disk space.** Multi-stage builds cache large layers; clean up periodically with `docker image prune` and `docker builder prune` (ask the user before pruning). +- **Long-running operation.** Run as an async terminal so the conversation is not blocked, and poll output with `get_terminal_output`. Never sleep/poll inside the shell. +- **Do not run `bazel` or `make ovms_builder_image` inside the `-build` container.** This skill is host-side only. diff --git a/.github/skills/run-make-style/SKILL.md b/.github/skills/run-make-style/SKILL.md new file mode 100644 index 0000000000..640ef54683 --- /dev/null +++ b/.github/skills/run-make-style/SKILL.md @@ -0,0 +1,120 @@ +--- +name: run-make-style +description: "Use when running OVMS code-style and lint checks via the repository Makefile. The umbrella target `make style` chains spell, clang-format-check, cpplint, and cppclean. Also covers the individual sub-targets and how to fix the most common failures (uncommitted clang-format changes, cpplint warnings, cppclean unused includes, spelling/whitelist). Trigger phrases: 'run make style', 'check code style', 'cpplint', 'clang-format-check', 'cppclean', 'spell check', 'fix style', 'pre-commit style'." +--- + +# Run `make style` and Verify the Output + +Use this skill any time the user asks to run code-style / lint checks on OVMS using the repository `Makefile`. These checks are typically run on the developer host or CI runner rather than inside the `-build` container, because the build Dockerfiles may not include `clang-format` and other style tooling. The `venv-style` target creates `.venv-style/` in whatever environment `make` is executed from using `ci/style_requirements.txt`. + +## When to use + +- User asks to run `make style`, `make cpplint`, `make clang-format`, `make clang-format-check`, `make cppclean`, `make spell`, or `make sdl-check`. +- User asks to "check code style", "fix style", "verify before commit / push", "run lint", "format C++ files". +- A previous style run failed and the user wants to re-run it after fixes. + +## Do NOT use + +- For Bazel build/test invocations — use `build-bazel-target` / `run-single-gtest`. +- For SDL / security-only checks — call `make sdl-check` directly (covered briefly below). +- For modifying source files to *suppress* legitimate cpplint/cppclean findings without addressing them. + +## What `make style` actually runs + +`make style` (Makefile line 283) is an umbrella that depends on: + +1. **`venv-style`** — provisions `.venv-style/` and installs `ci/style_requirements.txt` (one-off per environment refresh). +2. **`spell`** — runs `codespell` over tracked + staged files, filtered through `spelling-whitelist.txt`. +3. **`clang-format-check`** — runs `clang-format -i` (target `clang-format`) on every `*.cpp/*.hpp/*.cc/*.cxx` under `src/`, then verifies the working tree and the index are clean. **It rewrites files in place** before checking — be aware. +4. **`cpplint`** — runs `cpplint` with the project options (`STYLE_CHECK_OPTS`, line length 120, the OVMS filter list) over `STYLE_CHECK_DIRS = src`. +5. **`cppclean`** — invokes `ci/cppclean.sh` to flag unused / redundant includes. + +`make style` exits non-zero on the first failing step. + +## Pre-flight + +1. Confirm the workspace is the OVMS repo root (contains `Makefile`, `ci/style_requirements.txt`, `spelling-whitelist.txt`). +2. Confirm `python3`, `git`, and `clang-format` are available on the host. +3. **Commit or stash any in-progress edits first.** `clang-format-check` will fail the run if `clang-format` rewrites tracked files, even if the rewrites were the right thing to do — so if you intend to lint your own changes, commit them first, then run `make style`, then commit any formatting changes on top. + +## Default invocation + +```bash +make style +``` + +The first run on a clean checkout also creates `.venv-style/` and installs the requirements; subsequent runs reuse it. + +## Targeted sub-targets + +Run these when iterating on a specific class of failure — they're much faster than the full chain. + +| Target | Purpose | +|--------|---------| +| `make spell` | Spell-check tracked + staged files via `codespell`, honoring `spelling-whitelist.txt`. | +| `make clang-format` | **Rewrite** all `src/**/*.{cpp,hpp,cc,cxx}` with the project clang-format style. | +| `make clang-format-check` | Same as above, then `git diff --exit-code` (working tree) and `git diff --exit-code --staged` (index) — **fails if it changed anything**. | +| `make cpplint` | Run cpplint only. | +| `make cppclean` | Run `ci/cppclean.sh` only (unused includes, etc.). | +| `make sdl-check` | hadolint (Dockerfiles) + bandit (Python) + license-headers (Apache 2.0). Slower; use only when the user asks for SDL. | + +## Recipes + +### 1. Full style pass before pushing + +```bash +make style +``` + +### 2. Auto-format everything, then re-verify + +```bash +make clang-format +git status # review the rewrites +git add -p # stage what you want +make clang-format-check # should now exit clean +``` + +### 3. Iterate on cpplint / cppclean only + +```bash +make cpplint +make cppclean +``` + +### 4. Spell-check only + +```bash +make spell +``` + +If a legitimate term is flagged, append it to `spelling-whitelist.txt` (lowercase) **as a separate commit** rather than trying to silence the failure inline. + +## Verifying the output + +For each step, look for these in the make output: + +| Step | Success signal | Failure signal & fix | +|------|----------------|----------------------| +| `spell` | `Spelling check completed.` | A list of `file:line: word ==> suggestion` lines. Either fix the typo or add the word to `spelling-whitelist.txt`. | +| `clang-format-check` | No diff (`make style` proceeds to `cpplint`). | `clang-format changes not committed. Commit those changes first` — run `git diff` to see what was rewritten, commit the formatting, then re-run. | +| `cpplint` | `make` proceeds to `cppclean` with no `Total errors found:` line. | Lines like `src/foo.cpp:123: [] [confidence]`. Address the warnings; do **not** add `// NOLINT` unless the project already does so for the same case. | +| `cppclean` | `make` finishes the umbrella step. | List of unused / redundant includes. Remove them per the OVMS include-what-you-use rule (forward-declare in headers, include in `.cpp`). | +| `make style` overall | Process exits with code `0` and no error lines preceding it. | Non-zero exit; the failing step is the last one whose output appears before the make `[Error N]` line. | + +When summarizing for the user, capture: + +- Which step failed (if any). +- The first 5–10 offending lines (the rest is usually repetitive). +- The concrete fix from the table above. + +## Tips & pitfalls + +- **`clang-format` rewrites files in place.** If you already had uncommitted edits, your edits will be intermixed with formatting changes. Commit (or stash) first, then run `make clang-format`, then commit the formatting separately. +- **`clang-format-check` depends on `clang-format`.** It is *not* a dry-run; it actually rewrites files and then asserts the working tree / index are clean. The failure message "Commit those changes first" really means "the formatter changed something — review and commit the formatting". +- **`STYLE_CHECK_DIRS = src`.** Only `src/` is checked. Code in `client/`, `demos/`, etc. is excluded from `make style`. +- **`spell` operates on tracked + staged files only.** A typo in an unstaged new file is invisible until it is at least `git add`-ed. +- **Filter list in `STYLE_CHECK_OPTS`.** Several cpplint categories are intentionally suppressed (`-build/c++11`, `-runtime/references`, `-whitespace/braces`, `-whitespace/indent`, `-build/include_order`, `-runtime/indentation_namespace`, `-build/namespaces`, `-whitespace/line_length`, `-runtime/string`, `-readability/casting`, `-runtime/explicit`, `-readability/todo`). Do not "fix" warnings that are explicitly disabled; if cpplint flags one of them, the project filter is the source of truth. +- **Line length is 120**, not 80 (set via `--linelength=120`). +- **`make style` does not run on the `-build` container.** It is host-side and relies on the host's `python3` + venv. The `-build` image is for Bazel work only. +- **License headers** are checked by `make sdl-check`, not by `make style`. New files still need the Apache 2.0 header — add it before pushing. diff --git a/.github/skills/run-single-gtest/SKILL.md b/.github/skills/run-single-gtest/SKILL.md new file mode 100644 index 0000000000..f1b5928bd2 --- /dev/null +++ b/.github/skills/run-single-gtest/SKILL.md @@ -0,0 +1,159 @@ +--- +name: run-single-gtest +description: "Use when running, re-running, filtering, or debugging a single OVMS C++ gtest fixture or test case from src/test/ inside the -build Docker container. Covers locating/starting the build container, invoking bazel test with --test_filter for one suite or one test, forwarding http_proxy/https_proxy/no_proxy, switching distro to redhat via --//:distro=redhat, and reading bazel-testlogs/src/ovms_test/test.log when output is truncated. Trigger phrases: 'run this test', 'run a single gtest', 'rerun failing test', 'bazel test --test_filter', 'ovms_test', 'TEST_F', 'gtest in container'." +--- + +# Run a Single GTest in the Build Container + + + +Use this skill any time the user asks to run **one** gtest fixture or test case (not the full `//src:ovms_test` suite). The full suite is time-expensive — always prefer a `--test_filter` first and only widen the filter after the targeted run is green. + +## Apply user defaults (every run) + +Before prompting the user for anything, parse the **USER DEFAULTS** comment block at the top of this file: + +1. If `DEFAULT_CONTAINER` is set, use it verbatim and skip the "choose container" questions. +2. If `DEFAULT_TEST_FILTER` is set and the user did not supply a different filter in the request, use it as the `--test_filter=` value. +3. If `DEFAULT_DISTRO` is `redhat`, add `--//:distro=redhat` to every Bazel command. +4. Otherwise (placeholder still ``), follow the normal interactive flow described in the sections below. + +Always tell the user which defaults were applied (e.g. "Using `DEFAULT_CONTAINER=ovms-build-fix_pull_all_dl_failed` and `DEFAULT_TEST_FILTER=HfPull.ResumeShutdown` from the skill file."). + +## When to use + +- User asks to run, re-run, or debug a specific test in `src/test/**/*.cpp` (e.g. `HfPull.ResumeShutdown`, `LibGit2LfsWipMarker.*`, `HfDownloaderClassTest.Methods`). +- User mentions `bazel test`, `ovms_test`, `--test_filter`, `TEST_F`, `TEST(`, or a fixture/test name. +- A previous test run failed and the user wants the same filter re-executed. + +## Do NOT use + +- For running the **whole** `//src:ovms_test` suite (skip the filter, run only after targeted tests pass). +- For Python functional tests (`make test_functional`). +- For build-only requests (`bazel build //src:ovms`) — no test invocation needed. + +## Pre-flight + +1. Confirm the workspace is the OVMS repo root (contains `WORKSPACE`, `src/`, `Makefile`). +2. **Resolve the image tag (`OVMS_CPP_IMAGE_TAG`) before creating a new container.** Do **not** silently fall back to `latest`. + ```bash + echo "${OVMS_CPP_IMAGE_TAG:-}" + ``` + - If `OVMS_CPP_IMAGE_TAG` is **already exported** in the user's shell, reuse that value verbatim and confirm with the user. + - Otherwise ask: "`OVMS_CPP_IMAGE_TAG` is not set. What tag should I use? (e.g. `latest-`, `latest-`)" + - Use the resolved tag when constructing the `docker run` image argument: `openvino/model_server-build:${OVMS_CPP_IMAGE_TAG}`. +3. Identify the target test: + - Suite + test: `--test_filter="SuiteName.TestName"` + - All tests in a suite: `--test_filter="SuiteName.*"` + - Multiple, colon-separated: `--test_filter="A.x:B.y:C.*"` +4. If the test needs models, ensure `make prepare_models` has been run at least once. LLM model regeneration: + ```bash + rm -rf src/test/llm_testing && make prepare_models + ``` + +## Choose the `-build` container + +Tests must run **inside** the `-build` Docker container with the repo mounted at `/ovms`. **Do not auto-discover or pick a running container.** If `DEFAULT_CONTAINER` is set in the defaults block above, use it. Otherwise, ask the user one of these two questions and wait for an answer before invoking `docker exec` / `docker run`: + +> **Question A — reuse:** "Which existing `-build` container should I use? Please give me its name or ID (e.g. from `docker ps --format '{{.Names}}\t{{.Image}}'`)." +> +> **Question B — create:** "No container provided — should I create a new one named `ovms-build-` from `` with `-v $(pwd):/ovms`? (suggest a name that makes the purpose obvious in `docker ps`)." + +Only proceed once `DEFAULT_CONTAINER` is available, or the user supplies a container name/ID, or confirms a new container with a specific name. + +### Step A — reuse a user-specified container + +Use the **name or ID the user gave you** verbatim. Do not substitute another container even if you see one in `docker ps`. + +```bash +docker exec -e http_proxy="$http_proxy" \ + -e https_proxy="$https_proxy" \ + -e no_proxy="$no_proxy" \ + \ + bash -lc 'cd /ovms && bazel test \ + --test_summary=detailed --test_output=streamed \ + --test_filter="SuiteName.TestName" \ + --test_env=http_proxy="${http_proxy}" \ + --test_env=https_proxy="${https_proxy}" \ + --test_env=no_proxy="${no_proxy}" \ + //src:ovms_test' +``` + +### Step B — create a new, clearly named container (only after user confirms) + +Always: +- Mount the **current** working directory: `-v "$(pwd)":/ovms` (run from the OVMS repo root). +- Give the container a **meaningful name** with `--name` so the user can identify it in `docker ps` (e.g. `ovms-build-`, `ovms-test-`, `ovms-redhat-build`). Confirm the name with the user before running. +- Prefer `-d` (detached) + `docker exec` for repeatable test runs, instead of `--rm` (which discards the container after one command and forces a new one for every re-run). + +```bash +# Confirm with user: name="ovms-test-", image="openvino/model_server-build:${OVMS_CPP_IMAGE_TAG}" +docker run -d -it \ + --name \ + -v "$(pwd)":/ovms \ + -w /ovms \ + -e http_proxy="$http_proxy" -e https_proxy="$https_proxy" -e no_proxy="$no_proxy" \ + openvino/model_server-build:${OVMS_CPP_IMAGE_TAG} bash + +# Then run the filtered test against that named container: +docker exec -e http_proxy="$http_proxy" \ + -e https_proxy="$https_proxy" \ + -e no_proxy="$no_proxy" \ + \ + bash -lc 'cd /ovms && bazel test \ + --test_summary=detailed --test_output=streamed \ + --test_filter="SuiteName.TestName" \ + --test_env=http_proxy="${http_proxy}" \ + --test_env=https_proxy="${https_proxy}" \ + --test_env=no_proxy="${no_proxy}" \ + //src:ovms_test' +``` + +User can verify which container is doing what at any time with: + +```bash +docker ps --format 'table {{.Names}}\t{{.Image}}\t{{.Status}}\t{{.Command}}' +``` + +## Red Hat (UBI9) builds + +Add `--//:distro=redhat` to **every** Bazel command (build and test): + +```bash +bazel test --//:distro=redhat \ + --test_summary=detailed --test_output=streamed \ + --test_filter="SuiteName.TestName" \ + //src:ovms_test +``` + +The `--//:distro=redhat` flag is **always required** in every Bazel command, regardless of whether you are running from the host or inside the container. While the `-build` container auto-detects the OS from `/etc/redhat-release` for environment setup, Bazel still requires the explicit flag to configure its build behavior. + +## After the run + +- **Pass**: report the filter, elapsed time, and `[ PASSED ]` count from the streamed output. +- **Fail or truncated output**: read the full log directly using the **same container name/ID the user provided**: + ```bash + docker exec -w /ovms \ + sh -c 'ls -la bazel-testlogs/src/ovms_test/test.log; \ + tail -n 200 bazel-testlogs/src/ovms_test/test.log' + ``` + Verify the log mtime matches the run that just finished — earlier runs leave stale logs at the same path. + +## Tips & pitfalls + +- **Do not run `bazel` on the host.** Always go through the `-build` container so the toolchain and OpenVINO paths match. +- **Network-dependent tests** (HuggingFace clone, etc.) need `http_proxy`/`https_proxy`/`no_proxy` forwarded **both** to `docker exec` (`-e ...`) **and** to Bazel test workers (`--test_env=...`). Forgetting the latter causes `Could not resolve host` failures. +- **Running fixtures in parallel forks** (`EXPECT_EXIT`, child processes spawned by tests like `HfPull.ResumeTerminate`) require `--test_output=streamed` to see the live output; `--test_output=errors` will hide partial progress. +- **Avoid `--cache_test_results=no`** unless the user specifically wants to bypass Bazel's test cache; Bazel will re-run a cached green test only when its inputs change. +- **One in-progress filter at a time**: if the user requests another run, kill the previous async terminal first to avoid two concurrent Bazel servers contending for the same `bazel-out`. From 3cbcf9160a46a1dee27f4009474d524fc5196493 Mon Sep 17 00:00:00 2001 From: Rafal Sapala Date: Mon, 4 May 2026 18:06:30 +0200 Subject: [PATCH 21/22] Fix tests --- .gitignore | 2 +- src/test/unit_tests.cpp | 27 +++++++++++++++++++++++++-- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index cd26093dfa..a76ff61a41 100644 --- a/.gitignore +++ b/.gitignore @@ -38,4 +38,4 @@ tmp/ *.zip *.tar.gz models -genhtml +genhtml \ No newline at end of file diff --git a/src/test/unit_tests.cpp b/src/test/unit_tests.cpp index 0b26f90f5b..aa356c6851 100644 --- a/src/test/unit_tests.cpp +++ b/src/test/unit_tests.cpp @@ -14,7 +14,26 @@ // limitations under the License. //***************************************************************************** -#include +#include + +namespace { + +// GTest death tests run assertions in a dedicated child process launched with +// an internal argument. Detect that child to skip PythonEnvironment setup, +// which avoids teardown-time crashes in exit-based death-test flows. +bool isDeathTestSubprocess(int argc, char** argv) { + for (int i = 0; i < argc; ++i) { + if (argv[i] == nullptr) { + continue; + } + if (std::string(argv[i]).find("gtest_internal_run_death_test") != std::string::npos) { + return true; + } + } + return false; +} + +} // namespace #include "environment.hpp" #include "gpuenvironment.hpp" @@ -22,12 +41,16 @@ #include "python_environment.hpp" int main(int argc, char** argv) { + // Check before InitGoogleTest because it can consume/rewrite internal args. + const bool deathTestSubprocess = isDeathTestSubprocess(argc, argv); ::testing::InitGoogleTest(&argc, argv); ::testing::InitGoogleMock(&argc, argv); ::testing::AddGlobalTestEnvironment(new Environment); ::testing::AddGlobalTestEnvironment(new GPUEnvironment); ::testing::AddGlobalTestEnvironment(new GGUFEnvironment); - ::testing::AddGlobalTestEnvironment(new PythonEnvironment); + if (!deathTestSubprocess) { + ::testing::AddGlobalTestEnvironment(new PythonEnvironment); + } ::testing::FLAGS_gtest_death_test_style = "threadsafe"; return RUN_ALL_TESTS(); } From 9d9ffb84a4bb2ac83bf679f006738d60da53d7a7 Mon Sep 17 00:00:00 2001 From: Rafal Sapala Date: Tue, 5 May 2026 10:11:49 +0200 Subject: [PATCH 22/22] Fix windows tests --- src/test/unit_tests.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/test/unit_tests.cpp b/src/test/unit_tests.cpp index aa356c6851..aebb349900 100644 --- a/src/test/unit_tests.cpp +++ b/src/test/unit_tests.cpp @@ -15,6 +15,7 @@ //***************************************************************************** #include +#include namespace { @@ -22,6 +23,11 @@ namespace { // an internal argument. Detect that child to skip PythonEnvironment setup, // which avoids teardown-time crashes in exit-based death-test flows. bool isDeathTestSubprocess(int argc, char** argv) { + const char* deathTestEnv = std::getenv("GTEST_INTERNAL_RUN_DEATH_TEST"); + if (deathTestEnv != nullptr && deathTestEnv[0] != '\0') { + return true; + } + for (int i = 0; i < argc; ++i) { if (argv[i] == nullptr) { continue; @@ -45,10 +51,12 @@ int main(int argc, char** argv) { const bool deathTestSubprocess = isDeathTestSubprocess(argc, argv); ::testing::InitGoogleTest(&argc, argv); ::testing::InitGoogleMock(&argc, argv); + + // Keep death-test subprocesses minimal to avoid teardown-time side effects. ::testing::AddGlobalTestEnvironment(new Environment); - ::testing::AddGlobalTestEnvironment(new GPUEnvironment); - ::testing::AddGlobalTestEnvironment(new GGUFEnvironment); if (!deathTestSubprocess) { + ::testing::AddGlobalTestEnvironment(new GPUEnvironment); + ::testing::AddGlobalTestEnvironment(new GGUFEnvironment); ::testing::AddGlobalTestEnvironment(new PythonEnvironment); } ::testing::FLAGS_gtest_death_test_style = "threadsafe";