-
Notifications
You must be signed in to change notification settings - Fork 666
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
Add FcntlArg::F_FULLFSYNC #407
Conversation
@@ -69,6 +71,8 @@ pub fn fcntl(fd: RawFd, arg: FcntlArg) -> Result<c_int> { | |||
F_ADD_SEALS(flag) => libc::fcntl(fd, ffi::F_ADD_SEALS, flag.bits()), | |||
#[cfg(target_os = "linux")] | |||
F_GET_SEALS => libc::fcntl(fd, ffi::F_GET_SEALS), | |||
#[cfg(target_os = "darwin")] | |||
F_FULLFSYNC => libc::fcntl(fd, ffi::F_FULLFSYNC), |
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.
Darwin is not a valid target_os
. You have to use either macos
, ios
or both, depending on where that flag exists. This is also, why this line does not cause our apple build to fail. Otherwise it would complain about an unkown identifier ffi::F_FULLFSYNC
(see the ffi
module definition further up in the file).
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.
Sorry, I was confused because of https://github.com/untitaker/nix/blob/69c5d686ae5c64ea758ceaab482e5a3789397ee2/nix-test/build.rs#L10
e7fffa0
to
aa4824f
Compare
I don't have Mac OS X, so I'll test on Travis. I don't know if iOS supports that flag, but this thread seems to imply so. |
aa4824f
to
ae6635f
Compare
Okay, the tests seem to pass now! |
Thank you. @homu r+ |
📌 Commit ae6635f has been approved by |
⚡ Test exempted - status |
https://developer.apple.com/library/ios/documentation/System/Conceptual/ManPages_iPhoneOS/man2/fsync.2.html