From 16fa43da405bf6390f3dcb6c6ff7b814926109ea Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Fri, 24 Jul 2026 23:13:16 +0200 Subject: [PATCH] ci(encode): lint and test the feature-gated encode backends MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pf-encode's GPU backends are off by default, so almost none of them were under `-D warnings`: - ci.yml lints/tests with default features, which do cover VAAPI, libav-NVENC and — via punktfunk-host's `default = ["pyrowave"]` — the PyroWave backends, but not `nvenc` or `vulkan-encode`. - deb.yml builds those two, but with `cargo build`, where warnings are not errors. That left enc/linux/nvenc_cuda.rs, enc/linux/vulkan_video.rs and the vendored vk_av1_encode / vk_valve_rgb bindings — ~8,150 lines carrying ~70 `unsafe` blocks — never linted anywhere, so the crate's own `#![deny(clippy::undocumented_unsafe_blocks)]`, its stated unsafe-proof gate, was never actually enforced on them. Linux gains a clippy+test leg at the SHIPPED feature set: deb.yml builds `punktfunk-host/nvenc,punktfunk-host/vulkan-encode` WITHOUT `--no-default-features`, so the .deb carries pyrowave too and that combination is what deserves the lint. GPU-free — the hardware tests are `#[ignore]`d and NVENC/CUDA dlopen their entry points, so the test binary links with no driver. Windows gains a separate `-p pf-encode --all-targets` lint. The existing lint is `-p punktfunk-host`, which never builds pf-encode's test targets — the blind spot that let the Linux twin's tests rot. It must be clippy rather than `cargo test`: on MSVC nvidia-video-codec-sdk link-imports NvEncodeAPICreateInstance / NvEncodeAPIGetMaxSupportedVersion, so a test binary cannot link without the driver's import lib. clippy type-checks without linking; ci.yml runs the tests. `--all-targets` is load-bearing, not decoration: without it the feature-gated `#[cfg(test)]` modules are never compiled at all. Also widens windows-host.yml's paths filter, which listed `crates/pf-encode/**` but none of the crates it compiles against (pf-frame, pf-gpu, pf-zerocopy, pf-host-config, pf-capture, ...), so a change reaching the Windows host through one of those triggered no Windows build. Co-Authored-By: Claude Opus 5 (1M context) --- .gitea/workflows/ci.yml | 37 +++++++++++++++++++++++++++++++ .gitea/workflows/windows-host.yml | 32 ++++++++++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml index c24bc729..87ff9baa 100644 --- a/.gitea/workflows/ci.yml +++ b/.gitea/workflows/ci.yml @@ -61,6 +61,43 @@ jobs: - name: Test (unit + loopback + proptest + C ABI harness) run: cargo test --workspace --locked + # The GPU encode backends are OFF by default, so every step above compiles ~none of them: + # `nvenc` gates enc/linux/nvenc_cuda.rs (+ nvenc_core/nvenc_status) and `vulkan-encode` gates + # enc/linux/vulkan_video.rs (+ the vendored vk_av1_encode/vk_valve_rgb bindings) — ~8,150 + # lines carrying ~70 `unsafe` blocks. Their ONLY prior CI coverage was deb.yml's + # `cargo build`, where warnings are not errors, so pf-encode's own + # `#![deny(clippy::undocumented_unsafe_blocks)]` — the crate's stated unsafe-proof gate — + # was never actually enforced on them. (`pyrowave` needs no extra step: punktfunk-host has + # `default = ["pyrowave"]`, so the steps above already cover it.) + # + # `--all-targets` is load-bearing, not decoration: without it the feature-gated + # `#[cfg(test)]` modules are never compiled, which is exactly how all ten + # `NvencCudaEncoder::open` call sites in nvenc_cuda.rs's tests drifted to the wrong arity + # (E0061 x10) without any job noticing. + # + # GPU-free: every test needing real hardware is `#[ignore]`d, and NVENC/CUDA resolve their + # entry points at RUNTIME (dlopen), so the test binary links without a driver present. + # (On MSVC the same crate link-imports those symbols instead, which is why windows-host.yml + # can only type-check these tests via clippy — see the note there.) + # + # Scoped to `-p pf-encode` with ITS OWN feature names: punktfunk-host has no code gated on + # `nvenc`/`vulkan-encode` (its only `cfg(feature)` sites are the two `pyrowave` ones in + # capture.rs, and pyrowave is default-on, so the steps above already cover them). Going + # through `--features punktfunk-host/...` would force punktfunk-host into the selection and + # re-run its entire test suite a second time for no extra coverage. + # + # `pyrowave` is listed explicitly even though it is punktfunk-host's default: selecting only + # `-p pf-encode` takes the host out of the resolution, and pf-encode's own default is empty. + # Naming it keeps this the SHIPPED Linux feature set — deb.yml builds + # `--features punktfunk-host/nvenc,punktfunk-host/vulkan-encode` WITHOUT + # `--no-default-features`, so the .deb carries nvenc + vulkan-encode + pyrowave together, and + # that combination is what deserves the lint. + - name: Clippy + test the feature-gated Linux encode backends + run: | + cargo clippy -p pf-encode --all-targets --locked \ + --features nvenc,vulkan-encode,pyrowave -- -D warnings + cargo test -p pf-encode --locked --features nvenc,vulkan-encode,pyrowave + - name: C ABI harness (standalone link proof) run: bash crates/punktfunk-core/tests/c/run.sh diff --git a/.gitea/workflows/windows-host.yml b/.gitea/workflows/windows-host.yml index d9eb4d7a..6788ae4a 100644 --- a/.gitea/workflows/windows-host.yml +++ b/.gitea/workflows/windows-host.yml @@ -50,6 +50,23 @@ on: # builds — without these, encoder changes only reached this workflow via Cargo.lock luck. - 'crates/pf-encode/**' - 'crates/libvpl-sys/**' + # …and the rest of the W6 subsystem crates this build compiles. pf-encode was listed while + # the crates it speaks (pf-frame's CapturedFrame/PixelFormat/dxgi vocabulary, pf-gpu's + # adapter selection, pf-zerocopy, pf-host-config) were not, so a change that broke the + # Windows host through one of THEM reached main with no Windows build at all — the same + # Cargo.lock-luck gap the two lines above were added to close. + - 'crates/pf-frame/**' + - 'crates/pf-gpu/**' + - 'crates/pf-zerocopy/**' + - 'crates/pf-host-config/**' + - 'crates/pf-capture/**' + - 'crates/pf-win-display/**' + - 'crates/pf-vdisplay/**' + - 'crates/pf-inject/**' + - 'crates/pf-paths/**' + - 'crates/pf-driver-proto/**' + - 'crates/pf-clipboard/**' + - 'crates/pyrowave-sys/**' - 'packaging/windows/**' - 'scripts/windows/**' - 'web/**' @@ -154,8 +171,23 @@ jobs: # build minutes earlier). Linting in release reuses those native build-script artifacts (no # openh264 rebuild), and keeps everything in one C:\t\release tree. Same reason # pf-vkhdr-layer's clippy below runs --release. + # + # pf-encode is linted SEPARATELY with --all-targets so its Windows `#[cfg(test)]` modules + # are type-checked — the AMF C-ABI layout assertions (`variant_layout_matches_c` and + # friends, which are the only guard on a hand-mirrored vtable ABI), the QSV tests, and the + # PyroWave-Windows smoke test. The host lint above cannot cover them: `-p punktfunk-host` + # only builds pf-encode as a dependency, so its test targets are never compiled, and that + # blind spot is what let the Linux twin's tests rot to the wrong arity unnoticed. + # NOTE: clippy (a check, no link step) is deliberately the vehicle here — `cargo test` + # with `nvenc` cannot LINK on MSVC: nvidia-video-codec-sdk link-imports + # NvEncodeAPICreateInstance / NvEncodeAPIGetMaxSupportedVersion, which resolve only against + # the driver's import lib. (On Linux the same crate dlopens them, so ci.yml can and does + # run the tests there.) Running them here would need an `--features amf-qsv,qsv` build + # without `nvenc`, i.e. a third full dep tree on a runner that already trips C1069 — not + # worth it while ci.yml executes the same tests. run: | cargo clippy --release -p punktfunk-host --features nvenc,amf-qsv,qsv -- -D warnings; if ($LASTEXITCODE) { throw "host clippy" } + cargo clippy --release -p pf-encode --all-targets --features nvenc,amf-qsv,qsv -- -D warnings; if ($LASTEXITCODE) { throw "pf-encode clippy" } cargo clippy --release -p punktfunk-tray -- -D warnings; if ($LASTEXITCODE) { throw "tray clippy" } - name: Build + lint the HDR Vulkan layer (pf-vkhdr-layer)