refactor(tvix/derivation): expose proper ValidateDerivationError

Use proper errors, instead of anyhow.

Change-Id: I6db14c72a6319b389b0136aac7b84f50a30fb366
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7847
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This commit is contained in:
Florian Klink 2023-01-16 15:24:02 +01:00 committed by flokli
parent 95cad95b93
commit d644ed389a
7 changed files with 109 additions and 30 deletions

1
tvix/Cargo.lock generated
View file

@ -2288,6 +2288,7 @@ dependencies = [
"sha2", "sha2",
"test-case", "test-case",
"test-generator", "test-generator",
"thiserror",
"tvix-store-bin", "tvix-store-bin",
] ]

View file

@ -6790,6 +6790,10 @@ rec {
name = "sha2"; name = "sha2";
packageId = "sha2"; packageId = "sha2";
} }
{
name = "thiserror";
packageId = "thiserror";
}
{ {
name = "tvix-store-bin"; name = "tvix-store-bin";
packageId = "tvix-store-bin"; packageId = "tvix-store-bin";

View file

@ -10,6 +10,7 @@ anyhow = "1.0.68"
glob = "0.3.0" glob = "0.3.0"
serde = { version = "1.0", features = ["derive"] } serde = { version = "1.0", features = ["derive"] }
sha2 = "0.10.6" sha2 = "0.10.6"
thiserror = "1.0.38"
tvix-store-bin = { path = "../store" } tvix-store-bin = { path = "../store" }
[dev-dependencies.test-generator] [dev-dependencies.test-generator]

View file

@ -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),
}

View file

@ -1,4 +1,5 @@
mod derivation; mod derivation;
mod errors;
mod nix_hash; mod nix_hash;
mod output; mod output;
mod string_escape; mod string_escape;
@ -11,4 +12,5 @@ mod tests;
// Public API of the crate. // Public API of the crate.
pub use derivation::Derivation; pub use derivation::Derivation;
pub use errors::ValidateDerivationError;
pub use output::{Hash, Output}; pub use output::{Hash, Output};

View file

@ -1,5 +1,7 @@
use serde::{Deserialize, Serialize}; 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)] #[derive(Clone, Debug, Default, Eq, PartialEq, Serialize, Deserialize)]
pub struct Output { pub struct Output {
@ -22,8 +24,10 @@ impl Output {
self.hash.is_some() self.hash.is_some()
} }
pub fn validate(&self) -> anyhow::Result<()> { pub fn validate(&self) -> Result<(), ParseStorePathError> {
StorePath::from_absolute_path(&self.path)?; if let Err(e) = StorePath::from_absolute_path(&self.path) {
return Err(e);
}
Ok(()) Ok(())
} }
} }

View file

@ -1,85 +1,108 @@
use crate::{derivation::Derivation, write::DOT_FILE_EXT}; use crate::{derivation::Derivation, write::DOT_FILE_EXT, ValidateDerivationError};
use anyhow::bail;
use tvix_store::store_path::StorePath; use tvix_store::store_path::StorePath;
impl Derivation { impl Derivation {
/// validate ensures a Derivation struct is properly populated, /// validate ensures a Derivation struct is properly populated,
/// and returns an error if not. /// and returns a [ValidateDerivationError] if not.
/// TODO(flokli): make this proper errors pub fn validate(&self) -> Result<(), ValidateDerivationError> {
pub fn validate(&self) -> anyhow::Result<()> {
// Ensure the number of outputs is > 1 // Ensure the number of outputs is > 1
if self.outputs.is_empty() { if self.outputs.is_empty() {
bail!("0 outputs"); return Err(ValidateDerivationError::NoOutputs());
} }
// Validate all outputs // Validate all outputs
for (output_name, output) in &self.outputs { for (output_name, output) in &self.outputs {
if output_name.is_empty() { 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 output.is_fixed() {
if self.outputs.len() != 1 { 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" { if output_name != "out" {
bail!("the fixed-output output name must be called 'out'"); return Err(ValidateDerivationError::InvalidOutputNameForFixed(
output_name.to_string(),
));
} }
break; break;
} }
output.validate()?; if let Err(e) = output.validate() {
return Err(ValidateDerivationError::InvalidOutputPath(
output_name.to_string(),
e,
));
};
} }
// Validate all input_derivations // Validate all input_derivations
for (input_derivation_path, output_names) in &self.input_derivations { for (input_derivation_path, output_names) in &self.input_derivations {
// Validate input_derivation_path // 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) { if !input_derivation_path.ends_with(DOT_FILE_EXT) {
bail!( return Err(ValidateDerivationError::InvalidInputDerivationPrefix(
"derivation {} does not end with .drv", input_derivation_path.to_string(),
input_derivation_path ));
);
} }
if output_names.is_empty() { if output_names.is_empty() {
bail!( return Err(ValidateDerivationError::EmptyInputDerivationOutputNames(
"output_names list for {} may not be empty", input_derivation_path.to_string(),
input_derivation_path ));
);
} }
for output_name in output_names.iter() { for output_name in output_names.iter() {
if output_name.is_empty() { if output_name.is_empty() {
bail!( return Err(ValidateDerivationError::InvalidInputDerivationOutputName(
"output name entry for {} may not be empty", input_derivation_path.to_string(),
input_derivation_path output_name.to_string(),
) ));
} }
// TODO: do we need to apply more name validation here?
} }
} }
// Validate all input_sources // Validate all input_sources
for input_source in self.input_sources.iter() { 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 // validate platform
if self.system.is_empty() { if self.system.is_empty() {
bail!("required attribute 'platform' missing"); return Err(ValidateDerivationError::InvalidPlatform(
self.system.to_string(),
));
} }
// validate builder // validate builder
if self.builder.is_empty() { 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. // validate env, none of the keys may be empty.
// We skip the `name` validation seen in go-nix. // We skip the `name` validation seen in go-nix.
for k in self.environment.keys() { for k in self.environment.keys() {
if k.is_empty() { if k.is_empty() {
bail!("found empty environment variable key"); return Err(ValidateDerivationError::InvalidEnvironmentKey(
k.to_string(),
));
} }
} }