From c5f1d0d76c6a328ba2c9e04a069b3b8a419efbc6 Mon Sep 17 00:00:00 2001 From: Mike Solar Date: Mon, 17 Aug 2026 00:51:36 +0800 Subject: [PATCH] refactor(engine): pure cdylib + undo-stack test race fix (M14 R4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - oakengine is now cdylib-only (no rlib/staticlib consumers anywhere; cargo tree verified) — the plugin/external C ABI layer; README and docs updated - cd.yml drops the dylib embedding/re-sign steps (the app no longer links it) - test race root-caused and fixed for good: the global undo stack lock is now a re-entrant mutex (parking_lot) shared by every test that drives the stack, including the previously unlocked node/render families; the render-manager serial-ordering bug (an earlier repro test initialized the global manager before the not-initialized test) is fixed with a shared SERIAL guard and a manager shutdown - 5 consecutive parallel runs clean; serial 209/209 --- .github/workflows/cd.yml | 61 +++----- Cargo.lock | 1 + Cargo.toml | 11 +- crates/oakengine/Cargo.toml | 6 +- crates/oakengine/README.md | 141 +++++++++--------- crates/oakengine/build.rs | 5 +- crates/oakengine/src/lib.rs | 60 ++++---- crates/oakengine/src/linkage.rs | 26 ++-- .../oakengine/src/test_support/common/mod.rs | 9 +- .../oakengine/src/test_support/it_export.rs | 5 +- .../oakengine/src/test_support/it_library.rs | 4 +- .../oakengine/src/test_support/it_storage.rs | 6 +- crates/oakengine/src/test_support/it_task.rs | 5 +- .../oakengine/src/test_support/it_timeline.rs | 5 +- crates/oakengine/src/test_support/it_undo.rs | 11 +- crates/oakengine/src/test_support/node.rs | 3 + crates/oakengine/src/test_support/render.rs | 33 +++- crates/oakengine/src/test_support/undo.rs | 3 +- docs/build.md | 7 +- docs/zh/plans/riir/M14-direct-rlib.md | 2 +- 20 files changed, 214 insertions(+), 190 deletions(-) diff --git a/.github/workflows/cd.yml b/.github/workflows/cd.yml index 94fe38f69..b78c24b73 100644 --- a/.github/workflows/cd.yml +++ b/.github/workflows/cd.yml @@ -77,13 +77,12 @@ jobs: rsvg-convert -w 512 -h 512 Oak_Icon.svg -o icons/icon.png file icons/icon.png - # oakengine is a workspace member but NOT a default member; the - # app/CLI/worker link scripts point the linker at the profile dir - # (target//), so build its cdylib there before the rest. + # Build the packaged binaries (default members: the app, oak-cli, + # oak-worker). oakengine is a workspace member but NOT a default + # member (M14 R4: pure cdylib for plugins/external consumers — no + # packaged binary links it), so it is not built here. - name: Build (release) - run: | - cargo build --release --locked -p oakengine - cargo build --release --locked + run: cargo build --release --locked - name: Package (deb, AppImage, pacman) run: cargo packager --release --formats deb,appimage,pacman @@ -112,11 +111,10 @@ jobs: if-no-files-found: error # # ------------------------------------------------------------------ -# macOS: Apple Silicon only. cargo-packager cannot fold liboakengine.dylib -# into the bundle (no post-packaging hook; the .app is rebuilt on every -# run), so we package the .app with cargo-packager, embed the dylib next -# to the binaries with install_name_tool, ad-hoc re-sign, then create the -# DMG with hdiutil (the same way the old C++ CD did). +# macOS: Apple Silicon only. The app/CLI/worker no longer link +# liboakengine (M14 R4: they call the module rlibs directly), so the +# .app bundle carries no dylib to fold in; package it with +# cargo-packager and create the DMG with hdiutil. # ------------------------------------------------------------------ macos: name: macOS DMG (Apple Silicon) @@ -175,42 +173,16 @@ macos: rsvg-convert -w 512 -h 512 Oak_Icon.svg -o icons/icon.png file icons/icon.png - # oakengine is a workspace member but NOT a default member; the - # app/CLI/worker link scripts point the linker at the profile dir - # (target//), so build its cdylib there before the rest. + # Build the packaged binaries (default members: the app, oak-cli, + # oak-worker). oakengine is a workspace member but NOT a default + # member (M14 R4: pure cdylib for plugins/external consumers — no + # packaged binary links it), so it is not built here. - name: Build (release) - run: | - cargo build --release --locked -p oakengine - cargo build --release --locked + run: cargo build --release --locked - name: Package .app bundle run: cargo packager --release --formats app - - name: Embed liboakengine.dylib and re-sign - run: | - set -euo pipefail - APP="target/release/Oak.app" - MACOS="$APP/Contents/MacOS" - # cargo may place the dylib in target/release/deps/ (dependency - # build) or target/release/ (workspace-member build); derive it - # from the main binary's own load reference so the bundled copy - # matches exactly what the binary was linked against. - REF="$(otool -L "$MACOS/oak-editor" | awk '/liboakengine\.dylib/ {print $1; exit}')" - test -n "$REF" && test -f "$REF" - cp "$REF" "$MACOS/liboakengine.dylib" - install_name_tool -id "@executable_path/liboakengine.dylib" "$MACOS/liboakengine.dylib" - # build.rs links the app against the dylib's absolute build path; - # point every binary that references it at the bundled copy. - for bin in oak-editor oak-cli oak-worker; do - if otool -L "$MACOS/$bin" 2>/dev/null | grep -q "liboakengine.dylib"; then - REF="$(otool -L "$MACOS/$bin" | awk '/liboakengine/ {print $1; exit}')" - install_name_tool -change "$REF" "@executable_path/liboakengine.dylib" "$MACOS/$bin" - fi - done - # install_name_tool invalidates the packager's ad-hoc signature. - codesign --force --deep --sign - "$APP" - codesign -dv "$APP" 2>&1 | head -3 - - name: Create DMG run: | rm -rf dmg-staging @@ -235,8 +207,9 @@ macos: # `-Wl` flags that the MSVC linker rejects. The NSIS packaging itself is # fine (cargo-packager downloads its own makensis, SHA-1 verified); the # blocker is the binary link. Re-enable this job once crates link on - # Windows; the job below is kept verbatim (with the -p oakengine prebuild) - # as the reference. + # Windows; the job below is kept verbatim as the reference (the + # `-p oakengine` prebuild is obsolete since M14 R4 — no packaged binary + # links the cdylib — and should be dropped when re-enabling). # ------------------------------------------------------------------ # windows: # name: Windows installer (NSIS) diff --git a/Cargo.lock b/Cargo.lock index d772e2205..5a2be1236 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4836,6 +4836,7 @@ dependencies = [ "oaktask", "oaktimeline", "oakundo", + "parking_lot", "sea-orm", "serde", "serde_json", diff --git a/Cargo.toml b/Cargo.toml index eeb3d6c82..4441867ac 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,10 +33,13 @@ exclude = ["gpui"] # dependency, so it builds with the app). Build/test it explicitly with # `cargo test -p oakstorage`. # NOTE: `crates/oakengine` is deliberately NOT a default member (it stays a -# workspace member, so `cargo test -p oakengine` works): its in-flight -# integration tests (`tests/it_*族.rs`, an ongoing rewrite) share temp files -# and process-global facade state, which makes the parallel default-members -# run flaky. The app builds it as a regular path dependency instead. +# workspace member, so `cargo test -p oakengine` / `cargo test --workspace` +# still run its tests): it is the plugin/external-consumer cdylib (M14 R4) — +# no crate in the workspace links it (app/cli/worker call the module rlibs +# directly), so default builds skip it. Its in-crate unit tests +# (src/test_support/, the former tests/*.rs) share temp files and +# process-global facade state, which makes the parallel default-members run +# flaky. default-members = [".", "crates/oak-cli", "crates/oak-worker"] resolver = "2" diff --git a/crates/oakengine/Cargo.toml b/crates/oakengine/Cargo.toml index 29baaa38f..3440011df 100644 --- a/crates/oakengine/Cargo.toml +++ b/crates/oakengine/Cargo.toml @@ -2,9 +2,12 @@ name = "oakengine" version = "0.1.0" edition = "2021" -description = "Oak Video Editor facade: re-exports the frozen oakengine_* C ABI over the module Rust APIs (Rust)" +description = "Oak Video Editor plugin/external-C-ABI layer: the frozen oakengine_* C ABI as a pure cdylib over the module crates' direct Rust APIs" license = "GPL-3.0-or-later" +# Pure cdylib (M14 R4): the only consumers are OFX plugins and external +# C-ABI embedders. No Rust crate depends on this crate, so no rlib/staticlib +# artifact is produced and it stays out of the workspace default-members. [lib] crate-type = ["cdylib"] @@ -52,6 +55,7 @@ oakplugin = { path = "../oakplugin" } oakstorage = { path = "../oakstorage" } [dev-dependencies] +parking_lot = "0.12" # The facade is cdylib-only, so the former tests/*.rs integration tests # now run as unit tests inside the crate (src/test_support/, pulled in # from src/lib.rs); the module crates are reachable through the normal diff --git a/crates/oakengine/README.md b/crates/oakengine/README.md index 64fc68ad8..42cd5eca2 100644 --- a/crates/oakengine/README.md +++ b/crates/oakengine/README.md @@ -1,64 +1,71 @@ -# oakfacade — the `liboakengine` facade (Rust) +# oakengine — the `liboakengine` cdylib (plugin / external C ABI) -Re-exports the frozen `oakengine_*` C ABI (`engine/include/oakengine/*.h`) -verbatim on top of the module C ABIs (`include//*.h`, implemented by -the oakundo/oaknode/oaktimeline/oakcodec/oakaudio/oakrender/oaktask/ -oakcommon/oakplugin crates). This is the M9 §4 assembly layer: every -module call crosses the module C ABI as an `extern "C"` import; the -facade itself owns only cross-cutting state. +The frozen `oakengine_*` C ABI (`engine/include/oakengine/*.h`) as a +**pure cdylib**. This is the plugin / external-consumer layer: OFX plugins +and third-party embedders link `liboakengine` and call the C ABI. The app, +oak-cli and oak-worker do not use it anymore (M14 R4) — they link the +module crates directly as Rust rlibs. The C ABI itself is frozen: only +additive changes, plus major version bumps. + +Downward, every export is a direct Rust call into the module crates +(oakundo/oaknode/oaktimeline/oakcodec/oakaudio/oakrender/oaktask/ +oakcommon/oakplugin/oakstorage/oakcore-rs) through `src/stubs.rs` (the +rewired replacement for the deleted `bridge/`). The facade itself owns +only the engine's box/unbox, buf/size and error-code conventions; +cross-cutting state that used to live here (the process-wide undo stack, +the open undo group) has sunk into the modules (M14 R1: +`oakundo::global`). ## Architecture ``` src/ - lib.rs crate docs, module list + lib.rs crate docs, module list, test-only test_link error.rs OAKENGINE error codes (module 00 → -1..-6) handle.rs CHandle mirror, OakEngine* opaque wrappers, box/unbox, catch_unwind guards, buf/size string helpers - bridge/ extern "C" imports per module crate (the only way the - facade talks to modules) - undo.rs engine/include/oakengine/undo.h + stubs.rs direct-Rust shims replacing the deleted bridge/ (the + engine's only downward path to the modules) + linkage.rs #[used] anchors pulling every module rlib into the cdylib + undo.rs engine/include/oakengine/undo.h (thin forward to + oakundo::global) common.rs config.h + videoparams.h (facade-static tables + POD↔handle) audio.rs audio.h (manager + sync + processor) - codec.rs encoding.h (metadata over oakcodec + facade params POD box) + codec.rs encoding.h + exporter.h (metadata, params POD, exporter) render.rs renderer.h + color.h + lut.h (renderer/frame/color processor) plugin.rs plugin.h node.rs node.h + project.h + footage.h (node graph / project / footage) timeline.rs timeline.h (sequences, clips, tracks, markers, workarea) task.rs task.h (background tasks over oaktask) + storage.rs write-through session (project ↔ oakstorage backend) deferred.rs documented deferrals (stub detail lives here and in the family modules' stub bodies) -tests/ - common/mod.rs test support: force-link + oakcore/ffmpeg_bridge stubs - undo.rs, common.rs, audio.rs, codec.rs, render.rs, plugin.rs, linkage.rs + test_support/ the former tests/*.rs, now in-crate unit tests ``` ### Dependencies The facade's regular dependency is `thiserror` (the `Display` + `std::error::Error` impl for the facade error enum, `src/error.rs`). Every -module -call crosses the module C ABI as an `extern "C"` import (`src/bridge/`), -and the module crates themselves are real dependencies: [`linkage`](src/linkage.rs) -anchors them so their `#[no_mangle]` exports are linked into the -`liboakengine` cdylib — the dylib carries the module C ABIs (oakundo_*, -oakcommon_*, oaktimeline_*, oakcodec_*, oakaudio_*, oakrender_*, -oaktask_*, oakplugin_*, oaknode_*) next to the facade's oakengine_* -exports. The `oakcore_audioparams_*` accessors the audio paths read -through are implemented in the dylib too (src/stubs.rs, module `audio`, -M12 P5) — they used to be C++ host symbols left as runtime lookups -(macOS `-undefined dynamic_lookup`); the cdylib now carries no undefined -imports. +module call is a compile-time Rust call into the module crate's direct API +(`src/stubs.rs`); the module crates are real dependencies, and +[`linkage`](src/linkage.rs) anchors them so their rlibs are embedded in +the `liboakengine` cdylib next to the facade's `oakengine_*` exports. The +`oakcore_audioparams_*` accessors the audio paths read through are +implemented in the dylib too (src/stubs.rs, module `audio`, M12 P5) — +they used to be C++ host symbols left as runtime lookups (macOS +`-undefined dynamic_lookup`); the cdylib now carries no undefined imports. ### Handle mapping The engine headers' opaque pointers (`OakEngineNode*`, `OakEngineTrack*`, -...) are thin newtype wrappers around the module C ABI's `CHandle` +...) are thin newtype wrappers around the module layer's `CHandle` (`{ctx, addref, release, abi_version}`) values. A box is created by `handle::box_handle` and freed by `handle::free_box` (release + dealloc); consuming exports (`oakengine_*_free`, `oakengine_undo_push`, ...) free -their box, borrowed results never are. The facade's own process-wide undo -stack and open undo group live in `undo.rs` (module 00 analogues of +their box, borrowed results never are. The process-wide undo stack and +open undo group live in the oakundo module (`oakundo::global`); the +undo.h exports are thin forwards over it (module 00 analogues of `EngineCore::undo_stack()` and the C++ capi's `g_undo_group`). ### Error codes @@ -73,56 +80,56 @@ convention: the return value is the length excluding the NUL | Family | Header | Wrapped | Notes | |---|---|---|---| -| undo | undo.h | 37 | stack/group/command lifecycle + Qt leftovers (update_actions/actions → no-op/NULL) | +| undo | undo.h | 37 | stack/group/command lifecycle over oakundo (`oakundo::global` + undocommand) + Qt leftovers (update_actions/actions → no-op/NULL) | | common | config.h, videoparams.h | 34 | config over oakcommon; videoparams static tables ported from `engine/render/videoparams.cpp` | | audio | audio.h | 26 | manager + sync; processor convert/output_params stubbed (interface mismatch) | | plugin | plugin.h | 4 | callbacks are facade state; push_button stubbed (no module API) | -| codec | encoding.h | 81/85 | metadata family over oakcodec (`include/codec/format.h`); params handle is a facade box over the `oakcodec_encoding_params` POD; presets/load-save/export stubs | +| codec | encoding.h, exporter.h | 81/85 | metadata family over oakcodec (`include/codec/format.h`); params handle is a facade box over the `oakcodec_encoding_params` POD; the exporter family drives the export task synchronously (integration-tested, real mp4 via FFmpeg); presets/load-save deferred | | render | renderer.h, color.h, lut.h | 60/60 | renderer over oakrender tickets; frame accessors over `OakCodecFrame`; color processor over `oakrender_color_processor_*`; color-manager list queries + LUT library stubs | -| node | node.h, project.h, footage.h | 226/327 | the node graph, project and footage families over the oaknode C ABI (`include/node/*.h`); 101 documented stubs where the module lacks the surface (gizmos, plugin messages, input properties, brush, thumbnail/waveform caches, shape/subtitle, keyframe enumeration, ...) — see the stub bodies | -| timeline | timeline.h | 126/139 | sequences/clips/tracks/markers/workarea over oaknode + oaktimeline; 13 documented stubs (ripple-tracks command, default transitions, move-track/clip, marker-create, auto-cache, cache invalidation, multicam find/switch — module-surface gaps, see the stub bodies) | +| node | node.h, project.h, footage.h | 226/327 | the node graph, project and footage families over the oaknode crate; documented stubs where the module lacks the surface (gizmos, plugin messages, input properties, brush, thumbnail/waveform caches, shape/subtitle, keyframe enumeration, ...) — see the stub bodies | +| timeline | timeline.h | 126/139 | sequences/clips/tracks/markers/workarea over oaknode + oaktimeline; documented stubs (ripple-tracks command, default transitions, move-track/clip, marker-create, auto-cache, cache invalidation, multicam find/switch — module-surface gaps, see the stub bodies) | | task | task.h | 27 | the background-task system over oaktask (manager + load/save/import/export creators + result accessors); `create_proxy` stubbed (the module has no proxy-task C creator); start-time/is-cancelled are facade-approximated | -Deferred/stub detail lives in [`deferred`] and in the stub bodies' doc -comments. The worker/IPC families (`worker.h`/`ipc.h`) are **not** part of -the facade anymore: the frozen C++ ABI does not include them, so the -render worker's runtime and the shared-memory frame-slot transport moved -into the `oak-worker` crate (`crates/oak-worker/src/{worker,ipc}.rs`, -self-contained, direct Rust calls into oakrender); the ipc.h control-plane -message serializers remain unwrapped. +Deferred/stub detail lives in [`deferred`](src/deferred.rs) and in the +stub bodies' doc comments. The worker/IPC families (`worker.h`/`ipc.h`) +are **not** part of the facade anymore: the frozen C++ ABI does not +include them, so the render worker's runtime and the shared-memory +frame-slot transport moved into the `oak-worker` crate +(`crates/oak-worker/src/{worker,ipc}.rs`, self-contained, direct Rust +calls into oakrender); the ipc.h control-plane message serializers remain +unwrapped. ## Testing -The module crates are real dependencies, so `cargo test` links the same -rlibs the cdylib embeds; the dev-dependencies re-declare -`oakcommon`/`oakplugin` with their `test-stubs` features so the test -binaries keep the in-crate mocks (ffmpeg_bridge stub / render mocks): +The crate is cdylib-only (no rlib artifact), so integration tests cannot +link it as a crate; the former `tests/*.rs` moved into `src/test_support/` +and run as in-crate unit tests (pulled in from `src/lib.rs` under +`#[cfg(test)]`), addressing the modules through `crate::*`. The module +crates are real dependencies, so the test binary statically links the same +rlibs the cdylib embeds: -- `oaknode`/`oaktimeline`/`oaktask` are linked WITHOUT their `test-stubs` - features: their in-crate mocks would collide with the real oakundo rlib - in one test binary. Without test-stubs their real exports reference the - oaknode/oakundo/oakcommon C ABI symbols as link-time externs, which the - sibling crate rlibs provide; oaknode itself resolves cross-module - symbols at runtime with `dlsym(RTLD_DEFAULT)`. -- `tests/common/mod.rs` re-exports the facade's in-dylib - `oakcore_audioparams_*` accessors (M12 P5 folded them in) and - force-links the oakcommon XML writer/reader + the oakundo command - factory so the oaknode serializer's dlsym lookups resolve in every test - binary. -- `src/lib.rs`'s test-only `test_link` forces the oakrender/oaknode/ - oaktimeline/oaktask rlibs into the lib unit-test binary (the always-on - `src/linkage.rs` anchors are `#[cfg(not(test))]`; they are what embeds - the module C ABIs in the cdylib for `cargo build`). +- `src/linkage.rs` (always-on) anchors the module rlibs into the cdylib + for `cargo build`; the test-only `test_link` module in `src/lib.rs` + forces the oakrender/oaknode/oaktimeline/oaktask rlibs into the lib + unit-test binary, and `src/test_support/common/mod.rs::force_link` + covers the test-support module (same symbol list, so the anchor paths + stay proven against the current module layouts). +- Tests that touch process-wide state (the audio manager, the undo stack, + the task manager) take a shared serialization lock instead of relying + on the process isolation the old integration tests had + (`test_support/common::with_manager`, the per-family `SERIAL` mutexes). -Families whose wrapped behavior requires the real module dylibs carry -`#[ignore]` tests with a documented reason; the smoke tests here exercise -the module crates' real implementations. +The smoke tests exercise the module crates' real implementations. Where a +wrapped family needs module behavior the crates do not implement yet, the +engine function is a documented stub with its reason (see `deferred.rs`). ``` -cargo test # lib tests + integration families (undo, common, audio, - # plugin, codec, render, linkage, node, timeline, task) -cargo build # cdylib embeds the module C ABIs + the folded-in - # oakcore_audioparams_* accessors (no undefined imports) +cargo test # in-crate unit tests (undo, common, audio, codec, + # exporter, plugin, render, node, timeline, task, + # library/storage families) +cargo build # liboakengine cdylib embeds the module rlibs + the + # folded-in oakcore_audioparams_* accessors (no + # undefined imports) ``` ## FFI discipline diff --git a/crates/oakengine/build.rs b/crates/oakengine/build.rs index 9294715dd..db5423d8a 100644 --- a/crates/oakengine/build.rs +++ b/crates/oakengine/build.rs @@ -16,8 +16,9 @@ //! Build-time link configuration for the `liboakengine` cdylib. //! -//! The dylib carries the module C ABIs itself (oakundo_*, oakcommon_*, ... -//! — see Cargo.toml). The `oakcore_audioparams_*` accessors the audio +//! The dylib carries the module crates themselves (their direct-Rust +//! code, kept in the link by `src/linkage.rs` — see Cargo.toml). The +//! `oakcore_audioparams_*` accessors the audio //! paths read through used to be host-provided C++ liboakcore symbols, //! left as runtime lookups via `-undefined,dynamic_lookup`; M12 P5 //! implemented them inside the dylib (src/stubs.rs, module `audio`), so diff --git a/crates/oakengine/src/lib.rs b/crates/oakengine/src/lib.rs index 0278d7109..e5e838f56 100644 --- a/crates/oakengine/src/lib.rs +++ b/crates/oakengine/src/lib.rs @@ -14,24 +14,31 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -//! # oakengine — the `liboakengine` facade (Rust) +//! # oakengine — the `liboakengine` cdylib (plugin / external C ABI) //! -//! Re-exports the frozen `oakengine_*` C ABI -//! (`engine/include/oakengine/*.h`) verbatim on top of the module C ABIs -//! (`include//*.h`, implemented by the oakundo/oaknode/oaktimeline/ -//! oakcodec/oakaudio/oakrender/oaktask/oakcommon/oakplugin crates). It is -//! the M9 §4 assembly layer: every module call crosses the module C ABI as -//! an `extern "C"` import (see [`bridge`]); the facade itself owns only -//! cross-cutting state (the process-wide undo stack and the open undo -//! group, see [`undo`]). +//! The frozen `oakengine_*` C ABI (`engine/include/oakengine/*.h`) as a +//! **pure cdylib** (M14 R4). This is the plugin / external-consumer layer: +//! OFX plugins and third-party embedders link `liboakengine` and call the +//! C ABI; the app, oak-cli and oak-worker do not use it anymore — they +//! link the module crates directly as Rust rlibs. +//! +//! Downward, every `oakengine_*` export is a direct Rust call into the +//! module crates (oakundo/oaknode/oaktimeline/oakcodec/oakaudio/oakrender/ +//! oaktask/oakcommon/oakplugin/oakstorage/oakcore-rs) through [`stubs`] +//! (the rewired replacement for the deleted `bridge/`); the C ABI itself +//! stays frozen (only additive changes + major version bumps). +//! Cross-cutting state that used to live here (the process-wide undo stack, +//! the open undo group) has sunk into the modules (M14 R1: +//! [`oakundo::global`]); [`undo`] is a thin forward that adds the engine's +//! box/unbox, buf/size and error-code conventions. //! //! ## Handle mapping //! //! The engine headers' opaque pointers (`OakEngineNode*`, `OakEngineTrack*`, //! ...) become thin newtype wrappers around module [`handle::CHandle`] //! values (see [`handle`]). Each exported function keeps the exact -//! signature from the engine header; inside, it unboxes the module handle, -//! calls the module C ABI and boxes the result. +//! signature from the engine header; inside, it unboxes the module value, +//! calls the module's direct Rust API and boxes the result. //! //! ## FFI discipline //! @@ -42,29 +49,20 @@ //! //! ## Testing //! -//! The module crates are real dependencies (see Cargo.toml) and -//! [`linkage`] anchors them into every link of this crate, so the module -//! C ABIs are embedded in the `liboakengine` cdylib next to the facade's -//! own exports. `cargo test` links the same crates' rlibs (plus the -//! `test-stubs` feature union declared in the dev-dependencies, which -//! compiles the oakcommon/oakplugin in-crate mocks); `tests/linkage.rs` -//! additionally references every crate for the integration-test binaries -//! and `test_link` (below) covers the unit-test binary. Where a wrapped -//! family needs module behavior the crates do not implement yet, the -//! engine function is a documented stub and its test carries `#[ignore]` -//! with a reason (see README.md). +//! The crate is cdylib-only, so the former `tests/*.rs` integration tests +//! run as in-crate unit tests under `src/test_support/` (pulled in from +//! here under `#[cfg(test)]`; they address the crate's modules through +//! `crate::*`). The module crates are real dependencies (see Cargo.toml), +//! so the test binary statically links the same rlibs the cdylib embeds; +//! [`linkage`] anchors every crate into the cdylib link, and the test-only +//! [`test_link`] module does the same for the unit-test binary. Where a +//! wrapped family needs module behavior the crates do not implement yet, +//! the engine function is a documented stub with its reason (see +//! `deferred.rs` and README.md). #![deny(unsafe_op_in_unsafe_fn)] #![warn(missing_docs)] -// Re-export the node crate so integration tests (and embedders) address -// the SAME compiled instance the facade uses: under `cargo test -// --workspace`, oaknode is built twice (oaknode's own dev-dependency -// enables oakcodec/test-stubs), and a test that mixes `oaknode::` direct -// imports with `oakengine::` re-exports gets two incompatible type -// instances (E0308 "multiple different versions of crate oaknode"). -pub use oaknode; - pub mod audio; pub mod codec; pub mod common; @@ -98,7 +96,7 @@ mod test_link { // The lib's own unit-test binary must link the module crates' rlibs to // satisfy the facade's imports that the unit tests compile in — e.g. // the render family's oakrender display renderer (src/render.rs). - // The integration tests do the same through tests/common/mod.rs + // The in-crate tests do the same through test_support/common/mod.rs // `force_link()`; this covers the `cargo test` unit-test binary. #![allow(dead_code)] fn force_link() -> usize { diff --git a/crates/oakengine/src/linkage.rs b/crates/oakengine/src/linkage.rs index b05e2fa1a..0784559b2 100644 --- a/crates/oakengine/src/linkage.rs +++ b/crates/oakengine/src/linkage.rs @@ -16,20 +16,22 @@ //! Linkage anchors — force the module crates' rlibs into every link. //! -//! The facade talks to the modules exclusively through `extern "C"` -//! imports (src/bridge/), so rustc would otherwise consider the module -//! crates unused and prune their rlibs from the link. This module -//! references one `#[no_mangle]` export of every module crate (and -//! `oakcore-rs`) from a `#[used]` static, which (a) marks each crate as -//! used so its rlib reaches the linker and (b) keeps the anchor alive so -//! the referenced object files are pulled. For the `liboakengine` cdylib -//! this is what actually embeds the module C ABIs (oakundo_*, -//! oakcommon_*, ...) into the dylib next to the facade's own oakengine_* -//! exports. +//! The facade calls the module crates' direct Rust APIs (single-lib; the +//! deleted `src/bridge/` no longer exists), and most modules reach the +//! linker through those normal references. The anchors below reference one +//! direct-Rust symbol of every module crate (and `oakcore-rs`) from a +//! `#[used]` static, which (a) marks each crate as used so its rlib +//! reaches the linker even when the facade only touches it indirectly and +//! (b) keeps the anchor alive so the referenced object files are pulled. +//! For the `liboakengine` cdylib this is what embeds the module crates +//! next to the facade's own `oakengine_*` exports — including the +//! oakcommon XML/undo symbols oaknode's serializer resolves at runtime +//! via dlsym(RTLD_DEFAULT) (see the per-anchor comments in `force_link`). //! //! The per-crate symbol mirrors the test-force-link in -//! tests/common/mod.rs (same paths, same `as usize` cast idiom), so the -//! crate/module paths are proven against the current module layouts. +//! test_support/common/mod.rs (same paths, same `as usize` cast idiom), +//! so the crate/module paths are proven against the current module +//! layouts. #![allow(dead_code)] diff --git a/crates/oakengine/src/test_support/common/mod.rs b/crates/oakengine/src/test_support/common/mod.rs index 272899081..a500cc862 100644 --- a/crates/oakengine/src/test_support/common/mod.rs +++ b/crates/oakengine/src/test_support/common/mod.rs @@ -20,10 +20,11 @@ //! Two jobs: //! //! 1. **Force rustc to link every module crate's rlib** into the test -//! binary ([`force_link`]). The facade itself only references the -//! modules through `extern "C"` imports (see src/bridge), so rustc -//! would otherwise drop the dev-dependency rlibs from the link and -//! leave the imports undefined. +//! binary ([`force_link`]). The test-support files import the module +//! crates directly (single-lib; the deleted `src/bridge` no longer +//! exists), and the array doubles as a compile-time proof that the +//! anchor paths in `crates/oakengine/src/linkage.rs` match the current +//! module layouts. //! //! 2. **Re-export the folded-in `oakcore_audioparams_*` accessors** for //! the former mock call sites (`common::oakcore_audioparams_*`). The diff --git a/crates/oakengine/src/test_support/it_export.rs b/crates/oakengine/src/test_support/it_export.rs index 525ac878a..1f4f6c6a4 100644 --- a/crates/oakengine/src/test_support/it_export.rs +++ b/crates/oakengine/src/test_support/it_export.rs @@ -85,7 +85,7 @@ struct SerialGuard { _task: std::sync::MutexGuard<'static, ()>, /// The facade-wide undo-stack lock, so the `oakengine_project_new` /// calls in these tests never race the it_undo / it_storage stack tests. - _stack: std::sync::MutexGuard<'static, ()>, + _stack: parking_lot::ReentrantMutexGuard<'static, ()>, } /// Take the [`SERIAL`] lock AND the global undo-stack lock, recovering @@ -93,8 +93,7 @@ struct SerialGuard { fn serial() -> SerialGuard { let _task = SERIAL.lock().unwrap_or_else(|e| e.into_inner()); let _stack = super::it_undo::GLOBAL_STACK_LOCK - .lock() - .unwrap_or_else(|e| e.into_inner()); + .lock(); SerialGuard { _task, _stack } } diff --git a/crates/oakengine/src/test_support/it_library.rs b/crates/oakengine/src/test_support/it_library.rs index b5985ae1c..b031c36f9 100644 --- a/crates/oakengine/src/test_support/it_library.rs +++ b/crates/oakengine/src/test_support/it_library.rs @@ -81,7 +81,7 @@ fn journal_rows(db: &Path, uuid: &str) -> usize { /// the storage-config lock for the whole body, then point the library at a /// temp SQLite file. fn with_library(db: &Path, f: impl FnOnce() -> R) -> R { - let _stack = GLOBAL_STACK_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let _stack = GLOBAL_STACK_LOCK.lock(); let _config = common::STORAGE_CONFIG_LOCK .lock() .unwrap_or_else(|e| e.into_inner()); @@ -394,7 +394,7 @@ fn export_then_import_round_trip() { /// mutating call fails with E_STATE. #[test] fn disabled_backend_degrades_gracefully() { - let _stack = GLOBAL_STACK_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let _stack = GLOBAL_STACK_LOCK.lock(); let _off = common::storage_off_guard(); assert_eq!(list_json(), "[]", "no library configured reads as empty"); diff --git a/crates/oakengine/src/test_support/it_storage.rs b/crates/oakengine/src/test_support/it_storage.rs index 3e4e6f167..d855ac91a 100644 --- a/crates/oakengine/src/test_support/it_storage.rs +++ b/crates/oakengine/src/test_support/it_storage.rs @@ -67,7 +67,7 @@ const MATH: &str = "org.olivevideoeditor.Olive.math"; /// the storage-config lock for the whole body, then point the write-through /// backend at a temp library. fn with_storage(db: &Path, interval: i32, f: impl FnOnce() -> R) -> R { - let _stack = GLOBAL_STACK_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let _stack = GLOBAL_STACK_LOCK.lock(); let _config = common::STORAGE_CONFIG_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let store = oakcommon::configstore::ConfigStore::instance(); store.set(Some("Storage"), "Backend", "sqlite"); @@ -80,7 +80,7 @@ fn with_storage(db: &Path, interval: i32, f: impl FnOnce() -> R) -> R { /// "off"`): projects bind to nothing and the undo stack stays untouched by /// write-throughs. fn with_storage_off(f: impl FnOnce() -> R) -> R { - let _stack = GLOBAL_STACK_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let _stack = GLOBAL_STACK_LOCK.lock(); let _config = common::STORAGE_CONFIG_LOCK.lock().unwrap_or_else(|e| e.into_inner()); oakcommon::configstore::ConfigStore::instance().set(Some("Storage"), "Backend", "off"); f() @@ -624,7 +624,7 @@ fn unwritable_library_records_last_error() { #[test] fn default_library_path_and_backend() { common::force_link(); - let _stack = GLOBAL_STACK_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let _stack = GLOBAL_STACK_LOCK.lock(); let _config = common::STORAGE_CONFIG_LOCK.lock().unwrap_or_else(|e| e.into_inner()); // The default path is a plain absolute `…/library.db`. diff --git a/crates/oakengine/src/test_support/it_task.rs b/crates/oakengine/src/test_support/it_task.rs index 205a7cb59..98f415c22 100644 --- a/crates/oakengine/src/test_support/it_task.rs +++ b/crates/oakengine/src/test_support/it_task.rs @@ -97,7 +97,7 @@ pub(crate) struct SerialGuard { /// The facade's process-wide undo-stack lock (it_undo's), so the /// `oakengine_project_new` calls in these tests (which clear the stack) /// never race the it_undo / it_storage stack tests. - _stack: std::sync::MutexGuard<'static, ()>, + _stack: parking_lot::ReentrantMutexGuard<'static, ()>, } /// Take the [`SERIAL`] lock AND the global undo-stack lock, recovering @@ -105,8 +105,7 @@ pub(crate) struct SerialGuard { pub(crate) fn serial() -> SerialGuard { let _task = SERIAL.lock().unwrap_or_else(|e| e.into_inner()); let _stack = super::it_undo::GLOBAL_STACK_LOCK - .lock() - .unwrap_or_else(|e| e.into_inner()); + .lock(); SerialGuard { _task, _stack } } diff --git a/crates/oakengine/src/test_support/it_timeline.rs b/crates/oakengine/src/test_support/it_timeline.rs index af8e6bdbf..3485890a3 100644 --- a/crates/oakengine/src/test_support/it_timeline.rs +++ b/crates/oakengine/src/test_support/it_timeline.rs @@ -72,7 +72,7 @@ struct SerialGuard { /// The [`SERIAL`] lock. _task: std::sync::MutexGuard<'static, ()>, /// The facade-wide undo-stack lock. - _stack: std::sync::MutexGuard<'static, ()>, + _stack: parking_lot::ReentrantMutexGuard<'static, ()>, } /// Take the [`SERIAL`] lock AND the global undo-stack lock, recovering @@ -80,8 +80,7 @@ struct SerialGuard { fn serial() -> SerialGuard { let _task = SERIAL.lock().unwrap_or_else(|e| e.into_inner()); let _stack = super::it_undo::GLOBAL_STACK_LOCK - .lock() - .unwrap_or_else(|e| e.into_inner()); + .lock(); SerialGuard { _task, _stack } } diff --git a/crates/oakengine/src/test_support/it_undo.rs b/crates/oakengine/src/test_support/it_undo.rs index c624a92bb..b0de779a9 100644 --- a/crates/oakengine/src/test_support/it_undo.rs +++ b/crates/oakengine/src/test_support/it_undo.rs @@ -92,7 +92,8 @@ unsafe fn read_str(buf: *const c_char) -> String { /// group cannot be shared, so each of those tests holds this lock for its /// whole body. Public so the write-through tests (it_storage.rs), which /// push commands on the same global stack, serialize on the SAME lock. -pub static GLOBAL_STACK_LOCK: Mutex<()> = Mutex::new(()); +pub static GLOBAL_STACK_LOCK: parking_lot::ReentrantMutex<()> = + parking_lot::ReentrantMutex::new(()); static LIFECYCLE_REDO: AtomicI32 = AtomicI32::new(0); static LIFECYCLE_UNDO: AtomicI32 = AtomicI32::new(0); @@ -570,7 +571,7 @@ fn free_contracts() { /// group begin/end/abort lifecycle. #[test] fn undo_stack_integration() { - let _lock = GLOBAL_STACK_LOCK.lock().unwrap(); + let _lock = GLOBAL_STACK_LOCK.lock(); common::force_link(); // --- Baseline: clear() resets to the single "New/Open Project" row. @@ -834,7 +835,7 @@ fn undo_stack_integration() { /// label") and the facade docs. #[test] fn null_name_push_repro() { - let _lock = GLOBAL_STACK_LOCK.lock().unwrap(); + let _lock = GLOBAL_STACK_LOCK.lock(); common::force_link(); assert_eq!(unsafe { oakengine_undo_clear() }, 0); @@ -866,7 +867,7 @@ fn null_name_push_repro() { /// is safe. #[test] fn null_name_group_repro() { - let _lock = GLOBAL_STACK_LOCK.lock().unwrap(); + let _lock = GLOBAL_STACK_LOCK.lock(); common::force_link(); assert_eq!(unsafe { oakengine_undo_clear() }, 0); @@ -905,7 +906,7 @@ unsafe extern "C" fn abort_undo_cb(_ud: *mut c_void) { /// satisfied by a leftover value from an earlier jump.) #[test] fn group_abort_undoes_children_repro() { - let _lock = GLOBAL_STACK_LOCK.lock().unwrap(); + let _lock = GLOBAL_STACK_LOCK.lock(); common::force_link(); assert_eq!(unsafe { oakengine_undo_clear() }, 0); diff --git a/crates/oakengine/src/test_support/node.rs b/crates/oakengine/src/test_support/node.rs index 320a21573..bdac6b4d3 100644 --- a/crates/oakengine/src/test_support/node.rs +++ b/crates/oakengine/src/test_support/node.rs @@ -108,6 +108,7 @@ unsafe fn find_node(project: *mut crate::handle::OakEngineProject, id: &str) -> /// process-wide (the same serialization the undo family uses). #[test] fn project_node_keyframe_lifecycle() { + let _stack = super::it_undo::GLOBAL_STACK_LOCK.lock(); common::force_link(); let _ = force_oakundo_command_link(); // The undo commands below would bind the project to the default user @@ -399,6 +400,7 @@ fn project_node_keyframe_lifecycle() { /// NULL handles yield -1 and out-of-range indexes yield -4. #[test] fn node_failure_paths() { + let _stack = super::it_undo::GLOBAL_STACK_LOCK.lock(); common::force_link(); // NULL node → OAKENGINE_E_INVALID (-1). @@ -437,6 +439,7 @@ fn node_failure_paths() { /// Footage probe/import/borrow failure paths (no media required). #[test] fn footage_failure_paths() { + let _stack = super::it_undo::GLOBAL_STACK_LOCK.lock(); common::force_link(); // Probing a nonexistent path → NULL + a non-empty last error. diff --git a/crates/oakengine/src/test_support/render.rs b/crates/oakengine/src/test_support/render.rs index b68fbe52a..4553434e4 100644 --- a/crates/oakengine/src/test_support/render.rs +++ b/crates/oakengine/src/test_support/render.rs @@ -19,11 +19,17 @@ //! tests, so the manager/cacher families exercise the module's STATE //! error path and the renderer/color families exercise the NULL/invalid //! argument paths (real rendering needs the deferred node family plus an -//! initialized render manager). +//! initialized render manager). The empty-sequence repro at the bottom +//! brings the process-global manager up for its run and shuts it back +//! down, restoring this invariant for the tests that follow it; the two +//! manager-touching tests serialize on [`SERIAL`] so the repro never +//! overlaps `render_manager_not_initialized` (the `it_task`/`it_export` +//! pattern). use super::common; use std::ffi::{c_char, c_double}; +use std::sync::{Mutex, MutexGuard}; use crate::render::{ oakengine_color_last_error, oakengine_color_manager_get_config_filename, @@ -38,10 +44,23 @@ use crate::render::{ OakColorTransformPod, }; +/// Serialize the manager-touching render tests (the pattern in +/// `it_task`/`it_export`). The empty-sequence repro brings the +/// process-global render manager up (and back down) inside its run; +/// without the lock it overlaps `render_manager_not_initialized`, whose +/// STATE paths then see an initialized manager. +static SERIAL: Mutex<()> = Mutex::new(()); + +fn serial() -> MutexGuard<'static, ()> { + SERIAL.lock().unwrap_or_else(|e| e.into_inner()) +} + /// Render manager state without initialization: the module reports its /// STATE error, passed through untranslated (-70002). #[test] fn render_manager_not_initialized() { + let _stack = super::it_undo::GLOBAL_STACK_LOCK.lock(); + let _g = serial(); assert_eq!( unsafe { oakengine_render_manager_set_aggressive_garbage_collection(1) }, -70002 @@ -62,6 +81,7 @@ fn render_manager_not_initialized() { /// Renderer lifecycle: NULL sequence is rejected; mode validation. #[test] fn renderer_lifecycle() { + let _stack = super::it_undo::GLOBAL_STACK_LOCK.lock(); // NULL seq → NULL renderer. let r = unsafe { oakengine_renderer_create( @@ -92,6 +112,7 @@ fn renderer_lifecycle() { /// Frame accessors on NULL / empty handles report zero/NULL safely. #[test] fn frame_accessors_null_safe() { + let _stack = super::it_undo::GLOBAL_STACK_LOCK.lock(); assert_eq!(unsafe { oakengine_frame_width(std::ptr::null()) }, 0); assert_eq!(unsafe { oakengine_frame_height(std::ptr::null()) }, 0); assert_eq!( @@ -107,6 +128,7 @@ fn frame_accessors_null_safe() { /// NULL; freeing is safe either way. #[test] fn color_processor_lifecycle() { + let _stack = super::it_undo::GLOBAL_STACK_LOCK.lock(); // NULL input → NULL. let p = unsafe { oakengine_color_processor_create(std::ptr::null(), std::ptr::null(), std::ptr::null(), 0) @@ -157,6 +179,7 @@ fn color_processor_lifecycle() { /// reports STATE; the last-error string starts empty. #[test] fn color_manager_and_error() { + let _stack = super::it_undo::GLOBAL_STACK_LOCK.lock(); let mut buf = [0 as c_char; 64]; let rc = unsafe { oakengine_color_manager_get_config_filename(std::ptr::null(), buf.as_mut_ptr(), 64) @@ -170,6 +193,7 @@ fn color_manager_and_error() { /// LUT library stubs report the documented neutral values. #[test] fn lut_library_stubs() { + let _stack = super::it_undo::GLOBAL_STACK_LOCK.lock(); assert_eq!(unsafe { oakengine_lut_directory_count() }, 0); assert_eq!( unsafe { oakengine_lut_set_directories(std::ptr::null(), 0) }, @@ -180,6 +204,8 @@ fn lut_library_stubs() { // repro: render_audio on an empty sequence (playback tick on an empty timeline). #[test] fn render_audio_empty_sequence_no_crash() { + let _stack = super::it_undo::GLOBAL_STACK_LOCK.lock(); + let _g = serial(); super::common::force_link(); unsafe { assert_eq!(crate::render::oakengine_render_manager_init(), 0); @@ -200,5 +226,10 @@ fn render_audio_empty_sequence_no_crash() { } crate::render::oakengine_renderer_free(r); crate::node::oakengine_project_free(project); + // The repro initialized the process-global render manager; tear it + // down so the tests after this one keep the module header's + // documented "manager not initialized" contract (the STATE paths + // asserted by `render_manager_not_initialized`). + assert_eq!(crate::render::oakengine_render_manager_shutdown(), 0); } } diff --git a/crates/oakengine/src/test_support/undo.rs b/crates/oakengine/src/test_support/undo.rs index 4fbfc79b0..48a6bde17 100644 --- a/crates/oakengine/src/test_support/undo.rs +++ b/crates/oakengine/src/test_support/undo.rs @@ -157,8 +157,7 @@ fn undo_stack_lifecycle() { // write-through tests (the facade's stack is process-wide): without it // a concurrent test's pushes break the exact-count assertions below. let _stack = super::it_undo::GLOBAL_STACK_LOCK - .lock() - .unwrap_or_else(|e| e.into_inner()); + .lock(); // Reset to a clean "New/Open Project" base row. assert_eq!(unsafe { oakengine_undo_clear() }, 0); diff --git a/docs/build.md b/docs/build.md index 6f4366f28..cf9c4004c 100644 --- a/docs/build.md +++ b/docs/build.md @@ -3,8 +3,11 @@ This document describes how to build Oak Video Editor from source on Windows, Linux, and macOS. > **Note (2026):** Oak is now a Rust workspace; `cargo build` at the -> repository root produces the app, the CLI, the worker and -> `liboakengine`. The CMake instructions below are kept for historical +> repository root produces the app, the CLI and the worker +> (`liboakengine` — the plugin/external-consumer cdylib — is not a +> default member, M14 R4; build it explicitly with +> `cargo build -p oakengine`). The CMake instructions below are kept for +> historical > reference only. Rust dependencies are pulled from crates.io; the only > native libraries still needed are FFmpeg (see the next section), > OpenColorIO (optional; `ocio-sys` builds a stub without it) and a diff --git a/docs/zh/plans/riir/M14-direct-rlib.md b/docs/zh/plans/riir/M14-direct-rlib.md index 2314dfef0..ca24ea969 100644 --- a/docs/zh/plans/riir/M14-direct-rlib.md +++ b/docs/zh/plans/riir/M14-direct-rlib.md @@ -45,7 +45,7 @@ | R1 | 胶水下沉：oakundo 全局栈+通知、oakstorage 会话管理器、oaktask 任务面补齐、oaknode ops 组合函数 | 各模块自带测试绿；facade 改为转发下沉版（行为不变，全量绿） | | R2 | oak-cli + oak-worker 切换（小，先蹚路） | 两 crate 零 oakengine 依赖，测试绿 | | R3 | app 切换：src/oakui/ffi.rs、host_syms.rs 删除，real.rs 全量改 Rust 调用；AppEngine trait 不动（Mock 保留） | `cargo test`（根 crate）全绿；真机冒烟（导入/播放/编辑/导出） | -| R4 | oakengine 转纯 cdylib（workspace default-members 移除 rlib 使用点清零）、根 build.rs 链接逻辑删除、CD 去掉 app 内嵌 dylib | CI 绿；dmg 体积显著缩小（不再嵌 37-56MB dylib） | +| R4 | oakengine 转纯 cdylib（workspace default-members 移除 rlib 使用点清零）、根 build.rs 链接逻辑删除、CD 去掉 app 内嵌 dylib（**已完成，2026-08**：crate-type 已是 cdylib-only、零 Rust 依赖方、cd.yml 去掉 embed 步骤与 `-p oakengine` 预构建） | CI 绿；dmg 体积显著缩小（不再嵌 37-56MB dylib） | | R5 | M13 遗留的 CHandle 内部清除逐模块完成（bridge 已删，剩 handle.rs 与模块内 CHandle 传参） | 模块内部无 CHandle 传参；C ABI 导出层（oakengine）独占 CHandle | 依赖：R1→R2→R3→R4；R5 与 R2-R4 可并行。