From 84f8b7a98b5e8e66dc2635cac03955f63921af82 Mon Sep 17 00:00:00 2001 From: mhead Date: Thu, 18 Jul 2024 15:09:56 +0530 Subject: [PATCH] proper error message when inter-partition copying fails with test --- src/uu/mv/src/mv.rs | 12 +++++- tests/by-util/test_mv.rs | 81 +++++++++++++++++++++++++++++++++++++--- 2 files changed, 86 insertions(+), 7 deletions(-) diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index 9155e3500..7edecb960 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -662,7 +662,17 @@ fn rename_with_fallback( } } else { if to.is_symlink() { - fs::remove_file(to)?; + fs::remove_file(to).map_err(|err| { + let to = to.to_string_lossy(); + let from = from.to_string_lossy(); + io::Error::new( + err.kind(), + format!( + "inter-device move failed: '{from}' to '{to}'\ + ; unable to remove target: {err}" + ), + ) + })?; } #[cfg(all(unix, not(any(target_os = "macos", target_os = "redox"))))] fs::copy(from, to) diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index 94b182c5c..b6943a311 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -1629,21 +1629,25 @@ fn test_acl() { fn test_mv_unlinks_dest_symlink() { let scene = TestScenario::new(util_name!()); let at = &scene.fixtures; + // create a file in the current partition. at.write("src", "src contents"); // create a folder in another partition. - let other_fs_tempdir = tempfile::TempDir::new_in("/dev/shm/").unwrap(); + let other_fs_tempdir = + tempfile::TempDir::new_in("/dev/shm/").expect("Unable to create temp directory"); // create a file inside that folder. - let file_path = other_fs_tempdir.path().join("other_fs_file.txt"); - let mut file = std::fs::File::create(&file_path).expect("Unable to create file"); + let other_fs_file_path = other_fs_tempdir.path().join("other_fs_file"); + let mut file = + std::fs::File::create(&other_fs_file_path).expect("Unable to create other_fs_file"); std::io::Write::write_all(&mut file, b"other fs file contents") - .expect("Unable to write to file"); + .expect("Unable to write to other_fs_file"); // create a symlink to the file inside the same directory. - let symlink_path = other_fs_tempdir.path().join("symlink_to_file.txt"); - std::os::unix::fs::symlink(&file_path, &symlink_path).unwrap(); + let symlink_path = other_fs_tempdir.path().join("symlink_to_file"); + std::os::unix::fs::symlink(&other_fs_file_path, &symlink_path) + .expect("Unable to create symlink_to_file"); // mv src to symlink in another partition scene @@ -1651,8 +1655,73 @@ fn test_mv_unlinks_dest_symlink() { .arg("src") .arg(symlink_path.to_str().unwrap()) .succeeds(); + // make sure that src got removed. assert!(!at.file_exists("src")); + // make sure symlink got unlinked assert!(!symlink_path.is_symlink()); + + // make sure that file contents in other_fs_file didn't change. + let mut new_contents = String::new(); + std::io::Read::read_to_string( + &mut std::fs::File::open(&other_fs_file_path).expect("Unable to open other_fs_file"), + &mut new_contents, + ) + .expect("Unable to read other_fs_file"); + assert_eq!(new_contents, "other fs file contents"); + + // make sure that src file contents got copied into new file created in symlink_path . + let mut new_contents = String::new(); + std::io::Read::read_to_string( + &mut std::fs::File::open(&symlink_path).expect("Unable to open file"), + &mut new_contents, + ) + .expect("Unable to read file"); + assert_eq!(new_contents, "src contents"); +} + +// In an inter-partition move if unlinking the destination symlink fails, ensure +// that it would output the proper error message. +#[cfg(target_os = "linux")] +#[test] +fn test_mv_unlinks_dest_symlink_error_message() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + // create a file in the current partition. + at.write("src", "src contents"); + + // create a folder in another partition. + let other_fs_tempdir = + tempfile::TempDir::new_in("/dev/shm/").expect("Unable to create temp directory"); + + // create a file inside that folder. + let other_fs_file_path = other_fs_tempdir.path().join("other_fs_file"); + let mut file = + std::fs::File::create(&other_fs_file_path).expect("Unable to create other_fs_file"); + std::io::Write::write_all(&mut file, b"other fs file contents") + .expect("Unable to write to other_fs_file"); + + // create a symlink to the file inside the same directory. + let symlink_path = other_fs_tempdir.path().join("symlink_to_file"); + std::os::unix::fs::symlink(&other_fs_file_path, &symlink_path) + .expect("Unable to create symlink_to_file"); + + // disable write for the target folder so that when mv tries to remove the + // the destination symlink inside the target directory it would fail. + std::fs::set_permissions( + other_fs_tempdir.path(), + std::os::unix::fs::PermissionsExt::from_mode(0o555), + ) + .expect("Unable to set permissions for temp directory"); + + // mv src to symlink in another partition + scene + .ucmd() + .arg("src") + .arg(symlink_path.to_str().unwrap()) + .fails() + .stderr_contains("inter-device move failed:") + .stderr_contains("Permission denied"); }