Ver Fonte

melib/utils: fix test_fd_locks() on platforms without OFD support

Not all platforms support open file description locks.

It is part of POSIX-2024 [1] so hopefully this will improve in the future.

[1]: "The Open Group Base Specifications Issue 8 IEEE Std 1003.1-2024" <https://pubs.opengroup.org/onlinepubs/9799919799/basedefs/fcntl.h.html>

Fixes #522 "utils::tests::test_fd_locks fails on macOS"

Resolves: <https://git.meli-email.org/meli/meli/issues/522>
Signed-off-by: Manos Pitsidianakis <manos@pitsidianak.is>
Manos Pitsidianakis há 8 meses atrás
pai
commit
b7e215f9c2
2 ficheiros alterados com 107 adições e 15 exclusões
  1. 23 12
      melib/src/utils/lock.rs
  2. 84 3
      melib/src/utils/tests.rs

+ 23 - 12
melib/src/utils/lock.rs

@@ -46,16 +46,26 @@ impl FileLockOptions {
 #[derive(Debug)]
 pub struct FileLock<T: AsRawFd>(T);
 
-// F_OFD_SETLKW
-#[cfg(any(target_os = "linux", target_os = "android"))]
-const F_SETLKW: libc::c_int = 38;
-// F_OFD_SETLK
-#[cfg(any(target_os = "linux", target_os = "android"))]
-const F_SETLK: libc::c_int = 37;
-#[cfg(not(any(target_os = "linux", target_os = "android")))]
-const F_SETLKW: libc::c_int = libc::F_SETLKW;
-#[cfg(not(any(target_os = "linux", target_os = "android")))]
-const F_SETLK: libc::c_int = libc::F_SETLK;
+// OFD: fuchsia, hermit, netbsd, openbsd and solaris don't have it,
+cfg_if::cfg_if! {
+    if #[cfg(any(
+            target_os = "android",
+            target_os = "freebsd",
+            target_os = "illumos",
+            target_os = "ios",
+            target_os = "linux",
+            target_os = "macos"
+            ))] {
+        const F_SETLKW: libc::c_int = libc::F_OFD_SETLKW;
+        const F_SETLK: libc::c_int = libc::F_OFD_SETLK;
+    } else if #[cfg(target_os = "freebsd")] {
+        const F_SETLKW: libc::c_int = libc::F_OSETLKW;
+        const F_SETLK: libc::c_int = libc::F_OSETLK;
+    } else {
+        const F_SETLKW: libc::c_int = libc::F_SETLKW;
+        const F_SETLK: libc::c_int = libc::F_SETLK;
+    }
+}
 
 impl<T> Drop for FileLock<T>
 where
@@ -89,9 +99,10 @@ where
     // # man fcntl
     fn lock(self, options: FileLockOptions, path: &Path) -> Result<FileLock<T>> {
         let fd: libc::c_int = self.as_raw_fd();
+        #[allow(clippy::as_underscore)]
         let mut flock: libc::flock = libc::flock {
-            l_type: libc::F_WRLCK as libc::c_short,
-            l_whence: libc::SEEK_SET as libc::c_short,
+            l_type: libc::F_WRLCK as _,
+            l_whence: libc::SEEK_SET as _,
             l_start: 0,
             l_len: 0, /* Specifying 0 for l_len has the special meaning: lock all bytes starting
                        * at the location specified by l_whence and l_start through to the end of

+ 84 - 3
melib/src/utils/tests.rs

@@ -30,8 +30,6 @@ use std::{
 use sealed_test::prelude::*;
 use tempfile::TempDir;
 
-use crate::error::{Errno, ErrorKind};
-
 #[sealed_test]
 #[ignore]
 #[test]
@@ -265,9 +263,21 @@ fn test_shellexpandtrait_impls() {
     _ = tmp_dir.close();
 }
 
+// Only test on platforms that support OFD locking.
+#[cfg(any(
+    target_os = "android",
+    target_os = "freebsd",
+    target_os = "illumos",
+    target_os = "ios",
+    target_os = "linux",
+    target_os = "macos",
+))]
 #[test]
 fn test_fd_locks() {
-    use crate::utils::lock::*;
+    use crate::{
+        error::{Errno, ErrorKind},
+        utils::lock::*,
+    };
 
     fn create_file_util(path: &Path) -> File {
         use super::shellexpand::ShellExpandTrait;
@@ -335,6 +345,77 @@ fn test_fd_locks() {
         .unwrap();
 }
 
+// Test that locking is a no-op on platforms that don't support OFD locking.
+#[cfg(not(any(
+    target_os = "android",
+    target_os = "freebsd",
+    target_os = "illumos",
+    target_os = "ios",
+    target_os = "linux",
+    target_os = "macos",
+)))]
+#[test]
+fn test_fd_locks() {
+    use crate::utils::lock::*;
+
+    fn create_file_util(path: &Path) -> File {
+        use super::shellexpand::ShellExpandTrait;
+
+        let path = Path::new(path).expand();
+        let mut f = File::create(&path).unwrap();
+        let mut permissions = f.metadata().unwrap().permissions();
+        permissions.set_mode(0o600); // Read/write for owner only.
+        f.set_permissions(permissions).unwrap();
+        f.write_all(b"\n").unwrap();
+        f.flush().unwrap();
+        assert!(path.exists());
+        f
+    }
+
+    let tmp_dir = TempDir::new().unwrap();
+
+    let file_path = tmp_dir.path().join("test.txt");
+
+    let f = create_file_util(&file_path)
+        .lock(FileLockOptions::Blocking, &file_path)
+        .unwrap();
+    let open = || {
+        OpenOptions::new()
+            .read(true)
+            .write(true)
+            .open(&file_path)
+            .unwrap()
+    };
+    let f2 = open();
+    f2.lock(
+        FileLockOptions::Nonblocking {
+            max_tries: 0,
+            try_wait: None,
+        },
+        &file_path,
+    )
+    .unwrap();
+    open()
+        .lock(
+            FileLockOptions::Nonblocking {
+                max_tries: 1,
+                try_wait: None,
+            },
+            &file_path,
+        )
+        .unwrap();
+    drop(f);
+    open()
+        .lock(
+            FileLockOptions::Nonblocking {
+                max_tries: 0,
+                try_wait: None,
+            },
+            &file_path,
+        )
+        .unwrap();
+}
+
 #[test]
 fn test_fnmatch() {
     use crate::utils::fnmatch::*;