about summary refs log tree commit diff
diff options
context:
space:
mode:
authorFlorian Klink <flokli@flokli.de>2023-03-16T22·54+0100
committerclbot <clbot@tvl.fyi>2023-03-17T07·49+0000
commitb025ebb2a1ae989afc5ee3ae69cb920e97a4efdc (patch)
tree4cd74f7486140d82c86976d2a594fe49d6f04038
parent985f842e32852f0e1a110fbfdf64be654e9ced09 (diff)
refactor(tvix/store/nar): pass in &proto::node::Node r/6019
Passing in a &proto::node::Node into all this allows us consumers to
keep ownership of the proto::node::Node.

Change-Id: I44882a86c46826b06a8a8a0b24c18adfc7052662
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8316
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: flokli <flokli@flokli.de>
-rw-r--r--tvix/store/src/nar/mod.rs2
-rw-r--r--tvix/store/src/nar/non_caching_calculation_service.rs2
-rw-r--r--tvix/store/src/nar/renderer.rs34
-rw-r--r--tvix/store/src/proto/grpc_pathinfoservice_wrapper.rs2
-rw-r--r--tvix/store/src/tests/nar_renderer.rs10
5 files changed, 29 insertions, 21 deletions
diff --git a/tvix/store/src/nar/mod.rs b/tvix/store/src/nar/mod.rs
index d9e0c8143b..d6a2f0889a 100644
--- a/tvix/store/src/nar/mod.rs
+++ b/tvix/store/src/nar/mod.rs
@@ -31,6 +31,6 @@ pub enum RenderError {
 pub trait NARCalculationService {
     fn calculate_nar(
         &self,
-        root_node: proto::node::Node,
+        root_node: &proto::node::Node,
     ) -> Result<proto::CalculateNarResponse, RenderError>;
 }
diff --git a/tvix/store/src/nar/non_caching_calculation_service.rs b/tvix/store/src/nar/non_caching_calculation_service.rs
index 880d126369..f77f0b30d6 100644
--- a/tvix/store/src/nar/non_caching_calculation_service.rs
+++ b/tvix/store/src/nar/non_caching_calculation_service.rs
@@ -35,7 +35,7 @@ impl<BS: BlobService, CS: ChunkService + Clone, DS: DirectoryService> NARCalcula
 {
     fn calculate_nar(
         &self,
-        root_node: proto::node::Node,
+        root_node: &proto::node::Node,
     ) -> Result<proto::CalculateNarResponse, RenderError> {
         let h = Sha256::new();
         let mut cw = CountWrite::from(h);
diff --git a/tvix/store/src/nar/renderer.rs b/tvix/store/src/nar/renderer.rs
index f0708ab739..335f336434 100644
--- a/tvix/store/src/nar/renderer.rs
+++ b/tvix/store/src/nar/renderer.rs
@@ -36,7 +36,7 @@ impl<BS: BlobService, CS: ChunkService + Clone, DS: DirectoryService> NARRendere
     pub fn write_nar<W: std::io::Write>(
         &self,
         w: &mut W,
-        proto_root_node: proto::node::Node,
+        proto_root_node: &proto::node::Node,
     ) -> Result<(), RenderError> {
         // Initialize NAR writer
         let nar_root_node = nar::writer::open(w).map_err(RenderError::NARWriterError)?;
@@ -49,7 +49,7 @@ impl<BS: BlobService, CS: ChunkService + Clone, DS: DirectoryService> NARRendere
     fn walk_node(
         &self,
         nar_node: nar::writer::Node,
-        proto_node: proto::node::Node,
+        proto_node: &proto::node::Node,
     ) -> Result<(), RenderError> {
         match proto_node {
             proto::node::Node::Symlink(proto_symlink_node) => {
@@ -59,7 +59,7 @@ impl<BS: BlobService, CS: ChunkService + Clone, DS: DirectoryService> NARRendere
             }
             proto::node::Node::File(proto_file_node) => {
                 // get the digest we're referring to
-                let digest = proto_file_node.digest;
+                let digest = &proto_file_node.digest;
                 // query blob_service for blob_meta
                 let resp = self
                     .blob_service
@@ -73,15 +73,18 @@ impl<BS: BlobService, CS: ChunkService + Clone, DS: DirectoryService> NARRendere
                 match resp {
                     // if it's None, that's an error!
                     None => {
-                        return Err(RenderError::BlobNotFound(digest, proto_file_node.name));
+                        return Err(RenderError::BlobNotFound(
+                            digest.to_vec(),
+                            proto_file_node.name.to_owned(),
+                        ));
                     }
                     Some(blob_meta) => {
                         // make sure the blob_meta size matches what we expect from proto_file_node
                         let blob_meta_size = blob_meta.chunks.iter().fold(0, |acc, e| acc + e.size);
                         if blob_meta_size != proto_file_node.size {
                             return Err(RenderError::UnexpectedBlobMeta(
-                                digest,
-                                proto_file_node.name,
+                                digest.to_vec(),
+                                proto_file_node.name.to_owned(),
                                 proto_file_node.size,
                                 blob_meta_size,
                             ));
@@ -103,11 +106,16 @@ impl<BS: BlobService, CS: ChunkService + Clone, DS: DirectoryService> NARRendere
             }
             proto::node::Node::Directory(proto_directory_node) => {
                 // get the digest we're referring to
-                let digest: [u8; 32] = proto_directory_node.digest.try_into().map_err(|_e| {
-                    RenderError::StoreError(crate::Error::StorageError(
-                        "invalid digest len in directory node".to_string(),
-                    ))
-                })?;
+                let digest: [u8; 32] =
+                    proto_directory_node
+                        .digest
+                        .to_owned()
+                        .try_into()
+                        .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
@@ -120,7 +128,7 @@ impl<BS: BlobService, CS: ChunkService + Clone, DS: DirectoryService> NARRendere
                     None => {
                         return Err(RenderError::DirectoryNotFound(
                             digest.to_vec(),
-                            proto_directory_node.name,
+                            proto_directory_node.name.to_owned(),
                         ))
                     }
                     Some(proto_directory) => {
@@ -134,7 +142,7 @@ impl<BS: BlobService, CS: ChunkService + Clone, DS: DirectoryService> NARRendere
                             let child_node = nar_node_directory
                                 .entry(proto_node.get_name())
                                 .map_err(RenderError::NARWriterError)?;
-                            self.walk_node(child_node, proto_node)?;
+                            self.walk_node(child_node, &proto_node)?;
                         }
 
                         // close the directory
diff --git a/tvix/store/src/proto/grpc_pathinfoservice_wrapper.rs b/tvix/store/src/proto/grpc_pathinfoservice_wrapper.rs
index c7df6b643a..21a65185de 100644
--- a/tvix/store/src/proto/grpc_pathinfoservice_wrapper.rs
+++ b/tvix/store/src/proto/grpc_pathinfoservice_wrapper.rs
@@ -64,7 +64,7 @@ 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) {
+            Some(root_node) => match self.nar_calculation_service.calculate_nar(&root_node) {
                 Ok(resp) => Ok(Response::new(resp)),
                 Err(e) => Err(e.into()),
             },
diff --git a/tvix/store/src/tests/nar_renderer.rs b/tvix/store/src/tests/nar_renderer.rs
index 056cfaf5fb..2d612d9c27 100644
--- a/tvix/store/src/tests/nar_renderer.rs
+++ b/tvix/store/src/tests/nar_renderer.rs
@@ -23,7 +23,7 @@ fn single_symlink() {
     renderer
         .write_nar(
             &mut buf,
-            crate::proto::node::Node::Symlink(SymlinkNode {
+            &crate::proto::node::Node::Symlink(SymlinkNode {
                 name: "doesntmatter".to_string(),
                 target: "/nix/store/somewhereelse".to_string(),
             }),
@@ -47,7 +47,7 @@ fn single_file_missing_blob() {
     let e = renderer
         .write_nar(
             &mut buf,
-            crate::proto::node::Node::File(FileNode {
+            &crate::proto::node::Node::File(FileNode {
                 name: "doesntmatter".to_string(),
                 digest: HELLOWORLD_BLOB_DIGEST.to_vec(),
                 size: HELLOWORLD_BLOB_CONTENTS.len() as u32,
@@ -94,7 +94,7 @@ fn single_file_wrong_blob_size() {
     let e = renderer
         .write_nar(
             &mut buf,
-            crate::proto::node::Node::File(FileNode {
+            &crate::proto::node::Node::File(FileNode {
                 name: "doesntmatter".to_string(),
                 digest: HELLOWORLD_BLOB_DIGEST.to_vec(),
                 size: 42, // <- note the wrong size here!
@@ -151,7 +151,7 @@ fn single_file() {
     renderer
         .write_nar(
             &mut buf,
-            crate::proto::node::Node::File(FileNode {
+            &crate::proto::node::Node::File(FileNode {
                 name: "doesntmatter".to_string(),
                 digest: HELLOWORLD_BLOB_DIGEST.to_vec(),
                 size: HELLOWORLD_BLOB_CONTENTS.len() as u32,
@@ -196,7 +196,7 @@ fn test_complicated() {
     renderer
         .write_nar(
             &mut buf,
-            crate::proto::node::Node::Directory(DirectoryNode {
+            &crate::proto::node::Node::Directory(DirectoryNode {
                 name: "doesntmatter".to_string(),
                 digest: DIRECTORY_COMPLICATED.digest().to_vec(),
                 size: DIRECTORY_COMPLICATED.size(),