Merge #681

681: Remove feature flags r=Susurrus

These are vestiges of the initial push to get this working on Rust 1.0. These feature flags are undocumented and so hard to discover (only learned about them today!), prevent functions being included that should be and this also affects documentation on docs.rs, and none of the features are tested in CI and the `execvpe` has been broken for forever.

The solution is to conditionally compile everything supported for a given platform and do away completely with the feature flags. The `execvpe` function is completely removed as it's not available for *nix platforms in libc and is already broken, so no loss removing it. We'll add it back once it's back in libc (rust-lang/libc#670).

Closes #98.
Closes #206.
Closes #306.
Closes #308.

Author: bors[bot]

CHANGELOG.md

@@ -46,10 +46,15 @@ This project adheres to [Semantic Versioning](http://semver.org/).
   Also file system type constants like `nix::sys::statfs::ADFS_SUPER_MAGIC` were removed in favor of the libc equivalent.
   ([#561](https://github.com/nix-rust/nix/pull/561))
 - Revised the termios API including additional tests and documentation and exposed it on iOS. ([#527](https://github.com/nix-rust/nix/pull/527))
+- `eventfd`, `signalfd`, and `pwritev`/`preadv` functionality is now included by default for all
+  supported platforms. ([#681](https://github.com/nix-rust/nix/pull/561))
 
 ### Removed
 - Removed `io::Error` from `nix::Error` and the conversion from `nix::Error` to `Errno`
   ([#614](https://github.com/nix-rust/nix/pull/614))
+- All feature flags have been removed in favor of conditional compilation on supported platforms.
+  `execvpe` is no longer supported, but this was already broken and will be added back in the next
+  release. ([#681](https://github.com/nix-rust/nix/pull/561))
 
 ### Fixed
 - Fixed multiple issues compiling under different archetectures and OSes.

Cargo.toml

@@ -15,12 +15,6 @@ exclude     = [
   "test/**/*"
 ]
 
-[features]
-eventfd = []
-execvpe = []
-preadv_pwritev = []
-signalfd = []
-
 [dependencies]
 libc = "0.2.25"
 bitflags = "0.9"
@@ -38,12 +32,6 @@ nix-test = { path = "nix-test", version = "0.0.1" }
 name = "test"
 path = "test/test.rs"
 
-[[test]]
-name = "test-signalfd"
-path = "test/test_signalfd.rs"
-harness = false
-test = true
-
 [[test]]
 name = "test-mount"
 path = "test/test_mount.rs"

src/sys/mod.rs

@@ -9,8 +9,7 @@ pub mod epoll;
     target_os = "dragonfly", target_os = "openbsd", target_os = "netbsd"))]
 pub mod event;
 
-// TODO: switch from feature flags to conditional builds
-#[cfg(feature = "eventfd")]
+#[cfg(target_os = "linux")]
 pub mod eventfd;
 
 #[cfg(target_os = "linux")]
@@ -24,8 +23,8 @@ pub mod sendfile;
 
 pub mod signal;
 
-#[cfg(any(target_os = "linux", target_os = "android"))]
-#[cfg(feature = "signalfd")]
+// FIXME: Add to Android once libc#671 lands in a release
+#[cfg(target_os = "linux")]
 pub mod signalfd;
 
 pub mod socket;

src/sys/signalfd.rs

@@ -60,12 +60,10 @@ pub fn signalfd(fd: RawFd, mask: &SigSet, flags: SfdFlags) -> Result<RawFd> {
 /// # Examples
 ///
 /// ```
-/// use nix::sys::signalfd::*;
-///
+/// # use nix::sys::signalfd::*;
+/// // Set the thread to block the SIGUSR1 signal, otherwise the default handler will be used
 /// let mut mask = SigSet::empty();
-/// mask.add(signal::SIGUSR1).unwrap();
-///
-/// // Block the signal, otherwise the default handler will be invoked instead.
+/// mask.add(signal::SIGUSR1);
 /// mask.thread_block().unwrap();
 ///
 /// // Signals are queued up on the file descriptor
@@ -74,11 +72,9 @@ pub fn signalfd(fd: RawFd, mask: &SigSet, flags: SfdFlags) -> Result<RawFd> {
 /// match sfd.read_signal() {
 ///     // we caught a signal
 ///     Ok(Some(sig)) => (),
-///
 ///     // there were no signals waiting (only happens when the SFD_NONBLOCK flag is set,
 ///     // otherwise the read_signal call blocks)
 ///     Ok(None) => (),
-///
 ///     Err(err) => (), // some error happend
 /// }
 /// ```

src/sys/uio.rs

@@ -18,7 +18,7 @@ pub fn readv(fd: RawFd, iov: &mut [IoVec<&mut [u8]>]) -> Result<usize> {
     Errno::result(res).map(|r| r as usize)
 }
 
-#[cfg(feature = "preadv_pwritev")]
+#[cfg(target_os = "linux")]
 pub fn pwritev(fd: RawFd, iov: &[IoVec<&[u8]>],
                offset: off_t) -> Result<usize> {
     let res = unsafe {
@@ -28,7 +28,7 @@ pub fn pwritev(fd: RawFd, iov: &[IoVec<&[u8]>],
     Errno::result(res).map(|r| r as usize)
 }
 
-#[cfg(feature = "preadv_pwritev")]
+#[cfg(target_os = "linux")]
 pub fn preadv(fd: RawFd, iov: &mut [IoVec<&mut [u8]>],
               offset: off_t) -> Result<usize> {
     let res = unsafe {
@@ -57,7 +57,7 @@ pub fn pread(fd: RawFd, buf: &mut [u8], offset: off_t) -> Result<usize>{
 }
 
 #[repr(C)]
-pub struct IoVec<T>(libc::iovec, PhantomData<T>); 
+pub struct IoVec<T>(libc::iovec, PhantomData<T>);
 
 impl<T> IoVec<T> {
     #[inline]

src/unistd.rs

@@ -1596,9 +1596,6 @@ mod linux {
     use {Errno, Result, NixPath};
     use super::{Uid, Gid};
 
-    #[cfg(feature = "execvpe")]
-    use std::ffi::CString;
-
     pub fn pivot_root<P1: ?Sized + NixPath, P2: ?Sized + NixPath>(
             new_root: &P1, put_old: &P2) -> Result<()> {
         let res = try!(try!(new_root.with_nix_path(|new_root| {
@@ -1643,23 +1640,4 @@ mod linux {
 
         Errno::result(res).map(drop)
     }
-
-    #[inline]
-    #[cfg(feature = "execvpe")]
-    pub fn execvpe(filename: &CString, args: &[CString], env: &[CString]) -> Result<()> {
-        use std::ptr;
-        use libc::c_char;
-
-        let mut args_p: Vec<*const c_char> = args.iter().map(|s| s.as_ptr()).collect();
-        args_p.push(ptr::null());
-
-        let mut env_p: Vec<*const c_char> = env.iter().map(|s| s.as_ptr()).collect();
-        env_p.push(ptr::null());
-
-        unsafe {
-            super::ffi::execvpe(filename.as_ptr(), args_p.as_ptr(), env_p.as_ptr())
-        };
-
-        Err(Error::Sys(Errno::last()))
-    }
 }

test/sys/mod.rs

@@ -2,6 +2,8 @@ mod test_signal;
 #[cfg(any(target_os = "freebsd", target_os = "dragonfly", target_os = "ios",
           target_os = "netbsd", target_os = "macos", target_os = "linux"))]
 mod test_aio;
+#[cfg(target_os = "linux")]
+mod test_signalfd;
 mod test_socket;
 mod test_sockopt;
 mod test_termios;

test/sys/test_aio.rs

@@ -244,7 +244,7 @@ extern fn sigfunc(_: c_int) {
 #[cfg_attr(any(all(target_env = "musl", target_arch = "x86_64"), target_arch = "mips"), ignore)]
 fn test_write_sigev_signal() {
     #[allow(unused_variables)]
-    let m = ::SIGUSR2_MTX.lock().expect("Mutex got poisoned by another test");
+    let m = ::SIGNAL_MTX.lock().expect("Mutex got poisoned by another test");
     let sa = SigAction::new(SigHandler::Handler(sigfunc),
                             SA_RESETHAND,
                             SigSet::empty());
@@ -375,7 +375,7 @@ fn test_lio_listio_nowait() {
 #[cfg_attr(any(target_arch = "mips", target_env = "musl"), ignore)]
 fn test_lio_listio_signal() {
     #[allow(unused_variables)]
-    let m = ::SIGUSR2_MTX.lock().expect("Mutex got poisoned by another test");
+    let m = ::SIGNAL_MTX.lock().expect("Mutex got poisoned by another test");
     const INITIAL: &'static [u8] = b"abcdef123456";
     const WBUF: &'static [u8] = b"CDEF";
     let rbuf = Rc::new(vec![0; 4].into_boxed_slice());

test/sys/test_signalfd.rs

@@ -0,0 +1,26 @@
+#[test]
+fn test_signalfd() {
+    use nix::sys::signalfd::SignalFd;
+    use nix::sys::signal::{self, raise, Signal, SigSet};
+
+    // Grab the mutex for altering signals so we don't interfere with other tests.
+    #[allow(unused_variables)]
+    let m = ::SIGNAL_MTX.lock().expect("Mutex got poisoned by another test");
+
+    // Block the SIGUSR1 signal from automatic processing for this thread
+    let mut mask = SigSet::empty();
+    mask.add(signal::SIGUSR1);
+    mask.thread_block().unwrap();
+
+    let mut fd = SignalFd::new(&mask).unwrap();
+
+    // Send a SIGUSR1 signal to the current process. Note that this uses `raise` instead of `kill`
+    // because `kill` with `getpid` isn't correct during multi-threaded execution like during a
+    // cargo test session. Instead use `raise` which does the correct thing by default.
+    raise(signal::SIGUSR1).ok().expect("Error: raise(SIGUSR1) failed");
+
+    // And now catch that same signal.
+    let res = fd.read_signal().unwrap().unwrap();
+    let signo = Signal::from_c_int(res.ssi_signo as i32).unwrap();
+    assert_eq!(signo, signal::SIGUSR1);
+}

test/sys/test_uio.rs

@@ -129,7 +129,7 @@ fn test_pread() {
 }
 
 #[test]
-#[cfg(feature = "preadv_pwritev")]
+#[cfg(target_os = "linux")]
 fn test_pwritev() {
     use std::io::Read;
 
@@ -159,7 +159,7 @@ fn test_pwritev() {
 }
 
 #[test]
-#[cfg(feature = "preadv_pwritev")]
+#[cfg(target_os = "linux")]
 fn test_preadv() {
     use std::io::Write;
 

test/test.rs

@@ -11,7 +11,7 @@ extern crate nix_test as nixtest;
 
 mod sys;
 mod test_fcntl;
-#[cfg(any(target_os = "linux"))]
+#[cfg(target_os = "linux")]
 mod test_mq;
 mod test_net;
 mod test_nix_path;
@@ -45,8 +45,8 @@ lazy_static! {
     /// Any test that creates child processes must grab this mutex, regardless
     /// of what it does with those children.
     pub static ref FORK_MTX: Mutex<()> = Mutex::new(());
-    /// Any test that registers a SIGUSR2 handler must grab this mutex
-    pub static ref SIGUSR2_MTX: Mutex<()> = Mutex::new(());
+    /// Any test that alters signal handling must grab this mutex.
+    pub static ref SIGNAL_MTX: Mutex<()> = Mutex::new(());
 }
 
 #[test]

test/test_mq.rs

@@ -8,9 +8,6 @@ use std::ffi::CString;
 use std::str;
 use libc::c_long;
 
-use nix::unistd::{fork, read, write, pipe};
-use nix::unistd::ForkResult::*;
-use nix::sys::wait::*;
 use nix::errno::Errno::*;
 use nix::Error::Sys;
 

test/test_signalfd.rs

@@ -227,10 +227,6 @@ fn test_lseek64() {
 
 execve_test_factory!(test_execve, execve, b"/bin/sh", b"/system/bin/sh");
 
-#[cfg(any(target_os = "linux", target_os = "android"))]
-#[cfg(feature = "execvpe")]
-execve_test_factory!(test_execvpe, execvpe, b"sh", b"sh");
-
 #[test]
 fn test_fpathconf_limited() {
     let f = tempfile().unwrap();