From 6e995724c10bc9c5f524df2575c8f3fe1a7df875 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 6 Aug 2024 21:39:19 +0200 Subject: [PATCH] add keep() function to builder that suppresses delete-on-drop behavior (#293) fixes #194 --- src/dir/imp/any.rs | 7 +++++- src/dir/imp/unix.rs | 7 +++++- src/dir/mod.rs | 8 +++++-- src/file/imp/unix.rs | 3 +-- src/file/imp/windows.rs | 3 +-- src/file/mod.rs | 19 +++++++++++++--- src/lib.rs | 49 +++++++++++++++++++++++++++++------------ src/util.rs | 5 ++--- tests/namedtempfile.rs | 17 ++++++++++++++ tests/tempdir.rs | 9 ++++++++ 10 files changed, 99 insertions(+), 28 deletions(-) diff --git a/src/dir/imp/any.rs b/src/dir/imp/any.rs index 015f9f87d..512a54e2b 100644 --- a/src/dir/imp/any.rs +++ b/src/dir/imp/any.rs @@ -7,7 +7,11 @@ fn not_supported(msg: &str) -> io::Result { Err(io::Error::new(io::ErrorKind::Other, msg)) } -pub fn create(path: PathBuf, permissions: Option<&std::fs::Permissions>) -> io::Result { +pub fn create( + path: PathBuf, + permissions: Option<&std::fs::Permissions>, + keep: bool, +) -> io::Result { if permissions.map_or(false, |p| p.readonly()) { return not_supported("changing permissions is not supported on this platform"); } @@ -15,5 +19,6 @@ pub fn create(path: PathBuf, permissions: Option<&std::fs::Permissions>) -> io:: .with_err_path(|| &path) .map(|_| TempDir { path: path.into_boxed_path(), + keep, }) } diff --git a/src/dir/imp/unix.rs b/src/dir/imp/unix.rs index 47dd125c1..d69dac389 100644 --- a/src/dir/imp/unix.rs +++ b/src/dir/imp/unix.rs @@ -3,7 +3,11 @@ use crate::TempDir; use std::io; use std::path::PathBuf; -pub fn create(path: PathBuf, permissions: Option<&std::fs::Permissions>) -> io::Result { +pub fn create( + path: PathBuf, + permissions: Option<&std::fs::Permissions>, + keep: bool, +) -> io::Result { let mut dir_options = std::fs::DirBuilder::new(); #[cfg(not(target_os = "wasi"))] { @@ -17,5 +21,6 @@ pub fn create(path: PathBuf, permissions: Option<&std::fs::Permissions>) -> io:: .with_err_path(|| &path) .map(|_| TempDir { path: path.into_boxed_path(), + keep, }) } diff --git a/src/dir/mod.rs b/src/dir/mod.rs index ea9017d4c..067c74a6e 100644 --- a/src/dir/mod.rs +++ b/src/dir/mod.rs @@ -177,6 +177,7 @@ pub fn tempdir_in>(dir: P) -> io::Result { /// [`std::process::exit()`]: http://doc.rust-lang.org/std/process/fn.exit.html pub struct TempDir { path: Box, + keep: bool, } impl TempDir { @@ -425,15 +426,18 @@ impl fmt::Debug for TempDir { impl Drop for TempDir { fn drop(&mut self) { - let _ = remove_dir_all(self.path()); + if !self.keep { + let _ = remove_dir_all(self.path()); + } } } pub(crate) fn create( path: PathBuf, permissions: Option<&std::fs::Permissions>, + keep: bool, ) -> io::Result { - imp::create(path, permissions) + imp::create(path, permissions, keep) } mod imp; diff --git a/src/file/imp/unix.rs b/src/file/imp/unix.rs index 2a3cd9ab1..117d76a41 100644 --- a/src/file/imp/unix.rs +++ b/src/file/imp/unix.rs @@ -82,8 +82,7 @@ fn create_unix(dir: &Path) -> io::Result { OsStr::new(".tmp"), OsStr::new(""), crate::NUM_RAND_CHARS, - None, - |path, _| create_unlinked(&path), + |path| create_unlinked(&path), ) } diff --git a/src/file/imp/windows.rs b/src/file/imp/windows.rs index bed62090f..e7f603843 100644 --- a/src/file/imp/windows.rs +++ b/src/file/imp/windows.rs @@ -45,8 +45,7 @@ pub fn create(dir: &Path) -> io::Result { OsStr::new(".tmp"), OsStr::new(""), crate::NUM_RAND_CHARS, - None, - |path, _permissions| { + |path| { OpenOptions::new() .create_new(true) .read(true) diff --git a/src/file/mod.rs b/src/file/mod.rs index 2780ae14e..8930da042 100644 --- a/src/file/mod.rs +++ b/src/file/mod.rs @@ -124,9 +124,11 @@ impl error::Error for PathPersistError { /// This is useful when the temporary file needs to be used by a child process, /// for example. /// -/// When dropped, the temporary file is deleted. +/// When dropped, the temporary file is deleted unless `keep(true)` was called +/// on the builder that constructed this value. pub struct TempPath { path: Box, + keep: bool, } impl TempPath { @@ -273,7 +275,6 @@ impl TempPath { /// Keep the temporary file from being deleted. This function will turn the /// temporary file into a non-temporary file without moving it. /// - /// /// # Errors /// /// On some platforms (e.g., Windows), we need to mark the file as @@ -320,6 +321,14 @@ impl TempPath { pub fn from_path(path: impl Into) -> Self { Self { path: path.into().into_boxed_path(), + keep: false, + } + } + + pub(crate) fn new(path: PathBuf, keep: bool) -> Self { + Self { + path: path.into_boxed_path(), + keep, } } } @@ -332,7 +341,9 @@ impl fmt::Debug for TempPath { impl Drop for TempPath { fn drop(&mut self) { - let _ = fs::remove_file(&self.path); + if !self.keep { + let _ = fs::remove_file(&self.path); + } } } @@ -1008,6 +1019,7 @@ pub(crate) fn create_named( mut path: PathBuf, open_options: &mut OpenOptions, permissions: Option<&std::fs::Permissions>, + keep: bool, ) -> io::Result { // Make the path absolute. Otherwise, changing directories could cause us to // delete the wrong file. @@ -1019,6 +1031,7 @@ pub(crate) fn create_named( .map(|file| NamedTempFile { path: TempPath { path: path.into_boxed_path(), + keep, }, file, }) diff --git a/src/lib.rs b/src/lib.rs index 057b82b31..ca657cbf1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -171,6 +171,7 @@ pub struct Builder<'a, 'b> { suffix: &'b OsStr, append: bool, permissions: Option, + keep: bool, } impl<'a, 'b> Default for Builder<'a, 'b> { @@ -181,6 +182,7 @@ impl<'a, 'b> Default for Builder<'a, 'b> { suffix: OsStr::new(""), append: false, permissions: None, + keep: false, } } } @@ -403,6 +405,27 @@ impl<'a, 'b> Builder<'a, 'b> { self } + /// Set the file/folder to be kept even when the [`NamedTempFile`]/[`TempDir`] goes out of + /// scope. + /// + /// By default, the file/folder is automatically cleaned up in the destructor of + /// [`NamedTempFile`]/[`TempDir`]. When `keep` is set to `true`, this behavior is supressed. + /// + /// # Examples + /// + /// ``` + /// use tempfile::Builder; + /// + /// let named_tempfile = Builder::new() + /// .keep(true) + /// .tempfile()?; + /// # Ok::<(), std::io::Error>(()) + /// ``` + pub fn keep(&mut self, keep: bool) -> &mut Self { + self.keep = keep; + self + } + /// Create the named temporary file. /// /// # Security @@ -463,9 +486,13 @@ impl<'a, 'b> Builder<'a, 'b> { self.prefix, self.suffix, self.random_len, - self.permissions.as_ref(), - |path, permissions| { - file::create_named(path, OpenOptions::new().append(self.append), permissions) + |path| { + file::create_named( + path, + OpenOptions::new().append(self.append), + self.permissions.as_ref(), + self.keep, + ) }, ) } @@ -528,14 +555,9 @@ impl<'a, 'b> Builder<'a, 'b> { dir = &storage; } - util::create_helper( - dir, - self.prefix, - self.suffix, - self.random_len, - self.permissions.as_ref(), - dir::create, - ) + util::create_helper(dir, self.prefix, self.suffix, self.random_len, |path| { + dir::create(path, self.permissions.as_ref(), self.keep) + }) } /// Attempts to create a temporary file (or file-like object) using the @@ -656,11 +678,10 @@ impl<'a, 'b> Builder<'a, 'b> { self.prefix, self.suffix, self.random_len, - None, - move |path, _permissions| { + move |path| { Ok(NamedTempFile::from_parts( f(&path)?, - TempPath::from_path(path), + TempPath::new(path, self.keep), )) }, ) diff --git a/src/util.rs b/src/util.rs index e91150f0a..2f6f381a5 100644 --- a/src/util.rs +++ b/src/util.rs @@ -24,8 +24,7 @@ pub fn create_helper( prefix: &OsStr, suffix: &OsStr, random_len: usize, - permissions: Option<&std::fs::Permissions>, - mut f: impl FnMut(PathBuf, Option<&std::fs::Permissions>) -> io::Result, + mut f: impl FnMut(PathBuf) -> io::Result, ) -> io::Result { let num_retries = if random_len != 0 { crate::NUM_RETRIES @@ -35,7 +34,7 @@ pub fn create_helper( for _ in 0..num_retries { let path = base.join(tmpname(prefix, suffix, random_len)); - return match f(path, permissions) { + return match f(path) { Err(ref e) if e.kind() == io::ErrorKind::AlreadyExists && num_retries > 1 => continue, // AddrInUse can happen if we're creating a UNIX domain socket and // the path already exists. diff --git a/tests/namedtempfile.rs b/tests/namedtempfile.rs index 34ddc44bc..06b6e86d2 100644 --- a/tests/namedtempfile.rs +++ b/tests/namedtempfile.rs @@ -345,6 +345,23 @@ fn test_keep() { std::fs::remove_file(&path).unwrap(); } +#[test] +fn test_builder_keep() { + let mut tmpfile = Builder::new().keep(true).tempfile().unwrap(); + write!(tmpfile, "abcde").unwrap(); + let path = tmpfile.path().to_owned(); + drop(tmpfile); + + { + // Try opening it again. + let mut f = File::open(&path).unwrap(); + let mut buf = String::new(); + f.read_to_string(&mut buf).unwrap(); + assert_eq!("abcde", buf); + } + std::fs::remove_file(&path).unwrap(); +} + #[test] fn test_make() { let tmpfile = Builder::new().make(|path| File::create(path)).unwrap(); diff --git a/tests/tempdir.rs b/tests/tempdir.rs index 2bc4001df..93f86ab42 100644 --- a/tests/tempdir.rs +++ b/tests/tempdir.rs @@ -163,6 +163,14 @@ fn pass_as_asref_path() { } } +fn test_keep() { + let tmpdir = Builder::new().keep(true).tempdir().unwrap(); + let path = tmpdir.path().to_owned(); + drop(tmpdir); + assert!(path.exists()); + fs::remove_dir(path).unwrap(); +} + #[test] fn main() { in_tmpdir(test_tempdir); @@ -172,4 +180,5 @@ fn main() { in_tmpdir(test_rm_tempdir_close); in_tmpdir(dont_double_panic); in_tmpdir(pass_as_asref_path); + in_tmpdir(test_keep); }