Merge pull request #1679 from sylvestre/ln-n

feature(ln): Implement -n

src/uu/ln/src/ln.rs

@@ -27,11 +27,13 @@ use uucore::fs::{canonicalize, CanonicalizeMode};
 pub struct Settings {
     overwrite: OverwriteMode,
     backup: BackupMode,
+    force: bool,
     suffix: String,
     symbolic: bool,
     relative: bool,
     target_dir: Option<String>,
     no_target_dir: bool,
+    no_dereference: bool,
     verbose: bool,
 }
 
@@ -81,6 +83,7 @@ static OPT_B: &str = "b";
 static OPT_BACKUP: &str = "backup";
 static OPT_FORCE: &str = "force";
 static OPT_INTERACTIVE: &str = "interactive";
+static OPT_NO_DEREFERENCE: &str = "no-dereference";
 static OPT_SYMBOLIC: &str = "symbolic";
 static OPT_SUFFIX: &str = "suffix";
 static OPT_TARGET_DIRECTORY: &str = "target-directory";
@@ -136,11 +139,18 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
                 .long(OPT_INTERACTIVE)
                 .help("prompt whether to remove existing destination files"),
         )
+        .arg(
+            Arg::with_name(OPT_NO_DEREFERENCE)
+                .short("n")
+                .long(OPT_NO_DEREFERENCE)
+                .help(
+                    "treat LINK_executable!() as a normal file if it is a \
+                                                    symbolic link to a directory",
+                ),
+        )
         // TODO: opts.arg(
         //    Arg::with_name(("L", "logical", "dereference TARGETs that are symbolic links");
-        // TODO: opts.arg(
+        //
-        //    Arg::with_name(("n", "no-dereference", "treat LINK_executable!() as a normal file if it is a \
-        //                                            symbolic link to a directory");
         // TODO: opts.arg(
         //    Arg::with_name(("P", "physical", "make hard links directly to symbolic links");
         .arg(
@@ -192,7 +202,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         )
         .get_matches_from(args);
 
-    /*  the list of files */
+    /* the list of files */
 
     let paths: Vec<PathBuf> = matches
         .values_of(ARG_FILES)
@@ -234,11 +244,13 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
     let settings = Settings {
         overwrite: overwrite_mode,
         backup: backup_mode,
+        force: matches.is_present(OPT_FORCE),
         suffix: backup_suffix.to_string(),
         symbolic: matches.is_present(OPT_SYMBOLIC),
         relative: matches.is_present(OPT_RELATIVE),
         target_dir: matches.value_of(OPT_TARGET_DIRECTORY).map(String::from),
         no_target_dir: matches.is_present(OPT_NO_TARGET_DIRECTORY),
+        no_dereference: matches.is_present(OPT_NO_DEREFERENCE),
         verbose: matches.is_present(OPT_VERBOSE),
     };
 
@@ -299,24 +311,47 @@ fn link_files_in_dir(files: &[PathBuf], target_dir: &PathBuf, settings: &Setting
 
     let mut all_successful = true;
     for srcpath in files.iter() {
-        let targetpath = match srcpath.as_os_str().to_str() {
+        let targetpath = if settings.no_dereference && settings.force {
-            Some(name) => {
+            // In that case, we don't want to do link resolution
-                match Path::new(name).file_name() {
+            // We need to clean the target
-                    Some(basename) => target_dir.join(basename),
+            if is_symlink(target_dir) {
-                    // This can be None only for "." or "..". Trying
+                if target_dir.is_file() {
-                    // to create a link with such name will fail with
+                    match fs::remove_file(target_dir) {
-                    // EEXIST, which agrees with the behavior of GNU
+                        Err(e) => show_error!("Could not update {}: {}", target_dir.display(), e),
-                    // coreutils.
+                        _ => (),
-                    None => target_dir.join(name),
+                    };
+                }
+                if target_dir.is_dir() {
+                    // Not sure why but on Windows, the symlink can be
+                    // considered as a dir
+                    // See test_ln::test_symlink_no_deref_dir
+                    match fs::remove_dir(target_dir) {
+                        Err(e) => show_error!("Could not update {}: {}", target_dir.display(), e),
+                        _ => (),
+                    };
                 }
             }
-            None => {
+            target_dir.clone()
-                show_error!(
+        } else {
-                    "cannot stat '{}': No such file or directory",
+            match srcpath.as_os_str().to_str() {
-                    srcpath.display()
+                Some(name) => {
-                );
+                    match Path::new(name).file_name() {
-                all_successful = false;
+                        Some(basename) => target_dir.join(basename),
-                continue;
+                        // This can be None only for "." or "..". Trying
+                        // to create a link with such name will fail with
+                        // EEXIST, which agrees with the behavior of GNU
+                        // coreutils.
+                        None => target_dir.join(name),
+                    }
+                }
+                None => {
+                    show_error!(
+                        "cannot stat '{}': No such file or directory",
+                        srcpath.display()
+                    );
+                    all_successful = false;
+                    continue;
+                }
             }
         };
 
@@ -389,6 +424,12 @@ fn link(src: &PathBuf, dst: &PathBuf, settings: &Settings) -> Result<()> {
         }
     }
 
+    if settings.no_dereference && settings.force {
+        if dst.exists() {
+            fs::remove_file(dst)?;
+        }
+    }
+
     if settings.symbolic {
         symlink(&source, dst)?;
     } else {

tests/by-util/test_ln.rs

@@ -321,8 +321,7 @@ fn test_symlink_errors() {
 
     // $ ln -T -t a b
     // ln: cannot combine --target-directory (-t) and --no-target-directory (-T)
-    ucmd.args(&["-T", "-t", dir, file_a, file_b])
+    ucmd.args(&["-T", "-t", dir, file_a, file_b]).fails();
-        .fails();
 }
 
 #[test]
@@ -484,3 +483,95 @@ fn test_symlink_relative_dir() {
     assert!(at.is_symlink(link));
     assert_eq!(at.resolve_link(link), dir);
 }
+
+#[test]
+fn test_symlink_no_deref_dir() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    let dir1 = "foo";
+    let dir2 = "bar";
+    let link = "baz";
+
+    at.mkdir(dir1);
+    at.mkdir(dir2);
+    scene
+        .ucmd()
+        .args(&["-s", dir2, link])
+        .succeeds()
+        .no_stderr();
+    assert!(at.dir_exists(dir1));
+    assert!(at.dir_exists(dir2));
+    assert!(at.is_symlink(link));
+    assert_eq!(at.resolve_link(link), dir2);
+
+    // try the normal behavior
+    scene
+        .ucmd()
+        .args(&["-sf", dir1, link])
+        .succeeds()
+        .no_stderr();
+    assert!(at.dir_exists(dir1));
+    assert!(at.dir_exists(dir2));
+    assert!(at.is_symlink("baz/foo"));
+    assert_eq!(at.resolve_link("baz/foo"), dir1);
+
+    // Doesn't work without the force
+    scene.ucmd().args(&["-sn", dir1, link]).fails();
+
+    // Try with the no-deref
+    let result = scene.ucmd().args(&["-sfn", dir1, link]).run();
+    println!("stdout {}", result.stdout);
+    println!("stderr {}", result.stderr);
+    assert!(result.success);
+    assert!(at.dir_exists(dir1));
+    assert!(at.dir_exists(dir2));
+    assert!(at.is_symlink(link));
+    assert_eq!(at.resolve_link(link), dir1);
+}
+
+#[test]
+fn test_symlink_no_deref_file() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    let file1 = "foo";
+    let file2 = "bar";
+    let link = "baz";
+
+    at.touch(file1);
+    at.touch(file2);
+    scene
+        .ucmd()
+        .args(&["-s", file2, link])
+        .succeeds()
+        .no_stderr();
+    assert!(at.file_exists(file1));
+    assert!(at.file_exists(file2));
+    assert!(at.is_symlink(link));
+    assert_eq!(at.resolve_link(link), file2);
+
+    // try the normal behavior
+    scene
+        .ucmd()
+        .args(&["-sf", file1, link])
+        .succeeds()
+        .no_stderr();
+    assert!(at.file_exists(file1));
+    assert!(at.file_exists(file2));
+    assert!(at.is_symlink("baz"));
+    assert_eq!(at.resolve_link("baz"), file1);
+
+    // Doesn't work without the force
+    scene.ucmd().args(&["-sn", file1, link]).fails();
+
+    // Try with the no-deref
+    let result = scene.ucmd().args(&["-sfn", file1, link]).run();
+    println!("stdout {}", result.stdout);
+    println!("stderr {}", result.stderr);
+    assert!(result.success);
+    assert!(at.file_exists(file1));
+    assert!(at.file_exists(file2));
+    assert!(at.is_symlink(link));
+    assert_eq!(at.resolve_link(link), file1);
+}