From 2beabe968ca70ce2aef8def08d7dab7340979ea6 Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Tue, 20 Aug 2024 16:52:07 +0300 Subject: refactor(nix-compat/store_path): make StorePath generic on S 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` (for now). I briefly thought about only publicly exporting `StorePath` 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 Tested-by: BuildkiteCI Reviewed-by: Connor Brewster --- tvix/store/src/import.rs | 6 +++--- tvix/store/src/pathinfoservice/fs/mod.rs | 3 ++- tvix/store/src/proto/mod.rs | 10 +++++++--- tvix/store/src/proto/tests/pathinfo.rs | 8 ++++---- 4 files changed, 16 insertions(+), 11 deletions(-) (limited to 'tvix/store/src') 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( directory_service: DS, path_info_service: PS, nar_calculation_service: NS, -) -> Result +) -> Result where P: AsRef + 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 + Send + Sync, { async fn get_by_basename(&self, name: &PathComponent) -> Result, 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 { + pub fn validate(&self) -> Result, 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, - #[case] exp_result: Result, + #[case] exp_result: Result, 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, + #[case] exp_result: Result, 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, + #[case] exp_result: Result, 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, + #[case] exp_result: Result, ValidatePathInfoError>, ) { // construct the PathInfo object let p = PathInfo { -- cgit 1.4.1