fix(tvix/glue/import): builtins.storeDir fixes r/8808

This didn't support store paths with a subpath joined to them, while
Nix does.

Use state.path_exists, which does. This also means we can drop the
`store_path_exists` helper, which was only used here.

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

3 files changed, 23 insertions, 39 deletions

diff --git a/tvix/glue/src/builtins/errors.rs b/tvix/glue/src/builtins/errors.rs
index af8a24e6abb8..d41c7d8e4998 100644
--- a/tvix/glue/src/builtins/errors.rs
+++ b/tvix/glue/src/builtins/errors.rs
@@ -69,8 +69,8 @@ pub enum ImportError {
     #[error("hash mismatch at ingestion of '{0}', expected: '{1}', got: '{2}'")]
     HashMismatch(String, NixHash, NixHash),
 
-    #[error("path '{}' is not in the Nix store", .0.display())]
-    PathNotInStore(PathBuf),
+    #[error("path '{}' is not absolute or invalid", .0.display())]
+    PathNotAbsoluteOrInvalid(PathBuf),
 }
 
 impl From<ImportError> for tvix_eval::ErrorKind {
diff --git a/tvix/glue/src/builtins/import.rs b/tvix/glue/src/builtins/import.rs
index 833e38bd577c..d2e2e6c5bd7d 100644
--- a/tvix/glue/src/builtins/import.rs
+++ b/tvix/glue/src/builtins/import.rs
@@ -1,6 +1,6 @@
 //! Implements builtins used to import paths in the store.
 
-use crate::builtins::errors::ImportError;
+use crate::tvix_store_io::TvixStoreIO;
 use std::path::Path;
 use tvix_castore::import::ingest_entries;
 use tvix_castore::Node;
@@ -106,15 +106,15 @@ async fn filtered_ingest(
 
 #[builtins(state = "Rc<TvixStoreIO>")]
 mod import_builtins {
-    use std::os::unix::ffi::OsStrExt;
-    use std::rc::Rc;
-
     use super::*;
 
+    use crate::builtins::ImportError;
     use crate::tvix_store_io::TvixStoreIO;
+    use bstr::ByteSlice;
     use nix_compat::nixhash::{CAHash, NixHash};
     use nix_compat::store_path::StorePathRef;
     use sha2::Digest;
+    use std::rc::Rc;
     use tokio::io::AsyncWriteExt;
     use tvix_eval::builtins::coerce_value_to_path;
     use tvix_eval::generators::Gen;
@@ -367,39 +367,33 @@ mod import_builtins {
         co: GenCo,
         path: Value,
     ) -> Result<Value, ErrorKind> {
-        let p = std::str::from_utf8(match &path {
-            Value::String(s) => s.as_bytes(),
-            Value::Path(p) => p.as_os_str().as_bytes(),
+        let p = match &path {
+            Value::String(s) => Path::new(s.as_bytes().to_os_str()?),
+            Value::Path(p) => p.as_path(),
             _ => {
                 return Err(ErrorKind::TypeError {
                     expected: "string or path",
                     actual: path.type_of(),
                 })
             }
-        })?;
+        };
 
-        let path_exists =
-            if let Ok((store_path, sub_path)) = StorePathRef::from_absolute_path_full(p) {
-                if !sub_path.as_os_str().is_empty() {
-                    false
-                } else {
-                    state.store_path_exists(store_path.as_ref()).await?
-                }
-            } else {
-                false
-            };
+        // For this builtin, the path needs to start with an absolute store path.
+        let (store_path, _sub_path) = StorePathRef::from_absolute_path_full(p)
+            .map_err(|_e| ImportError::PathNotAbsoluteOrInvalid(p.to_path_buf()))?;
 
-        if !path_exists {
-            return Err(ImportError::PathNotInStore(p.into()).into());
+        if state.path_exists(p)? {
+            Ok(Value::String(NixString::new_context_from(
+                [NixContextElement::Plain(store_path.to_absolute_path())].into(),
+                p.as_os_str().as_encoded_bytes(),
+            )))
+        } else {
+            Err(ErrorKind::IO {
+                path: Some(p.to_path_buf()),
+                error: Rc::new(std::io::ErrorKind::NotFound.into()),
+            })
         }
-
-        Ok(Value::String(NixString::new_context_from(
-            [NixContextElement::Plain(p.into())].into(),
-            p,
-        )))
     }
 }
 
 pub use import_builtins::builtins as import_builtins;
-
-use crate::tvix_store_io::TvixStoreIO;
diff --git a/tvix/glue/src/tvix_store_io.rs b/tvix/glue/src/tvix_store_io.rs
index 7e2ab38d7b6b..b5877566d524 100644
--- a/tvix/glue/src/tvix_store_io.rs
+++ b/tvix/glue/src/tvix_store_io.rs
@@ -1,7 +1,6 @@
 //! This module provides an implementation of EvalIO talking to tvix-store.
 use bytes::Bytes;
 use futures::{StreamExt, TryStreamExt};
-use nix_compat::store_path::StorePathRef;
 use nix_compat::{nixhash::CAHash, store_path::StorePath};
 use std::collections::BTreeMap;
 use std::{
@@ -435,15 +434,6 @@ impl TvixStoreIO {
         let path_info = self.node_to_path_info(name, path, ca, root_node).await?;
         Ok(self.path_info_service.as_ref().put(path_info).await?)
     }
-
-    pub async fn store_path_exists<'a>(&'a self, store_path: StorePathRef<'a>) -> io::Result<bool> {
-        Ok(self
-            .path_info_service
-            .as_ref()
-            .get(*store_path.digest())
-            .await?
-            .is_some())
-    }
 }
 
 impl EvalIO for TvixStoreIO {