Use rustix for uname; drop uname crate#7799
Conversation
📝 WalkthroughWalkthroughWorkspace dependency features for Changesuname implementation migration from uname crate to rustix
Sequence Diagram(s)sequenceDiagram
participant Client as POSIX module
participant Old as uname crate
participant New as rustix::system
participant Data as _os::UnameResultData
Client->>Old: uname(vm) via uname::uname()
Old-->>Client: PyResult<Uname>
Client->>New: uname() via rustix::system::uname()
New-->>Client: Uname (OsString fields)
Client->>Data: to_string_lossy() -> _os::UnameResultData
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Cargo.toml (1)
264-264:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winEnable the
"system"feature forrustixincrates/vm/Cargo.toml.The code at
crates/vm/src/stdlib/posix.rscallsrustix::system::uname(), which requires the"system"feature to be enabled. The rustix crate documents thatrustix::systemis "Available on crate featuresystemonly."Currently,
crates/vm/Cargo.tomldeclaresrustix = { workspace = true }, which inherits the workspace-level config that only includesfeatures = ["event"]. Add the"system"feature to the workspace rustix declaration or override it incrates/vm/Cargo.tomlto unblock compilation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Cargo.toml` at line 264, The workspace-level rustix dependency only enables the "event" feature, but crates/vm/src/stdlib/posix.rs calls rustix::system::uname() which requires the "system" feature; update the rustix dependency so the "system" feature is enabled for the VM crate by either adding "system" to the workspace rustix features or overriding in crates/vm/Cargo.toml (replace or extend rustix = { workspace = true } with a declaration that includes features = ["event", "system"] or rustix = { version = "...", features = ["event","system"] } so rustix::system::uname() builds).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@Cargo.toml`:
- Line 264: The workspace-level rustix dependency only enables the "event"
feature, but crates/vm/src/stdlib/posix.rs calls rustix::system::uname() which
requires the "system" feature; update the rustix dependency so the "system"
feature is enabled for the VM crate by either adding "system" to the workspace
rustix features or overriding in crates/vm/Cargo.toml (replace or extend rustix
= { workspace = true } with a declaration that includes features = ["event",
"system"] or rustix = { version = "...", features = ["event","system"] } so
rustix::system::uname() builds).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 6fbb84b5-0173-4157-8261-026ea0711671
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
Cargo.tomlcrates/vm/Cargo.tomlcrates/vm/src/stdlib/posix.rs
💤 Files with no reviewable changes (1)
- crates/vm/Cargo.toml
joshuamegnauth54
force-pushed
the
use-rustix-for-uname
branch
from
47f2f90 to
00b93d6
Compare
3 participants
Nothing fancy here.
unameis a small crate that hasn't been touched in ten years. It doesn't do anything spectacular. Here is its source.rustix, which is already being used by RustPython, provides auname()wrapper that we can use instead.I also dropped the
oldtimefeature fromchrono. According to thechronodocs, that feature doesn't do anything anymore.Summary by CodeRabbit