From 47c5bba409fc249dfd201b6b1b2572fb5587d447 Mon Sep 17 00:00:00 2001
From: Jeffrey Finkelstein <jeffrey.finkelstein@protonmail.com>
Date: Thu, 1 Sep 2022 22:12:16 -0400
Subject: [PATCH] cp: correctly copy ancestor dirs in --parents mode

Fix a bug where `cp` failed to copy ancestor directories when using
the `--parents` option. For example, before this commit:

    $ mkdir -p a/b/c d
    $ cp --parents a/b/c d
    $ find d
    d
    d/c

After this commit

    $ mkdir -p a/b/c d
    $ cp --parents a/b/c d
    $ find d
    d
    d/a
    d/a/b
    d/a/b/c

This commit also adds the correct messages for `--verbose` mode:

    $ cp -r --parents --verbose 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      | 37 ++++++++++++++++++++++++++++++++++++-
 tests/by-util/test_cp.rs | 24 ++++++++++++++++++++++++
 2 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs
index 1d1fac9a2ed..ab0ac98c26c 100644
--- a/src/uu/cp/src/cp.rs
+++ b/src/uu/cp/src/cp.rs
@@ -1097,6 +1097,38 @@ fn copy_directory(
         .into());
     }
 
+    // If in `--parents` mode, create all the necessary ancestor directories.
+    //
+    // For example, if the command is `cp --parents a/b/c d`, that
+    // means we need to copy the two ancestor directories first:
+    //
+    // a -> d/a
+    // a/b -> d/a/b
+    //
+    let tmp = if options.parents {
+        if let Some(parent) = root.parent() {
+            let new_target = target.join(parent);
+            std::fs::create_dir_all(&new_target)?;
+
+            if options.verbose {
+                let mut ancestors = vec![];
+                for p in parent.ancestors() {
+                    ancestors.push(p);
+                }
+                for p in ancestors.iter().rev().skip(1) {
+                    println!("{}", context_for(p, &target.join(p)));
+                }
+            }
+
+            new_target
+        } else {
+            target.to_path_buf()
+        }
+    } else {
+        target.to_path_buf()
+    };
+    let target = tmp.as_path();
+
     let current_dir =
         env::current_dir().unwrap_or_else(|e| crash!(1, "failed to get current directory {}", e));
 
@@ -1155,7 +1187,10 @@ fn copy_directory(
             if target.is_file() {
                 return Err("cannot overwrite non-directory with directory".into());
             }
-            fs::create_dir_all(local_to_target)?;
+            fs::create_dir_all(&local_to_target)?;
+            if options.verbose {
+                println!("{}", context_for(p.path(), &local_to_target));
+            }
         } else if !path.is_dir() {
             if preserve_hard_links {
                 let mut found_hard_link = false;
diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs
index 7f9b0aab90b..ef6b3289cf5 100644
--- a/tests/by-util/test_cp.rs
+++ b/tests/by-util/test_cp.rs
@@ -1990,6 +1990,30 @@ fn test_copy_same_symlink_no_dereference_dangling() {
     ucmd.args(&["-d", "a", "b"]).succeeds();
 }
 
+#[test]
+fn test_cp_parents_2_dirs() {
+    let (at, mut ucmd) = at_and_ucmd!();
+    at.mkdir_all("a/b/c");
+    at.mkdir("d");
+    // TODO We should iron out exactly what the `--verbose` behavior
+    // should be on Windows. Currently, we have it printing, for
+    // example,
+    //
+    //     a/b -> d\a/b
+    //
+    // Should the path separators all be forward slashes? All
+    // backslashes?
+    #[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", "-a", "--parents", "a/b/c", "d"])
+        .succeeds()
+        .no_stderr()
+        .stdout_is(expected_stdout);
+    assert!(at.dir_exists("d/a/b/c"));
+}
+
 #[test]
 #[ignore = "issue #3332"]
 fn test_cp_parents_2() {