-
Notifications
You must be signed in to change notification settings - Fork 672
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable fcntl OFD commands on macos #2300
base: master
Are you sure you want to change the base?
Conversation
Hi, thanks for your interest in contributing to Nix! If you want to bump the MSRV, I think this should be done in a separate PR, and you should also update this info in:
|
@@ -2,8 +2,9 @@ | |||
name = "nix" | |||
description = "Rust friendly bindings to *nix APIs" | |||
edition = "2021" | |||
resolver = "2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that this is already enabled since we are using Rust 2021?
@@ -110,7 +106,7 @@ impl<'fd> FdSet<'fd> { | |||
pub fn fds(&self, highest: Option<RawFd>) -> Fds { | |||
Fds { | |||
set: self, | |||
range: 0..highest.map(|h| h as usize + 1).unwrap_or(FD_SETSIZE), | |||
range: 0..highest.map(|h| h + 1).unwrap_or(FD_SETSIZE) as usize, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, they changed the type of this constant from usize
to c_int
: https://github.com/rust-lang/libc/pull/3356/files
@@ -6,7 +6,7 @@ use std::mem; | |||
use std::os::unix::io::{AsFd, AsRawFd, FromRawFd, OwnedFd, RawFd}; | |||
|
|||
libc_bitflags!( | |||
pub struct EpollFlags: c_int { | |||
pub struct EpollFlags: u32 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, another type change: rust-lang/libc#3466
And, for changes (logic changes and clippy fixes) requested by a newer version libc, I also recommend doing them in a separate PR.
|
I'll follow this up when the libc PR goes through, as it depends heavily on that. Also there are some issues in using 64bit apis on 32bit systems with nix which requires that I work around it for now. |
Feel free to run this to completion. I might revisit it when the libc stuff is merged. I have stopped depending on nix in the meanwhile as it deviates too much from libc. |
You mean those libc breaking changes? They only exist in branch |
I that why I was having trouble? Am I trying to use nix with the wrong branch of libc? |
I am not sure about the trouble you have, but if you want to use libc, you can use the one re-exported by Nix, which is guaranteed to be compatible with Nix. |
What does this PR do
See title. Also note I'm waiting on the constants to be exposed in libc, see rust-lang/libc#3563. When those are added I'll adjust the libc deps in Cargo.toml to match.
Checklist:
CONTRIBUTING.md