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`.