mdebray/tvl-depot
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

feat(tvix/store/proto): avoid clone in PathInfo::validate()

Browse source 
Have this return a StorePathRef<'_>, rather than a StorePath, and leave
it up to the caller to possibly convert it to a owned StorePath.

This avoids some allocations, if we only want to validate.

Change-Id: I5cf8e246fe02bd4e631f46a5cb86d3f77a728a0d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11361
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: Connor Brewster <cbrewster@hey.com>
This commit is contained in:
Florian Klink 2024-04-06 01:20:22 +03:00 committed by clbot
parent 9e81063050
commit 39276dc5b4
2 changed files with 13 additions and 18 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

14
tvix/store/src/proto/mod.rs
View file

@ -74,23 +74,20 @@ pub enum ValidatePathInfoError {
/// Parses a root node name.
///
/// On success, this returns the parsed [store_path::StorePath].
/// On success, this returns the parsed [store_path::StorePathRef].
/// On error, it returns an error generated from the supplied constructor.
fn parse_node_name_root<E>(
name: &[u8],
err: fn(Vec<u8>, store_path::Error) -> E,
) -> Result<store_path::StorePath, E> {
match store_path::StorePath::from_bytes(name) {
Ok(np) => Ok(np),
Err(e) => Err(err(name.to_vec(), e)),
}
) -> Result<store_path::StorePathRef<'_>, E> {
store_path::StorePathRef::from_bytes(name).map_err(|e| err(name.to_vec(), e))
}
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::StorePathRef<'_>, 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 {
@ -154,8 +151,7 @@ impl PathInfo {
// converting to this field.
if let Some(deriver) = &narinfo.deriver {
store_path::StorePathRef::from_name_and_digest(&deriver.name, &deriver.digest)
.map_err(ValidatePathInfoError::InvalidDeriverField)?
.to_owned();
.map_err(ValidatePathInfoError::InvalidDeriverField)?;
}
}
}

17
tvix/store/src/proto/tests/pathinfo.rs
View file

@ -3,8 +3,7 @@ use crate::tests::fixtures::*;
use bytes::Bytes;
use data_encoding::BASE64;
use nix_compat::nixbase32;
use nix_compat::store_path::{self, StorePath, StorePathRef};
use std::str::FromStr;
use nix_compat::store_path::{self, StorePathRef};
use test_case::test_case;
use tvix_castore::proto as castorepb;
@ -20,7 +19,7 @@ use tvix_castore::proto as castorepb;
)]
fn validate_no_node(
t_node: Option<castorepb::Node>,
t_result: Result<StorePath, ValidatePathInfoError>,
t_result: Result<StorePathRef, ValidatePathInfoError>,
) {
// construct the PathInfo object
let p = PathInfo {
@ -36,7 +35,7 @@ fn validate_no_node(
digest: DUMMY_DIGEST.clone().into(),
size: 0,
},
Ok(StorePath::from_str(DUMMY_NAME).expect("must succeed"));
Ok(StorePathRef::from_bytes(DUMMY_NAME.as_bytes()).expect("must succeed"));
"ok"
)]
#[test_case(
@ -62,7 +61,7 @@ fn validate_no_node(
)]
fn validate_directory(
t_directory_node: castorepb::DirectoryNode,
t_result: Result<StorePath, ValidatePathInfoError>,
t_result: Result<StorePathRef, ValidatePathInfoError>,
) {
// construct the PathInfo object
let p = PathInfo {
@ -81,7 +80,7 @@ fn validate_directory(
size: 0,
executable: false,
},
Ok(StorePath::from_str(DUMMY_NAME).expect("must succeed"));
Ok(StorePathRef::from_bytes(DUMMY_NAME.as_bytes()).expect("must succeed"));
"ok"
)]
#[test_case(
@ -107,7 +106,7 @@ fn validate_directory(
)]
fn validate_file(
t_file_node: castorepb::FileNode,
t_result: Result<StorePath, ValidatePathInfoError>,
t_result: Result<StorePathRef, ValidatePathInfoError>,
) {
// construct the PathInfo object
let p = PathInfo {
@ -124,7 +123,7 @@ fn validate_file(
name: DUMMY_NAME.into(),
target: "foo".into(),
},
Ok(StorePath::from_str(DUMMY_NAME).expect("must succeed"));
Ok(StorePathRef::from_bytes(DUMMY_NAME.as_bytes()).expect("must succeed"));
"ok"
)]
#[test_case(
@ -140,7 +139,7 @@ fn validate_file(
)]
fn validate_symlink(
t_symlink_node: castorepb::SymlinkNode,
t_result: Result<StorePath, ValidatePathInfoError>,
t_result: Result<StorePathRef, ValidatePathInfoError>,
) {
// construct the PathInfo object
let p = PathInfo {