From f8a9338aa4793e527f9ba997b0d35335850275ec Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Sun, 6 Nov 2022 10:03:12 -0500 Subject: [PATCH] cp: correct --verbose --parents output for files This commit corrects the behavior of `cp --parents --verbose` when the source path is a file so that it prints the copied ancestor directories. For example, $ mkdir -p a/b d $ touch a/b/c $ cp --verbose --parents a/b/c d a -> d/a a/b -> d/a/b 'a/b/c' -> 'd/a/b/c' Fixes #3332. --- src/uu/cp/src/cp.rs | 101 ++++++++++++++++++++++++++++++++++----- tests/by-util/test_cp.rs | 38 ++++++--------- 2 files changed, 104 insertions(+), 35 deletions(-) diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 9664ba66f4d..50da71f5094 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -1294,6 +1294,47 @@ fn file_or_link_exists(path: &Path) -> bool { path.symlink_metadata().is_ok() } +/// Zip the ancestors of a source path and destination path. +/// +/// # Examples +/// +/// ```rust,ignore +/// let actual = aligned_ancestors(&Path::new("a/b/c"), &Path::new("d/a/b/c")); +/// let expected = vec![ +/// (Path::new("a"), Path::new("d/a")), +/// (Path::new("a/b"), Path::new("d/a/b")), +/// ]; +/// assert_eq!(actual, expected); +/// ``` +fn aligned_ancestors<'a>(source: &'a Path, dest: &'a Path) -> Vec<(&'a Path, &'a Path)> { + // Collect the ancestors of each. For example, if `source` is + // "a/b/c", then the ancestors are "a/b/c", "a/b", "a/", and "". + let source_ancestors: Vec<&Path> = source.ancestors().collect(); + let dest_ancestors: Vec<&Path> = dest.ancestors().collect(); + + // For this particular application, we don't care about the null + // path "" and we don't care about the full path (e.g. "a/b/c"), + // so we exclude those. + let n = source_ancestors.len(); + let source_ancestors = &source_ancestors[1..n - 1]; + + // Get the matching number of elements from the ancestors of the + // destination path (for example, get "d/a" and "d/a/b"). + let k = source_ancestors.len(); + let dest_ancestors = &dest_ancestors[1..1 + k]; + + // Now we have two slices of the same length, so we zip them. + let mut result = vec![]; + for (x, y) in source_ancestors + .iter() + .rev() + .zip(dest_ancestors.iter().rev()) + { + result.push((*x, *y)); + } + result +} + /// Copy the a file from `source` to `dest`. `source` will be dereferenced if /// `options.dereference` is set to true. `dest` will be dereferenced only if /// the source was not a symlink. @@ -1353,9 +1394,33 @@ fn copy_file( if let Some(pb) = progress_bar { // Suspend (hide) the progress bar so the println won't overlap with the progress bar. pb.suspend(|| { + if options.parents { + // For example, if copying file `a/b/c` and its parents + // to directory `d/`, then print + // + // a -> d/a + // a/b -> d/a/b + // + for (x, y) in aligned_ancestors(source, dest) { + println!("{} -> {}", x.display(), y.display()); + } + } + println!("{}", context_for(source, dest)); }); } else { + if options.parents { + // For example, if copying file `a/b/c` and its parents + // to directory `d/`, then print + // + // a -> d/a + // a/b -> d/a/b + // + for (x, y) in aligned_ancestors(source, dest) { + println!("{} -> {}", x.display(), y.display()); + } + } + println!("{}", context_for(source, dest)); } } @@ -1648,15 +1713,29 @@ fn disk_usage_directory(p: &Path) -> io::Result { Ok(total) } -#[test] -fn test_cp_localize_to_target() { - assert!( - localize_to_target( - Path::new("a/source/"), - Path::new("a/source/c.txt"), - Path::new("target/") - ) - .unwrap() - == Path::new("target/c.txt") - ); +#[cfg(test)] +mod tests { + + use crate::{aligned_ancestors, localize_to_target}; + use std::path::Path; + + #[test] + fn test_cp_localize_to_target() { + let root = Path::new("a/source/"); + let source = Path::new("a/source/c.txt"); + let target = Path::new("target/"); + let actual = localize_to_target(root, source, target).unwrap(); + let expected = Path::new("target/c.txt"); + assert_eq!(actual, expected); + } + + #[test] + fn test_aligned_ancestors() { + let actual = aligned_ancestors(&Path::new("a/b/c"), &Path::new("d/a/b/c")); + let expected = vec![ + (Path::new("a"), Path::new("d/a")), + (Path::new("a/b"), Path::new("d/a/b")), + ]; + assert_eq!(actual, expected); + } } diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 9ced1d130e9..0f32dedbf7f 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -24,7 +24,6 @@ use std::fs as std_fs; use std::thread::sleep; #[cfg(not(target_os = "freebsd"))] use std::time::Duration; -use uucore::display::Quotable; static TEST_EXISTING_FILE: &str = "existing_file.txt"; static TEST_HELLO_WORLD_SOURCE: &str = "hello_world.txt"; @@ -2052,46 +2051,37 @@ fn test_cp_parents_2_dirs() { } #[test] -#[ignore = "issue #3332"] fn test_cp_parents_2() { let (at, mut ucmd) = at_and_ucmd!(); at.mkdir_all("a/b"); at.touch("a/b/c"); at.mkdir("d"); - ucmd.args(&["--verbose", "-a", "--parents", "a/b/c", "d"]) + #[cfg(not(windows))] + let expected_stdout = "a -> d/a\na/b -> d/a/b\n'a/b/c' -> 'd/a/b/c'\n"; + #[cfg(windows)] + let expected_stdout = "a -> d\\a\na/b -> d\\a/b\n'a/b/c' -> 'd\\a/b/c'\n"; + ucmd.args(&["--verbose", "--parents", "a/b/c", "d"]) .succeeds() - .stdout_is(format!( - "{} -> {}\n{} -> {}\n{} -> {}\n", - "a", - path_concat!("d", "a"), - path_concat!("a", "b"), - path_concat!("d", "a", "b"), - path_concat!("a", "b", "c").quote(), - path_concat!("d", "a", "b", "c").quote() - )); + .stdout_only(expected_stdout); assert!(at.file_exists("d/a/b/c")); } #[test] -#[ignore = "issue #3332"] fn test_cp_parents_2_link() { let (at, mut ucmd) = at_and_ucmd!(); at.mkdir_all("a/b"); at.touch("a/b/c"); at.mkdir("d"); at.relative_symlink_file("b", "a/link"); - ucmd.args(&["--verbose", "-a", "--parents", "a/link/c", "d"]) + #[cfg(not(windows))] + let expected_stdout = "a -> d/a\na/link -> d/a/link\n'a/link/c' -> 'd/a/link/c'\n"; + #[cfg(windows)] + let expected_stdout = "a -> d\\a\na/link -> d\\a/link\n'a/link/c' -> 'd\\a/link/c'\n"; + ucmd.args(&["--verbose", "--parents", "a/link/c", "d"]) .succeeds() - .stdout_is(format!( - "{} -> {}\n{} -> {}\n{} -> {}\n", - "a", - path_concat!("d", "a"), - path_concat!("a", "link"), - path_concat!("d", "a", "link"), - path_concat!("a", "link", "c").quote(), - path_concat!("d", "a", "link", "c").quote() - )); - assert!(at.dir_exists("d/a/link") && !at.symlink_exists("d/a/link")); + .stdout_only(expected_stdout); + assert!(at.dir_exists("d/a/link")); + assert!(!at.symlink_exists("d/a/link")); assert!(at.file_exists("d/a/link/c")); }