Repository: pkolaczk/fclones (language: rust)

Issue:
Add flag to output symlink instead of target
It would be nice to have a flag to report on the symlink name instead of the target, or treat symlinks as the target files themselves. This would fill a niche where symlinks provide an alternative name that's more useful than the target file name. This also allows flcones to list duplicate symlinks so that they can be cleaned up when the target files are removed.

Say you have the following files structure:

- `/a/file`
- `/b/file`
- `/c/symlink`

Where `/a/file` and `/b/file` are identical files. `/c/symlink` is a relative symlink to `/a/file`.

```
mkdir a b c
dd if=/dev/urandom of=a/file bs=1M count=10
cp a/file b/file
ln -s ../a/file c/symlink
```

If you use `--follow-links`, fclones will report on the target file `/a/test` instead of `/c/symlink`.
```
[2022-02-28 08:34:34.340] fclones:  info: Started grouping
[2022-02-28 08:34:34.343] fclones:  info: Scanned 7 file entries
[2022-02-28 08:34:34.343] fclones:  info: Found 2 (21.0 MB) files matching selection criteria
[2022-02-28 08:34:34.343] fclones:  info: Found 1 (10.5 MB) candidates after grouping by size
[2022-02-28 08:34:34.343] fclones:  info: Found 1 (10.5 MB) candidates after grouping by paths and file identifiers
[2022-02-28 08:34:34.343] fclones:  info: Found 1 (10.5 MB) candidates after grouping by prefix
[2022-02-28 08:34:34.343] fclones:  info: Found 1 (10.5 MB) candidates after grouping by suffix
[2022-02-28 08:34:35.188] fclones:  info: Found 1 (10.5 MB) redundant files
# Report by fclones 0.17.1
# Timestamp: 2022-02-28 08:34:35.189 +0000
# Command: fclones group b c --follow-links
# Found 1 file groups
# 10485760 B (10.5 MB) in 1 redundant files can be removed
1c7a31e91b14879d2b050786667f814c, 10485760 B (10.5 MB) * 2:
    /home/user/test/b/test
    /home/user/test/a/test
```


Discussion / hints:

Fix patch:
diff --git a/src/config.rs b/src/config.rs
index c7de342..3d09ad9 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -157,14 +157,24 @@ pub struct GroupConfig {
     pub no_ignore: bool,
 
     /// Follows symbolic links.
+    ///
+    /// If this flag is set together with `--symbolic-links`, only links to
+    /// directories are followed.
     #[structopt(short = "L", long)]
     pub follow_links: bool,
 
-    /// Treats files reachable from multiple paths through
-    /// hard links as duplicates
+    /// Treats files reachable from multiple paths through hard links as duplicates.
     #[structopt(short = "H", long)]
     pub hard_links: bool,
 
+    /// Treats symbolic links to files as regular files.
+    /// Reports symbolic links, not their targets.
+    ///
+    /// When this flag is set, multiple symbolic links to the same file, as well as a file and
+    /// a link to it, will be considered duplicates.
+    #[structopt(short = "S", long)]
+    pub symbolic_links: bool,
+
     /// Don't count matching files found within the same directory argument as duplicates.
     #[structopt(short("I"), long, conflicts_with("follow-links"))]
     pub isolate: bool,
diff --git a/src/file.rs b/src/file.rs
index b3ef26a..997d609 100644
--- a/src/file.rs
+++ b/src/file.rs
@@ -288,7 +288,7 @@ pub struct FileMetadata {
 impl FileMetadata {
     pub fn new(path: &Path) -> io::Result<FileMetadata> {
         let path_buf = path.to_path_buf();
-        let metadata = fs::symlink_metadata(&path_buf)?;
+        let metadata = fs::metadata(&path_buf)?;
 
         #[cfg(windows)]
         {
diff --git a/src/group.rs b/src/group.rs
index 2f23d97..46f8c1a 100644
--- a/src/group.rs
+++ b/src/group.rs
@@ -551,6 +551,7 @@ fn scan_files(ctx: &GroupCtx<'_>) -> Vec<Vec<FileInfo>> {
     walk.depth = config.depth.unwrap_or(usize::MAX);
     walk.hidden = config.hidden;
     walk.follow_links = config.follow_links;
+    walk.report_links = config.symbolic_links;
     walk.no_ignore = config.no_ignore;
     walk.path_selector = ctx.path_selector.clone();
     walk.log = Some(ctx.log);
@@ -650,7 +651,7 @@ where
     for (_, file_group) in groups.into_iter() {
         if file_group.len() == 1 {
             files.extend(file_group.into_iter().inspect(|p| progress(&p.path)));
-        } else if ctx.config.hard_links {
+        } else if ctx.config.hard_links || ctx.config.symbolic_links {
             files.extend(
                 file_group
                     .into_iter()
@@ -1484,6 +1485,30 @@ mod test {
         });
     }
 
+    #[test]
+    #[cfg(unix)]
+    fn report_symbolic_links_to_files() {
+        with_dir("main/soft_links", |root| {
+            let file1 = root.join("file1");
+            let file2 = root.join("file2");
+            write_test_file(&file1, b"aaa", b"", b"");
+            std::os::unix::fs::symlink(&file1, &file2).unwrap();
+
+            let log = test_log();
+            let mut config = GroupConfig::default();
+            config.paths = vec![file1.into(), file2.into()];
+            config.symbolic_links = true;
+
+            let results = group_files(&config, &log).unwrap();
+            assert_eq!(results.len(), 1);
+            assert_eq!(results[0].files.len(), 2);
+
+            config.symbolic_links = false;
+            let results = group_files(&config, &log).unwrap();
+            assert_eq!(results.len(), 0);
+        });
+    }
+
     #[test]
     fn duplicate_input_files() {
         with_dir("main/duplicate_input_files", |root| {
diff --git a/src/walk.rs b/src/walk.rs
index 25da456..2518c9d 100644
--- a/src/walk.rs
+++ b/src/walk.rs
@@ -20,6 +20,20 @@ enum EntryType {
     Other,
 }
 
+impl EntryType {
+    pub fn from(file_type: FileType) -> EntryType {
+        if file_type.is_symlink() {
+            EntryType::SymLink
+        } else if file_type.is_file() {
+            EntryType::File
+        } else if file_type.is_dir() {
+            EntryType::Dir
+        } else {
+            EntryType::Other
+        }
+    }
+}
+
 /// A path to a file, directory or symbolic link.
 /// Provides an abstraction over `Path` and `DirEntry`
 #[derive(Debug)]
@@ -30,26 +44,9 @@ struct Entry {
 
 impl Entry {
     pub fn new(file_type: FileType, path: Path) -> Entry {
-        if file_type.is_symlink() {
-            Entry {
-                tpe: EntryType::SymLink,
-                path,
-            }
-        } else if file_type.is_file() {
-            Entry {
-                tpe: EntryType::File,
-                path,
-            }
-        } else if file_type.is_dir() {
-            Entry {
-                tpe: EntryType::Dir,
-                path,
-            }
-        } else {
-            Entry {
-                tpe: EntryType::Other,
-                path,
-            }
+        Entry {
+            tpe: EntryType::from(file_type),
+            path,
         }
     }
 
@@ -126,13 +123,24 @@ impl IgnoreStack {
 /// Describes walk configuration.
 /// Many walks can be initiated from the same instance.
 pub struct Walk<'a> {
+    /// Relative root paths are resolved against this dir.
     pub base_dir: Arc<Path>,
+    /// Maximum allowed directory nesting level. 0 means "do not descend to directories at all".
     pub depth: usize,
+    /// Include hidden files.
     pub hidden: bool,
+    /// Resolve symlinks to dirs and files.
+    /// For a symlink to a file, report the target file, unless `report_links` is true.
     pub follow_links: bool,
+    /// Don't follow symlinks to files, but report them.
+    pub report_links: bool,
+    /// Don't honor .gitignore and .fdignore.
     pub no_ignore: bool,
+    /// Controls selecting or ignoring files by matching file and path names with regexes / globs.
     pub path_selector: PathSelector,
+    /// The function to call for each visited file. The directories are not reported.
     pub on_visit: &'a (dyn Fn(&Path) + Sync + Send),
+    /// Warnings about inaccessible files or dirs are logged here, if defined.
     pub log: Option<&'a Log>,
 }
 
@@ -151,6 +159,7 @@ impl<'a> Walk<'a> {
             depth: usize::MAX,
             hidden: false,
             follow_links: false,
+            report_links: false,
             no_ignore: false,
             path_selector: PathSelector::new(base_dir),
             on_visit: &|_| {},
@@ -253,7 +262,7 @@ impl<'a> Walk<'a> {
         match entry.tpe {
             EntryType::File => self.visit_file(entry.path, state),
             EntryType::Dir => self.visit_dir(entry.path, scope, level, gitignore, state),
-            EntryType::SymLink => self.visit_link(&entry.path, scope, level, gitignore, state),
+            EntryType::SymLink => self.visit_link(entry.path, scope, level, gitignore, state),
             EntryType::Other => {}
         }
     }
@@ -272,7 +281,7 @@ impl<'a> Walk<'a> {
     /// If `follow_links` is set to false, does nothing.
     fn visit_link<'s, 'w, F>(
         &'s self,
-        path: &Path,
+        path: Path,
         scope: &Scope<'w>,
         level: usize,
         gitignore: IgnoreStack,
@@ -281,9 +290,14 @@ impl<'a> Walk<'a> {
         F: Fn(Path) + Sync + Send,
         's: 'w,
     {
-        if self.follow_links {
-            match self.resolve_link(path) {
-                Ok(target) => self.visit_path(target, scope, level, gitignore, state),
+        if self.follow_links || self.report_links {
+            match self.resolve_link(&path) {
+                Ok((_, EntryType::File)) if self.report_links => self.visit_file(path, state),
+                Ok((target, _)) => {
+                    if self.follow_links {
+                        self.visit_path(target, scope, level, gitignore, state)
+                    }
+                }
                 Err(e) => self.log_warn(format!("Failed to read link {}: {}", path.display(), e)),
             }
         }
@@ -361,22 +375,33 @@ impl<'a> Walk<'a> {
         dirs.into_iter().chain(links).chain(files)
     }
 
-    /// Returns the absolute target path of a symbolic link
-    fn resolve_link(&self, link: &Path) -> io::Result<Path> {
-        let target = Path::from(read_link(link.to_path_buf())?);
+    /// Returns the absolute target path of a symbolic link with the type of the target
+    fn resolve_link(&self, link: &Path) -> io::Result<(Path, EntryType)> {
+        let link_buf = link.to_path_buf();
+        let target = read_link(&link_buf)?;
+        let entry_type = EntryType::from(link_buf.metadata()?.file_type());
+        let target = Path::from(target);
         let resolved = if target.is_relative() {
             link.parent().unwrap().join(target)
         } else {
             target
         };
-        Ok(self.absolute(resolved))
+        Ok((self.absolute(resolved), entry_type))
     }
 
     /// Returns absolute path with removed `.` and `..` components.
     /// Relative paths are resolved against `self.base_dir`.
-    fn absolute(&self, path: Path) -> Path {
+    /// Symbolic links to directories are resolved.
+    /// File symlinks are not resolved, because we need
+    fn absolute(&self, mut path: Path) -> Path {
         if path.is_relative() {
-            self.base_dir.join(path).canonicalize()
+            path = self.base_dir.join(path)
+        }
+        if path.to_path_buf().is_file() {
+            // for files we are sure there will be a parent and a file name
+            let parent = path.parent().unwrap().canonicalize();
+            let file_name = path.file_name().unwrap();
+            Arc::new(parent).join(Path::from(file_name))
         } else {
             path.canonicalize()
         }
@@ -443,6 +468,29 @@ mod test {
         });
     }
 
+    #[test]
+    #[cfg(unix)]
+    fn report_rel_file_sym_links() {
+        with_dir("target/test/walk/report_symlinks/", |test_root| {
+            use std::os::unix::fs::symlink;
+            let file = test_root.join("file.txt");
+            let link1 = test_root.join("link1");
+            let link2 = test_root.join("link2");
+            File::create(&file).unwrap();
+            symlink(PathBuf::from("file.txt"), &link1).unwrap(); // link1 -> file.txt
+            symlink(PathBuf::from("link1"), &link2).unwrap(); // link2 -> link1
+
+            let mut walk1 = Walk::new();
+            walk1.report_links = true;
+            assert_eq!(run_walk(walk1, link1.clone()), vec![link1]);
+
+            // a link to a link should also be reported
+            let mut walk2 = Walk::new();
+            walk2.report_links = true;
+            assert_eq!(run_walk(walk2, link2.clone()), vec![link2]);
+        });
+    }
+
     #[test]
     #[cfg(unix)]
     fn follow_rel_dir_sym_links() {


Test patch: