diff --git a/src/uu/mv/locales/en-US.ftl b/src/uu/mv/locales/en-US.ftl index 4bb5339fe32..484d8af4548 100644 --- a/src/uu/mv/locales/en-US.ftl +++ b/src/uu/mv/locales/en-US.ftl @@ -32,6 +32,7 @@ mv-error-backup-might-destroy-source = backing up {$target} might destroy source mv-error-will-not-overwrite-just-created = will not overwrite just-created {$target} with {$source} mv-error-not-replacing = not replacing {$target} mv-error-cannot-move = cannot move {$source} to {$target} +mv-error-cannot-overwrite-non-empty-directory = cannot overwrite {$target} mv-error-directory-not-empty = Directory not empty mv-error-dangling-symlink = can't determine symlink type, since it is dangling mv-error-no-symlink-support = your operating system does not support symlinks diff --git a/src/uu/mv/locales/fr-FR.ftl b/src/uu/mv/locales/fr-FR.ftl index 9ea2f2114b1..5470d5f10ce 100644 --- a/src/uu/mv/locales/fr-FR.ftl +++ b/src/uu/mv/locales/fr-FR.ftl @@ -32,6 +32,7 @@ mv-error-backup-might-destroy-source = sauvegarder {$target} pourrait détruire mv-error-will-not-overwrite-just-created = ne va pas écraser le fichier qui vient d'être créé {$target} avec {$source} mv-error-not-replacing = ne remplace pas {$target} mv-error-cannot-move = impossible de déplacer {$source} vers {$target} +mv-error-cannot-overwrite-non-empty-directory = impossible d'écraser {$target} mv-error-directory-not-empty = Répertoire non vide mv-error-dangling-symlink = impossible de déterminer le type de lien symbolique, car il est suspendu mv-error-no-symlink-support = votre système d'exploitation ne prend pas en charge les liens symboliques diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index aa34a6294ae..f272af2a9ef 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -36,7 +36,7 @@ use crate::hardlink::{ }; use uucore::backup_control::{self, source_is_target_backup}; use uucore::display::Quotable; -use uucore::error::{FromIo, UResult, USimpleError, UUsageError, set_exit_code}; +use uucore::error::{FromIo, UError, UResult, USimpleError, UUsageError, set_exit_code}; #[cfg(unix)] use uucore::fs::display_permissions_unix; #[cfg(unix)] @@ -415,8 +415,20 @@ fn handle_two_paths(source: &Path, target: &Path, opts: &Options) -> UResult<()> hardlink_params.0, hardlink_params.1, ) - .map_err_context(|| { - translate!("mv-error-cannot-move", "source" => source.quote(), "target" => target.quote()) + .map_err(|e| -> Box { + let message = if is_directory_not_empty_error(&e) { + translate!( + "mv-error-cannot-overwrite-non-empty-directory", + "target" => target.quote() + ) + } else { + translate!( + "mv-error-cannot-move", + "source" => source.quote(), + "target" => target.quote() + ) + }; + e.map_err_context(|| message) }) } else { Err(MvError::DirectoryToNonDirectory(target.quote().to_string()).into()) @@ -680,20 +692,31 @@ fn move_files_into_dir(files: &[PathBuf], target_dir: &Path, options: &Options) ) { Err(e) if e.to_string().is_empty() => set_exit_code(1), Err(e) => { - let e = e.map_err_context(|| { - translate!("mv-error-cannot-move", "source" => sourcepath.quote(), "target" => targetpath.quote()) - }); + let message = if is_directory_not_empty_error(&e) { + translate!( + "mv-error-cannot-overwrite-non-empty-directory", + "target" => targetpath.quote() + ) + } else { + translate!( + "mv-error-cannot-move", + "source" => sourcepath.quote(), + "target" => targetpath.quote() + ) + }; + let e = e.map_err_context(|| message); match display_manager { Some(ref pb) => pb.suspend(|| show!(e)), None => show!(e), } } - Ok(()) => (), + Ok(()) => { + moved_destinations.insert(targetpath.clone()); + } } if let Some(ref pb) = count_progress { pb.inc(1); } - moved_destinations.insert(targetpath.clone()); } Ok(()) } @@ -761,7 +784,10 @@ fn rename( if is_empty_dir(to) { fs::remove_dir(to)?; } else { - return Err(io::Error::other(translate!("mv-error-directory-not-empty"))); + return Err(io::Error::new( + io::ErrorKind::DirectoryNotEmpty, + translate!("mv-error-directory-not-empty"), + )); } } } @@ -806,6 +832,10 @@ fn rename( Ok(()) } +fn is_directory_not_empty_error(err: &io::Error) -> bool { + err.kind() == io::ErrorKind::DirectoryNotEmpty +} + #[cfg(unix)] fn is_fifo(filetype: fs::FileType) -> bool { filetype.is_fifo() @@ -932,10 +962,7 @@ fn rename_symlink_fallback(from: &Path, to: &Path) -> io::Result<()> { #[cfg(not(any(windows, unix)))] fn rename_symlink_fallback(from: &Path, to: &Path) -> io::Result<()> { let path_symlink_points_to = fs::read_link(from)?; - Err(io::Error::new( - io::ErrorKind::Other, - translate!("mv-error-no-symlink-support"), - )) + Err(io::Error::other(translate!("mv-error-no-symlink-support"))) } fn rename_dir_fallback( diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index 3c69d65a78d..7b58c7d1d08 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -119,7 +119,7 @@ fn test_mv_move_file_into_file_with_target_arg() { .arg(file1) .arg(file2) .fails() - .stderr_is(format!("mv: target directory '{file1}': Not a directory\n")); + .stderr_only(format!("mv: target directory '{file1}': Not a directory\n")); assert!(at.file_exists(file1)); } @@ -139,7 +139,7 @@ fn test_mv_move_multiple_files_into_file() { .arg(file2) .arg(file3) .fails() - .stderr_is(format!("mv: target '{file3}': Not a directory\n")); + .stderr_only(format!("mv: target '{file3}': Not a directory\n")); assert!(at.file_exists(file1)); assert!(at.file_exists(file2)); @@ -332,7 +332,7 @@ fn test_mv_interactive_no_clobber_force_last_arg_wins() { .ucmd() .args(&[file_a, file_b, "-n", "-f", "-i"]) .fails() - .stderr_is(format!("mv: overwrite '{file_b}'? ")); + .stderr_only(format!("mv: overwrite '{file_b}'? ")); at.write(file_a, "aa"); @@ -518,7 +518,7 @@ fn test_mv_same_file() { ucmd.arg(file_a) .arg(file_a) .fails() - .stderr_is(format!("mv: '{file_a}' and '{file_a}' are the same file\n")); + .stderr_only(format!("mv: '{file_a}' and '{file_a}' are the same file\n")); } #[test] @@ -535,7 +535,7 @@ fn test_mv_same_hardlink() { ucmd.arg(file_a) .arg(file_b) .fails() - .stderr_is(format!("mv: '{file_a}' and '{file_b}' are the same file\n")); + .stderr_only(format!("mv: '{file_a}' and '{file_b}' are the same file\n")); } #[test] @@ -566,7 +566,7 @@ fn test_mv_same_symlink() { ucmd.arg(file_b) .arg(file_a) .fails() - .stderr_is(format!("mv: '{file_b}' and '{file_a}' are the same file\n")); + .stderr_only(format!("mv: '{file_b}' and '{file_a}' are the same file\n")); let (at2, mut ucmd2) = at_and_ucmd!(); at2.touch(file_a); @@ -596,7 +596,7 @@ fn test_mv_same_symlink() { .arg(file_c) .arg(file_a) .fails() - .stderr_is(format!("mv: '{file_c}' and '{file_a}' are the same file\n")); + .stderr_only(format!("mv: '{file_c}' and '{file_a}' are the same file\n")); } #[test] @@ -609,7 +609,7 @@ fn test_mv_same_broken_symlink() { ucmd.arg("broken") .arg("broken") .fails() - .stderr_is("mv: 'broken' and 'broken' are the same file\n"); + .stderr_only("mv: 'broken' and 'broken' are the same file\n"); } #[test] @@ -742,7 +742,7 @@ fn test_mv_same_file_not_dot_dir() { let dir = "test_mv_errors_dir"; at.mkdir(dir); - ucmd.arg(dir).arg(dir).fails().stderr_is(format!( + ucmd.arg(dir).arg(dir).fails().stderr_only(format!( "mv: cannot move '{dir}' to a subdirectory of itself, '{dir}/{dir}'\n", )); } @@ -754,7 +754,7 @@ fn test_mv_same_file_dot_dir() { ucmd.arg(".") .arg(".") .fails() - .stderr_is("mv: '.' and '.' are the same file\n"); + .stderr_only("mv: '.' and '.' are the same file\n"); } #[test] @@ -1487,15 +1487,34 @@ fn test_mv_overwrite_nonempty_dir() { at.mkdir(dir_a); at.mkdir(dir_b); at.touch(dummy); - // Not same error as GNU; the error message is a rust builtin - // TODO: test (and implement) correct error message (or at least decide whether to do so) - // Current: "mv: couldn't rename path (Directory not empty; from=a; to=b)" - // GNU: "mv: cannot move 'a' to 'b': Directory not empty" - // Verbose output for the move should not be shown on failure - let result = ucmd.arg("-vT").arg(dir_a).arg(dir_b).fails(); - result.no_stdout(); - assert!(!result.stderr_str().is_empty()); + ucmd.arg("-vT") + .arg(dir_a) + .arg(dir_b) + .fails() + .stderr_contains("cannot overwrite"); + + assert!(at.dir_exists(dir_a)); + assert!(at.dir_exists(dir_b)); +} + +#[test] +fn test_mv_overwrite_nonempty_dir_into_dir() { + let (at, mut ucmd) = at_and_ucmd!(); + let dir_a = "test_mv_overwrite_nonempty_dir_into_dir_a"; + let dir_b = "test_mv_overwrite_nonempty_dir_into_dir_b"; + let target_dir = format!("{dir_b}/{dir_a}"); + let dummy = format!("{target_dir}/file"); + + at.mkdir(dir_a); + at.mkdir(dir_b); + at.mkdir(&target_dir); + at.touch(&dummy); + + ucmd.arg(dir_a) + .arg(dir_b) + .fails() + .stderr_contains("cannot overwrite"); assert!(at.dir_exists(dir_a)); assert!(at.dir_exists(dir_b)); @@ -1534,7 +1553,6 @@ fn test_mv_errors() { at.touch(file_b); // $ mv -T -t a b - // mv: cannot combine --target-directory (-t) and --no-target-directory (-T) scene .ucmd() .arg("-T") @@ -1547,29 +1565,26 @@ fn test_mv_errors() { // $ at.touch file && at.mkdir dir // $ mv -T file dir - // err == mv: cannot overwrite directory 'dir' with non-directory scene .ucmd() .arg("-T") .arg(file_a) .arg(dir) .fails() - .stderr_is(format!( + .stderr_only(format!( "mv: cannot overwrite directory '{dir}' with non-directory\n" )); // $ at.mkdir dir && at.touch file // $ mv dir file - // err == mv: cannot overwrite non-directory 'file' with directory 'dir' - assert!( - !scene - .ucmd() - .arg(dir) - .arg(file_a) - .fails() - .stderr_str() - .is_empty() - ); + scene + .ucmd() + .arg(dir) + .arg(file_a) + .fails() + .stderr_only(format!( + "mv: cannot overwrite non-directory '{file_a}' with directory '{dir}'\n" + )); } #[test] @@ -1656,8 +1671,7 @@ fn test_mv_arg_interactive_skipped() { .pipe_in("N\n") .ignore_stdin_write_error() .fails() - .stderr_is("mv: overwrite 'b'? ") - .no_stdout(); + .stderr_only("mv: overwrite 'b'? "); } #[test] diff --git a/util/build-gnu.sh b/util/build-gnu.sh index 421b43d5e7e..c9cd3760feb 100755 --- a/util/build-gnu.sh +++ b/util/build-gnu.sh @@ -278,9 +278,6 @@ sed -i -e "s|---dis ||g" tests/tail/overlay-headers.sh "${SED}" -i -e "s/\$prog: multiple field specifications/error: the argument '--field ' cannot be used multiple times\n\nUsage: numfmt [OPTION]... [NUMBER]...\n\nFor more information, try '--help'./g" tests/numfmt/numfmt.pl "${SED}" -i -e "s/Try 'mv --help' for more information/For more information, try '--help'/g" -e "s/mv: missing file operand/error: the following required arguments were not provided:\n ...\n\nUsage: mv [OPTION]... [-T] SOURCE DEST\n mv [OPTION]... SOURCE... DIRECTORY\n mv [OPTION]... -t DIRECTORY SOURCE...\n/g" -e "s/mv: missing destination file operand after 'no-file'/error: The argument '...' requires at least 2 values, but only 1 was provided\n\nUsage: mv [OPTION]... [-T] SOURCE DEST\n mv [OPTION]... SOURCE... DIRECTORY\n mv [OPTION]... -t DIRECTORY SOURCE...\n/g" tests/mv/diag.sh -# our error message is better -"${SED}" -i -e "s|mv: cannot overwrite 'a/t': Directory not empty|mv: cannot move 'b/t' to 'a/t': Directory not empty|" tests/mv/dir2dir.sh - # GNU doesn't support width > INT_MAX # disable these test cases "${SED}" -i -E "s|^([^#]*2_31.*)$|#\1|g" tests/printf/printf-cov.pl