Repository: pkolaczk/fclones (language: rust)

Issue:
--stdin Parameter Not Working
Greetings!

I've been playing a bit with `fclones` this morning (super cool tool, BTW) and wanted to use the `--stdin` parameter to read the list of files to analyze from the output of `find`. Based on the documentation it seems like passing the input to `fclones group --stdin` should work, but whenever I try this I always get an error: `fclones: error: No input files`

Here's a simple, trivial example of what I mean:
```
localhost~ % fclones --version
fclones 0.12.2
localhost~ % mkdir blah
localhost~ % cd blah
localhost~/blah % touch {1,2,3}.c
localhost~/blah % find . -name '*.c'
./1.c
./2.c
./3.c
localhost~/blah % find . -name '*.c' | fclones group --stdin
[2021-06-19 15:01:12.126] fclones: error: No input files
```
I'm not sure if I'm doing something incorrectly - any ideas? Thanks in advance!


Discussion / hints:
Looks like a bug. 
I confirm, this is a bug, caused by incorrect error handling.

Fix patch:
diff --git a/Cargo.lock b/Cargo.lock
index 6aed047..03ee36d 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1,5 +1,7 @@
 # This file is automatically @generated by Cargo.
 # It is not intended for manual editing.
+version = 3
+
 [[package]]
 name = "ahash"
 version = "0.3.8"
@@ -356,7 +358,7 @@ checksum = "4443176a9f2c162692bd3d352d745ef9413eec5782a80d8fd6f8a1ac692a07f7"
 
 [[package]]
 name = "fclones"
-version = "0.12.2"
+version = "0.12.3"
 dependencies = [
  "atomic-counter",
  "block-utils",
diff --git a/Cargo.toml b/Cargo.toml
index 27a5051..d1cb037 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "fclones"
-version = "0.12.2"
+version = "0.12.3"
 description = "Finds duplicate, unique, under- or over-replicated files"
 authors = ["Piotr Kołaczkowski <pkolaczk@gmail.com>"]
 repository = "https://github.com/pkolaczk/fclones"
diff --git a/src/main.rs b/src/main.rs
index 997f6dd..afef538 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -1,9 +1,9 @@
 use std::collections::HashMap;
 use std::ffi::{OsStr, OsString};
 use std::fs::File;
-use std::io;
 use std::io::{stdin, Write};
 use std::process::exit;
+use std::{fs, io};
 
 use fallible_iterator::FallibleIterator;
 use itertools::Itertools;
@@ -50,28 +50,40 @@ fn configure_main_thread_pool(pool_sizes: &HashMap<OsString, Parallelism>) {
         .unwrap();
 }
 
-fn run_group(config: &GroupConfig, log: &mut Log) -> Result<(), Error> {
-    let mut access_error = false;
-    let mut has_input_files: bool = false;
-    for path in config.paths.iter() {
-        match std::fs::metadata(path) {
-            Ok(metadata) if metadata.is_dir() && config.depth == Some(0) => log.warn(format!(
-                "Skipping directory {} because recursive scan is disabled.",
-                path.display()
-            )),
+fn run_group(mut config: GroupConfig, log: &mut Log) -> Result<(), Error> {
+    if !config.stdin {
+        // If files aren't streamed on stdin, we can inspect all of them now
+        // and exit early on any access error. If depth is set to 0 (recursive scan disabled)
+        // we also want to filter out directories and terminate with an error if there are
+        // no files in the input.
+        // Unfortunately we can't fail fast here when the list of files
+        // is streamed from the standard input, because we'd have to collect all paths into a vector
+        // list first, but we don't want to do this because there may be many.
+        // In that case, we just let the lower layers handle eventual
+        // problems and report as warnings.
+        let mut access_error = false;
+        let depth = config.depth;
+        config.paths.retain(|p| match fs::metadata(&p) {
+            Ok(m) if m.is_dir() && depth == Some(0) => {
+                log.warn(format!(
+                    "Skipping directory {} because recursive scan is disabled.",
+                    p.display()
+                ));
+                false
+            }
             Err(e) => {
-                log.err(format!("Can't access {}: {}", path.display(), e));
+                log.err(format!("Can't access {}: {}", p.display(), e));
                 access_error = true;
+                false
             }
-            Ok(_) => has_input_files = true,
+            Ok(_) => true,
+        });
+        if access_error {
+            return Err(Error::from("Some input paths could not be accessed."));
+        }
+        if config.paths.is_empty() {
+            return Err(Error::from("No input files."));
         }
-    }
-
-    if access_error {
-        return Err(Error::from(""));
-    }
-    if !has_input_files {
-        return Err(Error::from("No input files"));
     }
 
     configure_main_thread_pool(&config.thread_pool_sizes());
@@ -187,7 +199,7 @@ fn main() {
     }
 
     let result = match config.command {
-        Command::Group(config) => run_group(&config, &mut log),
+        Command::Group(config) => run_group(config, &mut log),
         Command::Remove(config) => run_dedupe(DedupeOp::Remove, config, &mut log),
         Command::Link { config, soft: true } => run_dedupe(DedupeOp::SoftLink, config, &mut log),
         Command::Link {
diff --git a/src/walk.rs b/src/walk.rs
index 6b96a9c..636cb12 100644
--- a/src/walk.rs
+++ b/src/walk.rs
@@ -1,7 +1,7 @@
 use std::env::current_dir;
 use std::fs::{read_link, symlink_metadata, DirEntry, FileType, ReadDir};
-use std::io;
 use std::sync::Arc;
+use std::{fs, io};
 
 use dashmap::DashSet;
 use rayon::Scope;
@@ -113,7 +113,13 @@ impl<'a> Walk<'a> {
         rayon::scope(|scope| {
             for p in roots.into_iter() {
                 let p = self.absolute(p);
-                scope.spawn(|scope| self.visit_path(p, scope, 0, &state))
+                match fs::metadata(&p.to_path_buf()) {
+                    Ok(metadata) if metadata.is_dir() && self.depth == 0 => self.log_warn(format!(
+                        "Skipping directory {} because recursive scan is disabled.",
+                        p
+                    )),
+                    _ => scope.spawn(|scope| self.visit_path(p, scope, 0, &state)),
+                }
             }
         });
     }


Test patch: