From 4fc1f330c7d3c93001b84bc242331571c65320e4 Mon Sep 17 00:00:00 2001 From: Etienne Lescot Date: Mon, 27 Jul 2026 21:53:20 +0200 Subject: [PATCH] ci(rust): run the Rust workspace, which no workflow has ever built No GitHub workflow invokes cargo. crates/compositor is production code the app links, crates/compositor-view-napi is the addon it ships, and neither has ever been compiled by CI - the 76 unit tests in the workspace only run when someone remembers to run them locally, and nothing catches a crate that stops building. A runner cannot compile the workspace at all as things stand. FFMPEG_DIR in crates/.cargo/config.toml points at a tree that is gitignored: bindgen needs its include/ and the linker needs its lib/, and neither has ever been provisioned by anything. The steps were folklore - hand-place the archive, or symlink poc-d3d/thirdparty and hope. So fetch-ffmpeg.mjs now keeps that tree. It already downloads the shared build, verifies its pinned SHA-256 and asserts the licence is LGPL; it extracted include/ and lib/ and then deleted them with the temp dir, keeping only the DLLs. Landing them at FFMPEG_DIR reuses the pin, the digest and the licence check rather than adding a second download path to keep in sync, and it fixes the clean-checkout build for people too, not only for CI. The destination is read out of the cargo config the same way compositorViewService.ts reads it for the runtime PATH, so a rename cannot silently desynchronise them. It copies over the tree rather than replacing it: crates/thirdparty is commonly a junction into poc-d3d/thirdparty, and a recursive delete would follow it and take the real tree with it. The job is windows-latest by necessity - the compositor is D3D11 and links the windows crate, so there is nothing to run on ubuntu. It runs from crates/ rather than passing --manifest-path, because cargo resolves .cargo/config.toml against the working directory and would otherwise get neither FFMPEG_DIR nor LIBCLANG_PATH. --workspace because default-members = ["poc-d3d"] means a bare cargo test skips the library and the addon. --all-targets because it excludes doctests, and it must: bindgen copies ffmpeg's C doxygen comments into out/ffi.rs verbatim, so rustdoc finds 13 "examples" that are C, and fails them. They are generated and there is nothing in them to fix. Not included, deliberately. cargo fmt --check fails today on files nobody touched in this change (audio.rs among them), and clippy on FFI-heavy code with a 13-warning generated binding is a backlog, not a gate. Both are ratchets to add once someone has cleared them; the gate that pays for itself now is "it builds and the tests pass". Verified locally with the junction removed, so the runner's cold state: fetch-ffmpeg.mjs materialised the tree at crates/thirdparty, cargo clean -p openscreen-compositor forced bindgen to re-run against it, and cargo test --workspace --all-targets passed 76/76. Re-running the script is a no-op. --- .github/workflows/ci.yml | 37 +++++++++++++++++++++++++++++++++ scripts/fetch-ffmpeg.mjs | 45 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index adbb039db..77239fad3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -99,6 +99,43 @@ jobs: - uses: ./.github/actions/setup - run: npx vite build + rust: + name: Rust + # windows-only by necessity, not preference: crates/compositor is D3D11 and links the + # `windows` crate. There is nothing to run on ubuntu. + runs-on: windows-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version: 22 + # Provisions include/ + lib/ at the FFMPEG_DIR that crates/.cargo/config.toml names. + # bindgen and the linker both need them and the tree is gitignored, so without this a + # runner cannot compile the workspace at all. Same pinned, SHA-256-verified archive the + # packaged app ships — no second download path to keep in sync. + - run: node scripts/fetch-ffmpeg.mjs + # The test binary dlopens the same av*.dll set at run time; without them it dies with + # STATUS_DLL_NOT_FOUND (0xc0000135) before a single test runs, which reads like a build + # failure and isn't. This dir is the fixed one the script vendors to, so no second copy + # of the FFMPEG_DIR lookup lives here. + - run: Add-Content $env:GITHUB_PATH "$env:GITHUB_WORKSPACE\electron\native\bin\win32-x64" + - uses: Swatinem/rust-cache@v2 + with: + workspaces: crates + # From crates/, not `--manifest-path` from the root: cargo reads .cargo/config.toml + # relative to the working directory, and that file is what supplies FFMPEG_DIR and + # LIBCLANG_PATH. Invoked from the root the build gets neither. + # + # --workspace, because `default-members = ["poc-d3d"]` means a bare `cargo test` would + # quietly skip the library the app actually links and the napi addon it ships. + # + # --all-targets is what excludes doctests, and it has to: bindgen copies ffmpeg's C + # doxygen comments verbatim into out/ffi.rs, so rustdoc finds 13 "examples" that are C + # and fails to compile them as Rust. They are generated, not ours, and there is nothing + # to fix in them. Everything else — lib, bins, integration tests — still runs. + - run: cargo test --workspace --all-targets + working-directory: crates + semantic-pr: name: Validate PR title (semantic) runs-on: ubuntu-latest diff --git a/scripts/fetch-ffmpeg.mjs b/scripts/fetch-ffmpeg.mjs index be75a858d..10149445d 100644 --- a/scripts/fetch-ffmpeg.mjs +++ b/scripts/fetch-ffmpeg.mjs @@ -254,6 +254,23 @@ function findDlls(dir) { return out; } +/** + * Where the Rust workspace expects the shared build's headers and import libs, read out of + * `crates/.cargo/config.toml` rather than repeated here — the two drifting silently is exactly + * how you get a `cargo build` that links yesterday's ffmpeg. Same trick as `pinnedFfmpegDir()` + * in electron/native-bridge/services/compositorViewService.ts, which parses the same key for + * the runtime PATH. Returns null if the key is gone, so a rename fails loudly upstream. + */ +function cargoFfmpegDir() { + const config = path.join(ROOT, "crates", ".cargo", "config.toml"); + if (!fs.existsSync(config)) return null; + const match = /^FFMPEG_DIR\s*=\s*\{[^}]*value\s*=\s*"([^"]+)"/m.exec( + fs.readFileSync(config, "utf8"), + ); + // `relative = true` in that file means relative to the config's own dir, i.e. crates/. + return match ? path.join(ROOT, "crates", match[1]) : null; +} + /** Downloads `spec.asset`, verifies its pinned SHA-256, and extracts it into a fresh temp dir. */ async function downloadAndExtract(spec) { const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openscreen-ffmpeg-")); @@ -284,6 +301,12 @@ async function downloadAndExtract(spec) { * against, into the same `electron/native/bin//` dir as the static exe * — so both ship as one `extraResources` unit and * `compositorViewService.ts`'s PATH-prepend finds them. Windows only. + * + * Also lands the whole extracted tree at the Rust workspace's `FFMPEG_DIR`. The DLLs alone are + * enough to *run* the addon, never to *build* it: bindgen needs `include/` and the linker needs + * `lib/`, both of which this archive already carries and this script used to delete with the + * temp dir. Without it a clean checkout cannot compile the crates at all — CI could not, and a + * new contributor had to hand-place the tree from a wiki step nobody had written down. */ async function fetchSharedDlls(tag, binDir) { const spec = SHARED_PINNED[tag]; @@ -302,7 +325,12 @@ async function fetchSharedDlls(tag, binDir) { e.name.toLowerCase().endsWith(".dll") && e.name.toLowerCase().startsWith("av"), ); - if (alreadyVendored && !process.argv.includes("--force")) { + // The build tree is a second destination with its own lifetime: it is gitignored, so a fresh + // clone or a CI runner has the DLLs cached and the headers missing. Both must be present + // before this is a no-op, or that machine silently gets no build tree. + const buildDir = cargoFfmpegDir(); + const buildTreeReady = buildDir !== null && fs.existsSync(path.join(buildDir, "include")); + if (alreadyVendored && buildTreeReady && !process.argv.includes("--force")) { console.log(`\nShared ffmpeg DLLs already present in ${binDir}. Use --force to re-vendor.`); return; } @@ -329,6 +357,21 @@ async function fetchSharedDlls(tag, binDir) { fs.copyFileSync(dll, path.join(binDir, path.basename(dll))); } console.log(`Vendored ${dlls.length} DLL(s) -> ${binDir}`); + + if (buildDir) { + // `exe` is /bin/ffmpeg.exe, so its grandparent is the tree root — derived rather + // than reconstructed from the asset name, which carries a build suffix the directory + // the Rust side wants does not. + const extracted = path.dirname(path.dirname(exe)); + // Overwrite in place, never `rm -rf` first: `crates/thirdparty` is commonly a junction + // into poc-d3d/thirdparty, and a recursive delete would follow it and take the real + // tree with it. The pin is immutable, so same-name files have the same contents anyway. + fs.mkdirSync(path.dirname(buildDir), { recursive: true }); + fs.cpSync(extracted, buildDir, { recursive: true }); + console.log(`Build tree (include/ + lib/) -> ${buildDir}`); + } else { + console.log("No FFMPEG_DIR in crates/.cargo/config.toml — skipped the Rust build tree."); + } console.log("LGPL verified: safe to ship with an MIT app."); } finally { fs.rmSync(tmp, { recursive: true, force: true });