refactor(tvix/castore): add into_nodes(), implement consuming proto conv r/8507

Provide a into_nodes() function on a Directory, which consumes self and
returns owned PathComponent and Node.

Use it to provide a proper conversion from Directory to the proto
variant that doesn't clone.

There's no need for the one taking only &Directory, we don't use it
anywhere, and once someone needs that they might as well clone Directory
before converting it.

Update all other users of the `.nodes()` function to use `.into_nodes()`
where applicable, and avoid some more cloning there.

Change-Id: Id4577b9eb173c012e225337458898d3937112bcb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12218
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: Connor Brewster <cbrewster@hey.com>

5 files changed, 24 insertions, 29 deletions

diff --git a/tvix/castore/src/directoryservice/traverse.rs b/tvix/castore/src/directoryservice/traverse.rs
index 63b99ba4ea5e..0bd67e9bcf1f 100644
--- a/tvix/castore/src/directoryservice/traverse.rs
+++ b/tvix/castore/src/directoryservice/traverse.rs
@@ -36,7 +36,7 @@ where
 
                 // look for the component in the [Directory].
                 if let Some((_child_name, child_node)) = directory
-                    .nodes()
+                    .into_nodes()
                     .find(|(name, _node)| name.as_ref() == component)
                 {
                     // child node found, update prev_node to that and continue.
diff --git a/tvix/castore/src/fs/mod.rs b/tvix/castore/src/fs/mod.rs
index d196266ab438..e700a25d3966 100644
--- a/tvix/castore/src/fs/mod.rs
+++ b/tvix/castore/src/fs/mod.rs
@@ -201,12 +201,12 @@ where
                     let mut inode_tracker = self.inode_tracker.write();
 
                     let children: Vec<(u64, PathComponent, Node)> = directory
-                        .nodes()
+                        .into_nodes()
                         .map(|(child_name, child_node)| {
-                            let inode_data = InodeData::from_node(child_node);
+                            let inode_data = InodeData::from_node(&child_node);
 
                             let child_ino = inode_tracker.put(inode_data);
-                            (child_ino, child_name.to_owned(), child_node.clone())
+                            (child_ino, child_name, child_node)
                         })
                         .collect();
 
diff --git a/tvix/castore/src/nodes/directory.rs b/tvix/castore/src/nodes/directory.rs
index 03db30b52408..a11fbf314bd0 100644
--- a/tvix/castore/src/nodes/directory.rs
+++ b/tvix/castore/src/nodes/directory.rs
@@ -50,6 +50,12 @@ impl Directory {
         self.nodes.iter()
     }
 
+    /// Dissolves a Directory into its individual names and nodes.
+    /// The elements are sorted by their names.
+    pub fn into_nodes(self) -> impl Iterator<Item = (PathComponent, Node)> + Send + Sync {
+        self.nodes.into_iter()
+    }
+
     /// Adds the specified [Node] to the [Directory] with a given name.
     ///
     /// Inserting an element that already exists with the same name in the directory will yield an
diff --git a/tvix/castore/src/proto/mod.rs b/tvix/castore/src/proto/mod.rs
index f32f3a0687a7..a533c1c4f195 100644
--- a/tvix/castore/src/proto/mod.rs
+++ b/tvix/castore/src/proto/mod.rs
@@ -142,44 +142,38 @@ impl TryFrom<&Directory> for crate::Directory {
     }
 }
 
-// TODO: add a proper owned version here that moves various fields
 impl From<crate::Directory> for Directory {
     fn from(value: crate::Directory) -> Self {
-        (&value).into()
-    }
-}
-
-impl From<&crate::Directory> for Directory {
-    fn from(directory: &crate::Directory) -> Directory {
         let mut directories = vec![];
         let mut files = vec![];
         let mut symlinks = vec![];
 
-        for (name, node) in directory.nodes() {
+        for (name, node) in value.into_nodes() {
             match node {
                 crate::Node::File {
                     digest,
                     size,
                     executable,
                 } => files.push(FileNode {
-                    name: name.to_owned().into(),
-                    digest: digest.to_owned().into(),
-                    size: *size,
-                    executable: *executable,
+                    name: name.into(),
+                    digest: digest.into(),
+                    size,
+                    executable,
                 }),
                 crate::Node::Directory { digest, size } => directories.push(DirectoryNode {
-                    name: name.to_owned().into(),
-                    digest: digest.to_owned().into(),
-                    size: *size,
+                    name: name.into(),
+                    digest: digest.into(),
+                    size,
                 }),
                 crate::Node::Symlink { target } => {
                     symlinks.push(SymlinkNode {
-                        name: name.to_owned().into(),
-                        target: target.to_owned().into(),
+                        name: name.into(),
+                        target: target.into(),
                     });
                 }
             }
         }
+
         Directory {
             directories,
             files,
diff --git a/tvix/glue/src/tvix_store_io.rs b/tvix/glue/src/tvix_store_io.rs
index 0833fc45e132..b18e6633731d 100644
--- a/tvix/glue/src/tvix_store_io.rs
+++ b/tvix/glue/src/tvix_store_io.rs
@@ -561,16 +561,11 @@ impl EvalIO for TvixStoreIO {
                             async move { self.directory_service.as_ref().get(&digest).await }
                         })? {
                             let mut children: Vec<(bytes::Bytes, FileType)> = Vec::new();
-                            // TODO: into_nodes() to avoid cloning
-                            for (name, node) in directory.nodes() {
+                            for (name, node) in directory.into_nodes() {
                                 children.push(match node {
-                                    Node::Directory { .. } => {
-                                        (name.clone().into(), FileType::Directory)
-                                    }
+                                    Node::Directory { .. } => (name.into(), FileType::Directory),
                                     Node::File { .. } => (name.clone().into(), FileType::Regular),
-                                    Node::Symlink { .. } => {
-                                        (name.clone().into(), FileType::Symlink)
-                                    }
+                                    Node::Symlink { .. } => (name.into(), FileType::Symlink),
                                 })
                             }
                             Ok(children)