Repository: pkolaczk/fclones (language: rust)

Issue:
Remove dependency on pcre-sys
This dependency makes it harder to install fclones on some platforms. Let's switch to regex crate.


Discussion / hints:

Fix patch:
diff --git a/Cargo.lock b/Cargo.lock
index d19200b..04ed8f8 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -324,7 +324,7 @@ checksum = "a357d28ed41a50f9c765dbfe56cbc04a64e53e5fc58ba79fbc34c10ef3df831f"
 
 [[package]]
 name = "fclones"
-version = "0.9.1"
+version = "0.10.0"
 dependencies = [
  "atomic-counter",
  "block-utils",
@@ -347,7 +347,6 @@ dependencies = [
  "metrohash",
  "nix",
  "nom",
- "pcre2",
  "rand",
  "rayon",
  "regex",
@@ -622,27 +621,6 @@ version = "1.7.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "af8b08b04175473088b46763e51ee54da5f9a164bc162f615b91bc179dbf15a3"
 
-[[package]]
-name = "pcre2"
-version = "0.2.3"
-source = "git+https://github.com/pkolaczk/rust-pcre2.git#e15c59ac6e73cc0e7bce7f823004ebae759076ed"
-dependencies = [
- "libc",
- "log 0.4.14",
- "pcre2-sys",
- "thread_local",
-]
-
-[[package]]
-name = "pcre2-sys"
-version = "0.2.2"
-source = "git+https://github.com/pkolaczk/rust-pcre2.git#e15c59ac6e73cc0e7bce7f823004ebae759076ed"
-dependencies = [
- "cc",
- "libc",
- "pkg-config",
-]
-
 [[package]]
 name = "pkg-config"
 version = "0.3.19"
diff --git a/Cargo.toml b/Cargo.toml
index 003ff00..8878c5f 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,7 +1,7 @@
 [package]
 name = "fclones"
 description = "Finds duplicate, unique, under- or over-replicated files"
-version = "0.9.1"
+version = "0.10.0"
 authors = ["Piotr Kołaczkowski <pkolaczk@gmail.com>"]
 edition = "2018"
 license = "MIT"
@@ -27,9 +27,8 @@ lazy-init = "0.5.0"
 maplit = "1.0.2"
 metrohash = "1.0.6"
 nom = { version = "5.1.2", features = ["regexp"] }
-pcre2 = { git = "https://github.com/pkolaczk/rust-pcre2.git" }
 rayon = "1.4.0"
-regex = "1.3.7"
+regex = "1.4.5"
 serde = { version = "1", features = ["derive"] }
 serde_json = "1.0"
 smallvec = "1.6.1"
diff --git a/src/lib.rs b/src/lib.rs
index 937053d..3ecae8d 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -6,6 +6,7 @@ pub mod log;
 pub mod path;
 pub mod pattern;
 pub mod progress;
+pub mod regex;
 pub mod report;
 pub mod selector;
 pub mod semaphore;
diff --git a/src/main.rs b/src/main.rs
index 50b590c..43f3253 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -553,7 +553,6 @@ fn main() {
         `?(a|b)`    matches at most one occurrence of the pattern inside the brackets
         `+(a|b)`    matches at least occurrence of the patterns given inside the brackets
         `*(a|b)`    matches any number of occurrences of the patterns given inside the brackets
-        `!(a|b)`    matches anything that doesn't match any of the patterns given inside the brackets
         `{escape}`         escapes wildcards, e.g. `{escape}?` would match `?` literally
     "
     ));
diff --git a/src/pattern.rs b/src/pattern.rs
index 7f9f1c4..718bc84 100644
--- a/src/pattern.rs
+++ b/src/pattern.rs
@@ -9,9 +9,10 @@ use nom::combinator::{cond, map};
 use nom::multi::{many0, separated_list};
 use nom::sequence::tuple;
 use nom::IResult;
-use pcre2::bytes::{Regex, RegexBuilder};
 use regex::escape;
 
+use crate::regex::Regex;
+
 #[derive(Debug)]
 pub struct PatternError {
     cause: String,
@@ -20,7 +21,11 @@ pub struct PatternError {
 
 impl Display for PatternError {
     fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
-        write!(f, "Invalid pattern '{}': {}", self.input, self.cause)
+        write!(
+            f,
+            "Failed to compile pattern '{}': {}",
+            self.input, self.cause
+        )
     }
 }
 
@@ -77,22 +82,12 @@ impl Pattern {
     pub fn regex_with(pattern: &str, opts: &PatternOpts) -> Result<Pattern, PatternError> {
         let pattern = pattern.trim_start_matches('^');
         let pattern = pattern.trim_end_matches('$');
-        let mut builder = RegexBuilder::new();
-        builder.jit_if_available(true);
-
-        // We don't set caseless on builder,
-        // because we may wish to concatenate patterns of different case-sensitivities
-        let pattern = if opts.case_insensitive {
-            "(?i)".to_string() + pattern + "(?-i)"
-        } else {
-            pattern.to_string()
-        };
+        let pattern = pattern.to_string();
 
         let anchored_regex = "^".to_string() + &pattern + "$";
-        let anchored_regex = builder.build(anchored_regex.as_str());
-
+        let anchored_regex = Regex::new(anchored_regex.as_str(), opts.case_insensitive);
         let prefix_regex = "^".to_string() + &pattern;
-        let prefix_regex = builder.build(prefix_regex.as_str());
+        let prefix_regex = Regex::new(prefix_regex.as_str(), opts.case_insensitive);
 
         match anchored_regex {
             Ok(anchored_regex) => Ok(Pattern {
@@ -159,28 +154,23 @@ impl Pattern {
 
     /// Returns true if this pattern fully matches the given path
     pub fn matches(&self, path: &str) -> bool {
-        self.anchored_regex
-            .is_match(path.as_bytes())
-            .unwrap_or(false)
+        self.anchored_regex.is_match(path)
     }
 
     /// Returns true if a prefix of this pattern fully matches the given path
     pub fn matches_partially(&self, path: &str) -> bool {
-        self.anchored_regex
-            .is_partial_match(path.as_bytes())
-            .unwrap_or(false)
+        self.anchored_regex.is_partial_match(path)
     }
 
     /// Returns true if this pattern fully matches a prefix of the given path
     pub fn matches_prefix(&self, path: &str) -> bool {
-        self.prefix_regex.is_match(path.as_bytes()).unwrap_or(false)
+        self.prefix_regex.is_match(path)
     }
 
     /// Returns true if this pattern fully matches given file path
     pub fn matches_path(&self, path: &Path) -> bool {
         self.anchored_regex
-            .is_match(path.to_string_lossy().as_bytes())
-            .unwrap_or(false)
+            .is_match(path.to_string_lossy().as_ref())
     }
 
     /// Parses a UNIX glob and converts it to a regular expression
@@ -306,9 +296,10 @@ impl ToString for Pattern {
 
 #[cfg(test)]
 mod test {
-    use super::*;
     use std::path::PathBuf;
 
+    use super::*;
+
     fn glob_to_regex_str(glob: &str) -> String {
         Pattern::glob(glob).unwrap().to_string()
     }
@@ -460,15 +451,6 @@ mod test {
         assert!(g.matches(native_dir_sep("/a-foobar").as_str()));
     }
 
-    #[test]
-    fn ext_glob_at_least_never() {
-        let g = Pattern::glob("/a-!(foo)*").unwrap();
-        assert!(!g.matches(native_dir_sep("/a-foo-bar").as_str()));
-        assert!(!g.matches(native_dir_sep("/a-foo").as_str()));
-        assert!(g.matches(native_dir_sep("/a-bar").as_str()));
-        assert!(g.matches(native_dir_sep("/a-").as_str()));
-    }
-
     #[test]
     fn ext_glob_nested() {
         let g = Pattern::glob("/a-@(foo|bar?(baz))").unwrap();
@@ -516,8 +498,6 @@ mod test {
         assert!(g3.matches_partially(native_dir_sep("/a/b21").as_str()));
         assert!(g3.matches_partially(native_dir_sep("/a/b21/b22").as_str()));
         assert!(g3.matches_partially(native_dir_sep("/a/b21/b22/c").as_str()));
-        assert!(!g3.matches_partially(native_dir_sep("/a/b11/b21/c").as_str()));
-        assert!(!g3.matches_partially(native_dir_sep("/a/b21/c").as_str()));
     }
 
     #[test]
diff --git a/src/regex.rs b/src/regex.rs
new file mode 100644
index 0000000..d603878
--- /dev/null
+++ b/src/regex.rs
@@ -0,0 +1,149 @@
+use std::cmp::min;
+
+/// Adds poor-man's partial matching support to the standard regex::Regex
+/// Note this is very limited and slightly broken stub for partial matching.
+/// False positives for partial matching are allowed.
+#[derive(Clone, Debug)]
+pub struct Regex {
+    regex: regex::Regex,
+    fixed_prefix: String,
+    case_insensitive: bool,
+}
+
+impl Regex {
+    pub fn new(re: &str, case_insensitive: bool) -> Result<Regex, regex::Error> {
+        assert!(re.starts_with('^'));
+        let regex = regex::RegexBuilder::new(re)
+            .case_insensitive(case_insensitive)
+            .build()?;
+        let fixed_prefix = if case_insensitive {
+            Self::get_fixed_prefix(re).to_lowercase()
+        } else {
+            Self::get_fixed_prefix(re)
+        };
+        Ok(Regex {
+            regex,
+            fixed_prefix,
+            case_insensitive,
+        })
+    }
+
+    pub fn is_match(&self, s: &str) -> bool {
+        self.regex.is_match(s)
+    }
+
+    /// Returns true if given string `s` could match the pattern if extended
+    /// by more characters.
+    ///
+    /// Technically it checks if the string s matches the initial characters
+    /// in the fixed prefix of the regex, where fixed prefix are all characters up to the
+    /// first regex wildcard.
+    pub fn is_partial_match(&self, s: &str) -> bool {
+        let len = min(s.len(), self.fixed_prefix.len());
+        let truncated: String = s.chars().take(len).collect();
+        let pattern = if self.case_insensitive {
+            truncated.to_lowercase()
+        } else {
+            truncated
+        };
+        self.fixed_prefix.starts_with(&pattern)
+    }
+
+    /// Returns the initial fragment of the regex string that always matches
+    /// a fixed string. That fragment does not contain any wildcard characters (or all are escaped).
+    fn get_fixed_prefix(s: &str) -> String {
+        let mut escape = false;
+        let mut result = String::new();
+        let magic_chars = ['.', '^', '$', '(', ')', '{', '}', '[', ']', '|', '.', '+'];
+
+        for (i, c) in s.chars().enumerate() {
+            if c == '^' && i == 0 {
+                continue;
+            }
+            if magic_chars.contains(&c) && !escape {
+                break;
+            }
+            // these may make the previous character optional,
+            // so we erase the last added one
+            if ['?', '*'].contains(&c) && !escape {
+                result = result.chars().take(result.len() - 1).collect();
+                break;
+            }
+            // escaped alphabetic character means a character class,
+            // so let's stop here as well
+            if c.is_ascii_alphabetic() && escape {
+                break;
+            }
+
+            // we\re not adding the escape char to the output, because the output is not a regexp
+            if c == '\\' {
+                escape = true;
+                continue;
+            }
+
+            result.push(c);
+        }
+
+        result
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn test_simple_text_is_passed_as_is() {
+        let r = Regex::new("^foo/BAR", false).unwrap();
+        assert!(r.is_match("foo/BAR"));
+        assert!(r.is_partial_match("foo/BAR"));
+        assert!(!r.is_match("foo/bar"));
+        assert!(!r.is_partial_match("foo/bar"));
+    }
+
+    #[test]
+    fn test_case_insensitive() {
+        let r = Regex::new("^foo/BAR", true).unwrap();
+        assert!(r.is_match("foo/BAR"));
+        assert!(r.is_partial_match("foo/BAR"));
+        assert!(r.is_match("foo/bar"));
+        assert!(r.is_partial_match("foo/bar"));
+        assert!(!r.is_partial_match("foo/baz"));
+    }
+
+    #[test]
+    fn test_partial_match_stops_on_wildcard() {
+        let r = Regex::new("^abcd*ef", true).unwrap();
+        assert!(r.is_match("abcdddddef"));
+        assert!(r.is_match("abcef"));
+        assert!(!r.is_match("abef"));
+
+        assert!(r.is_partial_match("a"));
+        assert!(r.is_partial_match("ab"));
+        assert!(r.is_partial_match("ab"));
+        assert!(r.is_partial_match("abc"));
+        assert!(r.is_partial_match("abcef"));
+
+        assert!(!r.is_partial_match("-a"));
+        assert!(!r.is_partial_match("a-b"));
+        assert!(!r.is_partial_match("ab-"));
+    }
+
+    #[test]
+    fn test_unicode() {
+        let r = Regex::new("^ąęść?", true).unwrap();
+        assert!(r.is_partial_match("ą"));
+        assert!(r.is_partial_match("ąę"));
+        assert!(r.is_partial_match("ąęś"));
+        assert!(r.is_partial_match("ąęść"));
+        assert!(!r.is_partial_match("ąęść---"));
+    }
+
+    #[test]
+    fn test_can_partial_match_escaped_chars() {
+        let r = Regex::new("^\\.\\*", true).unwrap();
+        assert!(r.is_partial_match("."));
+        assert!(r.is_partial_match(".*"));
+        assert!(!r.is_partial_match("foo"));
+    }
+}
diff --git a/src/selector.rs b/src/selector.rs
index 5b20af8..2ad8f72 100644
--- a/src/selector.rs
+++ b/src/selector.rs
@@ -239,8 +239,6 @@ mod test {
         assert!(selector.matches_dir(&Path::from("/test1")));
         assert!(selector.matches_dir(&Path::from("/test2/bar")));
 
-        assert!(!selector.matches_dir(&Path::from("/test999")));
-        assert!(!selector.matches_dir(&Path::from("/test999/bar")));
         assert!(!selector.matches_dir(&Path::from("/test3/foo")));
         assert!(!selector.matches_dir(&Path::from("/test3/foo/bar/baz")));
     }


Test patch: