Skip to content

mknod: replace nix::sys::stat with rustix - #12321

Draft
mattsu2020 wants to merge 4 commits into
uutils:mainfrom
mattsu2020:mknod_rustix
Draft

mknod: replace nix::sys::stat with rustix#12321
mattsu2020 wants to merge 4 commits into
uutils:mainfrom
mattsu2020:mknod_rustix

Conversation

@mattsu2020

@mattsu2020 mattsu2020 commented May 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

  • Replace nix::sys::stat in mknod with rustix.
  • Use rustix::fs::mknodat, Mode, FileType, and rustix::process::umask.
  • Remove the direct nix dependency.
  • Add tests for mknod permissions and umask handling.

Depends on bytecodealliance/rustix#1625

@github-actions

github-actions Bot commented May 16, 2026 •
edited
Loading

Copy link
Copy Markdown

GNU testsuite comparison:

Congrats! The gnu test tests/id/setgid is no longer failing!

oech3
oech3 reviewed May 16, 2026
View reviewed changes
Comment thread src/uu/mknod/src/mknod.rs Outdated
oech3
oech3 reviewed May 16, 2026
View reviewed changes
Comment thread src/uu/mknod/src/mknod.rs Outdated
@codspeed

codspeed Bot commented May 16, 2026 •
edited
Loading

Copy link
Copy Markdown

Merging this PR will regress 1 benchmark

⚡ 1 improved benchmark
❌ 1 regressed benchmark
✅ 391 untouched benchmarks
⏩ 54 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
❌ Simulation five_38_bit_primes 1.8 s 2 s -10.93%
⚡ Simulation three_39_bit_primes 533 ms 400.8 ms +32.97%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.

Comparing mattsu2020:mknod_rustix (9ddd0cf) with main (2b85f6f)

Open in CodSpeed

Footnotes

  1. 54 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

oech3
oech3 reviewed May 16, 2026
View reviewed changes
Comment thread src/uu/mknod/Cargo.toml Outdated
@oech3

oech3 commented May 16, 2026

Copy link
Copy Markdown
Contributor

mkfifo is subset of mknod. So it should be able to remake existing mkfifo wrapper without unsafe by this one.

@mattsu2020

mattsu2020 commented May 16, 2026

Copy link
Copy Markdown
Contributor Author

mkfifo is subset of mknod. So it should be able to remake existing mkfifo wrapper without unsafe by this one.

Thanks for the suggestion! You're right
mkfifo is essentially mknod with S_IFIFO and dev=0, so the do_mknod helper could be reused there to eliminate the nix dependency and unsafe as well.

@oech3

oech3 commented May 16, 2026

Copy link
Copy Markdown
Contributor

Is it able to open a PR on rustix to support rustix::fs::mk*at on macOS?
I have no access on macOS now...

@mattsu2020

mattsu2020 commented May 16, 2026

Copy link
Copy Markdown
Contributor Author

Is it able to open a PR on rustix to support rustix::fs::mk*at on macOS? I have no access on macOS now...

I created the PR, but there are some errors on the Rustix side during CI, so I'll review the code again once the fixes are complete.
bytecodealliance/rustix#1625

@oech3

oech3 commented May 16, 2026

Copy link
Copy Markdown
Contributor

Thankyou

@xtqqczze

xtqqczze commented Sep 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Depends on bytecodealliance/rustix#1691.

@xtqqczze

xtqqczze commented Sep 28, 2026

Copy link
Copy Markdown
Collaborator

Compiles successfully on Redox:

cargo check -q -p uu_mknod --target x86_64-unknown-redox

@xtqqczze

This comment was marked as resolved.

Sign in to view
@mattsu2020
mattsu2020 marked this pull request as draft September 29, 2026 00:05
@mattsu2020 mattsu2020 changed the title mknod: replace nix::sys::stat with rustix + libc mknod: replace nix::sys::stat with rustix Sep 29, 2026
@mattsu2020

This comment was marked as resolved.

Sign in to view
@xtqqczze xtqqczze mentioned this pull request Sep 30, 2026
Closed
@xtqqczze

This comment was marked as resolved.

Sign in to view
@mattsu2020
mknod: replace nix::sys::stat with rustix + libc
7da29f2 
Replace `nix::sys::stat` usage with `rustix::fs` types and `libc::mknod`:

- `nix::sys::stat::Mode` → `rustix::fs::Mode`
- `nix::sys::stat::SFlag` → `rustix::fs::FileType`
- `nix::sys::stat::umask` → `rustix::process::umask` with RAII `UmaskGuard`
- `nix::sys::stat::mknod` → `libc::mknod` (rustix::fs::mknodat is unavailable on apple targets)
- `nix::libc::S_I*` constants → `0o666` literal

The `UmaskGuard` pattern (borrowed from mkdir) ensures umask is restored
on drop, even on panic — an improvement over the previous manual save/restore.
@mattsu2020
fix: add cspell ignore entries for RAII and mknodat
8dc3c54
@mattsu2020
fix: address PR review feedback
dd05e04 
- Rename `to_rustix` → `to_file_type` to avoid embedding crate name
- Use `let _ = writeln!(stderr, ...)` to prevent SIGABRT on /dev/full
- Sort Cargo.toml dependencies alphabetically (libc before rustix)
@mattsu2020
mknod: test upstream rustix Apple support with Cargo patch
9ddd0cf
@xtqqczze

xtqqczze commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator

@mattsu2020 Please add Depends on https://github.com/bytecodealliance/rustix/pull/1625 to the PR description, and edit to include only important information.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@oech3 oech3 oech3 left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mattsu2020 @oech3 @xtqqczze