diff options
| -rw-r--r-- | tvix/build/src/buildservice/build_request.rs | 2 | ||||
| -rw-r--r-- | tvix/build/src/buildservice/dummy.rs | 5 | ||||
| -rw-r--r-- | tvix/build/src/buildservice/grpc.rs | 7 | ||||
| -rw-r--r-- | tvix/build/src/buildservice/mod.rs | 4 | ||||
| -rw-r--r-- | tvix/build/src/buildservice/oci.rs | 24 | ||||
| -rw-r--r-- | tvix/build/src/oci/bundle.rs | 47 | ||||
| -rw-r--r-- | tvix/build/src/oci/mod.rs | 7 | ||||
| -rw-r--r-- | tvix/build/src/oci/spec.rs | 80 | ||||
| -rw-r--r-- | tvix/build/src/proto/grpc_buildservice_wrapper.rs | 4 | ||||
| -rw-r--r-- | tvix/build/src/proto/mod.rs | 51 | ||||
| -rw-r--r-- | tvix/glue/src/tvix_build.rs | 158 | ||||
| -rw-r--r-- | tvix/glue/src/tvix_store_io.rs | 7 |
12 files changed, 221 insertions, 175 deletions
diff --git a/tvix/build/src/buildservice/build_request.rs b/tvix/build/src/buildservice/build_request.rs index 78409c34e3b2..4d53ee55096c 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(Debug, Clone, PartialEq)] +#[derive(Default, 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. diff --git a/tvix/build/src/buildservice/dummy.rs b/tvix/build/src/buildservice/dummy.rs index d20444755e73..3368201376ed 100644 --- a/tvix/build/src/buildservice/dummy.rs +++ b/tvix/build/src/buildservice/dummy.rs @@ -2,7 +2,8 @@ use tonic::async_trait; use tracing::instrument; use super::BuildService; -use crate::proto::{Build, BuildRequest}; +use crate::buildservice::BuildRequest; +use crate::proto; #[derive(Default)] pub struct DummyBuildService {} @@ -10,7 +11,7 @@ pub struct DummyBuildService {} #[async_trait] impl BuildService for DummyBuildService { #[instrument(skip(self), ret, err)] - async fn do_build(&self, _request: BuildRequest) -> std::io::Result<Build> { + async fn do_build(&self, _request: BuildRequest) -> std::io::Result<proto::Build> { Err(std::io::Error::new( std::io::ErrorKind::Other, "builds are not supported with DummyBuildService", diff --git a/tvix/build/src/buildservice/grpc.rs b/tvix/build/src/buildservice/grpc.rs index 9d22d8397abf..14f06f0ee3e6 100644 --- a/tvix/build/src/buildservice/grpc.rs +++ b/tvix/build/src/buildservice/grpc.rs @@ -1,6 +1,7 @@ use tonic::{async_trait, transport::Channel}; -use crate::proto::{build_service_client::BuildServiceClient, Build, BuildRequest}; +use crate::buildservice::BuildRequest; +use crate::proto::{self, build_service_client::BuildServiceClient}; use super::BuildService; @@ -17,10 +18,10 @@ impl GRPCBuildService { #[async_trait] impl BuildService for GRPCBuildService { - async fn do_build(&self, request: BuildRequest) -> std::io::Result<Build> { + async fn do_build(&self, request: BuildRequest) -> std::io::Result<proto::Build> { let mut client = self.client.clone(); client - .do_build(request) + .do_build(Into::<proto::BuildRequest>::into(request)) .await .map(|resp| resp.into_inner()) .map_err(std::io::Error::other) diff --git a/tvix/build/src/buildservice/mod.rs b/tvix/build/src/buildservice/mod.rs index c3c2fd6c5715..b12db6b95d13 100644 --- a/tvix/build/src/buildservice/mod.rs +++ b/tvix/build/src/buildservice/mod.rs @@ -1,6 +1,6 @@ use tonic::async_trait; -use crate::proto::{self, Build}; +use crate::proto; pub mod build_request; pub use crate::buildservice::build_request::*; @@ -17,5 +17,5 @@ pub use from_addr::from_addr; #[async_trait] pub trait BuildService: Send + Sync { /// TODO: document - async fn do_build(&self, request: proto::BuildRequest) -> std::io::Result<Build>; + async fn do_build(&self, request: BuildRequest) -> std::io::Result<proto::Build>; } diff --git a/tvix/build/src/buildservice/oci.rs b/tvix/build/src/buildservice/oci.rs index 89efbb4285d0..52efede6597b 100644 --- a/tvix/build/src/buildservice/oci.rs +++ b/tvix/build/src/buildservice/oci.rs @@ -10,15 +10,15 @@ use tvix_castore::{ fs::fuse::FuseDaemon, import::fs::ingest_path, refscan::{ReferencePattern, ReferenceScanner}, - Node, PathComponent, }; use uuid::Uuid; +use crate::buildservice::BuildRequest; use crate::{ oci::{get_host_output_paths, make_bundle, make_spec}, - proto::{build::OutputNeedles, Build, BuildRequest}, + proto::{self, build::OutputNeedles}, }; -use std::{collections::BTreeMap, ffi::OsStr, path::PathBuf, process::Stdio}; +use std::{ffi::OsStr, path::PathBuf, process::Stdio}; use super::BuildService; @@ -95,7 +95,7 @@ where DS: DirectoryService + Clone + 'static, { #[instrument(skip_all, err)] - async fn do_build(&self, request: BuildRequest) -> std::io::Result<Build> { + async fn do_build(&self, request: BuildRequest) -> std::io::Result<proto::Build> { let _permit = self.concurrent_builds.acquire().await.unwrap(); let bundle_name = Uuid::new_v4(); @@ -128,26 +128,20 @@ where .map_err(std::io::Error::other)?; // assemble a BTreeMap of Nodes to pass into TvixStoreFs. - let root_nodes: BTreeMap<PathComponent, Node> = - BTreeMap::from_iter(request.inputs.iter().map(|input| { - // We know from validation this is Some. - input.clone().try_into_name_and_node().unwrap() - })); let patterns = ReferencePattern::new(request.refscan_needles.clone()); // NOTE: impl Drop for FuseDaemon unmounts, so if the call is cancelled, umount. let _fuse_daemon = tokio::task::spawn_blocking({ let blob_service = self.blob_service.clone(); let directory_service = self.directory_service.clone(); - debug!(inputs=?root_nodes.keys(), "got inputs"); - let dest = bundle_path.join("inputs"); + let root_nodes = Box::new(request.inputs.clone()); move || { let fs = tvix_castore::fs::TvixStoreFs::new( blob_service, directory_service, - Box::new(root_nodes), + root_nodes, true, false, ); @@ -223,7 +217,7 @@ where Ok::<_, std::io::Error>(( tvix_castore::proto::Node::from_name_and_node( - PathBuf::from(output_path) + output_path .file_name() .and_then(|s| s.to_str()) .map(|s| s.to_string()) @@ -240,8 +234,8 @@ where .into_iter() .unzip(); - Ok(Build { - build_request: Some(request.clone()), + Ok(proto::Build { + build_request: Some(request.into()), outputs, outputs_needles, }) diff --git a/tvix/build/src/oci/bundle.rs b/tvix/build/src/oci/bundle.rs index c3c2e83e89e5..52789362a58d 100644 --- a/tvix/build/src/oci/bundle.rs +++ b/tvix/build/src/oci/bundle.rs @@ -5,7 +5,7 @@ use std::{ }; use super::scratch_name; -use crate::proto::BuildRequest; +use crate::buildservice::BuildRequest; use anyhow::{bail, Context}; use tracing::{debug, instrument}; @@ -60,12 +60,10 @@ pub(crate) fn get_host_output_paths( for output_path in request.outputs.iter() { // calculate the location of the path. - if let Some((mp, relpath)) = - find_path_in_scratchs(output_path, request.scratch_paths.as_slice()) - { + if let Some((mp, relpath)) = find_path_in_scratchs(output_path, &request.scratch_paths) { host_output_paths.push(scratch_root.join(scratch_name(mp)).join(relpath)); } else { - bail!("unable to find path {}", output_path); + bail!("unable to find path {output_path:?}"); } } @@ -77,16 +75,18 @@ pub(crate) fn get_host_output_paths( /// relative path from there to the search_path. /// mountpoints must be sorted, so we can iterate over the list from the back /// and match on the prefix. -fn find_path_in_scratchs<'a, 'b>( - search_path: &'a str, - mountpoints: &'b [String], -) -> Option<(&'b str, &'a str)> { - mountpoints.iter().rev().find_map(|mp| { - Some(( - mp.as_str(), - search_path.strip_prefix(mp)?.strip_prefix('/')?, - )) - }) +fn find_path_in_scratchs<'a, 'b, I>( + search_path: &'a Path, + mountpoints: I, +) -> Option<(&'b Path, &'a Path)> +where + I: IntoIterator<Item = &'b PathBuf>, + I::IntoIter: DoubleEndedIterator, +{ + mountpoints + .into_iter() + .rev() + .find_map(|mp| Some((mp.as_path(), search_path.strip_prefix(mp).ok()?))) } #[cfg(test)] @@ -95,7 +95,7 @@ mod tests { use rstest::rstest; - use crate::{oci::scratch_name, proto::BuildRequest}; + use crate::{buildservice::BuildRequest, oci::scratch_name}; use super::{find_path_in_scratchs, get_host_output_paths}; @@ -108,7 +108,18 @@ mod tests { #[case] mountpoints: &[String], #[case] expected: Option<(&str, &str)>, ) { - assert_eq!(find_path_in_scratchs(search_path, mountpoints), expected); + let expected = expected.map(|e| (Path::new(e.0), Path::new(e.1))); + assert_eq!( + find_path_in_scratchs( + Path::new(search_path), + mountpoints + .iter() + .map(PathBuf::from) + .collect::<Vec<_>>() + .as_slice() + ), + expected + ); } #[test] @@ -125,7 +136,7 @@ mod tests { let mut expected_path = PathBuf::new(); expected_path.push("bundle-root"); expected_path.push("scratch"); - expected_path.push(scratch_name("nix/store")); + expected_path.push(scratch_name(Path::new("nix/store"))); expected_path.push("fhaj6gmwns62s6ypkcldbaj2ybvkhx3p-foo"); assert_eq!(vec![expected_path], paths) diff --git a/tvix/build/src/oci/mod.rs b/tvix/build/src/oci/mod.rs index 26dab3059a58..a2400c4a6eba 100644 --- a/tvix/build/src/oci/mod.rs +++ b/tvix/build/src/oci/mod.rs @@ -5,9 +5,12 @@ pub(crate) use bundle::get_host_output_paths; pub(crate) use bundle::make_bundle; pub(crate) use spec::make_spec; +use std::path::Path; + /// For a given scratch path, return the scratch_name that's allocated. // We currently use use lower hex encoding of the b3 digest of the scratch // path, so we don't need to globally allocate and pass down some uuids. -pub(crate) fn scratch_name(scratch_path: &str) -> String { - data_encoding::BASE32.encode(blake3::hash(scratch_path.as_bytes()).as_bytes()) +pub(crate) fn scratch_name(scratch_path: &Path) -> String { + data_encoding::BASE32 + .encode(blake3::hash(scratch_path.as_os_str().as_encoded_bytes()).as_bytes()) } diff --git a/tvix/build/src/oci/spec.rs b/tvix/build/src/oci/spec.rs index e80e442f0350..e3b6172def8a 100644 --- a/tvix/build/src/oci/spec.rs +++ b/tvix/build/src/oci/spec.rs @@ -1,11 +1,10 @@ //! Module to create a OCI runtime spec for a given [BuildRequest]. -use crate::proto::BuildRequest; +use crate::buildservice::{BuildConstraints, BuildRequest}; use oci_spec::{ runtime::{Capability, LinuxNamespace, LinuxNamespaceBuilder, LinuxNamespaceType}, OciSpecError, }; use std::{collections::HashSet, path::Path}; -use tvix_castore::proto as castorepb; use super::scratch_name; @@ -35,33 +34,26 @@ pub(crate) fn make_spec( rootless: bool, sandbox_shell: &str, ) -> Result<oci_spec::runtime::Spec, oci_spec::OciSpecError> { - // TODO: add BuildRequest validations. BuildRequest must contain strings as inputs - let allow_network = request .constraints - .as_ref() - .is_some_and(|c| c.network_access); + .contains(&BuildConstraints::NetworkAccess); // Assemble ro_host_mounts. Start with constraints.available_ro_paths. - let mut ro_host_mounts = request + let mut ro_host_mounts: Vec<_> = request .constraints - .as_ref() - .map(|constraints| { - constraints - .available_ro_paths - .iter() - .map(|e| (e.as_str(), e.as_str())) - .collect::<Vec<_>>() + .iter() + .filter_map(|constraint| match constraint { + BuildConstraints::AvailableReadOnlyPath(path) => Some((path.as_path(), path.as_path())), + _ => None, }) - .unwrap_or_default(); + .collect(); // If provide_bin_sh is set, mount sandbox_shell to /bin/sh if request .constraints - .as_ref() - .is_some_and(|c| c.provide_bin_sh) + .contains(&BuildConstraints::ProvideBinSh) { - ro_host_mounts.push((sandbox_shell, "/bin/sh")) + ro_host_mounts.push((Path::new(sandbox_shell), Path::new("/bin/sh"))) } oci_spec::runtime::SpecBuilder::default() @@ -92,9 +84,9 @@ pub(crate) fn make_spec( .mounts(configure_mounts( rootless, allow_network, - request.scratch_paths.iter().map(|e| e.as_str()), + request.scratch_paths.iter().map(|e| e.as_path()), request.inputs.iter(), - &request.inputs_dir, // TODO: validate + &request.inputs_dir, ro_host_mounts, )?) .build() @@ -106,7 +98,7 @@ pub(crate) fn make_spec( /// Capabilities are a bit more complicated in case rootless building is requested. fn configure_process<'a>( command_args: &[String], - cwd: &String, + cwd: &Path, env: impl IntoIterator<Item = (&'a str, String)>, rootless: bool, ) -> Result<oci_spec::runtime::Process, oci_spec::OciSpecError> { @@ -232,10 +224,11 @@ fn configure_linux( fn configure_mounts<'a>( rootless: bool, allow_network: bool, - scratch_paths: impl IntoIterator<Item = &'a str>, - inputs: impl Iterator<Item = &'a castorepb::Node>, - inputs_dir: &str, - ro_host_mounts: impl IntoIterator<Item = (&'a str, &'a str)>, + scratch_paths: impl IntoIterator<Item = &'a Path>, + inputs: impl Iterator<Item = (&'a tvix_castore::PathComponent, &'a tvix_castore::Node)>, + + inputs_dir: &Path, + ro_host_mounts: impl IntoIterator<Item = (&'a Path, &'a Path)>, ) -> Result<Vec<oci_spec::runtime::Mount>, oci_spec::OciSpecError> { let mut mounts: Vec<_> = if rootless { oci_spec::runtime::get_rootless_mounts() @@ -244,8 +237,8 @@ fn configure_mounts<'a>( }; mounts.push(configure_mount( - "tmpfs", - "/tmp", + Path::new("tmpfs"), + Path::new("/tmp"), "tmpfs", &["nosuid", "noatime", "mode=700"], )?); @@ -255,28 +248,19 @@ fn configure_mounts<'a>( for scratch_path in scratch_paths.into_iter() { let src = scratch_root.join(scratch_name(scratch_path)); mounts.push(configure_mount( - src.to_str().unwrap(), - Path::new("/").join(scratch_path).to_str().unwrap(), + &src, + &Path::new("/").join(scratch_path), "none", &["rbind", "rw"], )?); } // For each input, create a bind mount from inputs/$name into $inputs_dir/$name. - for input in inputs { - let (input_name, _input) = input - .clone() - .try_into_name_and_node() - .expect("invalid input name"); - + for (input_name, _input) in inputs { let input_name = std::str::from_utf8(input_name.as_ref()).expect("invalid input name"); mounts.push(configure_mount( - Path::new("inputs").join(input_name).to_str().unwrap(), - Path::new("/") - .join(inputs_dir) - .join(input_name) - .to_str() - .unwrap(), + &Path::new("inputs").join(input_name), + &Path::new("/").join(inputs_dir).join(input_name), "none", &[ "rbind", "ro", @@ -295,7 +279,11 @@ fn configure_mounts<'a>( // In case network is enabled, also mount in /etc/{resolv.conf,services,hosts} if allow_network { - for p in ["/etc/resolv.conf", "/etc/services", "/etc/hosts"] { + for p in [ + Path::new("/etc/resolv.conf"), + Path::new("/etc/services"), + Path::new("/etc/hosts"), + ] { mounts.push(configure_mount(p, p, "none", &["rbind", "ro"])?); } } @@ -305,15 +293,15 @@ fn configure_mounts<'a>( /// Helper function to produce a mount. fn configure_mount( - source: &str, - destination: &str, + source: &Path, + destination: &Path, typ: &str, options: &[&str], ) -> Result<oci_spec::runtime::Mount, oci_spec::OciSpecError> { oci_spec::runtime::MountBuilder::default() - .destination(destination.to_string()) + .destination(destination) .typ(typ.to_string()) - .source(source.to_string()) + .source(source) .options(options.iter().map(|e| e.to_string()).collect::<Vec<_>>()) .build() } diff --git a/tvix/build/src/proto/grpc_buildservice_wrapper.rs b/tvix/build/src/proto/grpc_buildservice_wrapper.rs index 024f075de9ad..8a2d36ac569f 100644 --- a/tvix/build/src/proto/grpc_buildservice_wrapper.rs +++ b/tvix/build/src/proto/grpc_buildservice_wrapper.rs @@ -27,7 +27,9 @@ where &self, request: tonic::Request<BuildRequest>, ) -> Result<tonic::Response<Build>, tonic::Status> { - match self.inner.do_build(request.into_inner()).await { + let request = TryInto::<crate::buildservice::BuildRequest>::try_into(request.into_inner()) + .map_err(|err| tonic::Status::new(tonic::Code::InvalidArgument, err.to_string()))?; + match self.inner.do_build(request).await { Ok(resp) => Ok(tonic::Response::new(resp)), Err(e) => Err(tonic::Status::internal(e.to_string())), } diff --git a/tvix/build/src/proto/mod.rs b/tvix/build/src/proto/mod.rs index 0e106bb4cf63..be757bb56272 100644 --- a/tvix/build/src/proto/mod.rs +++ b/tvix/build/src/proto/mod.rs @@ -118,6 +118,57 @@ where data.tuple_windows().all(|(a, b)| a <= b) } +fn path_to_string(path: &Path) -> String { + path.to_str() + .expect("Tvix Bug: unable to convert Path to String") + .to_string() +} + +impl From<crate::buildservice::BuildRequest> for BuildRequest { + fn from(value: crate::buildservice::BuildRequest) -> Self { + let constraints = if value.constraints.is_empty() { + None + } else { + let mut constraints = build_request::BuildConstraints::default(); + for constraint in value.constraints { + use crate::buildservice::BuildConstraints; + match constraint { + BuildConstraints::System(system) => constraints.system = system, + BuildConstraints::MinMemory(min_memory) => constraints.min_memory = min_memory, + BuildConstraints::AvailableReadOnlyPath(path) => { + constraints.available_ro_paths.push(path_to_string(&path)) + } + BuildConstraints::ProvideBinSh => constraints.provide_bin_sh = true, + BuildConstraints::NetworkAccess => constraints.network_access = true, + } + } + Some(constraints) + }; + Self { + inputs: value + .inputs + .into_iter() + .map(|(name, node)| { + tvix_castore::proto::Node::from_name_and_node(name.into(), node) + }) + .collect(), + command_args: value.command_args, + working_dir: path_to_string(&value.working_dir), + scratch_paths: value + .scratch_paths + .iter() + .map(|p| path_to_string(p)) + .collect(), + inputs_dir: path_to_string(&value.inputs_dir), + outputs: value.outputs.iter().map(|p| path_to_string(p)).collect(), + environment_vars: value.environment_vars.into_iter().map(Into::into).collect(), + constraints, + additional_files: value.additional_files.into_iter().map(Into::into).collect(), + refscan_needles: value.refscan_needles, + } + } +} + impl TryFrom<BuildRequest> for crate::buildservice::BuildRequest { type Error = ValidateBuildRequestError; fn try_from(value: BuildRequest) -> Result<Self, Self::Error> { diff --git a/tvix/glue/src/tvix_build.rs b/tvix/glue/src/tvix_build.rs index 6a3c6ab40ccc..fa73224992e6 100644 --- a/tvix/glue/src/tvix_build.rs +++ b/tvix/glue/src/tvix_build.rs @@ -1,16 +1,13 @@ //! This module contains glue code translating from -//! [nix_compat::derivation::Derivation] to [tvix_build::proto::BuildRequest]. +//! [nix_compat::derivation::Derivation] to [tvix_build::buildservice::BuildRequest]. -use std::collections::BTreeMap; +use std::collections::{BTreeMap, HashSet}; +use std::path::PathBuf; 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}, -}; +use tvix_build::buildservice::{AdditionalFile, BuildConstraints, BuildRequest, EnvVar}; use tvix_castore::Node; /// These are the environment variables that Nix sets in its sandbox for every @@ -31,7 +28,7 @@ const NIX_ENVIRONMENT_VARS: [(&str, &str); 12] = [ ]; /// Get an iterator of store paths whose nixbase32 hashes will be the needles for refscanning -/// Importantly, the returned order will match the one used by derivation_to_build_request +/// Importantly, the returned order will match the one used by [derivation_to_build_request] /// so users may use this function to map back from the found needles to a store path pub(crate) fn get_refscan_needles( derivation: &Derivation, @@ -44,7 +41,7 @@ pub(crate) fn get_refscan_needles( .chain(derivation.input_derivations.keys()) } -/// Takes a [Derivation] and turns it into a [proto::BuildRequest]. +/// Takes a [Derivation] and turns it into a [buildservice::BuildRequest]. /// It assumes the Derivation has been validated. /// It needs two lookup functions: /// - one translating input sources to a castore node @@ -53,8 +50,8 @@ pub(crate) fn get_refscan_needles( /// castore nodes of the selected outpus (`fn_input_drvs_to_output_nodes`). pub(crate) fn derivation_to_build_request( derivation: &Derivation, - inputs: BTreeMap<bytes::Bytes, Node>, -) -> std::io::Result<proto::BuildRequest> { + inputs: BTreeMap<StorePath<String>, Node>, +) -> std::io::Result<BuildRequest> { debug_assert!(derivation.validate(true).is_ok(), "drv must validate"); // produce command_args, which is builder and arguments in a Vec. @@ -62,16 +59,6 @@ pub(crate) fn derivation_to_build_request( command_args.push(derivation.builder.clone()); command_args.extend_from_slice(&derivation.arguments); - // produce output_paths, which is the absolute path of each output (sorted) - let mut output_paths: Vec<String> = derivation - .outputs - .values() - .map(|e| e.path_str()[1..].to_owned()) - .collect(); - - // Sort the outputs. We can use sort_unstable, as these are unique strings. - output_paths.sort_unstable(); - // Produce environment_vars and additional files. // We use a BTreeMap while producing, and only realize the resulting Vec // while populating BuildRequest, so we don't need to worry about ordering. @@ -100,28 +87,41 @@ pub(crate) fn derivation_to_build_request( // TODO: handle __json (structured attrs, provide JSON file and source-able bash script) // Produce constraints. - let constraints = Some(BuildConstraints { - system: derivation.system.clone(), - min_memory: 0, - available_ro_paths: vec![], - // in case this is a fixed-output derivation, allow network access. - network_access: derivation.outputs.len() == 1 - && derivation - .outputs - .get("out") - .expect("invalid derivation") - .is_fixed(), - provide_bin_sh: true, - }); + let mut constraints = HashSet::from([ + BuildConstraints::System(derivation.system.clone()), + BuildConstraints::ProvideBinSh, + ]); + + if derivation.outputs.len() == 1 + && derivation + .outputs + .get("out") + .expect("Tvix bug: Derivation has no out output") + .is_fixed() + { + constraints.insert(BuildConstraints::NetworkAccess); + } - let build_request = proto::BuildRequest { + Ok(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) .map(|path| nixbase32::encode(path.digest())) .collect(), command_args, - outputs: output_paths, + + outputs: { + // produce output_paths, which is the absolute path of each output (sorted) + let mut output_paths: Vec<PathBuf> = derivation + .outputs + .values() + .map(|e| PathBuf::from(e.path_str()[1..].to_owned())) + .collect(); + + // Sort the outputs. We can use sort_unstable, as these are unique strings. + output_paths.sort_unstable(); + output_paths + }, // Turn this into a sorted-by-key Vec<EnvVar>. environment_vars: environment_vars @@ -130,7 +130,15 @@ pub(crate) fn derivation_to_build_request( .collect(), inputs: inputs .into_iter() - .map(|(name, node)| tvix_castore::proto::Node::from_name_and_node(name, node)) + .map(|(path, node)| { + ( + path.to_string() + .as_str() + .try_into() + .expect("Tvix bug: unable to convert store path basename to PathComponent"), + node, + ) + }) .collect(), inputs_dir: nix_compat::store_path::STORE_DIR[1..].into(), constraints, @@ -138,17 +146,12 @@ pub(crate) fn derivation_to_build_request( scratch_paths: vec!["build".into(), "nix/store".into()], additional_files: additional_files .into_iter() - .map(|(path, contents)| AdditionalFile { path, contents }) + .map(|(path, contents)| AdditionalFile { + path: PathBuf::from(path), + contents, + }) .collect(), - }; - - // FUTUREWORK: switch this function to construct the stricter BuildRequest directly. - debug_assert!( - BuildRequest::try_from(build_request.clone()).is_ok(), - "Tvix bug: BuildRequest would not be valid" - ); - - Ok(build_request) + }) } /// handle passAsFile, if set. @@ -212,15 +215,13 @@ fn calculate_pass_as_file_env(k: &str) -> (String, String) { #[cfg(test)] mod test { use bytes::Bytes; - use nix_compat::derivation::Derivation; - use std::collections::BTreeMap; + use nix_compat::{derivation::Derivation, store_path::StorePath}; + use std::collections::{BTreeMap, HashSet}; use std::sync::LazyLock; - use tvix_build::proto::{ - build_request::{AdditionalFile, BuildConstraints, EnvVar}, - BuildRequest, - }; use tvix_castore::fixtures::DUMMY_DIGEST; - use tvix_castore::Node; + use tvix_castore::{Node, PathComponent}; + + use tvix_build::buildservice::{AdditionalFile, BuildConstraints, BuildRequest, EnvVar}; use crate::tvix_build::NIX_ENVIRONMENT_VARS; @@ -242,7 +243,10 @@ mod test { let build_request = derivation_to_build_request( &derivation, - BTreeMap::from([(INPUT_NODE_FOO_NAME.clone(), INPUT_NODE_FOO.clone())]), + BTreeMap::from([( + StorePath::<String>::from_bytes(&INPUT_NODE_FOO_NAME.clone()).unwrap(), + INPUT_NODE_FOO.clone(), + )]), ) .expect("must succeed"); @@ -281,18 +285,15 @@ mod test { command_args: vec![":".into()], outputs: vec!["nix/store/fhaj6gmwns62s6ypkcldbaj2ybvkhx3p-foo".into()], environment_vars: expected_environment_vars, - inputs: vec![tvix_castore::proto::Node::from_name_and_node( - INPUT_NODE_FOO_NAME.clone(), + inputs: BTreeMap::from([( + PathComponent::try_from(INPUT_NODE_FOO_NAME.clone()).unwrap(), INPUT_NODE_FOO.clone() - )], + )]), inputs_dir: "nix/store".into(), - constraints: Some(BuildConstraints { - system: derivation.system.clone(), - min_memory: 0, - network_access: false, - available_ro_paths: vec![], - provide_bin_sh: true, - }), + constraints: HashSet::from([ + BuildConstraints::System(derivation.system.clone()), + BuildConstraints::ProvideBinSh + ]), additional_files: vec![], working_dir: "build".into(), scratch_paths: vec!["build".into(), "nix/store".into()], @@ -357,15 +358,13 @@ mod test { command_args: vec![":".to_string()], outputs: vec!["nix/store/4q0pg5zpfmznxscq3avycvf9xdvx50n3-bar".into()], environment_vars: expected_environment_vars, - inputs: vec![], + inputs: BTreeMap::new(), inputs_dir: "nix/store".into(), - constraints: Some(BuildConstraints { - system: derivation.system.clone(), - min_memory: 0, - network_access: true, - available_ro_paths: vec![], - provide_bin_sh: true, - }), + constraints: HashSet::from([ + BuildConstraints::System(derivation.system.clone()), + BuildConstraints::NetworkAccess, + BuildConstraints::ProvideBinSh + ]), additional_files: vec![], working_dir: "build".into(), scratch_paths: vec!["build".into(), "nix/store".into()], @@ -431,15 +430,12 @@ mod test { command_args: vec![":".to_string()], outputs: vec!["nix/store/pp17lwra2jkx8rha15qabg2q3wij72lj-foo".into()], environment_vars: expected_environment_vars, - inputs: vec![], + inputs: BTreeMap::new(), inputs_dir: "nix/store".into(), - constraints: Some(BuildConstraints { - system: derivation.system.clone(), - min_memory: 0, - network_access: false, - available_ro_paths: vec![], - provide_bin_sh: true, - }), + constraints: HashSet::from([ + BuildConstraints::System(derivation.system.clone()), + BuildConstraints::ProvideBinSh, + ]), additional_files: vec![ // baz env AdditionalFile { diff --git a/tvix/glue/src/tvix_store_io.rs b/tvix/glue/src/tvix_store_io.rs index c1fe53aa873f..67a88e13c54b 100644 --- a/tvix/glue/src/tvix_store_io.rs +++ b/tvix/glue/src/tvix_store_io.rs @@ -1,5 +1,4 @@ //! This module provides an implementation of EvalIO talking to tvix-store. -use bytes::Bytes; use futures::{StreamExt, TryStreamExt}; use nix_compat::{nixhash::CAHash, store_path::StorePath}; use std::collections::BTreeMap; @@ -181,7 +180,7 @@ impl TvixStoreIO { // derivation_to_build_request needs castore nodes for all inputs. // Provide them, which means, here is where we recursively build // all dependencies. - let mut inputs: BTreeMap<Bytes, Node> = + let mut inputs: BTreeMap<StorePath<String>, Node> = futures::stream::iter(drv.input_derivations.iter()) .map(|(input_drv_path, output_names)| { // look up the derivation object @@ -224,7 +223,7 @@ impl TvixStoreIO { .await?; if let Some(node) = node { - Ok((output_path.to_string().into(), node)) + Ok((output_path, node)) } else { Err(io::Error::other("no node produced")) } @@ -250,7 +249,7 @@ impl TvixStoreIO { .store_path_to_node(&input_source, Path::new("")) .await?; if let Some(node) = node { - Ok((input_source.to_string().into(), node)) + Ok((input_source, node)) } else { Err(io::Error::other("no node produced")) } |