From 5364fcb12708667a2dc698a689d00d70d1bf75af Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Tue, 18 Jul 2023 20:46:55 +0300 Subject: [PATCH] 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 Reviewed-by: tazjin --- tvix/nix-compat/src/derivation/mod.rs | 2 +- tvix/nix-compat/src/store_path/mod.rs | 35 +++++++++---------------- tvix/nix-compat/src/store_path/utils.rs | 23 ++++++++-------- tvix/store/src/proto/tests/pathinfo.rs | 6 ++--- 4 files changed, 27 insertions(+), 39 deletions(-) diff --git a/tvix/nix-compat/src/derivation/mod.rs b/tvix/nix-compat/src/derivation/mod.rs index ab1471165..ab615d502 100644 --- a/tvix/nix-compat/src/derivation/mod.rs +++ b/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), ) })? }; diff --git a/tvix/nix-compat/src/store_path/mod.rs b/tvix/nix-compat/src/store_path/mod.rs index 818e11b40..f7d42b2f7 100644 --- a/tvix/nix-compat/src/store_path/mod.rs +++ b/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}")] - InvalidName(NameError), - #[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 length")] + InvalidLength(), #[error("Invalid name: {0}")] InvalidName(String), -} - -impl From for Error { - fn from(e: NameError) -> Self { - Self::InvalidName(e) - } + #[error("Tried to parse an absolute path which was missing the store dir prefix.")] + MissingStoreDir(), } /// 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!( diff --git a/tvix/nix-compat/src/store_path/utils.rs b/tvix/nix-compat/src/store_path/utils.rs index fd3785568..2d0134cc6 100644 --- a/tvix/nix-compat/src/store_path/utils.rs +++ b/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 super::{NameError, STORE_DIR}; +use thiserror; /// Errors that can occur when creating a content-addressed store path. /// -/// This wraps the main [Error] which is just about invalid store path names. -#[derive(Debug, PartialEq, Eq, Error)] +/// This wraps the main [Error].. +#[derive(Debug, PartialEq, Eq, thiserror::Error)] pub enum BuildStorePathError { - #[error("{0}")] - InvalidName(NameError), + #[error("Invalid Store Path: {0}")] + 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, I: IntoIterator, C: AsRef<[u8]>> name: &str, content: C, references: I, -) -> Result { +) -> Result { 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, I: IntoIterator>( 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, I: IntoIterator>( }, 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 { +) -> Result { 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 { +) -> Result { let fingerprint = String::from(ty) + ":" + &hash.to_nix_hash_string() + ":" + STORE_DIR + ":" + name; let digest = { diff --git a/tvix/store/src/proto/tests/pathinfo.rs b/tvix/store/src/proto/tests/pathinfo.rs index 54a76fc6c..57104a5fd 100644 --- a/tvix/store/src/proto/tests/pathinfo.rs +++ b/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