diff --git a/src/uu/cp/src/copydir.rs b/src/uu/cp/src/copydir.rs index c312b7cbb4a..aaeb73f5acf 100644 --- a/src/uu/cp/src/copydir.rs +++ b/src/uu/cp/src/copydir.rs @@ -404,8 +404,18 @@ pub(crate) fn copy_directory( Err(e) => show_error!("{}", e), } } + // Copy the attributes from the root directory to the target directory. - copy_attributes(root, target, &options.attributes)?; + if options.parents { + let dest = target.join(root.file_name().unwrap()); + copy_attributes(root, dest.as_path(), &options.attributes)?; + for (x, y) in aligned_ancestors(root, dest.as_path()) { + copy_attributes(x, y, &options.attributes)?; + } + } else { + copy_attributes(root, target, &options.attributes)?; + } + Ok(()) } diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 642bbcb145f..f7069f04f76 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -1151,14 +1151,20 @@ fn copy_source( } else { // Copy as file let dest = construct_dest_path(source_path, target, target_type, options)?; - copy_file( + let res = copy_file( progress_bar, source_path, dest.as_path(), options, symlinked_files, true, - ) + ); + if options.parents { + for (x, y) in aligned_ancestors(source, dest.as_path()) { + copy_attributes(x, y, &options.attributes)?; + } + } + res } } @@ -1704,11 +1710,6 @@ fn copy_file( } copy_attributes(source, dest, &options.attributes)?; - if options.parents && should_preserve_attribute(options) { - for (x, y) in aligned_ancestors(source, dest) { - copy_attributes(x, y, &options.attributes)?; - } - } if let Some(progress_bar) = progress_bar { progress_bar.inc(fs::metadata(source)?.len()); @@ -1758,27 +1759,6 @@ fn copy_helper( Ok(()) } -fn should_preserve_attribute(options: &Options) -> bool { - let checks = [ - &options.attributes.mode, - &options.attributes.timestamps, - &options.attributes.links, - &options.attributes.context, - &options.attributes.xattr, - ]; - - #[cfg(unix)] - let checks = [ - checks.as_slice(), - [&options.attributes.ownership].as_slice(), - ] - .concat(); - - checks - .iter() - .any(|attr| matches!(attr, Preserve::Yes { .. })) -} - // "Copies" a FIFO by creating a new one. This workaround is because Rust's // built-in fs::copy does not handle FIFOs (see rust-lang/rust/issues/79390). #[cfg(unix)] diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 86fbf9dcc0a..ef35f6c2ded 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -1115,6 +1115,48 @@ fn test_cp_parents_with_permissions_copy_file() { } } +#[test] +fn test_cp_parents_with_permissions_copy_dir() { + let (at, mut ucmd) = at_and_ucmd!(); + + let dir1 = "dir"; + let dir2 = "p1/p2"; + let file = "p1/p2/file"; + + at.mkdir(dir1); + at.mkdir_all(dir2); + at.touch(file); + + let p1_mode = 0o0777; + let p2_mode = 0o0711; + let file_mode = 0o0702; + + #[cfg(unix)] + { + at.set_mode("p1", p1_mode); + at.set_mode("p1/p2", p2_mode); + at.set_mode(file, file_mode); + } + + ucmd.arg("-p") + .arg("--parents") + .arg("-r") + .arg(dir2) + .arg(dir1) + .succeeds(); + + #[cfg(all(unix, not(target_os = "freebsd")))] + { + let p1_metadata = at.metadata("p1"); + let p2_metadata = at.metadata("p1/p2"); + let file_metadata = at.metadata(file); + + assert_metadata_eq!(p1_metadata, at.metadata("dir/p1")); + assert_metadata_eq!(p2_metadata, at.metadata("dir/p1/p2")); + assert_metadata_eq!(file_metadata, at.metadata("dir/p1/p2/file")); + } +} + #[test] #[cfg(unix)] fn test_cp_writable_special_file_permissions() {