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`. 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/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/create_package.sh b/create_package.sh index 78546e32d4..e216f7aba9 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 ; \ @@ -106,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..07bf369ffc 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 //src:python_runtime_library_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 41f7ac8116..ca4ed1002d 100644 --- a/src/BUILD +++ b/src/BUILD @@ -787,7 +787,9 @@ ovms_cc_library( ], deps = select({ "//:not_disable_python": [ - "//src/python:libovmspythonmodule", + "//src/python:pythonnoderesources", + "//src/python:pythonexecutorcalculator", + "//src/python:pytensorovtensorconvertercalculator", ], "//:disable_python": [] }) + select({ @@ -961,10 +963,13 @@ ovms_cc_library( "//conditions:default": ["-lOpenCL"], # TODO make as direct dependency "//src:windows" : ["/DEFAULTLIB:Rpcrt4.lib"],}), data = select({ - "//:not_disable_python": [ + "//:is_windows_and_python_is_enabled": [ + "//src/python/binding:pyovms.pyd", + ], + "//:disable_python": [], + "//conditions:default": [ "//src/python/binding:pyovms.so", ], - "//:disable_python": [] }) + select({ "//:is_windows_and_python_is_enabled": [ "//src/python/binding:copy_pyovms", @@ -2273,10 +2278,13 @@ cc_binary( "//:disable_mediapipe" : [], }), data = select({ - "//:not_disable_python": [ + "//:is_windows_and_python_is_enabled": [ + "//src/python/binding:pyovms.pyd", + ], + "//:disable_python": [], + "//conditions:default": [ "//src/python/binding:pyovms.so", ], - "//:disable_python": [] }), # linkstatic = False, # Use for dynamic linking when necessary ) @@ -2531,7 +2539,15 @@ cc_test( "//src:libcustom_node_image_transformation.so", "//src:libcustom_node_add_one.so", "//src:libcustom_node_horizontal_ocr.so", - ], + ] + select({ + "//:is_windows_and_python_is_enabled": [ + "//src/python:libovmspython.dll", + ], + "//:disable_python": [], + "//conditions:default": [ + "//src/python:libovmspython.so", + ], + }), deps = [ "optimum-cli", "//src:ovms_lib", @@ -2585,6 +2601,11 @@ cc_test( [ "serialization_common", ], + }) + select({ + "//:not_disable_python": [ + "//src/python:libovmspythonmodule", + ], + "//:disable_python": [], }), copts = COPTS_TESTS, local_defines = COMMON_LOCAL_DEFINES, @@ -2605,6 +2626,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",], @@ -2764,6 +2813,9 @@ cc_library( srcs = ["test/python_environment.cpp",], linkopts = [], deps = PYBIND_DEPS + [ + "//src/python:libovmspythonmodule", + "//src:cpp_headers", + "libovmsstatus", "@com_google_googletest//:gtest", ], local_defines = COMMON_LOCAL_DEFINES, diff --git a/src/kfs_frontend/kfs_graph_executor_impl.cpp b/src/kfs_frontend/kfs_graph_executor_impl.cpp index 034f6f0907..3952e4bc56 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_EXECUTION_ERROR, details); + } auto requestInputItr = request.inputs().begin(); auto status = getRequestInput(requestInputItr, requestedName, request); if (!status.ok()) { 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 8b5cb2e70d..c5d289d725 100644 --- a/src/python/BUILD +++ b/src/python/BUILD @@ -16,7 +16,97 @@ 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("@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", + "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", +] + +_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": _SHARED_LIB_COPTS_WINDOWS, +}) + 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 @@ -75,7 +165,7 @@ ovms_cc_library( "pythonexecutorcalculator_cc_proto", "utils", ], - visibility = ["//visibility:private"], + visibility = ["//src:__pkg__"], alwayslink = 1, data = ["//src/python/binding:pyovms.so"], ) @@ -91,7 +181,7 @@ ovms_cc_library( "pytensorovtensorconvertercalculator_cc_proto", "pythonbackend", ], - visibility = ["//visibility:private"], + visibility = ["//src:__pkg__"], alwayslink = 1, data = ["//src/python/binding:pyovms.so"], ) @@ -110,7 +200,7 @@ ovms_cc_library( "//src/mediapipe_internal:node_initializer", "//src:libovmslogging", ], - visibility = ["//visibility:private"], + visibility = ["//src:__pkg__"], alwayslink = 1, data = ["//src/python/binding:pyovms.so"], ) @@ -145,3 +235,93 @@ ovms_cc_library( alwayslink = 1, 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 +# 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", + "pythoninterpretermodule.cpp", + "pythoninterpretermodule.hpp", + "python_runtime_entry.cpp", + "utils.hpp", + ], + deps = PYBIND_DEPS + [ + "//src:libovmslogging", + "//src:libovmsstatus", + "//src:libovms_module", + "//src:cpp_headers", + ], + copts = COPTS_SO, + linkopts = LINKOPTS_SO, + local_defines = COMMON_LOCAL_DEFINES, + 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_unix", + shared_library = ":libovmspython.so", + hdrs = [ + "ovms_py_tensor.hpp", + "python_backend.hpp", + "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/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 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(); diff --git a/src/python/python_runtime_entry.cpp b/src/python/python_runtime_entry.cpp new file mode 100644 index 0000000000..ee6a749f23 --- /dev/null +++ b/src/python/python_runtime_entry.cpp @@ -0,0 +1,77 @@ +//***************************************************************************** +// 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" + +#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 +#define PYTHON_RUNTIME_EXPORT __attribute__((visibility("default"))) +#endif + +extern "C" PYTHON_RUNTIME_EXPORT ovms::Module* OVMS_createPythonInterpreterModule() { + 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/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 3c9e8ad291..a1104e7b12 100644 --- a/src/servablemanagermodule.cpp +++ b/src/servablemanagermodule.cpp @@ -23,9 +23,6 @@ #include "metrics/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 bdf572f76c..3f1ca69f3d 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 @@ -71,13 +73,165 @@ #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 ValidatePythonEnvironmentFn = bool (*)(const char** errorMessage); -using grpc::ServerBuilder; +PythonLibraryHandle pythonRuntimeHandle = nullptr; +CreatePythonInterpreterModuleFn createPythonInterpreterModuleFn = nullptr; +ValidatePythonEnvironmentFn validatePythonEnvironmentFn = nullptr; -namespace ovms { +bool ensurePythonRuntimeLoaded() { + if (createPythonInterpreterModuleFn != nullptr && validatePythonEnvironmentFn != 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; + } + 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", + ".\\libovmspython.dll", + "src\\python\\libovmspython.dll", + ".\\src\\python\\libovmspython.dll", + "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) { + 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; + } + 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; + } +#ifdef __linux__ + dlclose(pythonRuntimeHandle); +#elif _WIN32 + FreeLibrary(pythonRuntimeHandle); +#endif + pythonRuntimeHandle = nullptr; +} +} // namespace +#endif Server& Server::instance() { static Server global; @@ -315,8 +469,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(); @@ -387,8 +545,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 @@ -427,12 +593,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 @@ -492,6 +658,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/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/python_runtime_library_test.cpp b/src/test/python_runtime_library_test.cpp new file mode 100644 index 0000000000..38f07bb5be --- /dev/null +++ b/src/test/python_runtime_library_test.cpp @@ -0,0 +1,285 @@ +//***************************************************************************** +// 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 + +#ifdef __linux__ +#include +#endif + +#ifdef _WIN32 +#include +#endif + +using testing::HasSubstr; + +namespace { + +using ValidatePythonEnvironmentFn = bool (*)(const char** errorMessage); + +class ScopedSharedLibrary { +public: +#ifdef _WIN32 + using HandleType = HMODULE; +#else + using HandleType = void*; +#endif + +private: + HandleType handle; + +public: + explicit ScopedSharedLibrary(const std::filesystem::path& path) : + 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 + } + } + + 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; + 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 _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; + + 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); + } + + 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 (...) { + } + + 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; + } + } + + return {}; +} + +std::filesystem::path findPyovmsBinding() { + std::vector searchPaths; + + 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" / bindingFilename); + searchPaths.emplace_back(srcDir / workspace / "bazel-bin" / "src/python/binding" / bindingFilename); + } + 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" / 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 (...) { + } + + 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)) { + return path; + } + } + + return {}; +} + +} // namespace + +TEST(PythonRuntimeLibrary, MissingLibraryPathFailsToLoad) { + const auto missingLibraryPath = std::filesystem::temp_directory_path() / ("missing_" + getRuntimeLibraryFilename()); + ASSERT_FALSE(std::filesystem::exists(missingLibraryPath)); + + ScopedSharedLibrary library(missingLibraryPath); + + EXPECT_EQ(library.get(), nullptr); +} + +TEST(PythonRuntimeLibrary, ExistingLibraryExportsRequiredSymbols) { + 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) << getLibraryLoadError(); + + EXPECT_NE(findSymbol(library.get(), "OVMS_createPythonInterpreterModule"), nullptr); + EXPECT_NE(findSymbol(library.get(), "OVMS_validatePythonEnvironment"), nullptr); +} + +TEST(PythonRuntimeLibrary, ValidationFailsWithoutBindingOnPythonPath) { + 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) << 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(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")); +} + +TEST(PythonRuntimeLibrary, ValidationSucceedsWithBindingOnPythonPath) { + 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 " << getBindingFilename(); + + ScopedSharedLibrary library(libraryPath); + ASSERT_NE(library.get(), nullptr) << getLibraryLoadError(); + + ScopedEnvironmentVariable pythonPathEnv("PYTHONPATH", bindingPath.parent_path().string()); + + 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); +} diff --git a/src/test/pythonnode_test.cpp b/src/test/pythonnode_test.cpp index 5b31146d19..b87336b409 100644 --- a/src/test/pythonnode_test.cpp +++ b/src/test/pythonnode_test.cpp @@ -39,8 +39,8 @@ #include "src/metrics/metric_config.hpp" #include "src/metrics/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" @@ -58,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; @@ -112,7 +113,19 @@ 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) { + auto* pythonBackend = pythonModule->getPythonBackend(); + if (pythonBackend != nullptr) { + return pythonBackend; + } + } + + auto* pythonBackend = getGlobalPythonBackend(); + if (pythonBackend == nullptr) { + throw std::runtime_error("Python backend is not available"); + } + return pythonBackend; } // --------------------------------------- OVMS initializing Python nodes tests diff --git a/src/test/unit_tests.cpp b/src/test/unit_tests.cpp index 4a841c6171..aebb349900 100644 --- a/src/test/unit_tests.cpp +++ b/src/test/unit_tests.cpp @@ -14,18 +14,51 @@ // 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) { + 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; + } + 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" #include "gguf_environment.hpp" #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); + + // Keep death-test subprocesses minimal to avoid teardown-time side effects. ::testing::AddGlobalTestEnvironment(new Environment); - ::testing::AddGlobalTestEnvironment(new GPUEnvironment); - ::testing::AddGlobalTestEnvironment(new GGUFEnvironment); - ::testing::AddGlobalTestEnvironment(new PythonEnvironment); + if (!deathTestSubprocess) { + ::testing::AddGlobalTestEnvironment(new GPUEnvironment); + ::testing::AddGlobalTestEnvironment(new GGUFEnvironment); + ::testing::AddGlobalTestEnvironment(new PythonEnvironment); + } ::testing::FLAGS_gtest_death_test_style = "threadsafe"; return RUN_ALL_TESTS(); } 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" diff --git a/windows_test.bat b/windows_test.bat index dfd1a8c424..345954f393 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" @@ -108,6 +112,13 @@ echo [INFO] install_ovms_service.bat unit tests passed. :: 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" @@ -120,6 +131,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