diff --git a/Cargo.lock b/Cargo.lock index be16f95a5..2fba0c7a7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -164,6 +164,7 @@ dependencies = [ "fish-gettext-maps", "fish-gettext-mo-file-parser", "fish-printf", + "fish-tempfile", "libc", "lru", "macro_rules_attribute", diff --git a/Cargo.toml b/Cargo.toml index fc144f1e7..76f0eacdb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -89,6 +89,7 @@ fish-build-man-pages = { workspace = true, optional = true } fish-gettext-extraction = { workspace = true, optional = true } fish-gettext-maps = { workspace = true, optional = true } fish-printf.workspace = true +fish-tempfile.workspace = true libc.workspace = true lru.workspace = true macro_rules_attribute = "0.2.2" @@ -170,7 +171,7 @@ rust.non_upper_case_globals = "allow" rust.unknown_lints = "allow" rust.unstable_name_collisions = "allow" rustdoc.private_intra_doc_links = "allow" -clippy.len_without_is_empty = "allow" # we're not a library crate +clippy.len_without_is_empty = "allow" # we're not a library crate clippy.let_and_return = "allow" clippy.manual_range_contains = "allow" clippy.needless_lifetimes = "allow" diff --git a/src/autoload.rs b/src/autoload.rs index d363fabd9..9c7214441 100644 --- a/src/autoload.rs +++ b/src/autoload.rs @@ -496,7 +496,7 @@ mod tests { #[serial] fn test_autoload() { let _cleanup = test_init(); - use crate::common::{charptr2wcstring, wcs2zstring}; + use crate::common::wcs2zstring; use crate::fds::wopen_cloexec; use nix::fcntl::OFlag; @@ -521,10 +521,10 @@ fn touch_file(path: &wstr) { file.write_all(b"Hello").unwrap(); } - let mut t1 = "/tmp/fish_test_autoload.XXXXXX\0".as_bytes().to_vec(); - let p1 = charptr2wcstring(unsafe { libc::mkdtemp(t1.as_mut_ptr().cast()) }); - let mut t2 = "/tmp/fish_test_autoload.XXXXXX\0".as_bytes().to_vec(); - let p2 = charptr2wcstring(unsafe { libc::mkdtemp(t2.as_mut_ptr().cast()) }); + let p1 = fish_tempfile::new_dir().unwrap(); + let p1 = WString::from(p1.path().to_str().unwrap()); + let p2 = fish_tempfile::new_dir().unwrap(); + let p2 = WString::from(p2.path().to_str().unwrap()); let paths = &[p1.clone(), p2.clone()]; let mut autoload = Autoload::new(L!("test_var")); @@ -603,8 +603,5 @@ fn touch_file(path: &wstr) { autoload.invalidate_cache(); assert!(autoload.resolve_command_impl(L!("file1"), paths).is_some()); autoload.mark_autoload_finished(L!("file1")); - - run!(L!("rm -Rf %s"), p1); - run!(L!("rm -Rf %s"), p2); } } diff --git a/src/highlight/file_tester.rs b/src/highlight/file_tester.rs index 897415620..51e99d134 100644 --- a/src/highlight/file_tester.rs +++ b/src/highlight/file_tester.rs @@ -425,49 +425,39 @@ mod tests { use crate::tests::prelude::*; use crate::wchar::prelude::*; - use crate::common::charptr2wcstring; use crate::redirection::RedirectionMode; use std::fs::{self, File, Permissions, create_dir_all}; use std::os::unix::fs::PermissionsExt; + use std::path::PathBuf; - struct TempDir { - basepath: WString, + struct TempDirWithCtx { + tempdir: fish_tempfile::TempDir, ctx: OperationContext<'static>, } - impl TempDir { - fn new() -> TempDir { - let mut t1 = *b"/tmp/fish_file_tester_dir.XXXXXX\0"; - let basepath_narrow = unsafe { libc::mkdtemp(t1.as_mut_ptr().cast()) }; - assert!(!basepath_narrow.is_null(), "mkdtemp failed"); - let basepath: WString = charptr2wcstring(basepath_narrow); - TempDir { - basepath, + impl TempDirWithCtx { + fn new() -> TempDirWithCtx { + TempDirWithCtx { + tempdir: fish_tempfile::new_dir().unwrap(), ctx: OperationContext::empty(), } } - fn filepath(&self, name: &str) -> String { - let mut result = self.basepath.to_string(); - result.push('/'); - result.push_str(name); - result + fn filepath(&self, name: &str) -> PathBuf { + self.tempdir.path().join(name) } fn file_tester(&self) -> FileTester<'_> { - FileTester::new(self.basepath.clone(), &self.ctx) - } - } - - impl Drop for TempDir { - fn drop(&mut self) { - let _ = std::fs::remove_dir_all(self.basepath.to_string()); + FileTester::new( + WString::from_str(self.tempdir.path().to_str().unwrap()), + &self.ctx, + ) } } #[test] fn test_ispath() { - let temp = TempDir::new(); + let temp = TempDirWithCtx::new(); let tester = temp.file_tester(); let file_path = temp.filepath("file.txt"); @@ -516,7 +506,7 @@ fn test_ispath() { #[test] fn test_iscdpath() { - let temp = TempDir::new(); + let temp = TempDirWithCtx::new(); let tester = temp.file_tester(); // Note cd (unlike file paths) should report IsErr for invalid cd paths, @@ -547,7 +537,7 @@ fn test_iscdpath() { #[test] fn test_redirections() { // Note we use is_ok and is_err since we don't care about the IsFile part. - let temp = TempDir::new(); + let temp = TempDirWithCtx::new(); let tester = temp.file_tester(); let file_path = temp.filepath("file.txt"); File::create(&file_path).unwrap(); diff --git a/src/history.rs b/src/history.rs index 2e9ed58ab..c7bce63a8 100644 --- a/src/history.rs +++ b/src/history.rs @@ -1836,7 +1836,6 @@ mod tests { use rand::Rng; use rand::rngs::SmallRng; use std::collections::VecDeque; - use std::ffi::CString; use std::io::BufReader; use std::os::unix::ffi::OsStrExt; use std::sync::Arc; @@ -2314,21 +2313,18 @@ fn test_history_merge() { fn test_history_path_detection() { let _cleanup = test_init(); // Regression test for #7582. - let tmpdirbuff = CString::new("/tmp/fish_test_history.XXXXXX").unwrap(); - let tmpdir = unsafe { libc::mkdtemp(tmpdirbuff.into_raw()) }; - let tmpdir = unsafe { CString::from_raw(tmpdir) }; - let mut tmpdir = bytes2wcstring(tmpdir.to_bytes()); - if !tmpdir.ends_with('/') { - tmpdir.push('/'); - } + let tmpdir = fish_tempfile::new_dir().unwrap(); // Place one valid file in the directory. let filename = L!("testfile"); - std::fs::write(wcs2osstring(&(tmpdir.clone() + filename)), []).unwrap(); + let file_path = tmpdir.path().join(filename.to_string()); + let wfile_path = WString::from(file_path.to_str().unwrap()); + std::fs::write(&file_path, []).unwrap(); + let wdir_path = WString::from(tmpdir.path().to_str().unwrap()); let test_vars = EnvStack::new(); - test_vars.set_one(L!("PWD"), EnvMode::GLOBAL, tmpdir.clone()); - test_vars.set_one(L!("HOME"), EnvMode::GLOBAL, tmpdir.clone()); + test_vars.set_one(L!("PWD"), EnvMode::GLOBAL, wdir_path.clone()); + test_vars.set_one(L!("HOME"), EnvMode::GLOBAL, wdir_path.clone()); let history = History::with_name(L!("path_detection")); history.clear(); @@ -2344,7 +2340,7 @@ fn test_history_path_detection() { PersistenceMode::Disk, ); history.clone().add_pending_with_file_detection( - &(L!("cmd2 ").to_owned() + &tmpdir[..] + L!("/") + filename), + &(L!("cmd2 ").to_owned() + &wfile_path[..]), &test_vars, PersistenceMode::Disk, ); @@ -2385,15 +2381,15 @@ fn test_history_path_detection() { // Expected sets of paths. let expected_paths = [ - vec![], // cmd0 - vec![filename.to_owned()], // cmd1 - vec![tmpdir.clone() + L!("/") + filename], // cmd2 - vec![L!("$HOME/").to_owned() + filename], // cmd3 - vec![], // cmd4 - vec![L!("~/").to_owned() + filename], // cmd5 - vec![], // cmd6 - vec![], // cmd7 - we do not expand globs - vec![], // cmd8 + vec![], // cmd0 + vec![filename.to_owned()], // cmd1 + vec![wfile_path], // cmd2 + vec![L!("$HOME/").to_owned() + filename], // cmd3 + vec![], // cmd4 + vec![L!("~/").to_owned() + filename], // cmd5 + vec![], // cmd6 + vec![], // cmd7 - we do not expand globs + vec![], // cmd8 ]; let maxlap = 128; @@ -2414,7 +2410,6 @@ fn test_history_path_detection() { std::thread::sleep(std::time::Duration::from_millis(2)); } history.clear(); - let _ = std::fs::remove_dir_all(wcs2osstring(&tmpdir)); } fn install_sample_history(name: &wstr) { diff --git a/src/universal_notifier/inotify.rs b/src/universal_notifier/inotify.rs index 114872faa..4a40df344 100644 --- a/src/universal_notifier/inotify.rs +++ b/src/universal_notifier/inotify.rs @@ -68,23 +68,16 @@ fn notification_fd_became_readable(&self, fd: RawFd) -> bool { #[cfg(test)] mod tests { use super::InotifyNotifier; - use crate::universal_notifier::{UniversalNotifier, test_helpers::test_notifiers}; + use crate::{ + universal_notifier::{UniversalNotifier, test_helpers::test_notifiers}, + wchar::WString, + }; #[test] fn test_inotify_notifiers() { - use crate::common::{cstr2wcstring, wcs2osstring}; - use std::ffi::CString; - use std::fs::remove_dir_all; - use std::path::PathBuf; - - let template = CString::new("/tmp/fish_inotify_XXXXXX").unwrap(); - let temp_dir_ptr = unsafe { libc::mkdtemp(template.into_raw() as *mut libc::c_char) }; - if temp_dir_ptr.is_null() { - panic!("failed to create temp dir"); - } - let tmp_dir = unsafe { CString::from_raw(temp_dir_ptr) }; - let fake_uvars_dir = cstr2wcstring(tmp_dir.as_bytes_with_nul()); - let fake_uvars_path = fake_uvars_dir.clone() + "/fish_variables"; + let temp_dir = fish_tempfile::new_dir().unwrap(); + let fake_uvars_path = + WString::from(temp_dir.path().join("fish_variables").to_str().unwrap()); let mut notifiers = Vec::new(); for _ in 0..16 { @@ -97,7 +90,5 @@ fn test_inotify_notifiers() { .map(|n| n as &dyn UniversalNotifier) .collect::>(); test_notifiers(¬ifiers, Some(&fake_uvars_path)); - - let _ = remove_dir_all(PathBuf::from(wcs2osstring(&fake_uvars_dir))); } } diff --git a/src/universal_notifier/kqueue.rs b/src/universal_notifier/kqueue.rs index 6bbda35bc..72f1ac5b7 100644 --- a/src/universal_notifier/kqueue.rs +++ b/src/universal_notifier/kqueue.rs @@ -182,24 +182,16 @@ fn notification_fd_became_readable(&self, fd: RawFd) -> bool { mod tests { use super::KqueueNotifier; use crate::common::wcs2osstring; - use crate::universal_notifier::{UniversalNotifier, test_helpers::test_notifiers}; + use crate::{ + universal_notifier::{UniversalNotifier, test_helpers::test_notifiers}, + wchar::WString, + }; #[test] fn test_kqueue_notifiers() { - use crate::common::cstr2wcstring; - use std::ffi::CStr; - use std::fs::remove_dir_all; - use std::path::PathBuf; - - let mut template: Box<[u8]> = Box::from(&b"/tmp/fish_kqueue_XXXXXX\0"[..]); - - let temp_dir_ptr = unsafe { libc::mkdtemp(template.as_mut_ptr().cast()) }; - if temp_dir_ptr.is_null() { - panic!("failed to create temp dir"); - } - let tmp_dir = unsafe { CStr::from_ptr(temp_dir_ptr) }; - let fake_uvars_dir = cstr2wcstring(tmp_dir.to_bytes_with_nul()); - let fake_uvars_path = fake_uvars_dir.clone() + "/fish_variables"; + let temp_dir = fish_tempfile::new_dir().unwrap(); + let fake_uvars_path = + WString::from(temp_dir.path().join("fish_variables").to_str().unwrap()); let mut notifiers = Vec::new(); for _ in 0..16 { @@ -211,7 +203,5 @@ fn test_kqueue_notifiers() { .map(|n| n as &dyn UniversalNotifier) .collect::>(); test_notifiers(¬ifiers, Some(&fake_uvars_path)); - - let _ = remove_dir_all(PathBuf::from(wcs2osstring(&fake_uvars_dir))); } } diff --git a/src/wutil/dir_iter.rs b/src/wutil/dir_iter.rs index 36bdc48af..5ae312a49 100644 --- a/src/wutil/dir_iter.rs +++ b/src/wutil/dir_iter.rs @@ -329,8 +329,10 @@ fn next(&mut self) -> Option { #[cfg(test)] mod tests { use super::{DirEntryType, DirIter}; - use crate::common::wcs2zstring; use crate::wchar::prelude::*; + use nix::sys::stat::Mode; + use std::fs::File; + use std::path::PathBuf; #[test] fn test_dir_iter_bad_path() { @@ -374,11 +376,8 @@ fn test_dots() { #[test] #[allow(clippy::if_same_then_else)] fn test_dir_iter() { - use crate::common::charptr2wcstring; - use crate::common::wcs2osstring; use crate::wchar::L; - use libc::{EACCES, ENOENT, O_CREAT, O_WRONLY}; - use std::ffi::CString; + use libc::{EACCES, ENOENT}; let baditer = DirIter::new(L!("/definitely/not/a/valid/directory/for/sure")); assert!(baditer.is_err()); @@ -388,17 +387,10 @@ fn test_dir_iter() { let err = err.raw_os_error().expect("Should have an errno value"); assert!(err == ENOENT || err == EACCES); - let mut t1: [u8; 31] = *b"/tmp/fish_test_dir_iter.XXXXXX\0"; - let basepath_narrow = unsafe { libc::mkdtemp(t1.as_mut_ptr().cast()) }; - assert!(!basepath_narrow.is_null(), "mkdtemp failed"); - let basepath: WString = charptr2wcstring(basepath_narrow); + let temp_dir = fish_tempfile::new_dir().unwrap(); + let basepath = WString::from(temp_dir.path().to_str().unwrap()); - let makepath = |s: &str| -> CString { - let mut tmp = basepath.clone(); - tmp.push('/'); - tmp.push_str(s); - wcs2zstring(&tmp) - }; + let makepath = |s: &str| -> PathBuf { temp_dir.path().join(s) }; let dirname = "dir"; let regname = "reg"; @@ -428,33 +420,18 @@ fn test_dir_iter() { }; // Make our different file types - unsafe { - let mut ret = libc::mkdir(makepath(dirname).as_ptr(), 0o700); - assert!(ret == 0); - ret = libc::open(makepath(regname).as_ptr(), O_CREAT | O_WRONLY, 0o600); - assert!(ret >= 0); - libc::close(ret); - #[cfg(not(cygwin))] - { - ret = libc::symlink(makepath(regname).as_ptr(), makepath(reglinkname).as_ptr()); - assert!(ret == 0); - ret = libc::symlink(makepath(dirname).as_ptr(), makepath(dirlinkname).as_ptr()); - assert!(ret == 0); - ret = libc::symlink( - c"/this/is/an/invalid/path".as_ptr().cast(), - makepath(badlinkname).as_ptr(), - ); - assert!(ret == 0); - ret = libc::symlink( - makepath(selflinkname).as_ptr(), - makepath(selflinkname).as_ptr(), - ); - assert!(ret == 0); - } + nix::unistd::mkdir(&makepath(dirname), Mode::from_bits(0o700).unwrap()).unwrap(); + File::create(makepath(regname)).unwrap(); + #[cfg(not(cygwin))] + { + use std::os::unix::fs::symlink; - ret = libc::mkfifo(makepath(fifoname).as_ptr(), 0o600); - assert!(ret == 0); + symlink(makepath(regname), makepath(reglinkname)).unwrap(); + symlink(makepath(dirname), makepath(dirlinkname)).unwrap(); + symlink("/this/is/an/invalid/path", makepath(badlinkname)).unwrap(); + symlink(makepath(selflinkname), makepath(selflinkname)).unwrap(); } + nix::unistd::mkfifo(&makepath(fifoname), Mode::from_bits(0o600).unwrap()).unwrap(); let mut iter1 = DirIter::new(&basepath).expect("Should be able to open directory"); let mut seen = 0; @@ -499,8 +476,5 @@ fn test_dir_iter() { ); } assert_eq!(seen, names.len()); - - // Clean up. - let _ = std::fs::remove_dir_all(wcs2osstring(&basepath)); } } diff --git a/src/wutil/mod.rs b/src/wutil/mod.rs index bd733f9e6..b3cd1ebc7 100644 --- a/src/wutil/mod.rs +++ b/src/wutil/mod.rs @@ -481,14 +481,13 @@ pub fn wstr_offset_in(cursor: &wstr, base: &wstr) -> usize { mod tests { use super::{normalize_path, wbasename, wdirname, wstr_offset_in, wwrite_to_fd}; use crate::common::wcs2bytes; + use crate::fds::AutoCloseFd; use crate::tests::prelude::*; use crate::util::get_rng; use crate::wchar::prelude::*; - use crate::{fds::AutoCloseFd, fs::create_temporary_file}; use libc::{O_CREAT, O_RDWR, O_TRUNC, SEEK_SET, c_void}; use rand::Rng; - use std::ffi::CString; - use std::ptr; + use std::{ffi::CString, ptr}; mod test_path_normalize_for_cd { use super::super::path_normalize_for_cd; @@ -664,8 +663,8 @@ fn test_wdirname_wbasename() { #[serial] fn test_wwrite_to_fd() { let _cleanup = test_init(); - let (_file, filename) = create_temporary_file(L!("/tmp/fish_test_wwrite.XXXXXX")).unwrap(); - let filename = CString::new(filename.to_string()).unwrap(); + let temp_file = fish_tempfile::new_file().unwrap(); + let filename = CString::new(temp_file.path().to_str().unwrap()).unwrap(); let mut rng = get_rng(); let sizes = [1, 2, 3, 5, 13, 23, 64, 128, 255, 4096, 4096 * 2]; for &size in &sizes { @@ -699,7 +698,6 @@ fn test_wwrite_to_fd() { assert!(usize::try_from(read_amt).unwrap() == narrow.len()); assert_eq!(&contents, &narrow); } - unsafe { libc::remove(filename.as_ptr()) }; } #[test]