Improve package and recipes API

The main changes here are to leverage PackageError and PackageName more.

I also fixed a bug I introduced by misunderstanding code in auto_deps. auto_deps should return PackageNames instead of Strings, but I converted the Strings too early.

Cargo.lock

@@ -1832,7 +1832,7 @@ dependencies = [
 
 [[package]]
 name = "redox-pkg"
-version = "0.2.5"
+version = "0.2.7"
 dependencies = [
  "anyhow",
  "ignore",
@@ -1867,7 +1867,7 @@ dependencies = [
  "pkgar 0.1.17",
  "pkgar-core 0.1.17",
  "pkgar-keys 0.1.17",
- "redox-pkg 0.2.5",
+ "redox-pkg 0.2.7",
  "redoxer",
  "serde",
  "tempfile",

src/bin/cook.rs

@@ -219,7 +219,20 @@ fn fetch_offline(recipe_dir: &Path, source: &Option<SourceRecipe>) -> Result<Pat
         Some(SourceRecipe::SameAs { same_as: _ }) | Some(SourceRecipe::Path { path: _ }) | None => {
             return fetch(recipe_dir, source);
         }
-        Some(SourceRecipe::Git { git: _, upstream: _, branch: _, rev: _, patches: _, script: _ }) | Some(SourceRecipe::Tar { tar: _, blake3: _, patches: _, script: _ }) => {
+        Some(SourceRecipe::Git {
+            git: _,
+            upstream: _,
+            branch: _,
+            rev: _,
+            patches: _,
+            script: _,
+        })
+        | Some(SourceRecipe::Tar {
+            tar: _,
+            blake3: _,
+            patches: _,
+            script: _,
+        }) => {
             if !source_dir.is_dir() {
                 return Err(format!(
                     "'{dir}' is not exist and unable to continue in offline mode",
@@ -596,12 +609,7 @@ fn auto_deps(
                 if let Ok(relative_path) = path.strip_prefix(stage_dir) {
                     eprintln!("DEBUG: {} needs {}", relative_path.display(), name);
                 }
-                if let Ok(name) = PackageName::new(name) {
-                    needed.insert(name);
-                } else {
-                    // AFAICT this shouldn't ever happen.
-                    eprintln!("ERROR: `{name}` is an invalid package name; please report");
-                }
+                needed.insert(name.to_string());
             }
         }
     }
@@ -1126,7 +1134,8 @@ fn cook(
     let source_dir = match is_offline {
         true => fetch_offline(recipe_dir, &recipe.source),
         false => fetch(recipe_dir, &recipe.source),
-    }.map_err(|err| format!("failed to fetch: {}", err))?;
+    }
+    .map_err(|err| format!("failed to fetch: {}", err))?;
 
     if fetch_only {
         return Ok(());

src/bin/pkg_deps.rs

@@ -1,18 +1,22 @@
-use std::{env::args, process::ExitCode};
+use std::env::args;
 
-use pkg::package::Package;
+use pkg::{
+    package::{Package, PackageError},
+    PackageName,
+};
 
 use cookbook::WALK_DEPTH;
 
-fn main() -> ExitCode {
-    let names = args().skip(1).collect::<Vec<String>>();
-    // TODO: Ugly vec
-    let names: Vec<&str> = names.iter().map(|s| s.as_str()).collect();
-    let packages = Package::new_recursive(&names, WALK_DEPTH).expect("package not found");
+fn main() -> Result<(), PackageError> {
+    let names: Vec<_> = args()
+        .skip(1)
+        .map(PackageName::new)
+        .collect::<Result<_, _>>()?;
 
+    let packages = Package::new_recursive(&names, WALK_DEPTH)?;
     for package in packages {
         println!("{}", package.name);
     }
 
-    ExitCode::SUCCESS
+    Ok(())
 }

src/recipe.rs

@@ -1,7 +1,10 @@
 use std::{convert::TryInto, fs, path::PathBuf};
 
-use pkg::{recipes, PackageName};
-use serde::{Deserialize, Serialize};
+use pkg::{package::PackageError, recipes, PackageName};
+use serde::{
+    de::{value::Error as DeError, Error as DeErrorT},
+    Deserialize, Serialize,
+};
 
 /// Specifies how to download the source for a recipe
 #[derive(Debug, Deserialize, PartialEq, Serialize)]
@@ -108,48 +111,33 @@ pub struct CookRecipe {
 }
 
 impl CookRecipe {
-    pub fn new(name: &str) -> Result<Self, String> {
-        let name: PackageName = name
-            .try_into()
-            .map_err(|e| format!("Invalid package name: {e}"))?;
-        let dir = recipes::find(name.as_str());
-        if dir.is_none() {
-            return Err(format!("failed to find recipe directory '{}'", name));
-        }
-        let dir = dir.unwrap();
+    pub fn new(
+        name: impl TryInto<PackageName, Error = PackageError>,
+    ) -> Result<Self, PackageError> {
+        let name: PackageName = name.try_into()?;
+        let dir = recipes::find(name.as_str())
+            .ok_or_else(|| PackageError::PackageNotFound(name.clone()))?;
         let file = dir.join("recipe.toml");
         if !file.is_file() {
-            return Err(format!("failed to find recipe file '{}'", file.display()));
+            return Err(PackageError::FileMissing(file));
         }
 
-        let toml = fs::read_to_string(&file).map_err(|err| {
-            format!(
-                "failed to read recipe file '{}': {}\n{:#?}",
-                file.display(),
-                err,
-                err
-            )
-        })?;
+        let toml = fs::read_to_string(&file)
+            .map_err(|err| PackageError::Parse(DeError::custom(err), Some(file.clone())))?;
 
-        let recipe: Recipe = toml::from_str(&toml).map_err(|err| {
-            format!(
-                "failed to parse recipe file '{}': {}\n{:#?}",
-                file.display(),
-                err,
-                err
-            )
-        })?;
+        let recipe: Recipe = toml::from_str(&toml)
+            .map_err(|err| PackageError::Parse(DeError::custom(err), Some(file)))?;
 
         let dir = dir.to_path_buf();
         Ok(Self { name, dir, recipe })
     }
 
-    pub fn new_recursive(names: &[PackageName], recursion: usize) -> Result<Vec<Self>, String> {
+    pub fn new_recursive(
+        names: &[PackageName],
+        recursion: usize,
+    ) -> Result<Vec<Self>, PackageError> {
         if recursion == 0 {
-            return Err(format!(
-                "recursion limit while processing build dependencies: {:#?}",
-                names
-            ));
+            return Err(PackageError::Recursion(Default::default()));
         }
 
         let mut recipes = Vec::new();
@@ -158,7 +146,10 @@ impl CookRecipe {
 
             let dependencies =
                 Self::new_recursive(&recipe.recipe.build.dependencies, recursion - 1).map_err(
-                    |err| format!("{}: failed on loading build dependencies:\n{}", name, err),
+                    |mut err| {
+                        err.append_recursion(name);
+                        err
+                    },
                 )?;
 
             for dependency in dependencies {
@@ -178,12 +169,9 @@ impl CookRecipe {
     pub fn get_package_deps_recursive(
         names: &[PackageName],
         recursion: usize,
-    ) -> Result<Vec<PackageName>, String> {
+    ) -> Result<Vec<PackageName>, PackageError> {
         if recursion == 0 {
-            return Err(format!(
-                "recursion limit while processing package dependencies: {:#?}",
-                names
-            ));
+            return Err(PackageError::Recursion(Default::default()));
         }
 
         let mut recipes: Vec<PackageName> = Vec::new();
@@ -194,7 +182,10 @@ impl CookRecipe {
                 &recipe.recipe.package.dependencies,
                 recursion - 1,
             )
-            .map_err(|err| format!("{}: failed on loading package dependencies:\n{}", name, err))?;
+            .map_err(|mut err| {
+                err.append_recursion(name);
+                err
+            })?;
 
             for dependency in dependencies {
                 if !recipes.contains(&dependency) {