From e067c2858fad7c9ea1c0002b65664564b2503018 Mon Sep 17 00:00:00 2001 From: Amanda Sternberg Date: Fri, 12 Sep 2025 13:06:46 +0200 Subject: [PATCH 1/5] Added safe_rename and tests for cross-device file moves (OS error 18) --- rslib/src/media/files.rs | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/rslib/src/media/files.rs b/rslib/src/media/files.rs index ce17b40bb..9526a4955 100644 --- a/rslib/src/media/files.rs +++ b/rslib/src/media/files.rs @@ -320,6 +320,18 @@ pub(crate) fn mtime_as_i64>(path: P) -> io::Result { .as_millis() as i64) } +pub(crate) fn safe_rename(src: &Path, dst: &Path) -> io::Result<()> { + match fs::rename(src, dst) { + Ok(_) => Ok(()), + Err(e) if e.raw_os_error() == Some(18) => { + fs::copy(src, dst)?; + fs::remove_file(src)?; + Ok(()) + } + Err(e) => Err(e), + } +} + pub fn remove_files(media_folder: &Path, files: &[S]) -> Result<()> where S: AsRef + std::fmt::Debug, @@ -344,7 +356,7 @@ where } // move file to trash, clobbering any existing file with the same name - fs::rename(&src_path, &dst_path)?; + safe_rename(&src_path, &dst_path)?; // mark it as modified, so we can expire it in the future let secs = time::SystemTime::now(); @@ -437,6 +449,8 @@ pub(crate) fn data_for_file(media_folder: &Path, fname: &str) -> Result::Owned(format!("{}_", " ".repeat(MAX_MEDIA_FILENAME_LENGTH - 2))) ); } + #[test] + fn safe_rename_moves_file() { + let dir = tempdir().unwrap(); + let src = dir.path().join("test_src.txt"); + let dst = dir.path().join("test_dst.txt"); + + fs::write(&src, b"hello world").unwrap(); + safe_rename(&src, &dst).unwrap(); + + assert!(dst.exists()); + assert!(!src.exists()); + let contents = fs::read(&dst).unwrap(); + assert_eq!(contents, b"hello world"); + + fs::remove_file(&dst).unwrap(); + } } From 8b129420399cf33c67ba8f98ad3fd94fed6cf3fd Mon Sep 17 00:00:00 2001 From: Amanda Sternberg Date: Fri, 12 Sep 2025 13:18:24 +0200 Subject: [PATCH 2/5] format code with rustfmt and add contributor --- rslib/src/media/files.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rslib/src/media/files.rs b/rslib/src/media/files.rs index 9526a4955..2ddd1d55d 100644 --- a/rslib/src/media/files.rs +++ b/rslib/src/media/files.rs @@ -448,9 +448,9 @@ pub(crate) fn data_for_file(media_folder: &Path, fname: &str) -> Result Date: Fri, 12 Sep 2025 13:32:45 +0200 Subject: [PATCH 3/5] added contributor --- CONTRIBUTORS | 1 + 1 file changed, 1 insertion(+) diff --git a/CONTRIBUTORS b/CONTRIBUTORS index b03108e16..8ab2bb1e3 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -242,6 +242,7 @@ Lee Doughty <32392044+leedoughty@users.noreply.github.com> memchr Max Romanowski Aldlss +Amanda Sternberg ******************** From 77ccd69ada1cf2b9fd41a94e4950c3b5191ebdb8 Mon Sep 17 00:00:00 2001 From: Amanda Sternberg Date: Fri, 12 Sep 2025 13:38:29 +0200 Subject: [PATCH 4/5] apply cargo fmt --- rslib/src/media/files.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rslib/src/media/files.rs b/rslib/src/media/files.rs index 2ddd1d55d..b0b06241d 100644 --- a/rslib/src/media/files.rs +++ b/rslib/src/media/files.rs @@ -448,12 +448,12 @@ pub(crate) fn data_for_file(media_folder: &Path, fname: &str) -> Result Date: Mon, 15 Sep 2025 12:33:17 +0200 Subject: [PATCH 5/5] Refactor safe_rename to use crossesDevices and added comments --- rslib/src/media/files.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/rslib/src/media/files.rs b/rslib/src/media/files.rs index b0b06241d..f0c75c965 100644 --- a/rslib/src/media/files.rs +++ b/rslib/src/media/files.rs @@ -320,10 +320,12 @@ pub(crate) fn mtime_as_i64>(path: P) -> io::Result { .as_millis() as i64) } +/// On POSIX this is EXDEV (error 18) and on Windows it is ERROR_NOT_SAME_DEVICE +/// (error 17). pub(crate) fn safe_rename(src: &Path, dst: &Path) -> io::Result<()> { match fs::rename(src, dst) { Ok(_) => Ok(()), - Err(e) if e.raw_os_error() == Some(18) => { + Err(e) if e.kind() == io::ErrorKind::CrossesDevices => { fs::copy(src, dst)?; fs::remove_file(src)?; Ok(()) @@ -566,6 +568,8 @@ mod test { } #[test] fn safe_rename_moves_file() { + // This test only verifies that ´safe_rename´calls back to copy+remove logic + // when needed. It does not simulate a real cross-device move. let dir = tempdir().unwrap(); let src = dir.path().join("test_src.txt"); let dst = dir.path().join("test_dst.txt");