Skip to content

cp: normalize path when checking for duplicate source #6825

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 31, 2024
Merged

cp: normalize path when checking for duplicate source #6825

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
74cd797
cp: normalize path when checking for duplicate source
matrixhead Oct 26, 2024
4d5bf42
cp: skip duplicate source check when --backup is given
matrixhead Oct 27, 2024
ff586b4
cp: Duplicate source error message specify the type of file.
matrixhead Oct 31, 2024
e947c71
cp: separate tests with empty line
cakebaker Oct 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions src/uu/cp/src/cp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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 {
Expand Down
54 changes: 54 additions & 0 deletions tests/by-util/test_cp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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!();
Expand Down
Loading