fix(hydrate): gate posix_fadvise to Linux/Android, not cfg(unix) - #1208
Merged
Conversation
`release.rs` gated three sites `#[cfg(unix)]` and called `libc::posix_fadvise` / `libc::POSIX_FADV_DONTNEED` inside them. `posix_fadvise` is a Linux/Android extension, not POSIX-universal — Apple's libc does not declare it — so the crate failed to compile on macOS with `error[E0425]: cannot find function 'posix_fadvise' in crate 'libc'`. `cfg(unix)` reads as "has fadvise"; it isn't. The three sites are now gated `#[cfg(any(target_os = "linux", target_os = "android"))]`, with the existing fallback arm widened to cover macOS/BSD, and a doc comment recording the rule: widen this gate only to targets whose libc actually declares the call. Verification: - cfg selection proved two-sidedly on the pinned 1.98.1 toolchain with a `#![no_std]` rustc probe — `x86_64-apple-darwin` excludes the Linux arm, `x86_64-unknown-linux-gnu` selects it. The failure is target-OS-driven, not arch-driven. - 39/39 crate tests green on Linux (incl. the 3 in `release.rs`); `cargo fmt --check` and `clippy -p lance-graph-hydrate --all-targets -- -D warnings` clean. - Not compiled for darwin end-to-end here: `ring` needs a C toolchain this sandbox lacks. The macOS half rests on the cfg proof plus CI. Board: files `ISS-NO-NON-LINUX-TARGET`. Neither existing issue covers this one — `ISS-NO-AARCH64-RUNNER` is about target arch, and `ISS-EXCLUDED-CRATES-UNBUILT` is about the member list, while `lance-graph-hydrate` is a member that CI compiles on every PR. The accurate statement is the union: CI builds exactly one (target_os, target_arch, crate-list) tuple, and this week's three defects each fall on a different one of those axes. Found by q2's newly-ungated macos-latest job — a downstream consumer's CI doing this repo's job, for the third time in one week. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AGVLyRZNEKKBSfBDJfbY3V
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Essentials Run ID: 📒 Files selected for processing (2)
Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour. 📝 WalkthroughWalkthroughThe release code now enables ChangesNon-Linux platform support
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This change prevents macOS and BSD builds from compiling an unavailable advisory API while preserving Linux and Android behavior. No actionable merge risk remains. Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_90aa7b8a-a29e-43d4-b009-e883f1a4db14) |
AdaWorldAPI
marked this pull request as ready for review
September 6, 2026 16:03
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
AdaWorldAPI
pushed a commit
to AdaWorldAPI/q2
that referenced
this pull request
Sep 6, 2026
macOS CI failed to compile cockpit-server:
error[E0425]: cannot find function `posix_fadvise` in crate `libc`
error[E0425]: cannot find value `POSIX_FADV_DONTNEED` in crate `libc`
error: could not compile `cockpit-server` (bin "q2-cockpit")
`advise_dontneed` in osm_slab_hydrate.rs was gated `#[cfg(unix)]` with a
`#[cfg(not(unix))]` no-op beside it. `posix_fadvise` is a Linux/Android
extension, not POSIX-universal — Apple's libc does not declare it — so
`cfg(unix)` reads as "has fadvise" and is not. The function's own doc
comment stated the wrong premise ("No portable equivalent exists ... on
non-Unix targets"), which is what made the gate look correct.
This is the SECOND instance of this exact defect today. The first was in
lance-graph (AdaWorldAPI/lance-graph#1208, merged); q2 carries its own
independent copy in this file. Fixing lance-graph's did not fix this one,
and the macOS job moved from failing in `lance-graph-hydrate` to failing
here — same error, different crate.
Fix (identical to #1208): gate both arms on the OS that actually has the
call, and record the rule in the doc comment so the gate is not re-widened
by intuition.
Both arms still bump the `#[cfg(test)]` FADVISE_ATTEMPTED counter, so the
reachability test is unaffected on either platform.
Verification — red-then-green on the REAL CI target (macos-latest is Apple
Silicon), compiling the gate pair against the real `libc` crate:
new gate, aarch64-apple-darwin compiles OK
new gate, x86_64-unknown-linux-gnu compiles OK
OLD cfg(unix) control, aarch64-darwin reproduces both E0425s exactly
Plus a two-sided `#![no_std]` cfg probe on the pinned 1.98.1 toolchain:
x86_64/aarch64-apple-darwin exclude the Linux arm; x86_64-unknown-linux-gnu
selects it. `cargo fmt -p cockpit-server -- --check` clean.
NOT verified here: a full `cargo check -p cockpit-server` in either
configuration. This sandbox cannot build that dependency tree (lance +
datafusion + deno exhaust the disk), and cross-compiling it for darwin
additionally needs a C toolchain for `ring`. The change is three cfg
attributes and a doc comment, and the gate pair itself is compile-proven
above against real libc on both targets.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AGVLyRZNEKKBSfBDJfbY3V
AdaWorldAPI
pushed a commit
that referenced
this pull request
Sep 7, 2026
`main` moved 2918421 -> d24ddbc (#1208/#1209/#1210) and made this PR `mergeable_state: dirty`. One conflict: both sides prepended a board entry to LATEST_STATE.md. Both survive -- this branch's Blocks V3 contract entry above main's #1209 record, in prepend order. 3384 lines against parents of 3324 and 3305, so no-shrink holds against both. Corrected in the same resolution rather than merged forward: this entry's citation-decay paragraph claimed the gate was red on origin/main too, 12 findings byte-identical, and that its prescribed fix would require editing past entries the append-only rule protects. That described one run and was carried forward instead of re-read. On dcfc820 the gate reported 1 NEW decay and it was this branch's -- registering the class moved the NodeRow size assert off the line ISSUES.md:1537 cited. Fixed in 91fb423 with a stable anchor; the "append-only forbids it" dilemma was false, since an anchor repair changes a citation's address, not a past entry's claim. citation-decay --since d24ddbc: 0 new decay(s), 147 pre-existing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011DN5odWu4eisdevH9bPWyz
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
2 participants
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.
The defect
crates/lance-graph-hydrate/src/release.rsgated three sites#[cfg(unix)]and calledlibc::posix_fadvise/libc::POSIX_FADV_DONTNEEDinside them.posix_fadviseis a Linux/Android extension, not POSIX-universal — Apple's libc does not declare it — so the crate fails to compile on macOS:cfg(unix)reads as "has fadvise" and is not. This is a real break onmain, not something this branch introduced.The fix
The three sites are gated
#[cfg(any(target_os = "linux", target_os = "android"))], and the existing fallback arm's negation widens to cover macOS/BSD. The doc comment records the rule so the gate is not re-widened by intuition:One file, +15/−6. No behaviour change on Linux.
Verification
#[no_std]rustc probe:x86_64-apple-darwincompiles clean (Linux arm excluded);x86_64-unknown-linux-gnuhits the deliberate marker error (Linux arm selected). So the failure is target-OS-driven, not arch-driven — an x86 macOS runner catches it, an aarch64 Linux one does not.release.rs.cargo fmt -p lance-graph-hydrate -- --checkandcargo clippy -p lance-graph-hydrate --all-targets -- -D warningsboth clean.Honest caveat: the crate was not compiled end-to-end for darwin here —
ringneeds a C toolchain this sandbox lacks. The macOS half rests on the cfg proof above plus CI.Board hygiene
Files
ISS-NO-NON-LINUX-TARGET. Neither existing issue covers this one, and the difference is the generalization:ISS-NO-AARCH64-RUNNERISS-EXCLUDED-CRATES-UNBUILTlance-graph-hydrateis a member CI compiles every PRThe accurate statement is the union: CI compiles exactly one
(target_os, target_arch, crate-list)tuple —(linux, x86_64, members). Everything outside it is text the parser skips. This week's three defects fall one on each axis, which is why fixing any single axis would not have caught the other two.Found by q2's newly-ungated
macos-latestjob — a downstream consumer's CI doing this repo's job, for the third time in one week.Not fixed here: the axis itself. A
macos-latestjob (or a cheapcargo check --target x86_64-apple-darwinon the existing x86 runners) is a CI policy change and an operator call, not a drive-by on a compile-fix PR. The cheap form needsrustup target add --toolchain 1.98.1 <target>— installing for the default toolchain while the repo pins 1.98.1 yields acan't find crate for 'core'that reads like a broken probe rather than a missing std.Downstream
Unblocks macOS CI on q2
mainand q2 #147.🤖 Generated with Claude Code
https://claude.ai/code/session_01AGVLyRZNEKKBSfBDJfbY3V
Generated by Claude Code
Summary by CodeRabbit
Bug Fixes
posix_fadvisesupport to Linux and Android.Documentation