Merge pull request #16 from niklasmohrin/nix-0.29

Bump nix to 0.29, bump MSRV, refactor tests

.github/workflows/ci.yml

@@ -13,7 +13,7 @@ jobs:
       fail-fast: false
       matrix:
         os: [ubuntu-latest, windows-latest, macos-latest]
-        rust: ["1.63.0", stable]
+        rust: ["1.69.0", stable]
     runs-on: ${{ matrix.os }}
     steps:
       - uses: actions/checkout@v4
@@ -22,7 +22,7 @@ jobs:
           toolchain: ${{ matrix.rust }}
           components: clippy
       - run: cargo build
-      - run: cargo test
+      - run: cargo test -F test-close-again -- --test-threads 1
       - run: cargo clippy
 
   build_extra:

Cargo.toml

@@ -3,7 +3,7 @@ name = "clircle"
 version = "0.5.0"
 authors = ["Niklas Mohrin <[email protected]>"]
 license = "MIT OR Apache-2.0"
-rust-version = "1.63"
+rust-version = "1.69"
 edition = "2021"
 description = "Detect IO circles in your CLI apps arguments."
 homepage = "https://github.com/niklasmohrin/clircle"
@@ -16,22 +16,20 @@ keywords = ["cycle", "arguments", "argv", "io"]
 [features]
 default = ["serde"]
 serde = ["dep:serde", "dep:serde_derive"]
+test-close-again = []
 
 [dependencies]
 serde = { version = "1.0.117", optional = true }
 serde_derive = { version = "1.0.117", optional = true }
 cfg-if = "1.0.0"
 
-[target.'cfg(not(windows))'.dependencies]
-libc = "0.2"
-
 [target.'cfg(windows)'.dependencies]
 winapi = { version = "0.3.9", features = ["winnt", "winbase", "processenv", "handleapi", "ntdef", "fileapi", "std"] }
 
 [dev-dependencies]
 tempfile = "3.1.0"
 
-[target.'cfg(not(windows))'.dev-dependencies.nix]
-version = "0.24.1"
+[target.'cfg(unix)'.dev-dependencies.nix]
+version = "0.29"
 default-features = false
 features = ["term"]

src/clircle_unix.rs

@@ -3,13 +3,11 @@ use crate::{Clircle, Stdio};
 use std::convert::TryFrom;
 use std::fs::File;
 use std::io::{self, Seek};
+use std::os::fd::AsRawFd;
 use std::os::unix::fs::MetadataExt;
 use std::os::unix::io::{FromRawFd, IntoRawFd, RawFd};
 use std::{cmp, hash, ops};
 
-/// Re-export of libc
-pub use libc;
-
 /// Implementation of `Clircle` for Unix.
 #[derive(Debug)]
 pub struct UnixIdentifier {
@@ -80,9 +78,9 @@ impl TryFrom<Stdio> for UnixIdentifier {
 
     fn try_from(stdio: Stdio) -> Result<Self, Self::Error> {
         let fd = match stdio {
-            Stdio::Stdin => libc::STDIN_FILENO,
-            Stdio::Stdout => libc::STDOUT_FILENO,
-            Stdio::Stderr => libc::STDERR_FILENO,
+            Stdio::Stdin => io::stdin().as_raw_fd(),
+            Stdio::Stdout => io::stdout().as_raw_fd(),
+            Stdio::Stderr => io::stderr().as_raw_fd(),
         };
         // Safety: It is okay to create the file, because it won't be dropped later since the
         // `owns_fd` field is not set.
@@ -133,88 +131,78 @@ impl hash::Hash for UnixIdentifier {
 mod tests {
     use super::*;
 
-    use std::error::Error;
     use std::io::Write;
+    use std::os::fd::OwnedFd;
 
     use nix::pty::{openpty, OpenptyResult};
     use nix::unistd::close;
 
     #[test]
-    fn test_fd_closing() -> Result<(), Box<dyn Error>> {
-        let dir = tempfile::tempdir().expect("Couldn't create tempdir.");
-        let dir_path = dir.path().to_path_buf();
-
-        // 1) Check that the file returned by into_inner is still valid
-        let file = File::create(dir_path.join("myfile"))?;
-        let ident = UnixIdentifier::try_from(file)?;
+    fn test_into_inner() {
+        let file = tempfile::tempfile().expect("failed to create tempfile");
+        file.metadata().expect("can stat file");
+        let ident = UnixIdentifier::try_from(file).expect("failed to create identifier");
         let mut file = ident
             .into_inner()
-            .ok_or("Did not get file back from identifier")?;
-        // Check if file can be written to without weird errors
-        file.write_all(b"Some test content")?;
-
-        // 2) Check that dropping the Identifier does not close the file, if owns_fd is false
-        let fd = file.into_raw_fd();
-        let ident = unsafe { UnixIdentifier::try_from_raw_fd(fd, false) };
-        if let Err(e) = ident {
-            let _ = dbg!(close(fd));
-            return Err(Box::new(e));
-        }
-        let ident = ident.unwrap();
-        drop(ident);
-        close(fd).map_err(|e| {
-            format!(
-                "Error closing file, that I told UnixIdentifier not to close: {}",
-                e
-            )
-        })?;
-
-        // 3) Check that the file is closed on drop, if owns_fd is true
-        let fd = File::open(dir_path.join("myfile"))?.into_raw_fd();
-        let ident = unsafe { UnixIdentifier::try_from_raw_fd(fd, true) };
-        if let Err(e) = ident {
-            let _ = dbg!(close(fd));
-            return Err(Box::new(e));
-        }
-        let ident = ident.unwrap();
+            .expect("failed to convert identifier to file");
+        file.write_all(b"some test content")
+            .expect("failed to write test content to file");
+    }
+
+    #[test]
+    fn test_borrowed_fd() {
+        let file = tempfile::tempfile().expect("failed to create tempfile");
+        let fd: OwnedFd = file.into();
+        let ident = unsafe { UnixIdentifier::try_from_raw_fd(fd.as_raw_fd(), false) }
+            .expect("failed to create identifier");
         drop(ident);
-        close(fd).expect_err("This file descriptor should have been closed already!");
+        let fd = fd.into_raw_fd();
+        close(fd).expect("error closing fd");
+        #[cfg(feature = "test-close-again")]
+        close(fd).expect_err("closing again should fail");
+    }
 
-        Ok(())
+    #[test]
+    fn test_owned_fd() {
+        let file = tempfile::tempfile().expect("failed to create tempfile");
+        let fd: OwnedFd = file.into();
+        let ident = unsafe { UnixIdentifier::try_from_raw_fd(fd.as_raw_fd(), true) }
+            .expect("failed to create identifier");
+        drop(ident);
+        #[cfg(feature = "test-close-again")]
+        close(fd.into_raw_fd())
+            .expect_err("the fd should have already been closed by dropping the identifier");
     }
 
     #[test]
-    fn test_pty_equal_but_not_conflicting() -> Result<(), &'static str> {
-        let OpenptyResult { master, slave } = openpty(None, None).expect("Could not open pty.");
-        let res = unsafe { UnixIdentifier::try_from_raw_fd(slave, false) }
-            .map_err(|_| "Error creating UnixIdentifier from pty fd")
-            .and_then(|ident| {
-                if !ident.eq(&ident) {
-                    return Err("ident != ident");
-                }
-                if ident.surely_conflicts_with(&ident) {
-                    return Err("pty fd does not conflict with itself, but conflict detected");
-                }
-
-                let second_ident = unsafe { UnixIdentifier::try_from_raw_fd(slave, false) }
-                    .map_err(|_| "Error creating second Identifier to pty")?;
-                if !ident.eq(&second_ident) {
-                    return Err("ident != second_ident");
-                }
-                if ident.surely_conflicts_with(&second_ident) {
-                    return Err(
-                        "Two Identifiers to the same pty should not conflict, but they do.",
-                    );
-                }
-                Ok(())
-            });
-
-        let r1 = close(master);
-        let r2 = close(slave);
-
-        r1.expect("Error closing master end of pty");
-        r2.expect("Error closing slave end of pty");
-
-        res
+    fn test_pty_equal_but_not_conflicting() {
+        let OpenptyResult {
+            master: parent,
+            slave: child,
+        } = openpty(None, None).expect("failed to open pty");
+
+        let parent_ident = unsafe { UnixIdentifier::try_from_raw_fd(parent.as_raw_fd(), false) }
+            .expect("failed to create parent identifier");
+
+        assert_eq!(parent_ident, parent_ident);
+
+        assert!(!parent_ident.surely_conflicts_with(&parent_ident));
+
+        let child_ident = unsafe { UnixIdentifier::try_from_raw_fd(child.as_raw_fd(), false) }
+            .expect("failed to create child identifier");
+
+        assert_ne!(parent_ident, child_ident);
+        assert!(!parent_ident.surely_conflicts_with(&child_ident));
+
+        drop(child_ident);
+        drop(parent_ident);
+        let child = child.into_raw_fd();
+        close(child).expect("failed to close child");
+        #[cfg(feature = "test-close-again")]
+        close(child).expect_err("closing child again should fail");
+        let parent = parent.into_raw_fd();
+        close(parent).expect("failed to close parent");
+        #[cfg(feature = "test-close-again")]
+        close(parent).expect_err("closing parent again should fail");
     }
 }

src/lib.rs

@@ -47,7 +47,7 @@
 cfg_if::cfg_if! {
     if #[cfg(unix)] {
         mod clircle_unix;
-        pub use clircle_unix::{libc, UnixIdentifier};
+        pub use clircle_unix::UnixIdentifier;
         /// Identifies a file. The type is aliased according to the target platform.
         pub type Identifier = UnixIdentifier;
     } else if #[cfg(windows)] {