From c3d2d1d094ea4bde31398855b78af20b19abc6f8 Mon Sep 17 00:00:00 2001 From: municorn Date: Fri, 24 Oct 2025 04:37:03 +0200 Subject: [PATCH 1/2] fix: handle cross-device link errors by falling back to copy+delete on Linux --- src/freedesktop.rs | 62 +++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 56 insertions(+), 6 deletions(-) diff --git a/src/freedesktop.rs b/src/freedesktop.rs index a5de1af..17681ef 100644 --- a/src/freedesktop.rs +++ b/src/freedesktop.rs @@ -11,7 +11,7 @@ use std::{ collections::HashSet, ffi::{OsStr, OsString}, fs::{self, File, OpenOptions}, - io::{BufRead, BufReader, Write}, + io::{BufRead, BufReader, ErrorKind, Write}, os::unix::{ ffi::{OsStrExt, OsStringExt}, fs::PermissionsExt, @@ -551,13 +551,36 @@ fn move_items_no_replace(src: impl AsRef, dst: impl AsRef) -> Result let dst = dst.as_ref(); try_creating_placeholders(src, dst)?; - std::fs::rename(src, dst).map_err(|e| (src.to_owned(), e))?; - // Once everything is moved, lets recursively remove the directory - if src.is_dir() { - std::fs::remove_dir_all(src).map_err(|e| (src.to_owned(), e))?; + // Try rename first (fastest option for same filesystem) + if let Err(e) = std::fs::rename(src, dst) { + // Check if this is a cross-device link error + if e.kind() == ErrorKind::CrossesDevices { + // Cross-device link error - need to copy and delete instead + debug!("Cross-device move detected, falling back to copy+delete for {:?}", src); + + // Copy the file/directory + if src.is_dir() { + copy_dir_all(src, dst)?; + } else { + std::fs::copy(src, dst).map_err(|e| (src.to_owned(), e))?; + } + + // Remove the source + if src.is_dir() { + std::fs::remove_dir_all(src).map_err(|e| (src.to_owned(), e))?; + } else { + std::fs::remove_file(src).map_err(|e| (src.to_owned(), e))?; + } + + Ok(()) + } else { + // Some other error, propagate it + Err((src.to_owned(), e)) + } + } else { + Ok(()) } - Ok(()) } fn try_creating_placeholders(src: impl AsRef, dst: impl AsRef) -> Result<(), FsError> { @@ -574,6 +597,33 @@ fn try_creating_placeholders(src: impl AsRef, dst: impl AsRef) -> Re Ok(()) } +/// Helper function to recursively copy a directory +fn copy_dir_all(src: impl AsRef, dst: impl AsRef) -> Result<(), FsError> { + let src = src.as_ref(); + let dst = dst.as_ref(); + + std::fs::create_dir_all(dst).map_err(|e| (dst.to_owned(), e))?; + + for entry in std::fs::read_dir(src).map_err(|e| (src.to_owned(), e))? { + let entry = entry.map_err(|e| (src.to_owned(), e))?; + let file_type = entry.file_type().map_err(|e| (entry.path(), e))?; + let src_path = entry.path(); + let dst_path = dst.join(entry.file_name()); + + if file_type.is_dir() { + copy_dir_all(&src_path, &dst_path)?; + } else if file_type.is_symlink() { + // Handle symlinks by copying the symlink itself, not the target + let target = std::fs::read_link(&src_path).map_err(|e| (src_path.clone(), e))?; + std::os::unix::fs::symlink(&target, &dst_path).map_err(|e| (dst_path.clone(), e))?; + } else { + std::fs::copy(&src_path, &dst_path).map_err(|e| (src_path.clone(), e))?; + } + } + + Ok(()) +} + fn decode_uri_path(path: impl AsRef) -> PathBuf { // Paths may be invalid Unicode on most Unixes so they should be treated as byte strings // A higher level crate, such as `url`, can't be used directly since its API intakes valid Rust From 1894bfe4ab6677ca833cc60bbd6e568480abbf77 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 24 Oct 2025 04:27:50 +0200 Subject: [PATCH 2/2] refactor and fixes * disable perma-failing macOS tests * some code refactoring --- Cargo.toml | 2 +- src/freedesktop.rs | 46 +++++++++++++++++++++------------------------- src/macos/mod.rs | 6 +++--- tests/trash.rs | 2 ++ 4 files changed, 27 insertions(+), 29 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 85886de..257e3b7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,6 +8,7 @@ description = "A library for moving files and folders to the Recycle Bin" keywords = ["remove", "trash", "rubbish", "recycle", "bin"] repository = "https://github.com/ArturKovacs/trash" edition = "2021" +rust-version = "1.85.0" include = ["src/**/*", "LICENSE.txt", "README.md", "CHANGELOG.md", "build.rs"] [features] @@ -23,7 +24,6 @@ log = "0.4" [dev-dependencies] serial_test = { version = "2.0.0", default-features = false } chrono = { version = "0.4.31", default-features = false, features = ["clock"] } -rand = "0.8.5" once_cell = "1.18.0" env_logger = "0.10.0" tempfile = "3.8.0" diff --git a/src/freedesktop.rs b/src/freedesktop.rs index 17681ef..99065d6 100644 --- a/src/freedesktop.rs +++ b/src/freedesktop.rs @@ -552,35 +552,31 @@ fn move_items_no_replace(src: impl AsRef, dst: impl AsRef) -> Result try_creating_placeholders(src, dst)?; - // Try rename first (fastest option for same filesystem) - if let Err(e) = std::fs::rename(src, dst) { - // Check if this is a cross-device link error - if e.kind() == ErrorKind::CrossesDevices { - // Cross-device link error - need to copy and delete instead - debug!("Cross-device move detected, falling back to copy+delete for {:?}", src); - - // Copy the file/directory - if src.is_dir() { - copy_dir_all(src, dst)?; - } else { - std::fs::copy(src, dst).map_err(|e| (src.to_owned(), e))?; - } + // Try to rename first (fastest option for same filesystem) + let Err(e) = std::fs::rename(src, dst) else { return Ok(()) }; - // Remove the source - if src.is_dir() { - std::fs::remove_dir_all(src).map_err(|e| (src.to_owned(), e))?; - } else { - std::fs::remove_file(src).map_err(|e| (src.to_owned(), e))?; - } + let needs_cross_device_copy = e.kind() == ErrorKind::CrossesDevices; + if !needs_cross_device_copy { + return Err((src.to_owned(), e)); + } - Ok(()) - } else { - // Some other error, propagate it - Err((src.to_owned(), e)) - } + debug!("Cross-device move detected, falling back to copy+delete for {:?}", src); + + // Copy the file/directory + if src.is_dir() { + copy_dir_all(src, dst)?; } else { - Ok(()) + std::fs::copy(src, dst).map_err(|e| (src.to_owned(), e))?; } + + // Remove the source + if src.is_dir() { + std::fs::remove_dir_all(src).map_err(|e| (src.to_owned(), e))?; + } else { + std::fs::remove_file(src).map_err(|e| (src.to_owned(), e))?; + } + + Ok(()) } fn try_creating_placeholders(src: impl AsRef, dst: impl AsRef) -> Result<(), FsError> { diff --git a/src/macos/mod.rs b/src/macos/mod.rs index f2ee902..824371d 100644 --- a/src/macos/mod.rs +++ b/src/macos/mod.rs @@ -84,7 +84,7 @@ impl TrashContext { fn delete_using_file_mgr>(full_paths: &[P]) -> Result<(), Error> { trace!("Starting delete_using_file_mgr"); - let file_mgr = unsafe { NSFileManager::defaultManager() }; + let file_mgr = NSFileManager::defaultManager(); for path in full_paths { let path = path.as_ref().as_os_str().as_encoded_bytes(); let path = match std::str::from_utf8(path) { @@ -93,11 +93,11 @@ fn delete_using_file_mgr>(full_paths: &[P]) -> Result<(), Error> }; trace!("Starting fileURLWithPath"); - let url = unsafe { NSURL::fileURLWithPath(&path) }; + let url = NSURL::fileURLWithPath(&path); trace!("Finished fileURLWithPath"); trace!("Calling trashItemAtURL"); - let res = unsafe { file_mgr.trashItemAtURL_resultingItemURL_error(&url, None) }; + let res = file_mgr.trashItemAtURL_resultingItemURL_error(&url, None); trace!("Finished trashItemAtURL"); if let Err(err) = res { diff --git a/tests/trash.rs b/tests/trash.rs index 817a24c..7ea478f 100644 --- a/tests/trash.rs +++ b/tests/trash.rs @@ -89,6 +89,7 @@ mod unix { // use crate::init_logging; #[test] + #[ignore = "permission denied in more recent macOS versions"] fn test_delete_symlink() { init_logging(); trace!("Started test_delete_symlink"); @@ -107,6 +108,7 @@ mod unix { } #[test] + #[ignore = "permission denied in more recent macOS versions"] fn test_delete_symlink_in_folder() { init_logging(); trace!("Started test_delete_symlink_in_folder");