fix(rvf-runtime,rvlite): read errno portably so every Unix target compiles - #757
Open
ohdearquant wants to merge 1 commit into
Open
fix(rvf-runtime,rvlite): read errno portably so every Unix target compiles#757ohdearquant wants to merge 1 commit into
ohdearquant wants to merge 1 commit into
Conversation
…piles
is_pid_alive runs under #[cfg(unix)] but the errno accessor it calls was defined
only for linux, android, macos, ios and freebsd. On any other Unix the caller is
compiled and the callee is not, so the build fails to resolve it:
error[E0425]: cannot find function `libc_errno` in this scope
--> crates/rvf/rvf-runtime/src/locking.rs:283:29
Reproduced with cargo check -p rvf-runtime --target x86_64-unknown-netbsd, and
via rvlite with --features rvf-backend, which reaches the same shape in
storage/writer_lease.rs. Extending the per-OS symbol list would leave the same
gap open for the next target, since the trigger condition is all of unix while
the hand-declared reach is a fixed list.
std::io::Error::last_os_error() reads errno through each platform's own
accessor, so it covers every Unix without naming any symbol, needs no new
dependency, and removes two unsafe blocks and four extern declarations.
Adds a direct test for rvlite's is_pid_alive. Replacing the errno comparison
with a constant true left rvlite's suite fully green, so that line had no
coverage; the new test fails under that mutation and passes with the fix.
rvf-runtime already had coverage: the same mutation fails stale_lock_detection.
This was referenced Aug 2, 2026
ohdearquant
marked this pull request as draft
August 2, 2026 14:37
ohdearquant
marked this pull request as ready for review
August 3, 2026 18:08
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.
The defect
is_pid_aliveis compiled under#[cfg(unix)], but the errno accessor it calls is defined onlyfor
linux,android,macos,iosandfreebsd. On any other Unix the caller is compiled andthe callee is not:
crates/rvlite/src/storage/writer_lease.rscarries the same shape, reachable with--features rvf-backend.This is adjacent to #746 but not the same defect. #746's symptom, the missing
__erroron macOS, isalready fixed on
mainand in the published 0.3.2 — I checked the shipped source rather than theversion number, and 0.3.2 went out on 2026-07-28, two days before that issue was filed. What the fix
left behind is narrower and quieter: the trigger condition is all of
unix, the reach is a fixedlist of five, and the targets in the gap moved from a link error to a compile error.
Why not just add the missing targets
Extending the list works until the next target. NetBSD and OpenBSD use
__errno, DragonFly uses__dfly_error, Solaris and illumos use___errno, and each one has to be right. The list is arestatement of libc internals that has to be maintained against platforms nobody here builds on,
which is how it drifted the first time.
std::io::Error::last_os_error()reads errno through each platform's own accessor. It covers everyUnix without naming a symbol, needs no new dependency, and removes two
unsafeblocks and fourexterndeclarations. TheEPERMcomparison is unchanged, so the behaviour is the same on theplatforms that already worked.
Verification
check -p rvf-runtime --target x86_64-unknown-netbsderror[E0425]check -p rvlite --features rvf-backend --target x86_64-unknown-netbsderror[E0425]check -p rvf-runtime --target x86_64-unknown-linux-gnucheck -p rvlite --features rvf-backend(host, macOS)test -p rvf-runtime --libtest -p rvlite --features rvf-backend --libfmt --all --checkclippy --workspace --all-targets --no-deps -- -D warningsruvector-solverThe clippy row is a comparison against the base tree with the same command, so the pre-existing
error is stated rather than silently inherited. This change neither adds nor fixes it.
About the added test
rvlite'sis_pid_alivehad no coverage. Replacing its errno comparison with a constanttrueleft the whole suite green at 117 passed, which means nothing in it observed that line. The added
test fails under that mutation and passes with the fix, so it holds the behaviour rather than
decorating it.
rvf-runtimeneeded no new test: the same mutation there already failslocking::tests::stale_lock_detection.Why this survived
Neither path is reachable by the workspace CI as configured, which is worth knowing independently of
this patch:
rvf-runtimeis not a workspace member, socargo clippy --workspaceandcargo test --workspacenever reach it.
cargo test -p rvf-runtimefrom the workspace root reports that it "requiresdev-dependencies and is not a member of the workspace".
rvlite'swriter_leasemodule is behind#[cfg(feature = "rvf-backend")]andrvlitedefaultsto
default = [], so a default build compiles it out. I confirmed that by putting acompile_error!in the file and watching a default-feature check still succeed.Both are cheap to close if you want them closed, and I'm happy to open that separately rather than
widen this PR.
One more limitation, on the test itself
A later review pass of my own turned up a gap worth stating rather than leaving for you to find:
the added test does not guard the thing this PR fixes. It exercises the runtime
ESRCHbehaviour, and it would still pass on Linux and macOS if the previous target-gated errno accessors
were restored, because those targets were never the broken ones. The defect this PR closes is a
compilation failure on Unix targets outside that gated set, and no test here can observe that
from a host build.
What would actually guard it is a CI
cargo checkagainst one uncovered Unix target — for examplex86_64-unknown-netbsd. I have not added that, because it needs a cross toolchain in CI and thatis a change to your build infrastructure rather than to this crate, so it seemed like your call
rather than mine. Happy to open it separately if you want it.
To be precise about the strength of the claim: I verified the reasoning above by reading the diff
and the two changed files, not by executing a build against an uncovered target. So "would still
pass with the old code restored" is a reasoned conclusion, not a measured one.
Note on this PR's CI.
Tests (core-and-rest)is red on every branch in this repository,including
main: the job is cancelled at its 240-minute cap while still compiling and neverreaches the test phase. #786 restores the exclusion list that the shard's
packages:value losesto a shell comment, #784 unblocks the
ruvector-filtertest target that the compiler cannotfinish, and #787 fixes a deadlock waiting behind both. That failure is not caused by this branch.