refactor(tvix/store): use bytes for node names and symlink targets r/6436

Some paths might use names that are not valid UTF-8. We should be able
to represent them.

We don't actually need to touch the PathInfo structures, as they need to
represent StorePaths, which come with their own harder restrictions,
which can't encode non-UTF8 data.

While this doesn't change any of the wire format of the gRPC messages,
it does however change the interface of tvix_eval::EvalIO - its
read_dir() method does now return a list of Vec<u8>, rather than
SmolStr. Maybe this should be OsString instead?

Change-Id: I821016d9a58ec441ee081b0b9f01c9240723af0b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8974
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI

4 files changed, 23 insertions, 23 deletions

diff --git a/tvix/store/src/fuse/inode_tracker.rs b/tvix/store/src/fuse/inode_tracker.rs
index 06434d25b3..8d91564712 100644
--- a/tvix/store/src/fuse/inode_tracker.rs
+++ b/tvix/store/src/fuse/inode_tracker.rs
@@ -13,7 +13,7 @@ pub struct InodeTracker {
     blob_digest_to_inode: HashMap<B3Digest, u64>,
 
     // lookup table for symlinks by their target
-    symlink_target_to_inode: HashMap<String, u64>,
+    symlink_target_to_inode: HashMap<Vec<u8>, u64>,
 
     // lookup table for directories by their B3Digest.
     // Note the corresponding directory may not be present in data yet.
@@ -171,7 +171,7 @@ impl InodeTracker {
                 self.blob_digest_to_inode.insert(digest.clone(), ino);
             }
             InodeData::Symlink(ref target) => {
-                self.symlink_target_to_inode.insert(target.to_string(), ino);
+                self.symlink_target_to_inode.insert(target.to_vec(), ino);
             }
             InodeData::Directory(DirectoryInodeData::Sparse(ref digest, _size)) => {
                 self.directory_digest_to_inode.insert(digest.clone(), ino);
@@ -251,7 +251,7 @@ mod tests {
     #[test]
     fn put_symlink() {
         let mut inode_tracker = InodeTracker::default();
-        let f = InodeData::Symlink("target".to_string());
+        let f = InodeData::Symlink("target".into());
 
         // put it in
         let ino = inode_tracker.put(f.clone());
@@ -260,7 +260,7 @@ mod tests {
         let data = inode_tracker.get(ino).expect("must be some");
         match *data {
             InodeData::Symlink(ref target) => {
-                assert_eq!("target", target);
+                assert_eq!(b"target".to_vec(), *target);
             }
             InodeData::Regular(..) | InodeData::Directory(..) => panic!("wrong type"),
         }
@@ -269,10 +269,7 @@ mod tests {
         assert_eq!(ino, inode_tracker.put(f));
 
         // inserting another file should return a different ino
-        assert_ne!(
-            ino,
-            inode_tracker.put(InodeData::Symlink("target2".to_string()))
-        );
+        assert_ne!(ino, inode_tracker.put(InodeData::Symlink("target2".into())));
     }
 
     // TODO: put sparse directory
diff --git a/tvix/store/src/fuse/inodes.rs b/tvix/store/src/fuse/inodes.rs
index 78756472be..bf883cb22f 100644
--- a/tvix/store/src/fuse/inodes.rs
+++ b/tvix/store/src/fuse/inodes.rs
@@ -5,7 +5,7 @@ use crate::{proto, B3Digest};
 #[derive(Clone, Debug)]
 pub enum InodeData {
     Regular(B3Digest, u32, bool),  // digest, size, executable
-    Symlink(String),               // target
+    Symlink(Vec<u8>),              // target
     Directory(DirectoryInodeData), // either [DirectoryInodeData:Sparse] or [DirectoryInodeData:Populated]
 }
 
diff --git a/tvix/store/src/fuse/mod.rs b/tvix/store/src/fuse/mod.rs
index dd2c479b3d..077c6ce048 100644
--- a/tvix/store/src/fuse/mod.rs
+++ b/tvix/store/src/fuse/mod.rs
@@ -19,6 +19,7 @@ use crate::{
 use fuser::{FileAttr, ReplyAttr, Request};
 use nix_compat::store_path::StorePath;
 use std::io::{self, Read, Seek};
+use std::os::unix::ffi::OsStrExt;
 use std::str::FromStr;
 use std::sync::Arc;
 use std::{collections::HashMap, time::Duration};
@@ -142,7 +143,7 @@ impl FUSE {
                     let root_node = path_info.node.unwrap().node.unwrap();
 
                     // The name must match what's passed in the lookup, otherwise we return nothing.
-                    if root_node.get_name() != store_path.to_string() {
+                    if root_node.get_name() != store_path.to_string().as_bytes() {
                         return Ok(None);
                     }
 
@@ -279,7 +280,9 @@ impl fuser::Filesystem for FUSE {
             let _enter = span.enter();
 
             // in the children, find the one with the desired name.
-            if let Some((child_ino, _)) = children.iter().find(|e| e.1.get_name() == name) {
+            if let Some((child_ino, _)) =
+                children.iter().find(|e| e.1.get_name() == name.as_bytes())
+            {
                 // lookup the child [InodeData] in [self.inode_tracker].
                 // We know the inodes for children have already been allocated.
                 let child_inode_data = self.inode_tracker.get(*child_ino).unwrap();
@@ -353,7 +356,7 @@ impl fuser::Filesystem for FUSE {
                         Node::File(_) => fuser::FileType::RegularFile,
                         Node::Symlink(_) => fuser::FileType::Symlink,
                     },
-                    child_node.get_name(),
+                    std::ffi::OsStr::from_bytes(child_node.get_name()),
                 );
                 if full {
                     break;
@@ -480,7 +483,7 @@ impl fuser::Filesystem for FUSE {
             InodeData::Directory(..) | InodeData::Regular(..) => {
                 reply.error(libc::EINVAL);
             }
-            InodeData::Symlink(ref target) => reply.data(target.as_bytes()),
+            InodeData::Symlink(ref target) => reply.data(target),
         }
     }
 }
diff --git a/tvix/store/src/fuse/tests.rs b/tvix/store/src/fuse/tests.rs
index 1a53b9f5dd..8577e062e9 100644
--- a/tvix/store/src/fuse/tests.rs
+++ b/tvix/store/src/fuse/tests.rs
@@ -57,7 +57,7 @@ fn populate_blob_a(
     let path_info = PathInfo {
         node: Some(proto::Node {
             node: Some(proto::node::Node::File(FileNode {
-                name: BLOB_A_NAME.to_string(),
+                name: BLOB_A_NAME.into(),
                 digest: fixtures::BLOB_A_DIGEST.to_vec(),
                 size: fixtures::BLOB_A.len() as u32,
                 executable: false,
@@ -83,7 +83,7 @@ fn populate_blob_b(
     let path_info = PathInfo {
         node: Some(proto::Node {
             node: Some(proto::node::Node::File(FileNode {
-                name: BLOB_B_NAME.to_string(),
+                name: BLOB_B_NAME.into(),
                 digest: fixtures::BLOB_B_DIGEST.to_vec(),
                 size: fixtures::BLOB_B.len() as u32,
                 executable: false,
@@ -103,8 +103,8 @@ fn populate_symlink(
     let path_info = PathInfo {
         node: Some(proto::Node {
             node: Some(proto::node::Node::Symlink(proto::SymlinkNode {
-                name: SYMLINK_NAME.to_string(),
-                target: BLOB_A_NAME.to_string(),
+                name: SYMLINK_NAME.into(),
+                target: BLOB_A_NAME.into(),
             })),
         }),
         ..Default::default()
@@ -123,8 +123,8 @@ fn populate_symlink2(
     let path_info = PathInfo {
         node: Some(proto::Node {
             node: Some(proto::node::Node::Symlink(proto::SymlinkNode {
-                name: SYMLINK_NAME2.to_string(),
-                target: "/nix/store/somewhereelse".to_string(),
+                name: SYMLINK_NAME2.into(),
+                target: "/nix/store/somewhereelse".into(),
             })),
         }),
         ..Default::default()
@@ -153,7 +153,7 @@ fn populate_directory_with_keep(
     let path_info = PathInfo {
         node: Some(proto::Node {
             node: Some(proto::node::Node::Directory(DirectoryNode {
-                name: DIRECTORY_WITH_KEEP_NAME.to_string(),
+                name: DIRECTORY_WITH_KEEP_NAME.into(),
                 digest: fixtures::DIRECTORY_WITH_KEEP.digest().to_vec(),
                 size: fixtures::DIRECTORY_WITH_KEEP.size(),
             })),
@@ -174,7 +174,7 @@ fn populate_pathinfo_without_directory(
     let path_info = PathInfo {
         node: Some(proto::Node {
             node: Some(proto::node::Node::Directory(DirectoryNode {
-                name: DIRECTORY_WITH_KEEP_NAME.to_string(),
+                name: DIRECTORY_WITH_KEEP_NAME.into(),
                 digest: fixtures::DIRECTORY_WITH_KEEP.digest().to_vec(),
                 size: fixtures::DIRECTORY_WITH_KEEP.size(),
             })),
@@ -194,7 +194,7 @@ fn populate_blob_a_without_blob(
     let path_info = PathInfo {
         node: Some(proto::Node {
             node: Some(proto::node::Node::File(FileNode {
-                name: BLOB_A_NAME.to_string(),
+                name: BLOB_A_NAME.into(),
                 digest: fixtures::BLOB_A_DIGEST.to_vec(),
                 size: fixtures::BLOB_A.len() as u32,
                 executable: false,
@@ -231,7 +231,7 @@ fn populate_directory_complicated(
     let path_info = PathInfo {
         node: Some(proto::Node {
             node: Some(proto::node::Node::Directory(DirectoryNode {
-                name: DIRECTORY_COMPLICATED_NAME.to_string(),
+                name: DIRECTORY_COMPLICATED_NAME.into(),
                 digest: fixtures::DIRECTORY_COMPLICATED.digest().to_vec(),
                 size: fixtures::DIRECTORY_COMPLICATED.size(),
             })),