feat: build and test natively on macOS - #18
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed the full diff. The centralization is clean: t4::mount/mount_with_options now appear only in src/core/src/store.rs, the two crates that dropped the t4 dependency no longer reference t4:: anywhere, and liquid-cache is already a direct dependency of both. The PerfEventCollector stub keeps the call site in inprocess_runner.rs:527-570 unchanged on every target, and the new utils.rs reference oracle is a genuine independent check rather than the existing impl-vs-impl differential.
Three non-blocking comments inline. Note that the macOS job itself is the only real verification of the non-Linux paths and it had not reported when this review started, so I have not confirmed the workspace builds on aarch64-darwin.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
📊 Benchmark ComparisonCurrent:
Compared Liquid vs DataFusionDefault on the same runner |
There was a problem hiding this comment.
Both cycle-1 nits are addressed: the memory bound now parses the committed .snap through the same memory_bytes predicate (with the expected.is_empty() guard that keeps the length check from passing vacuously), and the t4::mount/mount_with_options ban is now enforced by clippy in both config files, with src/core::store::mount as the single #[allow]ed call site. Confirmed clippy.toml and src/datafusion/clippy.toml are the only two config files in the tree, and no t4::mount* call sites remain outside src/core/src/store.rs.
No new findings. CI on this push was still queued/in progress when I reviewed, so I am not asserting the macOS job's result.
1 participant
Makes the workspace build, test and run natively on macOS: all 9 t4 mount sites now go through one platform-aware
liquid_cache::store::mount(byte-identical tot4::mounton Linux, buffered plus a one-time warning elsewhere), andperf-event2becomes a Linux-only dependency so thebenchmarkcrate compiles. Adds a macOS CI job, givessrc/datafusion/src/utils.rsits first aarch64 test coverage, and documents two things measurement depends on: buffered I/O means memory accounting understates residency off Linux, and FSST picks a different symbol table per architecture so compression ratios do not transfer between arm64 and x86_64.Closes #12.