From 9fd15ba506c86cd4c1d84aa43d5f3767e02761db Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Mon, 15 Jan 2024 14:25:11 +0200 Subject: refactor(tvix/glue): have derivation_to_build_request consume inputs Determining the inputs might trigger additional builds/substitutions, so answering these lookups via a lambda in a lazy fashion gets complicated. You end up assembling the list of input nodes upfront, and the lambda will just be a dumb lookup into that preassembled list. Rather than doing that, simply have derivation_to_build_request leave the work of determining the inputs to the caller. Change-Id: I75880132916c76b930807c989090da298b6891bd Reviewed-on: https://cl.tvl.fyi/c/depot/+/10626 Reviewed-by: raitobezarius Autosubmit: flokli Tested-by: BuildkiteCI --- tvix/glue/src/tvix_build.rs | 72 +++++++++------------------------------------ 1 file changed, 14 insertions(+), 58 deletions(-) (limited to 'tvix/glue/src/tvix_build.rs') diff --git a/tvix/glue/src/tvix_build.rs b/tvix/glue/src/tvix_build.rs index e8dab1056807..72e84c0c71a2 100644 --- a/tvix/glue/src/tvix_build.rs +++ b/tvix/glue/src/tvix_build.rs @@ -1,16 +1,16 @@ //! This module contains glue code translating from //! [nix_compat::derivation::Derivation] to [tvix_build::proto::BuildRequest]. -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; use bytes::Bytes; -use nix_compat::{derivation::Derivation, nixbase32, store_path::StorePathRef}; +use nix_compat::{derivation::Derivation, nixbase32}; use sha2::{Digest, Sha256}; use tvix_build::proto::{ build_request::{AdditionalFile, BuildConstraints, EnvVar}, BuildRequest, }; -use tvix_castore::proto::{self, node::Node, NamedNode}; +use tvix_castore::proto::{self, node::Node}; /// These are the environment variables that Nix sets in its sandbox for every /// build. @@ -37,15 +37,11 @@ const NIX_ENVIRONMENT_VARS: [(&str, &str); 12] = [ /// - one translating a tuple of drv path and (a subset of their) output names to /// castore nodes of the selected outpus (`fn_input_drvs_to_output_nodes`). #[allow(dead_code)] -pub(crate) fn derivation_to_build_request( +#[allow(clippy::mutable_key_type)] +pub(crate) fn derivation_to_build_request( derivation: &Derivation, - fn_input_sources_to_node: FIS, - fn_input_drvs_to_output_nodes: FID, -) -> std::io::Result -where - FIS: Fn(&StorePathRef) -> Node, - FID: Fn(&StorePathRef, &[&str]) -> Vec, -{ + inputs: BTreeSet, +) -> std::io::Result { debug_assert!(derivation.validate(true).is_ok(), "drv must validate"); // produce command_args, which is builder and arguments in a Vec. @@ -90,31 +86,6 @@ where // TODO: handle __json (structured attrs, provide JSON file and source-able bash script) - // Produce inputs. As we refer to the contents here, not just plain store path strings, - // we need to perform lookups. - // FUTUREWORK: should we also model input_derivations and input_sources with StorePath? - let mut inputs: Vec = Vec::new(); - - // populate inputs selected by input_sources. - for input_source in derivation.input_sources.iter() { - // since Derivation is validated, we know input sources can be parsed. - let sp = StorePathRef::from_absolute_path(input_source.as_bytes()) - .expect("invalid input source path"); - inputs.push(fn_input_sources_to_node(&sp)); - } - - // populate inputs selected by input_drv and (subset of) output names. - for (input_derivation, output_names) in derivation.input_derivations.iter() { - // since Derivation is validated, we know input derivations can be parsed. - let sp = StorePathRef::from_absolute_path(input_derivation.as_bytes()) - .expect("invalid input derivation path"); - let output_names: Vec<&str> = output_names.iter().map(|e| e.as_str()).collect(); - inputs.extend(fn_input_drvs_to_output_nodes(&sp, output_names.as_slice())); - } - - // sort inputs by their name - inputs.sort_by(|a, b| a.get_name().cmp(b.get_name())); - // Produce constraints. let constraints = Some(BuildConstraints { system: derivation.system.clone(), @@ -222,6 +193,8 @@ fn calculate_pass_as_file_env(k: &str) -> (String, String) { #[cfg(test)] mod test { + use std::collections::BTreeSet; + use bytes::Bytes; use nix_compat::derivation::Derivation; use tvix_build::proto::{ @@ -252,24 +225,9 @@ mod test { let derivation = Derivation::from_aterm_bytes(aterm_bytes).expect("must parse"); - let build_request = derivation_to_build_request( - &derivation, - |_| unreachable!(), - |input_drv, output_names| { - // expected to be called with ss2p4wmxijn652haqyd7dckxwl4c7hxx-bar.drv only - if input_drv.to_string() != "ss2p4wmxijn652haqyd7dckxwl4c7hxx-bar.drv" { - panic!("called with unexpected input_drv: {}", input_drv); - } - // expect to be called with ["out"] - if output_names != ["out"] { - panic!("called with unexpected output_names: {:?}", output_names); - } - - // all good, reply with INPUT_NODE_FOO - vec![INPUT_NODE_FOO.clone()] - }, - ) - .expect("must succeed"); + let build_request = + derivation_to_build_request(&derivation, BTreeSet::from([INPUT_NODE_FOO.clone()])) + .expect("must succeed"); let mut expected_environment_vars = vec![ EnvVar { @@ -332,8 +290,7 @@ mod test { let derivation = Derivation::from_aterm_bytes(aterm_bytes).expect("must parse"); let build_request = - derivation_to_build_request(&derivation, |_| unreachable!(), |_, _| unreachable!()) - .expect("must succeed"); + derivation_to_build_request(&derivation, BTreeSet::from([])).expect("must succeed"); let mut expected_environment_vars = vec![ EnvVar { @@ -403,8 +360,7 @@ mod test { let derivation = Derivation::from_aterm_bytes(aterm_bytes).expect("must parse"); let build_request = - derivation_to_build_request(&derivation, |_| unreachable!(), |_, _| unreachable!()) - .expect("must succeed"); + derivation_to_build_request(&derivation, BTreeSet::from([])).expect("must succeed"); let mut expected_environment_vars = vec![ // Note how bar and baz are not present in the env anymore, -- cgit 1.4.1