Merge pull request science-computing#429 from ammernico/verify-error-out

Implement package filtering as a repo method

src/commands/source/download.rs

@@ -233,38 +233,8 @@ pub async fn download(
         NUMBER_OF_MAX_CONCURRENT_DOWNLOADS,
     ));
 
-    let mut r = repo.packages()
-        .filter(|p| {
-            match (pname.as_ref(), pvers.as_ref(), matching_regexp.as_ref()) {
-                (None, None, None)              => true,
-                (Some(pname), None, None)       => p.name() == pname,
-                (Some(pname), Some(vers), None) => p.name() == pname && vers.matches(p.version()),
-                (None, None, Some(regex))       => regex.is_match(p.name()),
-
-                (_, _, _) => {
-                    panic!("This should not be possible, either we select packages by name and (optionally) version, or by regex.")
-                },
-            }
-        }).peekable();
-
-    // check if the iterator is empty
-    if r.peek().is_none() {
-        let pname = matches.get_one::<String>("package_name");
-        let pvers = matches.get_one::<String>("package_version");
-        let matching_regexp = matches.get_one::<String>("matching");
-
-        match (pname, pvers, matching_regexp) {
-            (Some(pname), None, None) => return Err(anyhow!("{} not found", pname)),
-            (Some(pname), Some(vers), None) => return Err(anyhow!("{} {} not found", pname, vers)),
-            (None, None, Some(regex)) => return Err(anyhow!("{} regex not found", regex)),
-
-            (_, _, _) => {
-                panic!("This should not be possible, either we select packages by name and (optionally) version, or by regex.")
-            }
-        }
-    }
-
-    let r = r
+    let r = repo
+        .search_packages(&pname, &pvers, &matching_regexp)?
         .flat_map(|p| {
             sc.sources_for(p).into_iter().map(|source| {
                 let download_sema = download_sema.clone();

src/commands/source/mod.rs

@@ -74,21 +74,7 @@ pub async fn verify(
         .map(|s| crate::commands::util::mk_package_name_regex(s.as_ref()))
         .transpose()?;
 
-    let packages = repo
-        .packages()
-        .filter(|p| {
-            match (pname.as_ref(), pvers.as_ref(), matching_regexp.as_ref()) {
-                (None, None, None)              => true,
-                (Some(pname), None, None)       => p.name() == pname,
-                (Some(pname), Some(vers), None) => p.name() == pname && vers.matches(p.version()),
-                (None, None, Some(regex))       => regex.is_match(p.name()),
-
-                (_, _, _) => {
-                    panic!("This should not be possible, either we select packages by name and (optionally) version, or by regex.")
-                },
-            }
-        })
-        .inspect(|p| trace!("Found for verification: {} {}", p.name(), p.version()));
+    let packages = repo.search_packages(&pname, &pvers, &matching_regexp)?;
 
     verify_impl(packages, &sc, &progressbars).await
 }

src/repository/repository.rs

@@ -17,6 +17,7 @@ use anyhow::anyhow;
 use anyhow::Context;
 use anyhow::Error;
 use anyhow::Result;
+use regex::Regex;
 use resiter::AndThen;
 use resiter::FilterMap;
 use resiter::Map;
@@ -270,6 +271,44 @@ impl Repository {
     pub fn packages(&self) -> impl Iterator<Item = &Package> {
         self.inner.values()
     }
+
+    pub fn search_packages<'a>(
+        &'a self,
+        pname: &'a Option<PackageName>,
+        pvers: &'a Option<PackageVersionConstraint>,
+        matching_regexp: &'a Option<Regex>,
+    ) -> Result<impl Iterator<Item = &Package> + 'a> {
+        let mut r = self.inner.values()
+        .filter(move |p| {
+            match (pname, pvers, matching_regexp) {
+                (None, None, None)              => true,
+                (Some(pname), None, None)       => p.name() == pname,
+                (Some(pname), Some(vers), None) => p.name() == pname && vers.matches(p.version()),
+                (None, None, Some(regex))       => regex.is_match(p.name()),
+
+                (_, _, _) => {
+                    panic!("This should not be possible, either we select packages by name and (optionally) version, or by regex.")
+                },
+            }
+        }).peekable();
+
+        // check if the iterator is empty
+        if r.peek().is_none() {
+            match (pname, pvers, matching_regexp) {
+                (Some(pname), None, None) => return Err(anyhow!("{} not found", pname)),
+                (Some(pname), Some(vers), None) => {
+                    return Err(anyhow!("{} {} not found", pname, vers))
+                }
+                (None, None, Some(regex)) => return Err(anyhow!("{} regex not found", regex)),
+
+                (_, _, _) => {
+                    panic!("This should not be possible, either we select packages by name and (optionally) version, or by regex.")
+                }
+            }
+        }
+
+        Ok(r)
+    }
 }
 
 #[cfg(test)]