diff options
| -rw-r--r-- | tvix/build/src/buildservice/build_request.rs | 8 | ||||
| -rw-r--r-- | tvix/build/src/proto/mod.rs | 104 | ||||
| -rw-r--r-- | tvix/glue/src/tvix_build.rs | 15 |
3 files changed, 12 insertions, 115 deletions
diff --git a/tvix/build/src/buildservice/build_request.rs b/tvix/build/src/buildservice/build_request.rs index a93fef5b393c..78409c34e3b2 100644 --- a/tvix/build/src/buildservice/build_request.rs +++ b/tvix/build/src/buildservice/build_request.rs @@ -35,7 +35,7 @@ use tvix_castore::{Node, PathComponent}; /// /// As of now, we're okay to accept this, but it prevents uploading an /// entirely-non-IFD subgraph of BuildRequests eagerly. -#[derive(Clone, PartialEq)] +#[derive(Debug, Clone, PartialEq)] pub struct BuildRequest { /// The list of all root nodes that should be visible in `inputs_dir` at the /// time of the build. @@ -94,7 +94,7 @@ pub struct BuildRequest { pub refscan_needles: Vec<String>, } -#[derive(Clone, PartialEq)] +#[derive(Debug, Clone, PartialEq)] pub struct EnvVar { /// name of the environment variable. Must not contain =. pub key: String, @@ -105,7 +105,7 @@ pub struct EnvVar { /// Constraints can be things like required architecture and minimum amount of memory. /// The required input paths are *not* represented in here, because it /// wouldn't be hermetic enough - see the comment around inputs too. -#[derive(Clone, PartialEq, Eq, Hash)] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub enum BuildConstraints { /// The system that's needed to execute the build. /// Must not be empty. @@ -124,7 +124,7 @@ pub enum BuildConstraints { ProvideBinSh, } -#[derive(Clone, PartialEq)] +#[derive(Debug, Clone, PartialEq)] pub struct AdditionalFile { pub path: PathBuf, pub contents: Bytes, diff --git a/tvix/build/src/proto/mod.rs b/tvix/build/src/proto/mod.rs index 97d56c6e625d..e6c94f5b56c9 100644 --- a/tvix/build/src/proto/mod.rs +++ b/tvix/build/src/proto/mod.rs @@ -118,90 +118,6 @@ where data.tuple_windows().all(|(a, b)| a <= b) } -impl BuildRequest { - /// Ensures the build request is valid. - /// This means, all input nodes need to be valid, paths in lists need to be sorted, - /// and all restrictions around paths themselves (relative, clean, …) need - // to be fulfilled. - pub fn validate(&self) -> Result<(), ValidateBuildRequestError> { - // validate names. Make sure they're sorted - - let mut last_name: bytes::Bytes = "".into(); - for (i, node) in self.inputs.iter().enumerate() { - // TODO(flokli): store result somewhere - let (name, _node) = node - .clone() - .into_name_and_node() - .map_err(|e| ValidateBuildRequestError::InvalidInputNode(i, e))?; - - if name.as_ref() <= last_name.as_ref() { - return Err(ValidateBuildRequestError::InputNodesNotSorted); - } else { - last_name = name.into() - } - } - - // validate working_dir - if !is_clean_relative_path(&self.working_dir) { - Err(ValidateBuildRequestError::InvalidWorkingDir)?; - } - - // validate scratch paths - for (i, p) in self.scratch_paths.iter().enumerate() { - if !is_clean_relative_path(p) { - Err(ValidateBuildRequestError::InvalidScratchPath(i))? - } - } - if !is_sorted(self.scratch_paths.iter().map(|e| e.as_bytes())) { - Err(ValidateBuildRequestError::ScratchPathsNotSorted)?; - } - - // validate inputs_dir - if !is_clean_relative_path(&self.inputs_dir) { - Err(ValidateBuildRequestError::InvalidInputsDir)?; - } - - // validate outputs - for (i, p) in self.outputs.iter().enumerate() { - if !is_clean_relative_path(p) { - Err(ValidateBuildRequestError::InvalidOutputPath(i))? - } - } - if !is_sorted(self.outputs.iter().map(|e| e.as_bytes())) { - Err(ValidateBuildRequestError::OutputsNotSorted)?; - } - - // validate environment_vars. - for (i, e) in self.environment_vars.iter().enumerate() { - if e.key.is_empty() || e.key.contains('=') { - Err(ValidateBuildRequestError::InvalidEnvVar(i))? - } - } - if !is_sorted(self.environment_vars.iter().map(|e| e.key.as_bytes())) { - Err(ValidateBuildRequestError::EnvVarNotSorted)?; - } - - // validate build constraints - if let Some(constraints) = self.constraints.as_ref() { - constraints - .validate() - .map_err(ValidateBuildRequestError::InvalidBuildConstraints)?; - } - - // validate additional_files - for (i, additional_file) in self.additional_files.iter().enumerate() { - if !is_clean_relative_path(&additional_file.path) { - Err(ValidateBuildRequestError::InvalidAdditionalFilePath(i))? - } - } - if !is_sorted(self.additional_files.iter().map(|e| e.path.as_bytes())) { - Err(ValidateBuildRequestError::AdditionalFilesNotSorted)?; - } - - Ok(()) - } -} - impl TryFrom<BuildRequest> for crate::buildservice::BuildRequest { type Error = ValidateBuildRequestError; fn try_from(value: BuildRequest) -> Result<Self, Self::Error> { @@ -311,26 +227,6 @@ pub enum ValidateBuildConstraintsError { AvailableRoPathsNotSorted, } -impl build_request::BuildConstraints { - pub fn validate(&self) -> Result<(), ValidateBuildConstraintsError> { - // validate system - if self.system.is_empty() { - Err(ValidateBuildConstraintsError::InvalidSystem)?; - } - // validate available_ro_paths - for (i, p) in self.available_ro_paths.iter().enumerate() { - if !is_clean_absolute_path(p) { - Err(ValidateBuildConstraintsError::InvalidAvailableRoPaths(i))? - } - } - if !is_sorted(self.available_ro_paths.iter().map(|e| e.as_bytes())) { - Err(ValidateBuildConstraintsError::AvailableRoPathsNotSorted)?; - } - - Ok(()) - } -} - impl From<build_request::EnvVar> for crate::buildservice::EnvVar { fn from(value: build_request::EnvVar) -> Self { Self { diff --git a/tvix/glue/src/tvix_build.rs b/tvix/glue/src/tvix_build.rs index e832dcf4b1c6..6a3c6ab40ccc 100644 --- a/tvix/glue/src/tvix_build.rs +++ b/tvix/glue/src/tvix_build.rs @@ -6,9 +6,10 @@ use std::collections::BTreeMap; use bytes::Bytes; use nix_compat::{derivation::Derivation, nixbase32, store_path::StorePath}; use sha2::{Digest, Sha256}; +use tvix_build::buildservice::BuildRequest; use tvix_build::proto::{ + self, build_request::{AdditionalFile, BuildConstraints, EnvVar}, - BuildRequest, }; use tvix_castore::Node; @@ -43,7 +44,7 @@ pub(crate) fn get_refscan_needles( .chain(derivation.input_derivations.keys()) } -/// Takes a [Derivation] and turns it into a [BuildRequest]. +/// Takes a [Derivation] and turns it into a [proto::BuildRequest]. /// It assumes the Derivation has been validated. /// It needs two lookup functions: /// - one translating input sources to a castore node @@ -53,7 +54,7 @@ pub(crate) fn get_refscan_needles( pub(crate) fn derivation_to_build_request( derivation: &Derivation, inputs: BTreeMap<bytes::Bytes, Node>, -) -> std::io::Result<BuildRequest> { +) -> std::io::Result<proto::BuildRequest> { debug_assert!(derivation.validate(true).is_ok(), "drv must validate"); // produce command_args, which is builder and arguments in a Vec. @@ -113,7 +114,7 @@ pub(crate) fn derivation_to_build_request( provide_bin_sh: true, }); - let build_request = BuildRequest { + let build_request = proto::BuildRequest { // Importantly, this must match the order of get_refscan_needles, since users may use that // function to map back from the found needles to a store path refscan_needles: get_refscan_needles(derivation) @@ -141,10 +142,10 @@ pub(crate) fn derivation_to_build_request( .collect(), }; + // FUTUREWORK: switch this function to construct the stricter BuildRequest directly. debug_assert!( - build_request.validate().is_ok(), - "invalid BuildRequest: {}", - build_request.validate().unwrap_err() + BuildRequest::try_from(build_request.clone()).is_ok(), + "Tvix bug: BuildRequest would not be valid" ); Ok(build_request) |