Skip to content

fix(ci): keep native media dependencies out of the workspace hack - #7713

Merged
kixelated merged 6 commits into
mainfrom
quest/m0/ci/hakari-followups
Nov 5, 2026
Merged

kixelated merged 6 commits into
mainfrom
quest/m0/ci/hakari-followups

Conversation

@kixelated

@kixelated kixelated commented Nov 3, 2026 •

Copy link
Copy Markdown
Collaborator

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

  • Derive the trybuild rule from Cargo metadata, including renamed and target-specific dev-dependencies. Reject any hack edge and allow the safe absence of one, independently of the manual exclusions.
  • Add narrow hakari final exclusions for pipewire, libspa, and clang-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.
  • Run the new fixtures and Linux x86_64/ARM non-media dependency-graph checks in just gh test.
  • Keep this quest open only for the scheduled Linux cache comparison. The FFI alignment and macOS/Windows evidence in moq-dev/moq#7666 stay separate; none of its unmerged work is included.

Impact

  • Public API: none.
  • Wire: none.
  • Build: unrelated Linux tests no longer need PipeWire or libclang through the hack. Explicit media features retain their own dependencies.

Cache evidence

Local Cargo 1.98.1 build --locked -Z unstable-options --unit-graph --tests, using RUSTC_BOOTSTRAP=1 only 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 pipewire on Linux x86_64.

Build Before units After units
moq-json tests 455 425
PipeWire video tests 501 501
PipeWire audio tests 515 486
Distinct third-party identities across all three 531 531

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 vaapi and moq-video --features pipewire, against main ac0f04e5dc, 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 uses clang_6_0, whereas PipeWire's bindgen 0.72 uses clang_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

  • Reproduced the guard failure with an actual local Cargo workspace: the original script accepted a planted hack edge on a newly added, renamed trybuild dev-dependency, including a target-specific edge. Fixed fixtures cover safe absence and rejection, both with and without manual configuration exclusions.
  • Linux x86_64/ARM dependency-graph safeguards pass. With the cache launcher and shell environment reload disabled, the original manifests fail on a refused native media pkg-config probe; the fixed manifests pass all 188 moq-json tests with the same probe restriction and an invalid LIBCLANG_PATH.
  • quest gate just rs check -p moq-video -p moq-audio --features moq-video/pipewire,moq-audio/pipewire passes compilation, clippy, docs, and dependency hygiene.
  • After merging main, nix develop --command quest gate just ci origin/main at 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_map in this host's Nix builder.
  • Reproduced that same strict flake failure on unchanged origin/main at ac0f04e5dcca9a9a07d15135dc2e904215c494f3 with CHECK_MODULES=flake nix develop --command quest gate just ci --all; it fails building the identical github-runner derivation. just ci sets CI_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, and git diff --check pass.

Review

quest review --base origin/main --branch quest/m0/ci/hakari-followups, Claude Code / Claude Opus 5.5, medium effort, session d1822bda-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 flake skips 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.md under ## 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)

kixelated and others added 4 commits November 3, 2026 01:48
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>
@kixelated kixelated changed the title The workspace-hack refuses trybuild edges and spares non-media builds fix(ci): keep native media dependencies out of the workspace hack Nov 3, 2026
@kixelated
kixelated marked this pull request as ready for review November 4, 2026 22:55
@coderabbitai

coderabbitai Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You'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.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 7709ed0b-714e-410a-a70e-a6aa170548c1
Commits

Reviewing files that changed from the base of the PR and between ac0f04e and 50eb0a0.

Files ignored due to path filters: 1
  • Cargo.lock is excluded by !**/*.lock
Files selected for processing: 7
  • .config/hakari.toml
  • .github/justfile
  • quest/m0/ci/README.md
  • quest/m0/ci/hakari-followups.md
  • rs/workspace-hack/Cargo.toml
  • sh/rs/hakari.py
  • sh/rs/hakari.test.py
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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))

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

  • Derived trybuild guard (sh/rs/hakari.py): reads cargo metadata, so it matches the real package name for renamed aliases and covers target-specific dev-dependencies; a trybuild crate must not carry a workspace-hack edge, and a safe absence no longer depends on the manual exclude list.
  • Native media final exclusions (.config/hakari.toml): excludes pipewire, libspa, and clang-sys from 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 --diff reproduces 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/aarch64 cargo tree -p moq-json assertion 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.py passes both tests; python3 sh/rs/hakari.py passes.
  • cargo hakari generate --diff exits 0 with cargo-hakari 0.9.39, so the regenerated hack matches the new config.
  • cargo tree on Linux: moq-json reaches none of pipewire/libspa/clang-sys on either configured triple, while moq-video --features pipewire, moq-audio --features capture,pipewire, and moq-video --features vaapi still 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.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏

@kixelated
kixelated added this pull request to the merge queue Nov 5, 2026
Merged via the queue into main with commit 877c145 Nov 5, 2026
12 checks passed
@kixelated
kixelated deleted the quest/m0/ci/hakari-followups branch November 5, 2026 06:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant