Repository navigation
fix(ci): keep native media dependencies out of the workspace hack - #7713
Conversation
Quest-Claim: 65b310d43ab379af
Derive the trybuild exclusion from Cargo metadata and verify renamed and target-specific UI dependencies. Preserve the scheduled Linux cache measurement as the remaining quest work. Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 12 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed head 50eb0a060c5764313a5e4e86c1e5168e94064d30 against base ac0f04e5dcca9a9a07d15135dc2e904215c494f3.
No actionable findings. The trybuild guard checks Cargo metadata's actual package name and dev-dependency kind before the manual exclusions, covering renamed and target-specific dependencies while rejecting any workspace-hack edge. Its fixtures cover both rejection and safe absence. The native-dependency safeguard checks normal/build/dev graphs on both configured Linux triples and confirms the hack is present before excluding the native sys packages.
Direction: retain the narrow final exclusions. Media features still own their dependencies, and the documented VAAPI/PipeWire compilation split is an explicit tradeoff rather than a claimed cache improvement. Keep the scheduled pre/post-merge cache comparison open as planned.
Verification: GitHub-only static review of the diff, guard and fixtures, configuration, dependency manifests, CI wiring, quest and prior discussion. I did not run tests, regenerate hakari, export unit graphs, or measure cache performance. The exact-head hosted Check was still running when inspected; this is not a claim that the strict gate passed.
(Written by review (OpenAI))
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Derived trybuild guard (
sh/rs/hakari.py): readscargo metadata, so it matches the real package name for renamed aliases and covers target-specific dev-dependencies; a trybuild crate must not carry aworkspace-hackedge, and a safe absence no longer depends on the manual exclude list. - Native media final exclusions (
.config/hakari.toml): excludespipewire,libspa, andclang-sysfrom the generated hack. Verified across the media and non-media graphs below. - Regenerated hack and lockfile: the three entries are gone from the hack's Linux dependency lists and
Cargo.lock;cargo hakari generate --diffreproduces the checked-in file. - Fixtures wired into CI (
.github/justfile,sh/rs/hakari.test.py): covers trybuild rename, target-specific dev-dep, rejection, and safe absence, plus a Linux x86_64/aarch64cargo tree -p moq-jsonassertion that keeps the hack present while the native sys crates stay out. - Quest re-scope (
quest/m0/ci/hakari-followups.md, epic README): the remaining work is the post-merge Linux cache comparison; the implementation is recorded as done.
Independent verification
python3 sh/rs/hakari.test.pypasses both tests;python3 sh/rs/hakari.pypasses.cargo hakari generate --diffexits 0 with cargo-hakari 0.9.39, so the regenerated hack matches the new config.cargo treeon Linux:moq-jsonreaches none ofpipewire/libspa/clang-syson either configured triple, whilemoq-video --features pipewire,moq-audio --features capture,pipewire, andmoq-video --features vaapistill pull their native deps.
The VAAPI/PipeWire clang-sys split is a recorded, monitored cache tradeoff rather than a functional regression, and the post-merge comparison remains in ## Waiting as described.
deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏

Problem
The hakari guard accepts a new trybuild crate's workspace-hack edge unless someone separately excludes that crate by name. trybuild then copies the hack into each UI test project. The hack also makes non-media Linux test builds compile PipeWire bindings and require libpipewire/libclang.
Approach
pipewire,libspa, andclang-sys, regenerate the hack and lockfile, and retain feature unification for their other dependencies. cpal has no direct entry in the generated hack, so excluding it alone would not remove these paths.just gh test.Impact
Cache evidence
Local Cargo 1.98.1
build --locked -Z unstable-options --unit-graph --tests, usingRUSTC_BOOTSTRAP=1only for graph export. Compared package/target, mode, profile, features, platform, and dependencies recursively, replacing unit indices with the referenced identities. Commands select-p moq-json,-p moq-video --features pipewire, and-p moq-audio --features pipewireon Linux x86_64.JSON/video share 418 third-party identities before and 388 after: the 30 units removed from JSON must instead be supplied by a media build. Four identities change, for PipeWire/libspa without the hack's extra version features. The total number of combined third-party identities does not grow in this sample. This bounds the observed graph tradeoff; it is not a measured mbx hit rate or CPU saving. Recommendation: keep the narrow exclusions unless the scheduled measurements reveal a material cache cost.
The local review exposed a VAAPI cache tradeoff outside that three-build sample. Fresh Linux x86_64 test unit graphs for
moq-video --features vaapiandmoq-video --features pipewire, against mainac0f04e5dc, show VAAPI-only falling from 478 to 456 units and PipeWire staying at 491. Shared recursive third-party identities fall from 453 to 428; their union grows from 483 to 486. VAAPI's bindgen 0.70 usesclang_6_0, whereas PipeWire's bindgen 0.72 usesclang_11_0, splitting clang-sys and its two build-script units. The quest now records this cost and requires considering it in the scheduled comparison. Keeping clang-sys directly in the hack would retain libclang in non-media test graphs, defeating this PR's purpose. No CPU or hit-rate saving is claimed.The successful 2026-11-02 scheduled Linux Rust benchmark-tests job is a pre-merge candidate, not after evidence. Its 12 object-cache summaries total 986 hits, 721 misses, 323 not looked up, 8 bypassed, and 11 remote failures: 57.76% hits among attempted lookups. It also exhausted the 5 GiB download budget during the first build, so it is a noisy baseline. Its time-avoided fields are estimates, not measured CPU totals. Replace this candidate with the last scheduled run before this PR merges.
Alternatives
Traversal exclusions would stop unifying unrelated transitive dependencies too. Keeping the current hack would keep native media build requirements on every non-media test. The final exclusions remove just the three entries responsible for the observed dependency paths.
Choices made within the settled plan: use metadata's actual package names rather than manifest aliases, retain the existing kio-macros configuration exclusion for hakari itself, and test both Linux triples without cross-compiling them.
Validation
moq-jsontests with the same probe restriction and an invalidLIBCLANG_PATH.quest gate just rs check -p moq-video -p moq-audio --features moq-video/pipewire,moq-audio/pipewirepasses compilation, clippy, docs, and dependency hygiene.nix develop --command quest gate just ci origin/mainat 50eb0a060c: all selected lanes except strict flake pass, including Rust lint, 11,690 Rust tests (135 skipped), repository tooling, the trybuild fixtures, and both Linux dependency graphs. Strict flake fails because systemd-repart cannot write/proc/self/uid_mapin this host's Nix builder.origin/mainatac0f04e5dcca9a9a07d15135dc2e904215c494f3withCHECK_MODULES=flake nix develop --command quest gate just ci --all; it fails building the identicalgithub-runnerderivation.just cisetsCI_STRICT=1, so it deliberately bypasses the local capability skip. The full strict local gate is not reported as passing; the maintainer's hosted-CI decision below applies.quest check --base origin/main --branch quest/m0/ci/hakari-followups(590 documents), hakari regeneration, andgit diff --checkpass.Review
quest review --base origin/main --branch quest/m0/ci/hakari-followups, Claude Code / Claude Opus 5.5, medium effort, sessiond1822bda-a2fd-4dca-b42c-787d080882f2, round 2. Reviewed 50eb0a060c against main 0eee916849: 0 findings.Round 1's low finding on the VAAPI clang-sys feature split is fixed by measuring and recording the VAAPI-only/PipeWire graphs and their three extra combined identities in the quest and cache evidence above. The implementation is unchanged.
Decision
✅ Decided 2026-11-04 (maintainer, /quest-unblock): hosted CI suffices for the strict Nix gate. Local
check flakeskips on hosts that refuse user-namespace mapping, as expected since moq-dev/moq#7763. Requiring a namespace-capable local builder was not chosen. The strict check runs in hosted CI on this ready PR. Gate repairs moq-dev/moq#7654 and moq-dev/moq#7632 have landed.The scheduled Linux baseline versus the first two post-merge Nightlies remains in
quest/m0/ci/hakari-followups.mdunder## Waiting; it cannot run before merge and does not hold this PR in draft. Keep the narrow exclusions unless those measurements justify another design.Follow-ups
The existing quest retains the post-merge Linux cache comparison. The separate FFI/platform cache study remains with moq-dev/moq#7666.
Conflict check: moq-dev/moq#7877 has now merged. It retains Dependabot and documents workspace-hack regeneration by the landing fixer; it adds no competing exclusions. Merged
origin/main, retained its current quest links, and regenerated the hack with this PR's narrow exclusions.(Written by GPT-6.1 Sol with high effort)