diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 62509b7564..32168b0900 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -29,7 +29,7 @@ use platform::copy_on_write; use uucore::display::Quotable; use uucore::error::{set_exit_code, UClapError, UError, UResult, UUsageError}; use uucore::fs::{ - are_hardlinks_to_same_file, canonicalize, get_filename, is_symlink_loop, + are_hardlinks_to_same_file, canonicalize, get_filename, is_symlink_loop, normalize_path, path_ends_with_terminator, paths_refer_to_same_file, FileInformation, MissingHandling, ResolveMode, }; @@ -1264,9 +1264,17 @@ pub fn copy(sources: &[PathBuf], target: &Path, options: &Options) -> CopyResult }; for source in sources { - if seen_sources.contains(source) { - // FIXME: compare sources by the actual file they point to, not their path. (e.g. dir/file == dir/../dir/file in most cases) - show_warning!("source file {} specified more than once", source.quote()); + let normalized_source = normalize_path(source); + if options.backup == BackupMode::NoBackup && seen_sources.contains(&normalized_source) { + let file_type = if source.symlink_metadata()?.file_type().is_dir() { + "directory" + } else { + "file" + }; + show_warning!( + "source {file_type} {} specified more than once", + source.quote() + ); } else { let dest = construct_dest_path(source, target, target_type, options) .unwrap_or_else(|_| target.to_path_buf()); @@ -1309,7 +1317,7 @@ pub fn copy(sources: &[PathBuf], target: &Path, options: &Options) -> CopyResult copied_destinations.insert(dest.clone()); } } - seen_sources.insert(source); + seen_sources.insert(normalized_source); } if let Some(pb) = progress_bar { diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 0fba59672c..156daec1f1 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -121,6 +121,60 @@ fn test_cp_duplicate_files() { assert_eq!(at.read(TEST_COPY_TO_FOLDER_FILE), "Hello, World!\n"); } +#[test] +fn test_cp_duplicate_folder() { + let (at, mut ucmd) = at_and_ucmd!(); + ucmd.arg("-r") + .arg(TEST_COPY_FROM_FOLDER) + .arg(TEST_COPY_FROM_FOLDER) + .arg(TEST_COPY_TO_FOLDER) + .succeeds() + .stderr_contains(format!( + "source directory '{TEST_COPY_FROM_FOLDER}' specified more than once" + )); + assert!(at.dir_exists(format!("{TEST_COPY_TO_FOLDER}/{TEST_COPY_FROM_FOLDER}").as_str())); +} + +#[test] +fn test_cp_duplicate_files_normalized_path() { + let (at, mut ucmd) = at_and_ucmd!(); + ucmd.arg(TEST_HELLO_WORLD_SOURCE) + .arg(format!("./{TEST_HELLO_WORLD_SOURCE}")) + .arg(TEST_COPY_TO_FOLDER) + .succeeds() + .stderr_contains(format!( + "source file './{TEST_HELLO_WORLD_SOURCE}' specified more than once" + )); + assert_eq!(at.read(TEST_COPY_TO_FOLDER_FILE), "Hello, World!\n"); +} + +#[test] +fn test_cp_duplicate_files_with_plain_backup() { + let (_, mut ucmd) = at_and_ucmd!(); + ucmd.arg(TEST_HELLO_WORLD_SOURCE) + .arg(TEST_HELLO_WORLD_SOURCE) + .arg(TEST_COPY_TO_FOLDER) + .arg("--backup") + .fails() + // cp would skip duplicate src check and fail when it tries to overwrite the "just-created" file. + .stderr_contains( + "will not overwrite just-created 'hello_dir/hello_world.txt' with 'hello_world.txt", + ); +} + +#[test] +fn test_cp_duplicate_files_with_numbered_backup() { + let (at, mut ucmd) = at_and_ucmd!(); + // cp would skip duplicate src check and succeeds + ucmd.arg(TEST_HELLO_WORLD_SOURCE) + .arg(TEST_HELLO_WORLD_SOURCE) + .arg(TEST_COPY_TO_FOLDER) + .arg("--backup=numbered") + .succeeds(); + at.file_exists(TEST_COPY_TO_FOLDER_FILE); + at.file_exists(format!("{TEST_COPY_TO_FOLDER}.~1~")); +} + #[test] fn test_cp_same_file() { let (at, mut ucmd) = at_and_ucmd!();