make cp can copy folders contains dangling symbolic link (#5645)

* cp with no dangling link

* add -p to not follow symbolic link

* change comment

* add one more test case to check symblink body after copied

* better help message

crates/nu-command/src/filesystem/cp.rs

@@ -1,8 +1,9 @@
+use std::fs::read_link;
 use std::path::PathBuf;
 
 use nu_engine::env::current_dir;
 use nu_engine::CallExt;
-use nu_path::canonicalize_with;
+use nu_path::{canonicalize_with, expand_path_with};
 use nu_protocol::ast::Call;
 use nu_protocol::engine::{Command, EngineState, Stack};
 use nu_protocol::{
@@ -54,6 +55,11 @@ impl Command for Cp {
             // TODO: add back in additional features
             // .switch("force", "suppress error when no file", Some('f'))
             .switch("interactive", "ask user to confirm action", Some('i'))
+            .switch(
+                "no-dereference",
+                "If the -r option is specified, no symbolic links are followed.",
+                Some('p'),
+            )
             .category(Category::FileSystem)
     }
 
@@ -70,9 +76,9 @@ impl Command for Cp {
         let verbose = call.has_flag("verbose");
         let interactive = call.has_flag("interactive");
 
-        let path = current_dir(engine_state, stack)?;
-        let source = path.join(src.item.as_str());
-        let destination = path.join(dst.item.as_str());
+        let current_dir_path = current_dir(engine_state, stack)?;
+        let source = current_dir_path.join(src.item.as_str());
+        let destination = current_dir_path.join(dst.item.as_str());
 
         // check if destination is a dir and it exists
         let path_last_char = destination.as_os_str().to_string_lossy().chars().last();
@@ -140,7 +146,7 @@ impl Command for Cp {
             if entry.is_file() {
                 let sources = sources.paths_applying_with(|(source_file, _depth_level)| {
                     if destination.is_dir() {
-                        let mut dest = canonicalize_with(&dst.item, &path)?;
+                        let mut dest = canonicalize_with(&dst.item, &current_dir_path)?;
                         if let Some(name) = entry.file_name() {
                             dest.push(name);
                         }
@@ -153,7 +159,7 @@ impl Command for Cp {
                 for (src, dst) in sources {
                     if src.is_file() {
                         let res = if interactive && dst.exists() {
-                            interactive_copy_file(interactive, src, dst, span)
+                            interactive_copy(interactive, src, dst, span, copy_file)
                         } else {
                             copy_file(src, dst, span)
                         };
@@ -188,9 +194,23 @@ impl Command for Cp {
                     )
                 })?;
 
+                let not_follow_symlink = call.has_flag("no-dereference");
                 let sources = sources.paths_applying_with(|(source_file, depth_level)| {
                     let mut dest = destination.clone();
-                    let path = canonicalize_with(&source_file, &path)?;
+
+                    let path = if not_follow_symlink {
+                        expand_path_with(&source_file, &current_dir_path)
+                    } else {
+                        canonicalize_with(&source_file, &current_dir_path).or_else(|err| {
+                            // check if dangling symbolic link.
+                            let path = expand_path_with(&source_file, &current_dir_path);
+                            if path.is_symlink() && !path.exists() {
+                                Ok(path)
+                            } else {
+                                Err(err)
+                            }
+                        })?
+                    };
 
                     #[allow(clippy::needless_collect)]
                     let comps: Vec<_> = path
@@ -220,14 +240,21 @@ impl Command for Cp {
                         })?;
                     }
 
-                    if s.is_file() {
+                    if s.is_symlink() && not_follow_symlink {
                         let res = if interactive && d.exists() {
-                            interactive_copy_file(interactive, s, d, span)
+                            interactive_copy(interactive, s, d, span, copy_symlink)
+                        } else {
+                            copy_symlink(s, d, span)
+                        };
+                        result.push(res);
+                    } else if s.is_file() {
+                        let res = if interactive && d.exists() {
+                            interactive_copy(interactive, s, d, span, copy_file)
                         } else {
                             copy_file(s, d, span)
                         };
                         result.push(res);
-                    }
+                    };
                 }
             }
         }
@@ -265,7 +292,13 @@ impl Command for Cp {
     }
 }
 
-fn interactive_copy_file(interactive: bool, src: PathBuf, dst: PathBuf, span: Span) -> Value {
+fn interactive_copy(
+    interactive: bool,
+    src: PathBuf,
+    dst: PathBuf,
+    span: Span,
+    copy_impl: impl Fn(PathBuf, PathBuf, Span) -> Value,
+) -> Value {
     let (interaction, confirmed) =
         try_interaction(interactive, "cp: overwrite", &dst.to_string_lossy());
     if let Err(e) = interaction {
@@ -282,7 +315,7 @@ fn interactive_copy_file(interactive: bool, src: PathBuf, dst: PathBuf, span: Sp
         let msg = format!("{:} not copied to {:}", src.display(), dst.display());
         Value::String { val: msg, span }
     } else {
-        copy_file(src, dst, span)
+        copy_impl(src, dst, span)
     }
 }
 
@@ -293,13 +326,51 @@ fn copy_file(src: PathBuf, dst: PathBuf, span: Span) -> Value {
             Value::String { val: msg, span }
         }
         Err(e) => Value::Error {
-            error: ShellError::GenericError(
-                e.to_string(),
-                e.to_string(),
-                Some(span),
-                None,
-                Vec::new(),
-            ),
+            error: ShellError::FileNotFoundCustom(format!("copy file {src:?} failed: {e}"), span),
+        },
+    }
+}
+
+fn copy_symlink(src: PathBuf, dst: PathBuf, span: Span) -> Value {
+    let target_path = read_link(src.as_path());
+    let target_path = match target_path {
+        Ok(p) => p,
+        Err(err) => {
+            return Value::Error {
+                error: ShellError::GenericError(
+                    err.to_string(),
+                    err.to_string(),
+                    Some(span),
+                    None,
+                    vec![],
+                ),
+            }
+        }
+    };
+
+    let create_symlink = {
+        #[cfg(unix)]
+        {
+            std::os::unix::fs::symlink
+        }
+
+        #[cfg(windows)]
+        {
+            if !target_path.exists() || target_path.is_file() {
+                std::os::windows::fs::symlink_file
+            } else {
+                std::os::windows::fs::symlink_dir
+            }
+        }
+    };
+
+    match create_symlink(target_path.as_path(), dst.as_path()) {
+        Ok(_) => {
+            let msg = format!("copied {:} to {:}", src.display(), dst.display());
+            Value::String { val: msg, span }
+        }
+        Err(e) => Value::Error {
+            error: ShellError::GenericError(e.to_string(), e.to_string(), Some(span), None, vec![]),
         },
     }
 }

crates/nu-command/tests/commands/cp.rs

@@ -1,3 +1,4 @@
+use nu_test_support::fs::file_contents;
 use nu_test_support::fs::{files_exist_at, AbsoluteFile, Stub::EmptyFile};
 use nu_test_support::nu;
 use nu_test_support::playground::Playground;
@@ -249,3 +250,71 @@ fn copy_to_non_existing_dir() {
         assert!(actual.err.contains("destination directory does not exist"));
     });
 }
+
+#[test]
+fn copy_dir_contains_symlink_ignored() {
+    Playground::setup("cp_test_12", |_dirs, sandbox| {
+        sandbox
+            .within("tmp_dir")
+            .with_files(vec![EmptyFile("hello_there"), EmptyFile("good_bye")])
+            .within("tmp_dir")
+            .symlink("good_bye", "dangle_symlink");
+
+        // make symbolic link and copy.
+        nu!(
+            cwd: sandbox.cwd(),
+            "rm tmp_dir/good_bye; cp -r tmp_dir tmp_dir_2",
+        );
+
+        // check hello_there exists inside `tmp_dir_2`, and `dangle_symlink` don't exists inside `tmp_dir_2`.
+        let expected = sandbox.cwd().join("tmp_dir_2");
+        assert!(files_exist_at(vec!["hello_there"], expected.clone()));
+        let path = expected.join("dangle_symlink");
+        assert!(!path.exists() && !path.is_symlink());
+    });
+}
+
+#[test]
+fn copy_dir_contains_symlink() {
+    Playground::setup("cp_test_13", |_dirs, sandbox| {
+        sandbox
+            .within("tmp_dir")
+            .with_files(vec![EmptyFile("hello_there"), EmptyFile("good_bye")])
+            .within("tmp_dir")
+            .symlink("good_bye", "dangle_symlink");
+
+        // make symbolic link and copy.
+        nu!(
+            cwd: sandbox.cwd(),
+            "rm tmp_dir/good_bye; cp -r -p tmp_dir tmp_dir_2",
+        );
+
+        // check hello_there exists inside `tmp_dir_2`, and `dangle_symlink` also exists inside `tmp_dir_2`.
+        let expected = sandbox.cwd().join("tmp_dir_2");
+        assert!(files_exist_at(vec!["hello_there"], expected.clone()));
+        let path = expected.join("dangle_symlink");
+        assert!(path.is_symlink());
+    });
+}
+
+#[test]
+fn copy_dir_symlink_file_body_not_changed() {
+    Playground::setup("cp_test_14", |_dirs, sandbox| {
+        sandbox
+            .within("tmp_dir")
+            .with_files(vec![EmptyFile("hello_there"), EmptyFile("good_bye")])
+            .within("tmp_dir")
+            .symlink("good_bye", "dangle_symlink");
+
+        // make symbolic link and copy.
+        nu!(
+            cwd: sandbox.cwd(),
+            "rm tmp_dir/good_bye; cp -r -p tmp_dir tmp_dir_2; rm -r tmp_dir; cp -r -p tmp_dir_2 tmp_dir; echo hello_data | save tmp_dir/good_bye",
+        );
+
+        // check dangle_symlink in tmp_dir is no longer dangling.
+        let expected_file = sandbox.cwd().join("tmp_dir").join("dangle_symlink");
+        let actual = file_contents(expected_file);
+        assert!(actual.contains("hello_data"));
+    });
+}

crates/nu-test-support/src/playground/play.rs

@@ -228,7 +228,9 @@ impl<'a> Playground<'a> {
 
     pub fn within(&mut self, directory: &str) -> &mut Self {
         self.cwd.push(directory);
-        std::fs::create_dir(&self.cwd).expect("can not create directory");
+        if !(self.cwd.exists() && self.cwd.is_dir()) {
+            std::fs::create_dir(&self.cwd).expect("can not create directory");
+        }
         self
     }