Merge pull request #4184 from jfinkels/cp-verbose

cp: correct --verbose --parents output for both files and directories

src/uu/cp/src/copydir.rs

@@ -24,8 +24,8 @@ use uucore::uio_error;
 use walkdir::{DirEntry, WalkDir};
 
 use crate::{
-    copy_attributes, copy_file, copy_link, preserve_hardlinks, CopyResult, Error, Options,
+    aligned_ancestors, context_for, copy_attributes, copy_file, copy_link, preserve_hardlinks,
-    TargetSlice,
+    CopyResult, Error, Options, TargetSlice,
 };
 
 /// Ensure a Windows path starts with a `\\?`.
@@ -172,6 +172,27 @@ impl Entry {
     }
 }
 
+/// Decide whether the given path ends with `/.`.
+///
+/// # Examples
+///
+/// ```rust,ignore
+/// assert!(ends_with_slash_dot("/."));
+/// assert!(ends_with_slash_dot("./."));
+/// assert!(ends_with_slash_dot("a/."));
+///
+/// assert!(!ends_with_slash_dot("."));
+/// assert!(!ends_with_slash_dot("./"));
+/// assert!(!ends_with_slash_dot("a/.."));
+/// ```
+fn ends_with_slash_dot<P>(path: P) -> bool
+where
+    P: AsRef<Path>,
+{
+    // `path.ends_with(".")` does not seem to work
+    path.as_ref().display().to_string().ends_with("/.")
+}
+
 /// Copy a single entry during a directory traversal.
 fn copy_direntry(
     progress_bar: &Option<ProgressBar>,
@@ -196,7 +217,10 @@ fn copy_direntry(
 
     // If the source is a directory and the destination does not
     // exist, ...
-    if source_absolute.is_dir() && !local_to_target.exists() {
+    if source_absolute.is_dir()
+        && !ends_with_slash_dot(&source_absolute)
+        && !local_to_target.exists()
+    {
         if target_is_file {
             return Err("cannot overwrite non-directory with directory".into());
         } else {
@@ -205,7 +229,10 @@ fn copy_direntry(
             // `create_dir_all()` will have any benefit over
             // `create_dir()`, since all the ancestor directories
             // should have already been created.
-            fs::create_dir_all(local_to_target)?;
+            fs::create_dir_all(&local_to_target)?;
+            if options.verbose {
+                println!("{}", context_for(&source_relative, &local_to_target));
+            }
             return Ok(());
         }
     }
@@ -323,6 +350,19 @@ pub(crate) fn copy_directory(
         if let Some(parent) = root.parent() {
             let new_target = target.join(parent);
             std::fs::create_dir_all(&new_target)?;
+
+            if options.verbose {
+                // 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(root, &target.join(root)) {
+                    println!("{} -> {}", x.display(), y.display());
+                }
+            }
+
             new_target
         } else {
             target.to_path_buf()
@@ -392,3 +432,25 @@ pub fn path_has_prefix(p1: &Path, p2: &Path) -> io::Result<bool> {
 
     Ok(pathbuf1.starts_with(pathbuf2))
 }
+
+#[cfg(test)]
+mod tests {
+    use super::ends_with_slash_dot;
+
+    #[test]
+    fn test_ends_with_slash_dot() {
+        assert!(ends_with_slash_dot("/."));
+        assert!(ends_with_slash_dot("./."));
+        assert!(ends_with_slash_dot("../."));
+        assert!(ends_with_slash_dot("a/."));
+        assert!(ends_with_slash_dot("/a/."));
+
+        assert!(!ends_with_slash_dot(""));
+        assert!(!ends_with_slash_dot("."));
+        assert!(!ends_with_slash_dot("./"));
+        assert!(!ends_with_slash_dot(".."));
+        assert!(!ends_with_slash_dot("/.."));
+        assert!(!ends_with_slash_dot("a/.."));
+        assert!(!ends_with_slash_dot("/a/.."));
+    }
+}

src/uu/cp/src/cp.rs

@@ -1316,6 +1316,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.
@@ -1375,9 +1416,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));
         }
     }
@@ -1676,15 +1741,29 @@ fn disk_usage_directory(p: &Path) -> io::Result<u64> {
     Ok(total)
 }
 
-#[test]
+#[cfg(test)]
-fn test_cp_localize_to_target() {
+mod tests {
-    assert!(
+
-        localize_to_target(
+    use crate::{aligned_ancestors, localize_to_target};
-            Path::new("a/source/"),
+    use std::path::Path;
-            Path::new("a/source/c.txt"),
+
-            Path::new("target/")
+    #[test]
-        )
+    fn test_cp_localize_to_target() {
-        .unwrap()
+        let root = Path::new("a/source/");
-            == Path::new("target/c.txt")
+        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);
+    }
 }

tests/by-util/test_cp.rs

@@ -26,7 +26,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";
@@ -2054,49 +2053,70 @@ 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!(
+        .stdout_only(expected_stdout);
-            "{} -> {}\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()
-        ));
     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!(
+        .stdout_only(expected_stdout);
-            "{} -> {}\n{} -> {}\n{} -> {}\n",
+    assert!(at.dir_exists("d/a/link"));
-            "a",
+    assert!(!at.symlink_exists("d/a/link"));
-            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"));
     assert!(at.file_exists("d/a/link/c"));
 }
 
+#[test]
+fn test_cp_parents_2_dir() {
+    let (at, mut ucmd) = at_and_ucmd!();
+    at.mkdir_all("a/b/c");
+    at.mkdir("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", "-r", "--parents", "a/b/c", "d"])
+        .succeeds()
+        .stdout_only(expected_stdout);
+    assert!(at.dir_exists("d/a/b/c"));
+}
+
+#[test]
+fn test_cp_parents_2_deep_dir() {
+    let (at, mut ucmd) = at_and_ucmd!();
+    at.mkdir_all("a/b/c");
+    at.mkdir_all("d/e");
+    #[cfg(not(windows))]
+    let expected_stdout = "a -> d/e/a\na/b -> d/e/a/b\n'a/b/c' -> 'd/e/a/b/c'\n";
+    #[cfg(windows)]
+    let expected_stdout = "a -> d/e\\a\na/b -> d/e\\a/b\n'a/b/c' -> 'd/e\\a/b\\c'\n";
+    ucmd.args(&["--verbose", "-r", "--parents", "a/b/c", "d/e"])
+        .succeeds()
+        .stdout_only(expected_stdout);
+    assert!(at.dir_exists("d/e/a/b/c"));
+}
+
 #[test]
 fn test_cp_copy_symlink_contents_recursive() {
     let (at, mut ucmd) = at_and_ucmd!();
@@ -2419,3 +2439,18 @@ fn test_symbolic_link_file() {
         Path::new("src")
     );
 }
+
+#[test]
+fn test_src_base_dot() {
+    let ts = TestScenario::new(util_name!());
+    let at = ts.fixtures.clone();
+    at.mkdir("x");
+    at.mkdir("y");
+    let mut ucmd = UCommand::new(ts.bin_path, &Some(ts.util_name), at.plus("y"), true);
+
+    ucmd.args(&["--verbose", "-r", "../x/.", "."])
+        .succeeds()
+        .no_stderr()
+        .no_stdout();
+    assert!(!at.dir_exists("y/x"));
+}