Complete macOS support and cover it in CI - #19
Merged
Conversation
Every CI job ran on ubuntu-latest, so nothing exercised the macOS code paths. Building on an M2 surfaced a lint failure, an example that had drifted, and a memory field that was never populated. - kfd: `ComputeNode` and the non-Linux `nodes()` stub are only reachable from the Linux-gated `drm::detect`, so the lib target — built without `cfg(test)`, where the only caller lives — failed `-D warnings` on macOS. - metal: report used/free on Apple Silicon from `vm_stat`, counting active + wired + compressor pages, the figure Activity Monitor labels "Memory Used". Unified memory means system-wide usage is the GPU's usage; a discrete card on an Intel Mac reported its own VRAM and is left untouched. Read lazily, so such a Mac spawns no subprocess. - metal: map M5 to apple10, per Apple's Metal Feature Set Tables (May 21, 2026). Apple documents these per series, so one row covers M5, M5 Pro and M5 Max, as `1 => 7` already does for M1. - watch: print `vulkan_host()`, missing since the probe landed in #17. - ci: add a macos-latest job. The Linux jobs structurally cannot cover these paths — the modules are cfg'd per target, so code that is dead or broken only on macOS still compiles clean on ubuntu. Verified on an M2 MacBook Air (macOS 26.5): detects Apple M2 / apple8 / 8.0 GiB, with used/free matching an independent vm_stat calculation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Every job in
ci.ymlruns onubuntu-latest, so nothing has ever exercised the macOS code paths. Building the crate on an M2 MacBook Air (macOS 26.5) surfaced a lint failure, an example that had drifted, and a memory field that was never populated.Fixes
kfd.rs— the lib did not compile on macOS.ComputeNodeand the non-Linuxnodes()stub are only reachable fromdrm::detect, which is#[cfg(target_os = "linux")]. Dead-code analysis is per compilation unit, and the lib target is built withoutcfg(test)— where the stub's only caller lives — so both tripped-D warnings:metal.rs— used/free were never reported on Apple Silicon. Both were hardcodedNone, sowatchshowedusage unknownon every Apple machine. Now read fromvm_stat(no Metal linkage, no new dependencies, same subprocess approach as the existingsystem_profiler/sysctlcalls):usedisactive + wired + compressor, the figure Activity Monitor labels "Memory Used". Three details worth review:vm_statheader rather thanhw.pagesize, so counts and multiplier share a source. Apple Silicon is 16 KiB, Intel Macs 4 KiB.None. Thevm_statread is lazy, matching the deferraldrm.rsuses, so a discrete-only Mac spawns no subprocess.metal.rs— M5 support. Maps toapple10, confirmed against two sources rather than inferred, since this function's stated doctrine is that a wrong family gets acted on while an absent one does not:MTLGPUFamilyApple10 = 1010; the 15.4 SDK stops atApple9.M1-series → Apple7,M2-series → Apple8,M3-series → Apple9,M4-series → Apple9,M5-series → Apple10.Apple documents these per series, so the single row
5 => 10covers M5, M5 Pro, M5 Max and a future M5 Ultra — the same way1 => 7already covers M1 Max. The doc comment now cites that table instead of describing the mapping as name-derived guesswork, and a new test pinsapple10rendering sinceAppleFamilyhad only ever formatted single digits.watch.rs—vulkan_host()was never printed. The probe landed in #17 but the example was not updated, so the host block showed only oneapi/rocm/cuda. Also corrects a "Both rows" comment that was already stale at three probes.CI
Adds a
macos-latestjob running clippy with the same pedantic flags, tests, doctests,--no-default-features, and thedetectexample.Standalone rather than an
osdimension on the existing matrix: that matrix varies the toolchain, and this breakage is platform-specific, so three more macOS runners would buy little. It runs--no-default-featuresbecausenvidiais a default feature pulling in NVML that no Mac can use, so Apple-only consumers build that way and the check belongs on their platform. Thedetectstep prints the runner's GPU so probe regressions appear in the log rather than hiding behind an empty-result path.Had this job existed, it would have caught the
kfd.rsbreak directly.Verification
On an M2 MacBook Air, macOS 26.5, Rust 1.96.0 —
just ciexits 0:cargo fmt --check-D warnings)--no-default-featurescargo publish --dry-rundetect()reportsApple M2 (Apple, apple8): 8.0 GiB total, 1.9 GiB free, with used/free cross-checked against an independentawkcalculation overvm_stat(6.01/1.99 GiB, matching to sampling jitter).Known gap, not addressed here
vulkan.rsprobes onlylibvulkan.so*and Linux ICD paths, so MoltenVK is undetectable.Noneis correct on a Mac without the Vulkan SDK, but it is a false negative on one with it. Left for a follow-up.🤖 Generated with Claude Code