fix(build): stop hiding the shared dependencies behind cfg(linux) - #581
Conversation
Windows and macOS have not compiled since #559. My fault, and a plain editing mistake rather than a design one: adding `ash` and `wgpu-hal` put the `[target.'cfg(target_os = "linux")'.dependencies]` header immediately after `anyhow`, so everything below it — `wgpu`, `pollster`, `cosmic-text`, `ort`, `ndarray`, `serde`, `serde_json`, `image` — became Linux-only. error[E0432]: unresolved import `serde` error[E0433]: cannot find module or crate `ort` in this scope The two new crates belong under that header. Nothing else does. They are back in `[dependencies]`, with the header moved below them and a note saying what it is for, since the failure mode is silent on the platform you are developing on: the Linux build never stopped working, which is why this reached main. `cargo metadata` now reports exactly `ash` and `wgpu-hal` as target-gated, and `wgpu`, `serde`, `ort`, `image`, `pollster` as shared. The Linux build still compiles. I could not verify the Windows and macOS builds locally — those targets are not installed here — so this rests on the manifest structure and on CI.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe compositor manifest now declares shared dependencies for all platforms. The Linux target section contains only the ChangesCompositor dependency configuration
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to Shared compositor dependencies are again available on all platforms, while Vulkan-only bindings remain Linux-specific. No merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Description checkExplanation The description explains the build failure, identifies the affected dependencies, describes the manifest correction, and documents testing limits and CI follow-up. It does not use all template headings or checkbox fields, but it contains the required change and testing information. Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.) ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Windows and macOS have not compiled since #559. My fault, and an editing mistake rather than a design one.
Adding
ashandwgpu-halplaced the[target.'cfg(target_os = "linux")'.dependencies]header immediately afteranyhow, so every dependency below it became Linux-only:wgpu,pollster,cosmic-text,ort,ndarray,serde,serde_jsonandimageare back in[dependencies]. The header moved below them, with a note saying what it is for — because the failure mode is invisible on the platform you develop on. The Linux build never stopped working, which is exactly why this reached main.Verified by manifest rather than by assertion:
Linux still builds. I could not check the Windows and macOS builds locally — those targets are not installed on this machine — so this rests on the manifest structure and on CI. Worth a look at the two red checks on this PR before merging.
Found while checking why #579 and #580 were red: both showed the same two failures, and #579 only touches a
.jsfile, so they could not have been caused by either.Summary by CodeRabbit