diff options
| -rw-r--r-- | tvix/Cargo.lock | 1 | ||||
| -rw-r--r-- | tvix/Cargo.nix | 4 | ||||
| -rw-r--r-- | tvix/derivation/Cargo.toml | 1 | ||||
| -rw-r--r-- | tvix/derivation/src/errors.rs | 44 | ||||
| -rw-r--r-- | tvix/derivation/src/lib.rs | 2 | ||||
| -rw-r--r-- | tvix/derivation/src/output.rs | 10 | ||||
| -rw-r--r-- | tvix/derivation/src/validate.rs | 77 |
7 files changed, 109 insertions, 30 deletions
diff --git a/tvix/Cargo.lock b/tvix/Cargo.lock index 37e1a0f13998..36c9cef50f03 100644 --- a/tvix/Cargo.lock +++ b/tvix/Cargo.lock @@ -2288,6 +2288,7 @@ dependencies = [ "sha2", "test-case", "test-generator", + "thiserror", "tvix-store-bin", ] diff --git a/tvix/Cargo.nix b/tvix/Cargo.nix index 5fca533d1cb7..a27fe822f200 100644 --- a/tvix/Cargo.nix +++ b/tvix/Cargo.nix @@ -6791,6 +6791,10 @@ rec { packageId = "sha2"; } { + name = "thiserror"; + packageId = "thiserror"; + } + { name = "tvix-store-bin"; packageId = "tvix-store-bin"; } diff --git a/tvix/derivation/Cargo.toml b/tvix/derivation/Cargo.toml index 2d5ea74bdc79..12afcf92c1e8 100644 --- a/tvix/derivation/Cargo.toml +++ b/tvix/derivation/Cargo.toml @@ -10,6 +10,7 @@ anyhow = "1.0.68" glob = "0.3.0" serde = { version = "1.0", features = ["derive"] } sha2 = "0.10.6" +thiserror = "1.0.38" tvix-store-bin = { path = "../store" } [dev-dependencies.test-generator] diff --git a/tvix/derivation/src/errors.rs b/tvix/derivation/src/errors.rs new file mode 100644 index 000000000000..a43162c1460a --- /dev/null +++ b/tvix/derivation/src/errors.rs @@ -0,0 +1,44 @@ +use thiserror::Error; +use tvix_store::store_path::ParseStorePathError; + +/// Errors that can occur during the validation of Derivation structs. +#[derive(Debug, Error)] +pub enum ValidateDerivationError { + // outputs + #[error("No outputs defined.")] + NoOutputs(), + #[error("Invalid output name: {0}.")] + InvalidOutputName(String), + #[error("Encountered fixed-output derivation, but more than 1 output in total.")] + MoreThanOneOutputButFixed(), + #[error("Invalid output name for fixed-output derivation: {0}.")] + InvalidOutputNameForFixed(String), + #[error("Unable to parse path of output {0}: {1}.")] + InvalidOutputPath(String, ParseStorePathError), + + // input derivation + #[error("Unable to parse input derivation path {0}: {1}.")] + InvalidInputDerivationPath(String, ParseStorePathError), + #[error("Input Derivation {0} doesn't end with .drv.")] + InvalidInputDerivationPrefix(String), + #[error("Input Derivation {0} output names are empty.")] + EmptyInputDerivationOutputNames(String), + #[error("Input Derivation {0} output name {1} is invalid.")] + InvalidInputDerivationOutputName(String, String), + + // input sources + #[error("Unable to parse input sources path {0}: {1}.")] + InvalidInputSourcesPath(String, ParseStorePathError), + + // platform + #[error("Invalid platform field: {0}")] + InvalidPlatform(String), + + // builder + #[error("Invalid builder field: {0}")] + InvalidBuilder(String), + + // environment + #[error("Invalid environment key {0}")] + InvalidEnvironmentKey(String), +} diff --git a/tvix/derivation/src/lib.rs b/tvix/derivation/src/lib.rs index 0729ba0a6c6f..00a8e45022df 100644 --- a/tvix/derivation/src/lib.rs +++ b/tvix/derivation/src/lib.rs @@ -1,4 +1,5 @@ mod derivation; +mod errors; mod nix_hash; mod output; mod string_escape; @@ -11,4 +12,5 @@ mod tests; // Public API of the crate. pub use derivation::Derivation; +pub use errors::ValidateDerivationError; pub use output::{Hash, Output}; diff --git a/tvix/derivation/src/output.rs b/tvix/derivation/src/output.rs index 1236cd989f7f..cd77c00f2f41 100644 --- a/tvix/derivation/src/output.rs +++ b/tvix/derivation/src/output.rs @@ -1,5 +1,7 @@ use serde::{Deserialize, Serialize}; -use tvix_store::store_path::StorePath; +use tvix_store::store_path::{ParseStorePathError, StorePath}; + +use crate::ValidateDerivationError; #[derive(Clone, Debug, Default, Eq, PartialEq, Serialize, Deserialize)] pub struct Output { @@ -22,8 +24,10 @@ impl Output { self.hash.is_some() } - pub fn validate(&self) -> anyhow::Result<()> { - StorePath::from_absolute_path(&self.path)?; + pub fn validate(&self) -> Result<(), ParseStorePathError> { + if let Err(e) = StorePath::from_absolute_path(&self.path) { + return Err(e); + } Ok(()) } } diff --git a/tvix/derivation/src/validate.rs b/tvix/derivation/src/validate.rs index e225bd2fdb07..2ff23d52048f 100644 --- a/tvix/derivation/src/validate.rs +++ b/tvix/derivation/src/validate.rs @@ -1,85 +1,108 @@ -use crate::{derivation::Derivation, write::DOT_FILE_EXT}; -use anyhow::bail; +use crate::{derivation::Derivation, write::DOT_FILE_EXT, ValidateDerivationError}; use tvix_store::store_path::StorePath; impl Derivation { /// validate ensures a Derivation struct is properly populated, - /// and returns an error if not. - /// TODO(flokli): make this proper errors - pub fn validate(&self) -> anyhow::Result<()> { + /// and returns a [ValidateDerivationError] if not. + pub fn validate(&self) -> Result<(), ValidateDerivationError> { // Ensure the number of outputs is > 1 if self.outputs.is_empty() { - bail!("0 outputs"); + return Err(ValidateDerivationError::NoOutputs()); } // Validate all outputs for (output_name, output) in &self.outputs { if output_name.is_empty() { - bail!("output_name from outputs may not be empty") + return Err(ValidateDerivationError::InvalidOutputName( + output_name.to_string(), + )); } if output.is_fixed() { if self.outputs.len() != 1 { - bail!("encountered fixed-output, but there's more than 1 output in total"); + return Err(ValidateDerivationError::MoreThanOneOutputButFixed()); } if output_name != "out" { - bail!("the fixed-output output name must be called 'out'"); + return Err(ValidateDerivationError::InvalidOutputNameForFixed( + output_name.to_string(), + )); } break; } - output.validate()?; + if let Err(e) = output.validate() { + return Err(ValidateDerivationError::InvalidOutputPath( + output_name.to_string(), + e, + )); + }; } // Validate all input_derivations for (input_derivation_path, output_names) in &self.input_derivations { // Validate input_derivation_path - StorePath::from_absolute_path(input_derivation_path)?; + if let Err(e) = StorePath::from_absolute_path(input_derivation_path) { + return Err(ValidateDerivationError::InvalidInputDerivationPath( + input_derivation_path.to_string(), + e, + )); + } + if !input_derivation_path.ends_with(DOT_FILE_EXT) { - bail!( - "derivation {} does not end with .drv", - input_derivation_path - ); + return Err(ValidateDerivationError::InvalidInputDerivationPrefix( + input_derivation_path.to_string(), + )); } if output_names.is_empty() { - bail!( - "output_names list for {} may not be empty", - input_derivation_path - ); + return Err(ValidateDerivationError::EmptyInputDerivationOutputNames( + input_derivation_path.to_string(), + )); } for output_name in output_names.iter() { if output_name.is_empty() { - bail!( - "output name entry for {} may not be empty", - input_derivation_path - ) + return Err(ValidateDerivationError::InvalidInputDerivationOutputName( + input_derivation_path.to_string(), + output_name.to_string(), + )); } + // TODO: do we need to apply more name validation here? } } // Validate all input_sources for input_source in self.input_sources.iter() { - StorePath::from_absolute_path(input_source)?; + if let Err(e) = StorePath::from_absolute_path(input_source) { + return Err(ValidateDerivationError::InvalidInputSourcesPath( + input_source.to_string(), + e, + )); + } } // validate platform if self.system.is_empty() { - bail!("required attribute 'platform' missing"); + return Err(ValidateDerivationError::InvalidPlatform( + self.system.to_string(), + )); } // validate builder if self.builder.is_empty() { - bail!("required attribute 'builder' missing"); + return Err(ValidateDerivationError::InvalidBuilder( + self.builder.to_string(), + )); } // validate env, none of the keys may be empty. // We skip the `name` validation seen in go-nix. for k in self.environment.keys() { if k.is_empty() { - bail!("found empty environment variable key"); + return Err(ValidateDerivationError::InvalidEnvironmentKey( + k.to_string(), + )); } } |