refactor(nix-compat/store_path): use Path in from_absolute_path_full r/8807

These are not necessarily strings, and making it paths allows us to stop
converting them to lossy strings.

Change-Id: I11366c721dc5da1778aafe89092a1966b5a43178
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12617
Reviewed-by: Ilan Joselevich <personal@ilanjoselevich.com>
Reviewed-by: Jörg Thalheim <joerg@thalheim.io>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI

2 files changed, 17 insertions, 32 deletions

diff --git a/tvix/glue/src/tvix_store_io.rs b/tvix/glue/src/tvix_store_io.rs
index a09580e9b9ff..7e2ab38d7b6b 100644
--- a/tvix/glue/src/tvix_store_io.rs
+++ b/tvix/glue/src/tvix_store_io.rs
@@ -449,9 +449,7 @@ impl TvixStoreIO {
 impl EvalIO for TvixStoreIO {
     #[instrument(skip(self), ret(level = Level::TRACE), err)]
     fn path_exists(&self, path: &Path) -> io::Result<bool> {
-        if let Ok((store_path, sub_path)) =
-            StorePath::from_absolute_path_full(&path.to_string_lossy())
-        {
+        if let Ok((store_path, sub_path)) = StorePath::from_absolute_path_full(path) {
             if self
                 .tokio_handle
                 .block_on(self.store_path_to_node(&store_path, sub_path))?
@@ -471,9 +469,7 @@ impl EvalIO for TvixStoreIO {
 
     #[instrument(skip(self), err)]
     fn open(&self, path: &Path) -> io::Result<Box<dyn io::Read>> {
-        if let Ok((store_path, sub_path)) =
-            StorePath::from_absolute_path_full(&path.to_string_lossy())
-        {
+        if let Ok((store_path, sub_path)) = StorePath::from_absolute_path_full(path) {
             if let Some(node) = self
                 .tokio_handle
                 .block_on(async { self.store_path_to_node(&store_path, sub_path).await })?
@@ -527,9 +523,7 @@ impl EvalIO for TvixStoreIO {
 
     #[instrument(skip(self), ret(level = Level::TRACE), err)]
     fn file_type(&self, path: &Path) -> io::Result<FileType> {
-        if let Ok((store_path, sub_path)) =
-            StorePath::from_absolute_path_full(&path.to_string_lossy())
-        {
+        if let Ok((store_path, sub_path)) = StorePath::from_absolute_path_full(path) {
             if let Some(node) = self
                 .tokio_handle
                 .block_on(async { self.store_path_to_node(&store_path, sub_path).await })?
@@ -549,9 +543,7 @@ impl EvalIO for TvixStoreIO {
 
     #[instrument(skip(self), ret(level = Level::TRACE), err)]
     fn read_dir(&self, path: &Path) -> io::Result<Vec<(bytes::Bytes, FileType)>> {
-        if let Ok((store_path, sub_path)) =
-            StorePath::from_absolute_path_full(&path.to_string_lossy())
-        {
+        if let Ok((store_path, sub_path)) = StorePath::from_absolute_path_full(path) {
             if let Some(node) = self
                 .tokio_handle
                 .block_on(async { self.store_path_to_node(&store_path, sub_path).await })?
diff --git a/tvix/nix-compat/src/store_path/mod.rs b/tvix/nix-compat/src/store_path/mod.rs
index 546c34d93615..56c10dc41417 100644
--- a/tvix/nix-compat/src/store_path/mod.rs
+++ b/tvix/nix-compat/src/store_path/mod.rs
@@ -9,9 +9,6 @@ use std::{
 };
 use thiserror;
 
-#[cfg(target_family = "unix")]
-use std::os::unix::ffi::OsStrExt;
-
 mod utils;
 
 pub use utils::*;
@@ -160,31 +157,27 @@ where
     /// Decompose a string into a [StorePath] and a [PathBuf] containing the
     /// rest of the path, or an error.
     #[cfg(target_family = "unix")]
-    pub fn from_absolute_path_full<'a>(s: &'a str) -> Result<(Self, &'a Path), Error>
+    pub fn from_absolute_path_full<'a, P>(path: &'a P) -> Result<(Self, &'a Path), Error>
     where
         S: From<&'a str>,
+        P: AsRef<std::path::Path> + ?Sized,
     {
         // strip [STORE_DIR_WITH_SLASH] from s
+        let p = path
+            .as_ref()
+            .strip_prefix(STORE_DIR_WITH_SLASH)
+            .map_err(|_e| Error::MissingStoreDir)?;
 
-        match s.strip_prefix(STORE_DIR_WITH_SLASH) {
-            None => Err(Error::MissingStoreDir),
-            Some(rest) => {
-                let mut it = Path::new(rest).components();
+        let mut it = Path::new(p).components();
 
-                // The first component of the rest must be parse-able as a [StorePath]
-                if let Some(first_component) = it.next() {
-                    // convert first component to StorePath
-                    let store_path = StorePath::from_bytes(first_component.as_os_str().as_bytes())?;
+        // The first component of the rest must be parse-able as a [StorePath]
+        let first_component = it.next().ok_or(Error::InvalidLength)?;
+        let store_path = StorePath::from_bytes(first_component.as_os_str().as_encoded_bytes())?;
 
-                    // collect rest
-                    let rest_buf = it.as_path();
+        // collect rest
+        let rest_buf = it.as_path();
 
-                    Ok((store_path, rest_buf))
-                } else {
-                    Err(Error::InvalidLength) // Well, or missing "/"?
-                }
-            }
-        }
+        Ok((store_path, rest_buf))
     }
 
     /// Returns an absolute store path string.