Repository: pkolaczk/fclones (language: rust)

Issue:
23000 symlinks created...?
0.30.0

```
fclones group --cache /mnt/*T* > dupes.txt
fclones link < dupes.txt
```

Looks like it riddled all my disks with symlinks, preventing my containers from accessing the associated data... there's a switch for symlinks and I didn't use that. Is there a way to undo this to recover my damaged data (and prevent it from happening in the future).


Discussion / hints:
I also think that the automatic fallback to symbolic links is unexpected and therefore bad behavior. Per default fclones should not do any cross-device actions, because in this case it means deleting data!

To undo that you have to write a script to track down and fix these symlinks yourself, sorry.

Fix patch:
diff --git a/src/config.rs b/src/config.rs
index 731e7b4..49d3449 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -694,14 +694,13 @@ pub enum Command {
     /// The list of groups earlier produced by `fclones group` should be submitted
     /// on the standard input.
     ///
-    /// Unless `--soft` is specified, hard links are created for links within
-    /// the same file system. Soft links are always created to link files between
-    /// different file systems.
+    /// Unless `--soft` is specified, hard links are created.
+    /// Creating hard links to files on different mount points is expected to fail.
     Link {
         #[clap(flatten)]
         config: DedupeConfig,
 
-        /// Create soft links.
+        /// Create soft (symbolic) links.
         #[arg(short, long)]
         soft: bool,
     },
diff --git a/src/dedupe.rs b/src/dedupe.rs
index 047c72b..b063cbc 100644
--- a/src/dedupe.rs
+++ b/src/dedupe.rs
@@ -43,7 +43,7 @@ pub enum DedupeOp {
 }
 
 /// Convenience struct for holding a path to a file and its metadata together
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub struct PathAndMetadata {
     pub path: Path,
     pub metadata: FileMetadata,
@@ -73,6 +73,12 @@ impl AsRef<FileId> for PathAndMetadata {
     }
 }
 
+impl From<PathAndMetadata> for Path {
+    fn from(value: PathAndMetadata) -> Self {
+        value.path
+    }
+}
+
 impl Display for PathAndMetadata {
     fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
         f.pad(self.path.display().as_str())
@@ -684,18 +690,11 @@ impl PartitionedFileGroup {
         let mut commands = Vec::new();
         let retained_file = Arc::new(self.to_keep.swap_remove(0));
         for dropped_file in self.to_drop {
-            let devices_differ =
-                retained_file.metadata.device_id() != dropped_file.metadata.device_id();
             match strategy {
                 DedupeOp::SoftLink => commands.push(FsCommand::SoftLink {
                     target: retained_file.clone(),
                     link: dropped_file,
                 }),
-                // hard links are not supported between files on different file systems
-                DedupeOp::HardLink if devices_differ => commands.push(FsCommand::SoftLink {
-                    target: retained_file.clone(),
-                    link: dropped_file,
-                }),
                 DedupeOp::HardLink => commands.push(FsCommand::HardLink {
                     target: retained_file.clone(),
                     link: dropped_file,
@@ -724,42 +723,30 @@ impl PartitionedFileGroup {
 
 /// Attempts to retrieve the metadata of all the files in the file group.
 /// If metadata is inaccessible for a file, a warning is emitted to the log, and None gets returned.
-fn files_metadata<P>(group: FileGroup<P>, log: &dyn Log) -> Option<Vec<PathAndMetadata>>
+fn fetch_files_metadata<P>(group: FileGroup<P>, log: &dyn Log) -> Option<FileGroup<PathAndMetadata>>
 where
     P: Into<Path>,
 {
-    let mut last_error: Option<io::Error> = None;
-    let files: Vec<_> = group
-        .files
-        .into_iter()
-        .filter_map(|p| {
-            PathAndMetadata::new(p.into())
-                .map_err(|e| {
-                    log.warn(&e);
-                    last_error = Some(e);
-                })
-                .ok()
+    group
+        .try_map_all(|p| {
+            PathAndMetadata::new(p.into()).map_err(|e| {
+                log.warn(&e);
+            })
         })
-        .collect();
-
-    match last_error {
-        Some(_) => None,
-        None => Some(files),
-    }
+        .ok()
 }
 
 /// Partitions a group of files into files to keep and files that can be safely dropped
 /// (or linked).
-fn partition<P>(
-    group: FileGroup<P>,
+fn partition(
+    group: FileGroup<PathAndMetadata>,
     config: &DedupeConfig,
     log: &dyn Log,
-) -> Result<PartitionedFileGroup, Error>
-where
-    P: Into<Path>,
-{
+) -> Result<PartitionedFileGroup, Error> {
     let file_len = group.file_len;
     let file_hash = group.file_hash.clone();
+    let mut files = group.files;
+
     let error = |msg: &str| {
         Err(Error::from(format!(
             "Could not determine files to drop in group with hash {} and len {}: {}",
@@ -767,11 +754,6 @@ where
         )))
     };
 
-    let mut files = match files_metadata(group, log) {
-        Some(files) => files,
-        None => return error("Metadata of some files could not be obtained"),
-    };
-
     // We don't want to remove dirs or symlinks
     files.retain(|m| {
         let is_file = m.metadata.is_file();
@@ -882,19 +864,30 @@ pub fn dedupe<'a, I, P>(
 where
     I: IntoIterator<Item = FileGroup<P>> + 'a,
     I::IntoIter: Send,
-    P: Into<Path> + Send,
+    P: Into<Path> + AsRef<Path> + fmt::Debug + Send + 'a,
 {
     let devices = DiskDevices::new(&HashMap::new());
+    let disallow_cross_device = op == DedupeOp::HardLink || op == DedupeOp::RefLink;
     groups
         .into_iter()
         .enumerate()
         .par_bridge()
-        .map(move |(i, group)| match partition(group, config, log) {
-            Ok(group) => (i, group.dedupe_script(&op, &devices)),
-            Err(e) => {
-                log.warn(e);
-                (i, vec![])
+        .map(move |(i, group)| {
+            let mut commands = Vec::new();
+            if let Some(group) = fetch_files_metadata(group, log) {
+                let groups = if disallow_cross_device {
+                    group.partition_by_key(|p| p.metadata.device_id())
+                } else {
+                    vec![group]
+                };
+                for group in groups {
+                    match partition(group, config, log) {
+                        Ok(group) => commands.extend(group.dedupe_script(&op, &devices)),
+                        Err(e) => log.warn(e),
+                    }
+                }
             }
+            (i, commands)
         })
 }
 
@@ -1205,6 +1198,7 @@ mod test {
     fn test_partition_selects_files_for_removal() {
         with_dir("dedupe/partition/basic", |root| {
             let group = make_group(root, FileHash::from_str("00").unwrap());
+            let group = group.map(|p| PathAndMetadata::new(p).unwrap());
             let config = DedupeConfig::default();
             let partitioned = partition(group, &config, &StdLog::new()).unwrap();
             assert_eq!(partitioned.to_keep.len(), 1);
@@ -1216,6 +1210,7 @@ mod test {
     fn test_partition_bails_out_if_file_modified_too_late() {
         with_dir("dedupe/partition/modification", |root| {
             let group = make_group(root, FileHash::from_str("00").unwrap());
+            let group = group.map(|p| PathAndMetadata::new(p).unwrap());
             let config = DedupeConfig {
                 modified_before: Some(DateTime::from(Local::now() - Duration::days(1))),
                 ..DedupeConfig::default()
@@ -1235,6 +1230,7 @@ mod test {
                 priority: vec![Priority::MostRecentlyModified],
                 ..DedupeConfig::default()
             };
+            let group = group.map(|p| PathAndMetadata::new(p).unwrap());
             let partitioned = partition(group, &config, &StdLog::new()).unwrap();
             assert!(!partitioned.to_drop.iter().any(|m| m.path == path));
             assert!(!partitioned.to_keep.iter().any(|m| m.path == path));
@@ -1258,6 +1254,7 @@ mod test {
                 priority: vec![Priority::Newest],
                 ..DedupeConfig::default()
             };
+            let group = group.map(|p| PathAndMetadata::new(p).unwrap());
             let partitioned_1 = partition(group.clone(), &config, &StdLog::new()).unwrap();
             config.priority = vec![Priority::Oldest];
             let partitioned_2 = partition(group, &config, &StdLog::new()).unwrap();
@@ -1282,6 +1279,8 @@ mod test {
             let path = group.files[0].clone();
             write_file(&path.to_path_buf(), "foo");
 
+            // note that fetching metadata happens after we wrote the file
+            let group = group.map(|p| PathAndMetadata::new(p).unwrap());
             let config = DedupeConfig {
                 priority: vec![Priority::MostRecentlyModified],
                 ..DedupeConfig::default()
@@ -1309,20 +1308,22 @@ mod test {
     fn test_partition_respects_keep_patterns() {
         with_dir("dedupe/partition/keep", |root| {
             let group = make_group(root, FileHash::from_str("00").unwrap());
+            let group = group.map(|p| PathAndMetadata::new(p).unwrap());
             let mut config = DedupeConfig {
                 priority: vec![Priority::LeastRecentlyModified],
                 keep_name_patterns: vec![Pattern::glob("*_1").unwrap()],
                 ..DedupeConfig::default()
             };
+
             let p = partition(group.clone(), &config, &StdLog::new()).unwrap();
             assert_eq!(p.to_keep.len(), 1);
-            assert_eq!(&p.to_keep[0].path, &group.files[0]);
+            assert_eq!(&p.to_keep[0].path, &group.files[0].path);
 
             config.keep_name_patterns = vec![];
             config.keep_path_patterns = vec![Pattern::glob("**/file_1").unwrap()];
             let p = partition(group.clone(), &config, &StdLog::new()).unwrap();
             assert_eq!(p.to_keep.len(), 1);
-            assert_eq!(&p.to_keep[0].path, &group.files[0]);
+            assert_eq!(&p.to_keep[0].path, &group.files[0].path);
         })
     }
 
@@ -1330,6 +1331,7 @@ mod test {
     fn test_partition_respects_drop_patterns() {
         with_dir("dedupe/partition/drop", |root| {
             let group = make_group(root, FileHash::from_str("00").unwrap());
+            let group = group.map(|p| PathAndMetadata::new(p).unwrap());
             let mut config = DedupeConfig {
                 priority: vec![Priority::LeastRecentlyModified],
                 name_patterns: vec![Pattern::glob("*_3").unwrap()],
@@ -1337,13 +1339,13 @@ mod test {
             };
             let p = partition(group.clone(), &config, &StdLog::new()).unwrap();
             assert_eq!(p.to_drop.len(), 1);
-            assert_eq!(&p.to_drop[0].path, &group.files[2]);
+            assert_eq!(&p.to_drop[0].path, &group.files[2].path);
 
             config.name_patterns = vec![];
             config.path_patterns = vec![Pattern::glob("**/file_3").unwrap()];
             let p = partition(group.clone(), &config, &StdLog::new()).unwrap();
             assert_eq!(p.to_drop.len(), 1);
-            assert_eq!(&p.to_drop[0].path, &group.files[2]);
+            assert_eq!(&p.to_drop[0].path, &group.files[2].path);
         })
     }
 
@@ -1368,6 +1370,7 @@ mod test {
                 ..DedupeConfig::default()
             };
 
+            let group = group.map(|p| PathAndMetadata::new(p).unwrap());
             let p = partition(group, &config, &StdLog::new()).unwrap();
             assert_eq!(p.to_drop.len(), 3);
             assert!(p
@@ -1412,6 +1415,7 @@ mod test {
             };
 
             let config = DedupeConfig::default();
+            let group = group.map(|p| PathAndMetadata::new(p).unwrap());
             let p = partition(group, &config, &StdLog::new()).unwrap();
 
             // drop A files because file_a2 appears after file_b1 in the files vector
diff --git a/src/file.rs b/src/file.rs
index 39218a6..9d550fa 100644
--- a/src/file.rs
+++ b/src/file.rs
@@ -284,7 +284,7 @@ impl FileId {
 }
 
 /// Convenience wrapper for accessing OS-dependent metadata like inode and device-id
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub struct FileMetadata {
     id: FileId,
     metadata: fs::Metadata,
diff --git a/src/group.rs b/src/group.rs
index 970465e..d47e234 100644
--- a/src/group.rs
+++ b/src/group.rs
@@ -6,6 +6,7 @@ use std::collections::BTreeMap;
 use std::collections::HashMap;
 use std::env::{args_os, current_dir};
 use std::ffi::{OsStr, OsString};
+use std::fmt::Debug;
 use std::fs::File;
 use std::hash::Hash;
 use std::io;
@@ -240,10 +241,72 @@ impl<F> FileGroup<F> {
     pub fn total_size(&self) -> FileLen {
         self.file_len * self.file_count() as u64
     }
+
+    /// Maps the list of files in the group.
+    /// Preserves the group file len and hash.
+    pub fn map<R>(self, f: impl Fn(F) -> R) -> FileGroup<R> {
+        FileGroup {
+            file_len: self.file_len,
+            file_hash: self.file_hash,
+            files: self.files.into_iter().map(f).collect(),
+        }
+    }
+
+    /// Tries to map each file by given fallible function.
+    /// Does not stop processing on the first failure.
+    /// If mapping any of the files fails, then returns a vector of errors.
+    pub fn try_map_all<R: Debug, E: Debug>(
+        self,
+        f: impl Fn(F) -> Result<R, E>,
+    ) -> Result<FileGroup<R>, Vec<E>> {
+        let (ok, err): (Vec<_>, Vec<_>) = self.files.into_iter().map(f).partition(Result::is_ok);
+        if err.is_empty() {
+            Ok(FileGroup {
+                file_len: self.file_len,
+                file_hash: self.file_hash,
+                files: ok.into_iter().map(Result::unwrap).collect(),
+            })
+        } else {
+            Err(err.into_iter().map(Result::unwrap_err).collect())
+        }
+    }
+
+    /// Flat maps the list of files in the group.
+    /// Preserves the group file len and hash.
+    pub fn flat_map<R, I>(self, f: impl Fn(F) -> I) -> FileGroup<R>
+    where
+        I: IntoIterator<Item = R>,
+    {
+        FileGroup {
+            file_len: self.file_len,
+            file_hash: self.file_hash,
+            files: self.files.into_iter().flat_map(f).collect(),
+        }
+    }
+
+    /// Splits the group into one or more groups based on the key function applied to each file.
+    /// Files with the same key are placed in the same group.
+    /// The key is computed only once per item.
+    /// File len and file hash are preserved.
+    pub fn partition_by_key<K: Eq + Hash>(self, key_fn: impl Fn(&F) -> K) -> Vec<FileGroup<F>> {
+        let mut groups = HashMap::new();
+        for f in self.files {
+            let key = key_fn(&f);
+            groups.entry(key).or_insert_with(Vec::new).push(f);
+        }
+        groups
+            .into_values()
+            .map(|files| FileGroup {
+                file_len: self.file_len,
+                file_hash: self.file_hash.clone(),
+                files,
+            })
+            .collect()
+    }
 }
 
-#[cfg(test)]
-impl<F: AsRef<Path>> FileGroup<F> {
+impl<F: AsRef<Path> + core::fmt::Debug> FileGroup<F> {
+    #[cfg(test)]
     fn paths(&self) -> Vec<Path> {
         self.files.iter().map(|f| f.as_ref().clone()).collect_vec()
     }
@@ -1934,6 +1997,75 @@ mod test {
         )
     }
 
+    #[test]
+    fn partition() {
+        let fg = FileGroup {
+            file_len: FileLen::from(1u64),
+            file_hash: FileHash::from(1u128),
+            files: vec!["a1", "b1", "a2", "b2", "b3"],
+        };
+        let mut partitions = fg.partition_by_key(|f| f.chars().next().unwrap());
+        assert_eq!(partitions.len(), 2);
+        partitions.sort_by_key(|p| p.files.len());
+        assert_eq!(partitions[0].files, vec!["a1", "a2"]);
+        assert_eq!(partitions[1].files, vec!["b1", "b2", "b3"]);
+    }
+
+    #[test]
+    fn map() {
+        let fg = FileGroup {
+            file_len: FileLen::from(1u64),
+            file_hash: FileHash::from(1u128),
+            files: vec!["a", "b"],
+        };
+        let fg = fg.map(|f| format!("{f}.txt"));
+        assert_eq!(fg.files, vec![String::from("a.txt"), String::from("b.txt")]);
+    }
+
+    #[test]
+    fn try_map_all_happy_path() {
+        let fg = FileGroup {
+            file_len: FileLen::from(1u64),
+            file_hash: FileHash::from(1u128),
+            files: vec!["a", "b"],
+        };
+        let fg = fg.try_map_all(|f| Result::<_, ()>::Ok(format!("{f}.txt")));
+        assert!(fg.is_ok());
+        assert_eq!(
+            fg.unwrap().files,
+            vec![String::from("a.txt"), String::from("b.txt")]
+        );
+    }
+
+    #[test]
+    fn try_map_all_errors() {
+        let fg = FileGroup {
+            file_len: FileLen::from(1u64),
+            file_hash: FileHash::from(1u128),
+            files: vec!["a", "b"],
+        };
+        let fg = fg.try_map_all(|f| Result::<(), _>::Err(format!("error {f}")));
+        assert!(fg.is_err());
+        assert_eq!(
+            fg.unwrap_err(),
+            vec![String::from("error a"), String::from("error b")]
+        );
+    }
+
+    #[test]
+    fn flat_map() {
+        let fg = FileGroup {
+            file_len: FileLen::from(1u64),
+            file_hash: FileHash::from(1u128),
+            files: vec!["a1", "b1", "a2", "b2", "b3"],
+        };
+        let fg = fg.flat_map(|f| if f.starts_with('a') { Some(f) } else { None });
+        assert_eq!(fg.files, vec!["a1", "a2"]);
+
+        let fg = fg.flat_map(|f| vec![f, f]);
+        assert_eq!(fg.files, vec!["a1", "a1", "a2", "a2"]);
+    }
+
     fn write_test_file(path: &PathBuf, prefix: &[u8], mid: &[u8], suffix: &[u8]) {
         let mut file = OpenOptions::new()
             .write(true)


Test patch: