Merge pull request #144 from asomers/deny-warnings

Fix all warnings, and deny them in CI

.cirrus.yml

@@ -1,6 +1,8 @@
 cargo_cache:
   folder: $CARGO_HOME/registry
   fingerprint_script: cat Cargo.lock || echo ""
+env:
+  RUSTFLAGS: -D warnings
 
 build: &BUILD
   build_script:

rust/Cargo.toml

@@ -10,7 +10,6 @@ rand = { version = "0.8.5", features = ["min_const_gen"] }
 strum = { version = "0.24.0", features = ["derive"] }
 strum_macros = "0.24.0"
 anyhow = "1.0.57"
-once_cell = "1.12.0"
 paste = "1.0.7"
 gumdrop = "0.8.1"
 figment = { version = "0.10.6", features = ["toml"] }

rust/src/context.rs

@@ -240,7 +240,6 @@ impl<'a> TestContext<'a> {
         let remaining_chars = max_path_len - initial_path_len;
 
         let parts: Vec<_> = (0..remaining_chars / component_len)
-            .into_iter()
             .map(|_| Alphanumeric.sample_string(&mut rand::thread_rng(), component_len - 1))
             .collect();
 

rust/src/main.rs

@@ -1,3 +1,6 @@
+// https://github.com/rust-lang/rust-clippy/issues/1553
+#![allow(clippy::redundant_closure_call)]
+
 use std::{
     backtrace::{Backtrace, BacktraceStatus},
     collections::HashSet,
@@ -18,7 +21,6 @@ use nix::{
     sys::stat::{umask, Mode},
     unistd::Uid,
 };
-use once_cell::sync::OnceCell;
 use strum::{EnumMessage, IntoEnumIterator};
 
 use tempfile::{tempdir_in, TempDir};
@@ -36,8 +38,6 @@ use test::{FileSystemFeature, SerializedTestContext, TestCase, TestContext, Test
 
 use crate::utils::chmod;
 
-struct PanicLocation(u32, u32, String);
-
 static BACKTRACE: Mutex<Option<Backtrace>> = Mutex::new(None);
 
 #[derive(Debug, Options)]
@@ -92,7 +92,7 @@ fn main() -> anyhow::Result<()> {
         .path
         .ok_or_else(|| anyhow::anyhow!("cannot get current dir"))
         .or_else(|_| current_dir())?;
-    let base_dir = tempdir_in(&path)?;
+    let base_dir = tempdir_in(path)?;
 
     set_hook(Box::new(|_| {
         *BACKTRACE.lock().unwrap() = Some(Backtrace::capture());
@@ -147,7 +147,7 @@ fn run_test_cases(
 
     let is_root = Uid::current().is_root();
 
-    let enabled_features: HashSet<_> = config.features.fs_features.keys().into_iter().collect();
+    let enabled_features: HashSet<_> = config.features.fs_features.keys().collect();
 
     let entries = &config.dummy_auth.entries;
 

rust/src/tests/chflags.rs

@@ -1,12 +1,11 @@
-use std::{collections::HashSet, iter::once};
+use std::{collections::HashSet, iter::once, sync::OnceLock};
 
 use nix::{
     errno::Errno,
     libc::fflags_t,
     sys::stat::{lstat, stat, FileFlag},
     unistd::chflags,
 };
-use once_cell::sync::Lazy;
 
 #[cfg(any(target_os = "netbsd", target_os = "freebsd", target_os = "dragonfly"))]
 use crate::utils::lchflags;
@@ -27,29 +26,26 @@ use super::{
 
 //TODO: Split tests with unprivileged tests for user flags
 
-const USER_FLAGS: Lazy<HashSet<FileFlags>> = Lazy::new(|| {
-    HashSet::from([
+fn get_flags(ctx: &TestContext) -> (FileFlag, FileFlag, FileFlag) {
+    static USER_FLAGS: OnceLock<HashSet<FileFlags>> = OnceLock::new();
+    USER_FLAGS.get_or_init(|| HashSet::from([
         FileFlags::UF_NODUMP,
         FileFlags::UF_IMMUTABLE,
         FileFlags::UF_APPEND,
         #[cfg(any(target_os = "freebsd", target_os = "dragonfly"))]
         FileFlags::UF_NOUNLINK,
         #[cfg(any(target_os = "freebsd", target_os = "dragonfly"))]
         FileFlags::UF_OPAQUE,
-    ])
-});
-
-const SYSTEM_FLAGS: Lazy<HashSet<FileFlags>> = Lazy::new(|| {
-    HashSet::from([
+    ]));
+    static SYSTEM_FLAGS: OnceLock<HashSet<FileFlags>> = OnceLock::new();
+    SYSTEM_FLAGS.get_or_init(|| HashSet::from([
         FileFlags::SF_ARCHIVED,
         FileFlags::SF_IMMUTABLE,
         FileFlags::SF_APPEND,
         #[cfg(any(target_os = "freebsd", target_os = "dragonfly"))]
         FileFlags::SF_NOUNLINK,
-    ])
-});
+    ]));
 
-fn get_flags(ctx: &TestContext) -> (FileFlag, FileFlag, FileFlag) {
     let allflags: FileFlag = ctx
         .features_config()
         .file_flags
@@ -61,15 +57,15 @@ fn get_flags(ctx: &TestContext) -> (FileFlag, FileFlag, FileFlag) {
     let user_flags: FileFlag = ctx
         .features_config()
         .file_flags
-        .intersection(&USER_FLAGS)
+        .intersection(USER_FLAGS.get().unwrap())
         .copied()
         .map(Into::into)
         .collect();
 
     let system_flags: FileFlag = ctx
         .features_config()
         .file_flags
-        .intersection(&SYSTEM_FLAGS)
+        .intersection(SYSTEM_FLAGS.get().unwrap())
         .copied()
         .map(Into::into)
         .collect();
@@ -203,7 +199,7 @@ fn unchanged_ctime_failed(ctx: &mut SerializedTestContext, ft: FileType) {
 
     for flag in allflags.iter().chain(once(&FileFlag::empty())) {
         assert_ctime_unchanged(ctx, &file, || {
-            ctx.as_user(&user, None, || {
+            ctx.as_user(user, None, || {
                 assert_eq!(chflags(&file, *flag), Err(Errno::EPERM));
             })
         });
@@ -214,7 +210,7 @@ fn unchanged_ctime_failed(ctx: &mut SerializedTestContext, ft: FileType) {
     #[cfg(any(target_os = "netbsd", target_os = "freebsd", target_os = "dragonfly"))]
     for flag in allflags.into_iter().chain(once(FileFlag::empty())) {
         assert_ctime_unchanged(ctx, &file, || {
-            ctx.as_user(&user, None, || {
+            ctx.as_user(user, None, || {
                 assert_eq!(lchflags(&file, flag), Err(Errno::EPERM));
             })
         });

rust/src/tests/chmod.rs

@@ -9,7 +9,6 @@ use crate::{
 use crate::utils::lchmod;
 
 use nix::{
-    libc::mode_t,
     sys::stat::{lstat, stat, Mode},
     unistd::chown,
 };
@@ -25,7 +24,8 @@ use super::errors::{
     erofs::erofs_named_test_case,
 };
 
-const ALLPERMS_STICKY: mode_t = ALLPERMS | Mode::S_ISVTX.bits();
+#[cfg(any(target_os = "freebsd", target_os = "dragonfly"))]
+const ALLPERMS_STICKY: nix::libc::mode_t = ALLPERMS | Mode::S_ISVTX.bits();
 
 // chmod/00.t:L24
 crate::test_case! {
@@ -168,13 +168,13 @@ fn eftype(ctx: &mut SerializedTestContext, ft: FileType) {
     let new_mode = Mode::from_bits_truncate(0o644);
     let link = ctx.create(FileType::Symlink(Some(file.clone()))).unwrap();
 
-    ctx.as_user(&user, None, || {
+    ctx.as_user(user, None, || {
         assert_eq!(chmod(&file, new_mode | Mode::S_ISVTX), Err(Errno::EFTYPE));
     });
     let file_stat = stat(&file).unwrap();
     assert_eq!(file_stat.st_mode & ALLPERMS_STICKY, original_mode.bits());
 
-    ctx.as_user(&user, None, || {
+    ctx.as_user(user, None, || {
         assert_eq!(
             chmod(&link, original_mode | Mode::S_ISVTX),
             Err(Errno::EFTYPE)
@@ -186,7 +186,7 @@ fn eftype(ctx: &mut SerializedTestContext, ft: FileType) {
     // lchmod
 
     let mode = Mode::from_bits_truncate(0o621) | Mode::S_ISVTX;
-    ctx.as_user(&user, None, || {
+    ctx.as_user(user, None, || {
         assert_eq!(lchmod(&file, mode), Err(Errno::EFTYPE));
     });
 

rust/src/tests/mknod.rs

@@ -120,8 +120,8 @@ fn device_files(ctx: &mut TestContext, ft: FileType) {
 
     let stat = symlink_metadata(&file).unwrap();
     assert_eq!(stat.permissions().mode() as mode_t & ALLPERMS, mode);
-    assert_eq!(major(stat.rdev()) as u64, major_num as u64);
-    assert_eq!(minor(stat.rdev()) as u64, minor_num as u64);
+    assert_eq!(major(stat.rdev()), major_num);
+    assert_eq!(minor(stat.rdev()), minor_num);
     assert!(check(&stat.file_type()));
 }
 

rust/src/tests/mod.rs

@@ -294,163 +294,3 @@ where
         .path(path, CTIME)
         .execute(ctx, true, f)
 }
-
-/// Assert that a certain operation does not change the mtime of a file.
-fn assert_mtime_unchanged<F>(ctx: &TestContext, path: &Path, f: F)
-where
-    F: FnOnce(),
-{
-    assert_times_unchanged()
-        .path(path, MTIME)
-        .execute(ctx, false, f)
-}
-
-/// Guard to conditionally skip tests on platforms which do not support
-/// the required file flags.
-macro_rules! support_file_flags {
-    ($($flags: ident),*) => {
-        |config, _| {
-            let flags = &[ $(crate::flags::FileFlags::$flags),* ].iter().copied().collect();
-            if config.features.file_flags.is_superset(&flags) {
-                Ok(())
-            } else {
-                let unsupported_flags = flags.difference(&config.features.file_flags)
-                    .map(std::string::ToString::to_string)
-                    .collect::<Vec<_>>()
-                    .join(", ");
-
-                anyhow::bail!("file flags {unsupported_flags} aren't supported")
-            }
-        }
-    };
-}
-
-#[cfg(test)]
-mod t {
-    use crate::{config::Config, test::TestCase};
-
-    use super::*;
-
-    crate::test_case! {support_flags_empty; support_file_flags!()}
-    fn support_flags_empty(_: &mut TestContext) {}
-    #[test]
-    fn support_flags_test_empty() {
-        let config = Config::default();
-        let tc: &TestCase = inventory::iter::<TestCase>()
-            .find(|tc| tc.name == "pjdfstest::tests::t::support_flags_empty")
-            .unwrap();
-        assert_eq!(tc.guards.len(), 1);
-
-        let guard = &tc.guards[0];
-        assert!(guard(&config, Path::new("test")).is_ok());
-    }
-
-    #[cfg(any(
-        target_os = "openbsd",
-        target_os = "netbsd",
-        target_os = "freebsd",
-        target_os = "dragonfly",
-        target_os = "macos",
-        target_os = "ios",
-        target_os = "watchos",
-    ))]
-    crate::test_case! {support_flags_unique; support_file_flags!(SF_APPEND)}
-    #[cfg(any(
-        target_os = "openbsd",
-        target_os = "netbsd",
-        target_os = "freebsd",
-        target_os = "dragonfly",
-        target_os = "macos",
-        target_os = "ios",
-        target_os = "watchos",
-    ))]
-    fn support_flags_unique(_: &mut TestContext) {}
-    #[cfg(any(
-        target_os = "openbsd",
-        target_os = "netbsd",
-        target_os = "freebsd",
-        target_os = "dragonfly",
-        target_os = "macos",
-        target_os = "ios",
-        target_os = "watchos",
-    ))]
-    #[test]
-    fn support_flags_test_unique() {
-        use std::collections::HashSet;
-
-        use crate::flags::FileFlags;
-
-        let mut config = Config::default();
-        let tc: &TestCase = inventory::iter::<TestCase>()
-            .find(|tc| tc.name == "pjdfstest::tests::t::support_flags_unique")
-            .unwrap();
-        assert_eq!(tc.guards.len(), 1);
-
-        let guard = &tc.guards[0];
-        assert!(guard(&config, Path::new("test")).is_err());
-
-        config.features.file_flags = HashSet::from([FileFlags::SF_APPEND]);
-        assert!(guard(&config, Path::new("test")).is_ok());
-
-        config.features.file_flags = HashSet::from([FileFlags::SF_APPEND, FileFlags::UF_APPEND]);
-        assert!(guard(&config, Path::new("test")).is_ok());
-    }
-
-    #[cfg(any(
-        target_os = "openbsd",
-        target_os = "netbsd",
-        target_os = "freebsd",
-        target_os = "dragonfly",
-        target_os = "macos",
-        target_os = "ios",
-        target_os = "watchos",
-    ))]
-    crate::test_case! {support_flags_not_empty; support_file_flags!(SF_APPEND, UF_APPEND)}
-    #[cfg(any(
-        target_os = "openbsd",
-        target_os = "netbsd",
-        target_os = "freebsd",
-        target_os = "dragonfly",
-        target_os = "macos",
-        target_os = "ios",
-        target_os = "watchos",
-    ))]
-    fn support_flags_not_empty(_: &mut TestContext) {}
-    #[cfg(any(
-        target_os = "openbsd",
-        target_os = "netbsd",
-        target_os = "freebsd",
-        target_os = "dragonfly",
-        target_os = "macos",
-        target_os = "ios",
-        target_os = "watchos",
-    ))]
-    #[test]
-    fn support_flags_test_not_empty() {
-        use std::collections::HashSet;
-
-        use crate::flags::FileFlags;
-
-        let mut config = Config::default();
-        let tc: &TestCase = inventory::iter::<TestCase>()
-            .find(|tc| tc.name == "pjdfstest::tests::t::support_flags_not_empty")
-            .unwrap();
-        assert_eq!(tc.guards.len(), 1);
-
-        let guard = &tc.guards[0];
-        assert!(guard(&config, Path::new("test")).is_err());
-
-        config.features.file_flags = HashSet::from([FileFlags::SF_APPEND]);
-        assert!(guard(&config, Path::new("test")).is_err());
-
-        config.features.file_flags = HashSet::from([FileFlags::SF_APPEND, FileFlags::UF_APPEND]);
-        assert!(guard(&config, Path::new("test")).is_ok());
-
-        config.features.file_flags = HashSet::from([
-            FileFlags::SF_APPEND,
-            FileFlags::UF_APPEND,
-            FileFlags::SF_ARCHIVED,
-        ]);
-        assert!(guard(&config, Path::new("test")).is_ok());
-    }
-}