cp: link-deref gnu test fix (#6378)

matrixhead · BenWiederhake · web-flow · commit 2e16cbbd7a74 · 2024-05-19T10:14:29.000+02:00
Co-authored-by: Ben Wiederhake &lt;BenWiederhake.GitHub@gmx.de&gt;
diff --git a/src/uu/cp/src/copydir.rs b/src/uu/cp/src/copydir.rs
@@ -337,10 +337,6 @@ pub(crate) fn copy_directory(
     copied_files: &mut HashMap<FileInformation, PathBuf>,
     source_in_command_line: bool,
 ) -> CopyResult<()> {
-    if !options.recursive {
-        return Err(format!("-r not specified; omitting directory {}", root.quote()).into());
-    }
-
     // if no-dereference is enabled and this is a symlink, copy it as a file
     if !options.dereference(source_in_command_line) && root.is_symlink() {
         return copy_file(
@@ -355,6 +351,10 @@ pub(crate) fn copy_directory(
         );
     }
 
+    if !options.recursive {
+        return Err(format!("-r not specified; omitting directory {}", root.quote()).into());
+    }
+
     // check if root is a prefix of target
     if path_has_prefix(target, root)? {
         return Err(format!(
diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs
@@ -977,7 +977,9 @@ impl Options {
             dereference: !(matches.get_flag(options::NO_DEREFERENCE)
                 || matches.get_flag(options::NO_DEREFERENCE_PRESERVE_LINKS)
                 || matches.get_flag(options::ARCHIVE)
-                || recursive)
+                // cp normally follows the link only when not copying recursively or when
+                // --link (-l) is used
+                || (recursive && CopyMode::from_matches(matches)!= CopyMode::Link ))
                 || matches.get_flag(options::DEREFERENCE),
             one_file_system: matches.get_flag(options::ONE_FILE_SYSTEM),
             parents: matches.get_flag(options::PARENTS),
@@ -2063,7 +2065,13 @@ fn copy_file(
         } else {
             fs::symlink_metadata(source)
         };
-        result.context(context)?
+        // this is just for gnu tests compatibility
+        result.map_err(|err| {
+            if err.to_string().contains("No such file or directory") {
+                return format!("cannot stat {}: No such file or directory", source.quote());
+            }
+            err.to_string()
+        })?
     };
 
     let dest_permissions = calculate_dest_permissions(dest, &source_metadata, options, context)?;
diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs
@@ -5307,3 +5307,138 @@ mod same_file {
         assert_eq!(at.read(FILE_NAME), CONTENTS,);
     }
 }
+
+// the following tests are for how the cp should behave when the source is a symlink
+// and link option is given
+#[cfg(all(unix, not(target_os = "android")))]
+mod link_deref {
+
+    use crate::common::util::{AtPath, TestScenario};
+    use std::os::unix::fs::MetadataExt;
+
+    const FILE: &str = "file";
+    const FILE_LINK: &str = "file_link";
+    const DIR: &str = "dir";
+    const DIR_LINK: &str = "dir_link";
+    const DANG_LINK: &str = "dang_link";
+    const DST: &str = "dst";
+
+    fn setup_link_deref_tests(source: &str, at: &AtPath) {
+        match source {
+            FILE_LINK => {
+                at.touch(FILE);
+                at.symlink_file(FILE, FILE_LINK);
+            }
+            DIR_LINK => {
+                at.mkdir(DIR);
+                at.symlink_dir(DIR, DIR_LINK);
+            }
+            DANG_LINK => at.symlink_file("nowhere", DANG_LINK),
+            _ => {}
+        }
+    }
+
+    // cp --link shouldn't deref source if -P is given
+    #[test]
+    fn test_cp_symlink_as_source_with_link_and_no_deref() {
+        for src in [FILE_LINK, DIR_LINK, DANG_LINK] {
+            for r in [false, true] {
+                let scene = TestScenario::new(util_name!());
+                let at = &scene.fixtures;
+                setup_link_deref_tests(src, at);
+                let mut args = vec!["--link", "-P", src, DST];
+                if r {
+                    args.push("-R");
+                };
+                scene.ucmd().args(&args).succeeds().no_stderr();
+                at.is_symlink(DST);
+                let src_ino = at.symlink_metadata(src).ino();
+                let dest_ino = at.symlink_metadata(DST).ino();
+                assert_eq!(src_ino, dest_ino);
+            }
+        }
+    }
+
+    // Dereferencing should fail for dangling symlink.
+    #[test]
+    fn test_cp_dang_link_as_source_with_link() {
+        for option in ["", "-L", "-H"] {
+            for r in [false, true] {
+                let scene = TestScenario::new(util_name!());
+                let at = &scene.fixtures;
+                setup_link_deref_tests(DANG_LINK, at);
+                let mut args = vec!["--link", DANG_LINK, DST];
+                if r {
+                    args.push("-R");
+                };
+                if !option.is_empty() {
+                    args.push(option);
+                }
+                scene
+                    .ucmd()
+                    .args(&args)
+                    .fails()
+                    .stderr_contains("No such file or directory");
+            }
+        }
+    }
+
+    // Dereferencing should fail for the 'dir_link' without -R.
+    #[test]
+    fn test_cp_dir_link_as_source_with_link() {
+        for option in ["", "-L", "-H"] {
+            let scene = TestScenario::new(util_name!());
+            let at = &scene.fixtures;
+            setup_link_deref_tests(DIR_LINK, at);
+            let mut args = vec!["--link", DIR_LINK, DST];
+            if !option.is_empty() {
+                args.push(option);
+            }
+            scene
+                .ucmd()
+                .args(&args)
+                .fails()
+                .stderr_contains("cp: -r not specified; omitting directory");
+        }
+    }
+
+    // cp --link -R 'dir_link' should create a new directory.
+    #[test]
+    fn test_cp_dir_link_as_source_with_link_and_r() {
+        for option in ["", "-L", "-H"] {
+            let scene = TestScenario::new(util_name!());
+            let at = &scene.fixtures;
+            setup_link_deref_tests(DIR_LINK, at);
+            let mut args = vec!["--link", "-R", DIR_LINK, DST];
+            if !option.is_empty() {
+                args.push(option);
+            }
+            scene.ucmd().args(&args).succeeds();
+            at.dir_exists(DST);
+        }
+    }
+
+    //cp --link 'file_link' should create a hard link to the target.
+    #[test]
+    fn test_cp_file_link_as_source_with_link() {
+        for option in ["", "-L", "-H"] {
+            for r in [false, true] {
+                let scene = TestScenario::new(util_name!());
+                let at = &scene.fixtures;
+                setup_link_deref_tests(FILE_LINK, at);
+                let mut args = vec!["--link", "-R", FILE_LINK, DST];
+                if !option.is_empty() {
+                    args.push(option);
+                }
+                if r {
+                    args.push("-R");
+                }
+                scene.ucmd().args(&args).succeeds();
+                at.file_exists(DST);
+                let src_ino = at.symlink_metadata(FILE).ino();
+                let dest_ino = at.symlink_metadata(DST).ino();
+                assert_eq!(src_ino, dest_ino);
+            }
+        }
+    }
+}
