diff options
author | Florian Klink <flokli@flokli.de> | 2023-06-08T20·00+0300 |
---|---|---|
committer | clbot <clbot@tvl.fyi> | 2023-06-12T10·15+0000 |
commit | 8d05c0ceaa9bddb7fdaab436730f093eb16374a2 (patch) | |
tree | 33e034ae01b219d413247f1f93ea383133bd8062 /tvix | |
parent | 27ff98000b0cdf0ed30eb8837c7d44cd3e79d32f (diff) |
refactor(tvix/src/nar): drop NARCalculationService r/6270
There's only one way to calculate NAR files, by walking through them. Things like caching such replies should be done closer to where we use these, composing NARCalculationService doesn't actually give us much. Instead, expose two functions, `nar::calculate_size_and_sha256` and `nar::writer_nar`, the latter writing NAR to a writer, the former using write_nar to only keeping the NAR size and digest. Change-Id: Ie5d2cfea35470fdbb5cbf9da1136b0cdf0250266 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8723 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: flokli <flokli@flokli.de>
Diffstat (limited to 'tvix')
-rw-r--r-- | tvix/cli/src/main.rs | 13 | ||||
-rw-r--r-- | tvix/store/src/bin/tvix-store.rs | 19 | ||||
-rw-r--r-- | tvix/store/src/nar/grpc_nar_calculation_service.rs | 69 | ||||
-rw-r--r-- | tvix/store/src/nar/mod.rs | 15 | ||||
-rw-r--r-- | tvix/store/src/nar/non_caching_calculation_service.rs | 34 | ||||
-rw-r--r-- | tvix/store/src/nar/renderer.rs | 217 | ||||
-rw-r--r-- | tvix/store/src/pathinfoservice/mod.rs | 2 | ||||
-rw-r--r-- | tvix/store/src/proto/grpc_pathinfoservice_wrapper.rs | 42 | ||||
-rw-r--r-- | tvix/store/src/proto/tests/grpc_pathinfoservice.rs | 4 | ||||
-rw-r--r-- | tvix/store/src/store_io.rs | 25 | ||||
-rw-r--r-- | tvix/store/src/tests/nar_renderer.rs | 164 |
11 files changed, 258 insertions, 346 deletions
diff --git a/tvix/cli/src/main.rs b/tvix/cli/src/main.rs index 459177717c5a..0a1794006de2 100644 --- a/tvix/cli/src/main.rs +++ b/tvix/cli/src/main.rs @@ -73,20 +73,11 @@ fn interpret(code: &str, path: Option<PathBuf>, args: &Args, explain: bool) -> b let blob_service = MemoryBlobService::default(); let directory_service = MemoryDirectoryService::default(); - let path_info_service = MemoryPathInfoService::default(); - let nar_calculation_service = tvix_store::nar::NonCachingNARCalculationService::new( - Box::new(blob_service.clone()), - directory_service.clone(), - ); + let path_info_service = MemoryPathInfoService::default(); // TODO: update to pass in blob and directory svc eval.io_handle = Box::new(tvix_io::TvixIO::new( known_paths.clone(), - tvix_store::TvixStoreIO::new( - Box::new(blob_service), - directory_service, - path_info_service, - nar_calculation_service, - ), + tvix_store::TvixStoreIO::new(Box::new(blob_service), directory_service, path_info_service), )); // bundle fetchurl.nix (used in nixpkgs) by resolving <nix> to diff --git a/tvix/store/src/bin/tvix-store.rs b/tvix/store/src/bin/tvix-store.rs index 8fe62e5b5b2e..49c8c9ec34eb 100644 --- a/tvix/store/src/bin/tvix-store.rs +++ b/tvix/store/src/bin/tvix-store.rs @@ -11,8 +11,6 @@ use tvix_store::blobservice::GRPCBlobService; use tvix_store::blobservice::SledBlobService; use tvix_store::directoryservice::GRPCDirectoryService; use tvix_store::directoryservice::SledDirectoryService; -use tvix_store::nar::GRPCNARCalculationService; -use tvix_store::nar::NonCachingNARCalculationService; use tvix_store::pathinfoservice::GRPCPathInfoService; use tvix_store::pathinfoservice::SledPathInfoService; use tvix_store::proto::blob_service_client::BlobServiceClient; @@ -102,6 +100,8 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> { Commands::Daemon { listen_address } => { // initialize stores let blob_service = SledBlobService::new("blobs.sled".into())?; + let boxed_blob_service: Box<dyn BlobService> = Box::new(blob_service.clone()); + let boxed_blob_service2: Box<dyn BlobService> = Box::new(blob_service); let directory_service = SledDirectoryService::new("directories.sled".into())?; let path_info_service = SledPathInfoService::new("pathinfo.sled".into())?; @@ -112,22 +112,18 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> { let mut server = Server::builder(); - let nar_calculation_service = NonCachingNARCalculationService::new( - Box::new(blob_service.clone()), - directory_service.clone(), - ); - #[allow(unused_mut)] let mut router = server .add_service(BlobServiceServer::new(GRPCBlobServiceWrapper::from( - Box::new(blob_service) as Box<dyn BlobService>, + boxed_blob_service, ))) .add_service(DirectoryServiceServer::new( - GRPCDirectoryServiceWrapper::from(directory_service), + GRPCDirectoryServiceWrapper::from(directory_service.clone()), )) .add_service(PathInfoServiceServer::new(GRPCPathInfoServiceWrapper::new( path_info_service, - nar_calculation_service, + boxed_blob_service2, + directory_service, ))); #[cfg(feature = "reflection")] @@ -153,14 +149,11 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> { PathInfoServiceClient::connect("http://[::1]:8000").await?; let path_info_service = GRPCPathInfoService::from_client(path_info_service_client.clone()); - let nar_calculation_service = - GRPCNARCalculationService::from_client(path_info_service_client); let io = Arc::new(TvixStoreIO::new( Box::new(blob_service), directory_service, path_info_service, - nar_calculation_service, )); let tasks = paths diff --git a/tvix/store/src/nar/grpc_nar_calculation_service.rs b/tvix/store/src/nar/grpc_nar_calculation_service.rs deleted file mode 100644 index 429593743914..000000000000 --- a/tvix/store/src/nar/grpc_nar_calculation_service.rs +++ /dev/null @@ -1,69 +0,0 @@ -use super::NARCalculationService; -use crate::proto; -use tonic::transport::Channel; -use tonic::Status; - -/// A NAR calculation service which asks a remote tvix-store for NAR calculation -/// (via the gRPC PathInfoService). -#[derive(Clone)] -pub struct GRPCNARCalculationService { - /// A handle into the active tokio runtime. Necessary to spawn tasks. - tokio_handle: tokio::runtime::Handle, - - /// The internal reference to a gRPC client. - /// Cloning it is cheap, and it internally handles concurrent requests. - grpc_client: proto::path_info_service_client::PathInfoServiceClient<Channel>, -} - -impl GRPCNARCalculationService { - /// construct a new [GRPCNARCalculationService], by passing a handle to the - /// tokio runtime, and a gRPC client. - pub fn new( - tokio_handle: tokio::runtime::Handle, - grpc_client: proto::path_info_service_client::PathInfoServiceClient<Channel>, - ) -> Self { - Self { - tokio_handle, - grpc_client, - } - } - - /// construct a [GRPCNARCalculationService], from a [proto::path_info_service_client::PathInfoServiceClient<Channel>]. - /// panics if called outside the context of a tokio runtime. - pub fn from_client( - grpc_client: proto::path_info_service_client::PathInfoServiceClient<Channel>, - ) -> Self { - Self { - tokio_handle: tokio::runtime::Handle::current(), - grpc_client, - } - } -} - -impl NARCalculationService for GRPCNARCalculationService { - fn calculate_nar( - &self, - root_node: &proto::node::Node, - ) -> Result<(u64, [u8; 32]), super::RenderError> { - // Get a new handle to the gRPC client, and copy the root node. - let mut grpc_client = self.grpc_client.clone(); - let root_node = root_node.clone(); - - let task: tokio::task::JoinHandle<Result<_, Status>> = - self.tokio_handle.spawn(async move { - Ok(grpc_client - .calculate_nar(proto::Node { - node: Some(root_node), - }) - .await? - .into_inner()) - }); - - match self.tokio_handle.block_on(task).unwrap() { - Ok(resp) => Ok((resp.nar_size, resp.nar_sha256.to_vec().try_into().unwrap())), - Err(e) => Err(super::RenderError::StoreError(crate::Error::StorageError( - e.to_string(), - ))), - } - } -} diff --git a/tvix/store/src/nar/mod.rs b/tvix/store/src/nar/mod.rs index a29cc5451bae..c73e610f4ecb 100644 --- a/tvix/store/src/nar/mod.rs +++ b/tvix/store/src/nar/mod.rs @@ -1,14 +1,10 @@ -use crate::{proto, B3Digest}; +use crate::B3Digest; use data_encoding::BASE64; use thiserror::Error; -mod grpc_nar_calculation_service; -mod non_caching_calculation_service; mod renderer; - -pub use grpc_nar_calculation_service::GRPCNARCalculationService; -pub use non_caching_calculation_service::NonCachingNARCalculationService; -pub use renderer::NARRenderer; +pub use renderer::calculate_size_and_sha256; +pub use renderer::writer_nar; /// Errors that can encounter while rendering NARs. #[derive(Debug, Error)] @@ -28,8 +24,3 @@ pub enum RenderError { #[error("failure using the NAR writer: {0}")] NARWriterError(std::io::Error), } - -/// The base trait for something calculating NARs, and returning their size and sha256. -pub trait NARCalculationService { - fn calculate_nar(&self, root_node: &proto::node::Node) -> Result<(u64, [u8; 32]), RenderError>; -} diff --git a/tvix/store/src/nar/non_caching_calculation_service.rs b/tvix/store/src/nar/non_caching_calculation_service.rs deleted file mode 100644 index b743f264b0ff..000000000000 --- a/tvix/store/src/nar/non_caching_calculation_service.rs +++ /dev/null @@ -1,34 +0,0 @@ -use count_write::CountWrite; -use sha2::{Digest, Sha256}; - -use crate::blobservice::BlobService; -use crate::directoryservice::DirectoryService; -use crate::proto; - -use super::renderer::NARRenderer; -use super::{NARCalculationService, RenderError}; - -/// A NAR calculation service which simply renders the whole NAR whenever -/// we ask for the calculation. -pub struct NonCachingNARCalculationService<DS: DirectoryService> { - nar_renderer: NARRenderer<DS>, -} - -impl<DS: DirectoryService> NonCachingNARCalculationService<DS> { - pub fn new(blob_service: Box<dyn BlobService>, directory_service: DS) -> Self { - Self { - nar_renderer: NARRenderer::new(blob_service, directory_service), - } - } -} - -impl<DS: DirectoryService> NARCalculationService for NonCachingNARCalculationService<DS> { - fn calculate_nar(&self, root_node: &proto::node::Node) -> Result<(u64, [u8; 32]), RenderError> { - let h = Sha256::new(); - let mut cw = CountWrite::from(h); - - self.nar_renderer.write_nar(&mut cw, root_node)?; - - Ok((cw.count(), cw.into_inner().finalize().into())) - } -} diff --git a/tvix/store/src/nar/renderer.rs b/tvix/store/src/nar/renderer.rs index a9a6d989e1b8..6ea76e1429be 100644 --- a/tvix/store/src/nar/renderer.rs +++ b/tvix/store/src/nar/renderer.rs @@ -5,131 +5,140 @@ use crate::{ proto::{self, NamedNode}, B3Digest, }; +use count_write::CountWrite; use nix_compat::nar; +use sha2::{Digest, Sha256}; use std::io::{self, BufReader}; use tracing::warn; -/// A NAR renderer, using a blob_service, chunk_service and directory_service -/// to render a NAR to a writer. -pub struct NARRenderer<DS: DirectoryService> { - blob_service: Box<dyn BlobService>, +/// Invoke [render_nar], and return the size and sha256 digest of the produced +/// NAR output. +pub fn calculate_size_and_sha256<DS: DirectoryService + Clone>( + root_node: &proto::node::Node, + blob_service: &Box<dyn BlobService>, directory_service: DS, +) -> Result<(u64, [u8; 32]), RenderError> { + let h = Sha256::new(); + let mut cw = CountWrite::from(h); + + writer_nar(&mut cw, root_node, blob_service, directory_service)?; + + Ok((cw.count(), cw.into_inner().finalize().into())) } -impl<DS: DirectoryService> NARRenderer<DS> { - pub fn new(blob_service: Box<dyn BlobService>, directory_service: DS) -> Self { - Self { - blob_service, - directory_service, - } - } +/// Accepts a [proto::node::Node] pointing to the root of a (store) path, +/// and uses the passed blob_service and directory_service to +/// perform the necessary lookups as it traverses the structure. +/// The contents in NAR serialization are writen to the passed [std::io::Write]. +pub fn writer_nar<W: std::io::Write, DS: DirectoryService + Clone>( + w: &mut W, + proto_root_node: &proto::node::Node, + blob_service: &Box<dyn BlobService>, + directory_service: DS, +) -> Result<(), RenderError> { + // Initialize NAR writer + let nar_root_node = nar::writer::open(w).map_err(RenderError::NARWriterError)?; - /// Consumes a [proto::node::Node] pointing to the root of a (store) path, - /// and writes the contents in NAR serialization to the passed - /// [std::io::Write]. - /// - /// It uses the different clients in the struct to perform the necessary - /// lookups as it traverses the structure. - pub fn write_nar<W: std::io::Write>( - &self, - w: &mut W, - proto_root_node: &proto::node::Node, - ) -> Result<(), RenderError> { - // Initialize NAR writer - let nar_root_node = nar::writer::open(w).map_err(RenderError::NARWriterError)?; + walk_node( + nar_root_node, + proto_root_node, + blob_service, + directory_service, + ) +} - self.walk_node(nar_root_node, proto_root_node) - } +/// Process an intermediate node in the structure. +/// This consumes the node. +fn walk_node<DS: DirectoryService + Clone>( + nar_node: nar::writer::Node, + proto_node: &proto::node::Node, + blob_service: &Box<dyn BlobService>, + directory_service: DS, +) -> Result<(), RenderError> { + match proto_node { + proto::node::Node::Symlink(proto_symlink_node) => { + nar_node + .symlink(&proto_symlink_node.target) + .map_err(RenderError::NARWriterError)?; + } + proto::node::Node::File(proto_file_node) => { + let digest = B3Digest::from_vec(proto_file_node.digest.clone()).map_err(|_e| { + warn!( + file_node = ?proto_file_node, + "invalid digest length in file node", + ); - /// Process an intermediate node in the structure. - /// This consumes the node. - fn walk_node( - &self, - nar_node: nar::writer::Node, - proto_node: &proto::node::Node, - ) -> Result<(), RenderError> { - match proto_node { - proto::node::Node::Symlink(proto_symlink_node) => { - nar_node - .symlink(&proto_symlink_node.target) - .map_err(RenderError::NARWriterError)?; - } - proto::node::Node::File(proto_file_node) => { - let digest = B3Digest::from_vec(proto_file_node.digest.clone()).map_err(|_e| { - warn!( - file_node = ?proto_file_node, - "invalid digest length in file node", - ); + RenderError::StoreError(crate::Error::StorageError( + "invalid digest len in file node".to_string(), + )) + })?; + let mut blob_reader = match blob_service + .open_read(&digest) + .map_err(RenderError::StoreError)? + { + Some(blob_reader) => Ok(BufReader::new(blob_reader)), + None => Err(RenderError::NARWriterError(io::Error::new( + io::ErrorKind::NotFound, + format!("blob with digest {} not found", &digest), + ))), + }?; + + nar_node + .file( + proto_file_node.executable, + proto_file_node.size.into(), + &mut blob_reader, + ) + .map_err(RenderError::NARWriterError)?; + } + proto::node::Node::Directory(proto_directory_node) => { + let digest = + B3Digest::from_vec(proto_directory_node.digest.to_vec()).map_err(|_e| { RenderError::StoreError(crate::Error::StorageError( - "invalid digest len in file node".to_string(), + "invalid digest len in directory node".to_string(), )) })?; - let mut blob_reader = match self - .blob_service - .open_read(&digest) - .map_err(RenderError::StoreError)? - { - Some(blob_reader) => Ok(BufReader::new(blob_reader)), - None => Err(RenderError::NARWriterError(io::Error::new( - io::ErrorKind::NotFound, - format!("blob with digest {} not found", &digest), - ))), - }?; - - nar_node - .file( - proto_file_node.executable, - proto_file_node.size.into(), - &mut blob_reader, - ) - .map_err(RenderError::NARWriterError)?; - } - proto::node::Node::Directory(proto_directory_node) => { - let digest = - B3Digest::from_vec(proto_directory_node.digest.to_vec()).map_err(|_e| { - RenderError::StoreError(crate::Error::StorageError( - "invalid digest len in directory node".to_string(), - )) - })?; - - // look it up with the directory service - let resp = self - .directory_service - .get(&digest) - .map_err(RenderError::StoreError)?; + // look it up with the directory service + let resp = directory_service + .get(&digest) + .map_err(RenderError::StoreError)?; - match resp { - // if it's None, that's an error! - None => { - return Err(RenderError::DirectoryNotFound( - digest, - proto_directory_node.name.to_owned(), - )) - } - Some(proto_directory) => { - // start a directory node - let mut nar_node_directory = - nar_node.directory().map_err(RenderError::NARWriterError)?; - - // for each node in the directory, create a new entry with its name, - // and then invoke walk_node on that entry. - for proto_node in proto_directory.nodes() { - let child_node = nar_node_directory - .entry(proto_node.get_name()) - .map_err(RenderError::NARWriterError)?; - self.walk_node(child_node, &proto_node)?; - } + match resp { + // if it's None, that's an error! + None => { + return Err(RenderError::DirectoryNotFound( + digest, + proto_directory_node.name.to_owned(), + )) + } + Some(proto_directory) => { + // start a directory node + let mut nar_node_directory = + nar_node.directory().map_err(RenderError::NARWriterError)?; - // close the directory - nar_node_directory - .close() + // for each node in the directory, create a new entry with its name, + // and then invoke walk_node on that entry. + for proto_node in proto_directory.nodes() { + let child_node = nar_node_directory + .entry(proto_node.get_name()) .map_err(RenderError::NARWriterError)?; + walk_node( + child_node, + &proto_node, + blob_service, + directory_service.clone(), + )?; } + + // close the directory + nar_node_directory + .close() + .map_err(RenderError::NARWriterError)?; } } } - Ok(()) } + Ok(()) } diff --git a/tvix/store/src/pathinfoservice/mod.rs b/tvix/store/src/pathinfoservice/mod.rs index 6a34e09af478..ddede5851575 100644 --- a/tvix/store/src/pathinfoservice/mod.rs +++ b/tvix/store/src/pathinfoservice/mod.rs @@ -17,4 +17,6 @@ pub trait PathInfoService { /// Store a PathInfo message. Implementations MUST call validate and reject /// invalid messages. fn put(&self, path_info: proto::PathInfo) -> Result<proto::PathInfo, Error>; + + // TODO: add default impl for nar calculation here, and override from GRPC client! } diff --git a/tvix/store/src/proto/grpc_pathinfoservice_wrapper.rs b/tvix/store/src/proto/grpc_pathinfoservice_wrapper.rs index e82557b3a06c..c070b883fa16 100644 --- a/tvix/store/src/proto/grpc_pathinfoservice_wrapper.rs +++ b/tvix/store/src/proto/grpc_pathinfoservice_wrapper.rs @@ -1,19 +1,27 @@ -use crate::nar::RenderError; +use crate::blobservice::BlobService; +use crate::directoryservice::DirectoryService; +use crate::nar::{calculate_size_and_sha256, RenderError}; +use crate::pathinfoservice::PathInfoService; use crate::proto; -use crate::{nar::NARCalculationService, pathinfoservice::PathInfoService}; use tonic::{async_trait, Request, Response, Result, Status}; use tracing::{instrument, warn}; -pub struct GRPCPathInfoServiceWrapper<PS: PathInfoService, NS: NARCalculationService> { +pub struct GRPCPathInfoServiceWrapper<PS: PathInfoService, DS: DirectoryService> { path_info_service: PS, - nar_calculation_service: NS, + blob_service: Box<dyn BlobService>, + directory_service: DS, } -impl<PS: PathInfoService, NS: NARCalculationService> GRPCPathInfoServiceWrapper<PS, NS> { - pub fn new(path_info_service: PS, nar_calculation_service: NS) -> Self { +impl<PS: PathInfoService, DS: DirectoryService> GRPCPathInfoServiceWrapper<PS, DS> { + pub fn new( + path_info_service: PS, + blob_service: Box<dyn BlobService>, + directory_service: DS, + ) -> Self { Self { path_info_service, - nar_calculation_service, + blob_service, + directory_service, } } } @@ -21,8 +29,8 @@ impl<PS: PathInfoService, NS: NARCalculationService> GRPCPathInfoServiceWrapper< #[async_trait] impl< PS: PathInfoService + Send + Sync + 'static, - NS: NARCalculationService + Send + Sync + 'static, - > proto::path_info_service_server::PathInfoService for GRPCPathInfoServiceWrapper<PS, NS> + DS: DirectoryService + Send + Sync + Clone + 'static, + > proto::path_info_service_server::PathInfoService for GRPCPathInfoServiceWrapper<PS, DS> { #[instrument(skip(self))] async fn get( @@ -69,13 +77,19 @@ impl< ) -> Result<Response<proto::CalculateNarResponse>> { match request.into_inner().node { None => Err(Status::invalid_argument("no root node sent")), - Some(root_node) => match self.nar_calculation_service.calculate_nar(&root_node) { - Ok((nar_size, nar_sha256)) => Ok(Response::new(proto::CalculateNarResponse { + Some(root_node) => { + let (nar_size, nar_sha256) = calculate_size_and_sha256( + &root_node, + &self.blob_service, + self.directory_service.clone(), + ) + .expect("error during nar calculation"); // TODO: handle error + + Ok(Response::new(proto::CalculateNarResponse { nar_size, nar_sha256: nar_sha256.to_vec(), - })), - Err(e) => Err(e.into()), - }, + })) + } } } } diff --git a/tvix/store/src/proto/tests/grpc_pathinfoservice.rs b/tvix/store/src/proto/tests/grpc_pathinfoservice.rs index 11cab2c264cc..dbcdc5ced056 100644 --- a/tvix/store/src/proto/tests/grpc_pathinfoservice.rs +++ b/tvix/store/src/proto/tests/grpc_pathinfoservice.rs @@ -1,4 +1,3 @@ -use crate::nar::NonCachingNARCalculationService; use crate::proto::get_path_info_request::ByWhat::ByOutputHash; use crate::proto::node::Node::Symlink; use crate::proto::path_info_service_server::PathInfoService as GRPCPathInfoService; @@ -17,7 +16,8 @@ use tonic::Request; fn gen_grpc_service() -> impl GRPCPathInfoService { GRPCPathInfoServiceWrapper::new( gen_pathinfo_service(), - NonCachingNARCalculationService::new(gen_blob_service(), gen_directory_service()), + gen_blob_service(), + gen_directory_service(), ) } diff --git a/tvix/store/src/store_io.rs b/tvix/store/src/store_io.rs index a2967c06ff98..3820931e281f 100644 --- a/tvix/store/src/store_io.rs +++ b/tvix/store/src/store_io.rs @@ -16,7 +16,7 @@ use crate::{ blobservice::BlobService, directoryservice::{self, DirectoryService}, import, - nar::NARCalculationService, + nar::calculate_size_and_sha256, pathinfoservice::PathInfoService, proto::NamedNode, B3Digest, @@ -29,28 +29,23 @@ use crate::{ /// This is to both cover cases of syntactically valid store paths, that exist /// on the filesystem (still managed by Nix), as well as being able to read /// files outside store paths. -pub struct TvixStoreIO<DS: DirectoryService, PS: PathInfoService, NCS: NARCalculationService> { +pub struct TvixStoreIO<DS: DirectoryService, PS: PathInfoService> { blob_service: Box<dyn BlobService>, directory_service: DS, path_info_service: PS, - nar_calculation_service: NCS, std_io: StdIO, } -impl<DS: DirectoryService, PS: PathInfoService, NCS: NARCalculationService> - TvixStoreIO<DS, PS, NCS> -{ +impl<DS: DirectoryService + Clone, PS: PathInfoService> TvixStoreIO<DS, PS> { pub fn new( blob_service: Box<dyn BlobService>, directory_service: DS, path_info_service: PS, - nar_calculation_service: NCS, ) -> Self { Self { blob_service, directory_service, path_info_service, - nar_calculation_service, std_io: StdIO {}, } } @@ -109,10 +104,12 @@ impl<DS: DirectoryService, PS: PathInfoService, NCS: NARCalculationService> .expect("error during import_path"); // Render the NAR - let (nar_size, nar_sha256) = self - .nar_calculation_service - .calculate_nar(&root_node) - .expect("error during nar calculation"); // TODO: handle error + let (nar_size, nar_sha256) = calculate_size_and_sha256( + &root_node, + &self.blob_service, + self.directory_service.clone(), + ) + .expect("error during nar calculation"); // TODO: handle error // For given NAR sha256 digest and name, return the new [StorePath] this would have. let nar_hash_with_mode = @@ -178,9 +175,7 @@ fn calculate_nar_based_store_path(nar_sha256_digest: &[u8; 32], name: &str) -> S build_regular_ca_path(name, &nar_hash_with_mode, Vec::<String>::new(), false).unwrap() } -impl<DS: DirectoryService, PS: PathInfoService, NCS: NARCalculationService> EvalIO - for TvixStoreIO<DS, PS, NCS> -{ +impl<DS: DirectoryService + Clone, PS: PathInfoService> EvalIO for TvixStoreIO<DS, PS> { #[instrument(skip(self), ret, err)] fn path_exists(&self, path: &Path) -> Result<bool, io::Error> { if let Ok((store_path, sub_path)) = diff --git a/tvix/store/src/tests/nar_renderer.rs b/tvix/store/src/tests/nar_renderer.rs index 3d7cfd4a96a7..48c07e53c93e 100644 --- a/tvix/store/src/tests/nar_renderer.rs +++ b/tvix/store/src/tests/nar_renderer.rs @@ -1,28 +1,29 @@ use crate::directoryservice::DirectoryService; -use crate::nar::NARRenderer; +use crate::nar::calculate_size_and_sha256; +use crate::nar::writer_nar; use crate::proto::DirectoryNode; use crate::proto::FileNode; use crate::proto::SymlinkNode; use crate::tests::fixtures::*; use crate::tests::utils::*; +use sha2::{Digest, Sha256}; use std::io; #[test] fn single_symlink() { - let renderer = NARRenderer::new(gen_blob_service(), gen_directory_service()); - // don't put anything in the stores, as we don't actually do any requests. - let mut buf: Vec<u8> = vec![]; - renderer - .write_nar( - &mut buf, - &crate::proto::node::Node::Symlink(SymlinkNode { - name: "doesntmatter".to_string(), - target: "/nix/store/somewhereelse".to_string(), - }), - ) - .expect("must succeed"); + writer_nar( + &mut buf, + &crate::proto::node::Node::Symlink(SymlinkNode { + name: "doesntmatter".to_string(), + target: "/nix/store/somewhereelse".to_string(), + }), + // don't put anything in the stores, as we don't actually do any requests. + &gen_blob_service(), + gen_directory_service(), + ) + .expect("must succeed"); assert_eq!(buf, NAR_CONTENTS_SYMLINK.to_vec()); } @@ -30,20 +31,21 @@ fn single_symlink() { /// Make sure the NARRenderer fails if a referred blob doesn't exist. #[test] fn single_file_missing_blob() { - let renderer = NARRenderer::new(gen_blob_service(), gen_directory_service()); let mut buf: Vec<u8> = vec![]; - let e = renderer - .write_nar( - &mut buf, - &crate::proto::node::Node::File(FileNode { - name: "doesntmatter".to_string(), - digest: HELLOWORLD_BLOB_DIGEST.to_vec(), - size: HELLOWORLD_BLOB_CONTENTS.len() as u32, - executable: false, - }), - ) - .expect_err("must fail"); + let e = writer_nar( + &mut buf, + &crate::proto::node::Node::File(FileNode { + name: "doesntmatter".to_string(), + digest: HELLOWORLD_BLOB_DIGEST.to_vec(), + size: HELLOWORLD_BLOB_CONTENTS.len() as u32, + executable: false, + }), + // the blobservice is empty intentionally, to provoke the error. + &gen_blob_service(), + gen_directory_service(), + ) + .expect_err("must fail"); match e { crate::nar::RenderError::NARWriterError(e) => { @@ -68,22 +70,22 @@ fn single_file_wrong_blob_size() { .unwrap(); assert_eq!(HELLOWORLD_BLOB_DIGEST.clone(), writer.close().unwrap()); - let renderer = NARRenderer::new(blob_service, gen_directory_service()); - // Test with a root FileNode of a too big size { let mut buf: Vec<u8> = vec![]; - let e = renderer - .write_nar( - &mut buf, - &crate::proto::node::Node::File(FileNode { - name: "doesntmatter".to_string(), - digest: HELLOWORLD_BLOB_DIGEST.to_vec(), - size: 42, // <- note the wrong size here! - executable: false, - }), - ) - .expect_err("must fail"); + + let e = writer_nar( + &mut buf, + &crate::proto::node::Node::File(FileNode { + name: "doesntmatter".to_string(), + digest: HELLOWORLD_BLOB_DIGEST.to_vec(), + size: 42, // <- note the wrong size here! + executable: false, + }), + &blob_service, + gen_directory_service(), + ) + .expect_err("must fail"); match e { crate::nar::RenderError::NARWriterError(e) => { @@ -96,17 +98,19 @@ fn single_file_wrong_blob_size() { // Test with a root FileNode of a too small size { let mut buf: Vec<u8> = vec![]; - let e = renderer - .write_nar( - &mut buf, - &crate::proto::node::Node::File(FileNode { - name: "doesntmatter".to_string(), - digest: HELLOWORLD_BLOB_DIGEST.to_vec(), - size: 2, // <- note the wrong size here! - executable: false, - }), - ) - .expect_err("must fail"); + + let e = writer_nar( + &mut buf, + &crate::proto::node::Node::File(FileNode { + name: "doesntmatter".to_string(), + digest: HELLOWORLD_BLOB_DIGEST.to_vec(), + size: 2, // <- note the wrong size here! + executable: false, + }), + &blob_service, + gen_directory_service(), + ) + .expect_err("must fail"); match e { crate::nar::RenderError::NARWriterError(e) => { @@ -130,20 +134,20 @@ fn single_file() { .unwrap(); assert_eq!(HELLOWORLD_BLOB_DIGEST.clone(), writer.close().unwrap()); - let renderer = NARRenderer::new(blob_service, gen_directory_service()); let mut buf: Vec<u8> = vec![]; - renderer - .write_nar( - &mut buf, - &crate::proto::node::Node::File(FileNode { - name: "doesntmatter".to_string(), - digest: HELLOWORLD_BLOB_DIGEST.to_vec(), - size: HELLOWORLD_BLOB_CONTENTS.len() as u32, - executable: false, - }), - ) - .expect("must succeed"); + writer_nar( + &mut buf, + &crate::proto::node::Node::File(FileNode { + name: "doesntmatter".to_string(), + digest: HELLOWORLD_BLOB_DIGEST.to_vec(), + size: HELLOWORLD_BLOB_CONTENTS.len() as u32, + executable: false, + }), + &blob_service, + gen_directory_service(), + ) + .expect("must succeed"); assert_eq!(buf, NAR_CONTENTS_HELLOWORLD.to_vec()); } @@ -168,19 +172,35 @@ fn test_complicated() { .put(DIRECTORY_COMPLICATED.clone()) .unwrap(); - let renderer = NARRenderer::new(blob_service, directory_service); let mut buf: Vec<u8> = vec![]; - renderer - .write_nar( - &mut buf, - &crate::proto::node::Node::Directory(DirectoryNode { - name: "doesntmatter".to_string(), - digest: DIRECTORY_COMPLICATED.digest().to_vec(), - size: DIRECTORY_COMPLICATED.size(), - }), - ) - .expect("must succeed"); + writer_nar( + &mut buf, + &crate::proto::node::Node::Directory(DirectoryNode { + name: "doesntmatter".to_string(), + digest: DIRECTORY_COMPLICATED.digest().to_vec(), + size: DIRECTORY_COMPLICATED.size(), + }), + &blob_service, + directory_service.clone(), + ) + .expect("must succeed"); assert_eq!(buf, NAR_CONTENTS_COMPLICATED.to_vec()); + + // ensure calculate_nar does return the correct sha256 digest and sum. + let (nar_size, nar_digest) = calculate_size_and_sha256( + &crate::proto::node::Node::Directory(DirectoryNode { + name: "doesntmatter".to_string(), + digest: DIRECTORY_COMPLICATED.digest().to_vec(), + size: DIRECTORY_COMPLICATED.size(), + }), + &blob_service, + directory_service, + ) + .expect("must succeed"); + + assert_eq!(NAR_CONTENTS_COMPLICATED.len() as u64, nar_size); + let d = Sha256::digest(NAR_CONTENTS_COMPLICATED.clone()); + assert_eq!(d.as_slice(), nar_digest); } |