feat(tvix/nix-compat): fold NameError into Error

This being a nested error makes things more complicated than necessary.

Also, this caused BuildStorePathError to only hold NameError,
so refactor these utility functions to either return Error, or
BuildStorePathError.

Change-Id: I046fb403780cc5135df8b8833a291fc2a90fd913
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8972
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>

tvix/nix-compat/src/derivation/mod.rs

@@ -212,7 +212,7 @@ impl Derivation {
                 build_output_path(derivation_or_fod_hash, output_name, &path_name).map_err(|e| {
                     DerivationError::InvalidOutputDerivationPath(
                         output_name.to_string(),
-                        store_path::BuildStorePathError::InvalidName(e),
+                        store_path::BuildStorePathError::InvalidStorePath(e),
                     )
                 })?
             };

tvix/nix-compat/src/store_path/mod.rs

@@ -23,23 +23,12 @@ pub enum Error {
     MissingDash(),
     #[error("Hash encoding is invalid: {0}")]
     InvalidHashEncoding(Nixbase32DecodeError),
-    #[error("{0}")]
+    #[error("Invalid length")]
-    InvalidName(NameError),
+    InvalidLength(),
-    #[error("Tried to parse an absolute path which was missing the store dir prefix.")]
-    MissingStoreDir(),
-}
-
-/// Errors that can occur during the validation of name characters.
-#[derive(Debug, PartialEq, Eq, Error)]
-pub enum NameError {
     #[error("Invalid name: {0}")]
     InvalidName(String),
-}
+    #[error("Tried to parse an absolute path which was missing the store dir prefix.")]
-
+    MissingStoreDir(),
-impl From<NameError> for Error {
-    fn from(e: NameError) -> Self {
-        Self::InvalidName(e)
-    }
 }
 
 /// Represents a path in the Nix store (a direct child of [STORE_DIR]).
@@ -69,7 +58,7 @@ impl StorePath {
         // - 1 dash
         // - 1 character for the name
         if s.len() < ENCODED_DIGEST_SIZE + 2 {
-            Err(NameError::InvalidName("".to_string()))?;
+            Err(Error::InvalidLength())?
         }
 
         let digest = match nixbase32::decode(s[..ENCODED_DIGEST_SIZE].as_bytes()) {
@@ -120,10 +109,10 @@ impl StorePath {
                         let rest_buf: PathBuf = it.collect();
                         Ok((store_path, rest_buf))
                     } else {
-                        Err(Error::InvalidName(NameError::InvalidName("".to_string())))
+                        Err(Error::InvalidName("".to_string()))
                     }
                 } else {
-                    Err(Error::InvalidName(NameError::InvalidName("".to_string())))
+                    Err(Error::InvalidName("".to_string()))
                 }
             }
         }
@@ -137,7 +126,7 @@ impl StorePath {
     }
 
     /// Checks a given &str to match the restrictions for store path names.
-    pub fn validate_name(s: &str) -> Result<(), NameError> {
+    pub fn validate_name(s: &str) -> Result<(), Error> {
         for c in s.chars() {
             if c.is_ascii_alphanumeric()
                 || c == '-'
@@ -150,7 +139,7 @@ impl StorePath {
                 continue;
             }
 
-            return Err(NameError::InvalidName(s.to_string()));
+            return Err(Error::InvalidName(s.to_string()));
         }
 
         Ok(())
@@ -174,7 +163,7 @@ mod tests {
     use crate::store_path::{DIGEST_SIZE, ENCODED_DIGEST_SIZE};
     use test_case::test_case;
 
-    use super::{Error, NameError, StorePath};
+    use super::{Error, StorePath};
 
     #[test]
     fn encoded_digest_size() {
@@ -276,11 +265,11 @@ mod tests {
     #[test]
     fn from_absolute_path_errors() {
         assert_eq!(
-            Error::InvalidName(NameError::InvalidName("".to_string())),
+            Error::InvalidName("".into()),
             StorePath::from_absolute_path_full("/nix/store/").expect_err("must fail")
         );
         assert_eq!(
-            Error::InvalidName(NameError::InvalidName("".to_string())),
+            Error::InvalidLength(),
             StorePath::from_absolute_path_full("/nix/store/foo").expect_err("must fail")
         );
         assert_eq!(

tvix/nix-compat/src/store_path/utils.rs

@@ -1,18 +1,17 @@
+use super::{Error, STORE_DIR};
 use crate::nixbase32;
 use crate::nixhash::{HashAlgo, NixHash, NixHashWithMode};
 use crate::store_path::StorePath;
 use sha2::{Digest, Sha256};
-use thiserror::Error;
+use thiserror;
-
-use super::{NameError, STORE_DIR};
 
 /// Errors that can occur when creating a content-addressed store path.
 ///
-/// This wraps the main [Error] which is just about invalid store path names.
+/// This wraps the main [Error]..
-#[derive(Debug, PartialEq, Eq, Error)]
+#[derive(Debug, PartialEq, Eq, thiserror::Error)]
 pub enum BuildStorePathError {
-    #[error("{0}")]
+    #[error("Invalid Store Path: {0}")]
-    InvalidName(NameError),
+    InvalidStorePath(Error),
     /// This error occurs when we have references outside the SHA-256 +
     /// Recursive case. The restriction comes from upstream Nix. It may be
     /// lifted at some point but there isn't a pressing need to anticipate that.
@@ -46,7 +45,7 @@ pub fn build_text_path<S: AsRef<str>, I: IntoIterator<Item = S>, C: AsRef<[u8]>>
     name: &str,
     content: C,
     references: I,
-) -> Result<StorePath, NameError> {
+) -> Result<StorePath, Error> {
     build_store_path_from_fingerprint_parts(
         &make_type("text", references, false),
         // the nix_hash_string representation of the sha256 digest of some contents
@@ -79,7 +78,7 @@ pub fn build_regular_ca_path<S: AsRef<str>, I: IntoIterator<Item = S>>(
             hash,
             name,
         )
-        .map_err(BuildStorePathError::InvalidName),
+        .map_err(BuildStorePathError::InvalidStorePath),
         _ => {
             if references.into_iter().next().is_some() {
                 return Err(BuildStorePathError::InvalidReference());
@@ -105,7 +104,7 @@ pub fn build_regular_ca_path<S: AsRef<str>, I: IntoIterator<Item = S>>(
                 },
                 name,
             )
-            .map_err(BuildStorePathError::InvalidName)
+            .map_err(BuildStorePathError::InvalidStorePath)
         }
     }
 }
@@ -118,7 +117,7 @@ pub fn build_output_path(
     drv_hash: &NixHash,
     output_name: &str,
     output_path_name: &str,
-) -> Result<StorePath, NameError> {
+) -> Result<StorePath, Error> {
     build_store_path_from_fingerprint_parts(
         &(String::from("output:") + output_name),
         drv_hash,
@@ -138,7 +137,7 @@ fn build_store_path_from_fingerprint_parts(
     ty: &str,
     hash: &NixHash,
     name: &str,
-) -> Result<StorePath, NameError> {
+) -> Result<StorePath, Error> {
     let fingerprint =
         String::from(ty) + ":" + &hash.to_nix_hash_string() + ":" + STORE_DIR + ":" + name;
     let digest = {

tvix/store/src/proto/tests/pathinfo.rs

@@ -66,7 +66,7 @@ fn validate_no_node(
     },
     Err(ValidatePathInfoError::InvalidNodeName(
         "invalid".to_string(),
-        store_path::Error::InvalidName(store_path::NameError::InvalidName("".to_string()))
+        store_path::Error::InvalidLength()
     ));
     "invalid node name"
 )]
@@ -111,7 +111,7 @@ fn validate_directory(
     },
     Err(ValidatePathInfoError::InvalidNodeName(
         "invalid".to_string(),
-        store_path::Error::InvalidName(store_path::NameError::InvalidName("".to_string()))
+        store_path::Error::InvalidLength()
     ));
     "invalid node name"
 )]
@@ -141,7 +141,7 @@ fn validate_file(t_file_node: proto::FileNode, t_result: Result<StorePath, Valid
     },
     Err(ValidatePathInfoError::InvalidNodeName(
         "invalid".to_string(),
-        store_path::Error::InvalidName(store_path::NameError::InvalidName("".to_string()))
+        store_path::Error::InvalidLength()
     ));
     "invalid node name"
 )]