refactor(nix-compat/store_path): make StorePath generic on S r/8545

Similar to how cl/12253 already did this for `Signature`, we apply the
same logic to `StorePath`.

`StorePathRef<'a>'` is now a `StorePath<&'a str>`, and there's less
redundant code for the two different implementation.

`.as_ref()` returns a `StorePathRef<'_>`, `.to_owned()` gives a
`StorePath<String>` (for now).

I briefly thought about only publicly exporting `StorePath<String>`
as `StorePath`, but the diff is not too large and this will make it
easier to gradually introduce more flexibility in which store paths to
accept.

Also, remove some silliness in `StorePath::from_absolute_path_full`,
which now doesn't allocate anymore.

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

4 files changed, 16 insertions, 11 deletions

diff --git a/tvix/store/src/import.rs b/tvix/store/src/import.rs
index 5a21b375b575..1719669a4285 100644
--- a/tvix/store/src/import.rs
+++ b/tvix/store/src/import.rs
@@ -8,7 +8,7 @@ use tvix_castore::{
 
 use nix_compat::{
     nixhash::{CAHash, NixHash},
-    store_path::{self, StorePath},
+    store_path::{self, StorePathRef},
 };
 
 use crate::{
@@ -115,7 +115,7 @@ pub async fn import_path_as_nar_ca<BS, DS, PS, NS, P>(
     directory_service: DS,
     path_info_service: PS,
     nar_calculation_service: NS,
-) -> Result<StorePath, std::io::Error>
+) -> Result<StorePathRef, std::io::Error>
 where
     P: AsRef<Path> + std::fmt::Debug,
     BS: BlobService + Clone,
@@ -161,7 +161,7 @@ where
     // callers don't really need it.
     let _path_info = path_info_service.as_ref().put(path_info).await?;
 
-    Ok(output_path.to_owned())
+    Ok(output_path)
 }
 
 #[cfg(test)]
diff --git a/tvix/store/src/pathinfoservice/fs/mod.rs b/tvix/store/src/pathinfoservice/fs/mod.rs
index d97032b6658e..1f7fa8a8afce 100644
--- a/tvix/store/src/pathinfoservice/fs/mod.rs
+++ b/tvix/store/src/pathinfoservice/fs/mod.rs
@@ -1,5 +1,6 @@
 use futures::stream::BoxStream;
 use futures::StreamExt;
+use nix_compat::store_path::StorePathRef;
 use tonic::async_trait;
 use tvix_castore::fs::{RootNodes, TvixStoreFs};
 use tvix_castore::{blobservice::BlobService, directoryservice::DirectoryService};
@@ -48,7 +49,7 @@ where
     T: AsRef<dyn PathInfoService> + Send + Sync,
 {
     async fn get_by_basename(&self, name: &PathComponent) -> Result<Option<Node>, Error> {
-        let Ok(store_path) = nix_compat::store_path::StorePath::from_bytes(name.as_ref()) else {
+        let Ok(store_path) = StorePathRef::from_bytes(name.as_ref()) else {
             return Ok(None);
         };
 
diff --git a/tvix/store/src/proto/mod.rs b/tvix/store/src/proto/mod.rs
index e6b411664037..f3ea4b196946 100644
--- a/tvix/store/src/proto/mod.rs
+++ b/tvix/store/src/proto/mod.rs
@@ -87,7 +87,7 @@ impl PathInfo {
     /// validate performs some checks on the PathInfo struct,
     /// Returning either a [store_path::StorePath] of the root node, or a
     /// [ValidatePathInfoError].
-    pub fn validate(&self) -> Result<store_path::StorePath, ValidatePathInfoError> {
+    pub fn validate(&self) -> Result<store_path::StorePath<String>, ValidatePathInfoError> {
         // ensure the references have the right number of bytes.
         for (i, reference) in self.references.iter().enumerate() {
             if reference.len() != store_path::DIGEST_SIZE {
@@ -118,7 +118,7 @@ impl PathInfo {
             // parse references in reference_names.
             for (i, reference_name_str) in narinfo.reference_names.iter().enumerate() {
                 // ensure thy parse as (non-absolute) store path
-                let reference_names_store_path = store_path::StorePath::from_bytes(
+                let reference_names_store_path = store_path::StorePathRef::from_bytes(
                     reference_name_str.as_bytes(),
                 )
                 .map_err(|_| {
@@ -160,6 +160,10 @@ impl PathInfo {
         let root_nix_path = match &self.node {
             None => Err(ValidatePathInfoError::NoNodePresent)?,
             Some(node) => {
+                // NOTE: We could have some PathComponent not allocating here,
+                // so this can return StorePathRef.
+                // However, as this will get refactored away to stricter types
+                // soon anyways, there's no point.
                 let (name, _node) = node
                     .clone()
                     .into_name_and_node()
@@ -356,7 +360,7 @@ impl From<&nix_compat::narinfo::NarInfo<'_>> for NarInfo {
             signatures,
             reference_names: value.references.iter().map(|r| r.to_string()).collect(),
             deriver: value.deriver.as_ref().map(|sp| StorePath {
-                name: sp.name().to_owned(),
+                name: (*sp.name()).to_owned(),
                 digest: Bytes::copy_from_slice(sp.digest()),
             }),
             ca: value.ca.as_ref().map(|ca| ca.into()),
diff --git a/tvix/store/src/proto/tests/pathinfo.rs b/tvix/store/src/proto/tests/pathinfo.rs
index 253cafad01f4..eb47e592be8c 100644
--- a/tvix/store/src/proto/tests/pathinfo.rs
+++ b/tvix/store/src/proto/tests/pathinfo.rs
@@ -14,7 +14,7 @@ use tvix_castore::{DirectoryError, ValidateNodeError};
 
 fn validate_pathinfo(
     #[case] node: Option<castorepb::Node>,
-    #[case] exp_result: Result<StorePath, ValidatePathInfoError>,
+    #[case] exp_result: Result<StorePath<String>, ValidatePathInfoError>,
 ) {
     // construct the PathInfo object
     let p = PathInfo {
@@ -46,7 +46,7 @@ fn validate_pathinfo(
 )))]
 fn validate_directory(
     #[case] directory_node: castorepb::DirectoryNode,
-    #[case] exp_result: Result<StorePath, ValidatePathInfoError>,
+    #[case] exp_result: Result<StorePath<String>, ValidatePathInfoError>,
 ) {
     // construct the PathInfo object
     let p = PathInfo {
@@ -89,7 +89,7 @@ fn validate_directory(
 )]
 fn validate_file(
     #[case] file_node: castorepb::FileNode,
-    #[case] exp_result: Result<StorePath, ValidatePathInfoError>,
+    #[case] exp_result: Result<StorePath<String>, ValidatePathInfoError>,
 ) {
     // construct the PathInfo object
     let p = PathInfo {
@@ -121,7 +121,7 @@ fn validate_file(
 )]
 fn validate_symlink(
     #[case] symlink_node: castorepb::SymlinkNode,
-    #[case] exp_result: Result<StorePath, ValidatePathInfoError>,
+    #[case] exp_result: Result<StorePath<String>, ValidatePathInfoError>,
 ) {
     // construct the PathInfo object
     let p = PathInfo {