From 89882ff9b13ff1c25fc64605e3fc87ae7b9ab877 Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Tue, 9 Jan 2024 11:04:29 +0200 Subject: refactor(tvix): use AsRef instead of Deref MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removes some more needs for Arcs. Change-Id: I9a9f4b81641c271de260e9ffa98313a32944d760 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10578 Autosubmit: flokli Tested-by: BuildkiteCI Reviewed-by: raitobezarius --- tvix/castore/src/fs/mod.rs | 13 ++++++------- tvix/castore/src/fs/root_nodes.rs | 10 ++++++---- tvix/castore/src/fs/tests.rs | 5 ++--- tvix/castore/src/import.rs | 11 +++++------ tvix/castore/src/tests/import.rs | 26 ++++++++++---------------- tvix/glue/src/tvix_store_io.rs | 30 ++++++++++++++---------------- tvix/store/src/pathinfoservice/fs/mod.rs | 13 ++++++------- tvix/store/src/utils.rs | 19 +++++++++---------- 8 files changed, 58 insertions(+), 69 deletions(-) diff --git a/tvix/castore/src/fs/mod.rs b/tvix/castore/src/fs/mod.rs index 2da072329d34..632dad84cf86 100644 --- a/tvix/castore/src/fs/mod.rs +++ b/tvix/castore/src/fs/mod.rs @@ -23,7 +23,6 @@ use fuse_backend_rs::abi::fuse_abi::stat64; use fuse_backend_rs::api::filesystem::{Context, FileSystem, FsOptions, ROOT_ID}; use futures::StreamExt; use parking_lot::RwLock; -use std::ops::Deref; use std::{ collections::HashMap, io, @@ -101,8 +100,8 @@ pub struct TvixStoreFs { impl TvixStoreFs where - BS: Deref + Clone + Send, - DS: Deref + Clone + Send + 'static, + BS: AsRef + Clone + Send, + DS: AsRef + Clone + Send + 'static, RN: RootNodes + Clone + 'static, { pub fn new( @@ -157,7 +156,7 @@ where .block_on(self.tokio_handle.spawn({ let directory_service = self.directory_service.clone(); let parent_digest = parent_digest.to_owned(); - async move { directory_service.get(&parent_digest).await } + async move { directory_service.as_ref().get(&parent_digest).await } })) .unwrap()? .ok_or_else(|| { @@ -270,8 +269,8 @@ where impl FileSystem for TvixStoreFs where - BS: Deref + Clone + Send + 'static, - DS: Deref + Send + Clone + 'static, + BS: AsRef + Clone + Send + 'static, + DS: AsRef + Send + Clone + 'static, RN: RootNodes + Clone + 'static, { type Handle = u64; @@ -496,7 +495,7 @@ where let task = self .tokio_handle - .spawn(async move { blob_service.open_read(&blob_digest).await }); + .spawn(async move { blob_service.as_ref().open_read(&blob_digest).await }); let blob_reader = self.tokio_handle.block_on(task).unwrap(); diff --git a/tvix/castore/src/fs/root_nodes.rs b/tvix/castore/src/fs/root_nodes.rs index 312a0b2cdd7e..a603fd1b37d5 100644 --- a/tvix/castore/src/fs/root_nodes.rs +++ b/tvix/castore/src/fs/root_nodes.rs @@ -1,4 +1,4 @@ -use std::{collections::BTreeMap, ops::Deref, pin::Pin}; +use std::{collections::BTreeMap, pin::Pin}; use crate::{proto::node::Node, Error}; use bytes::Bytes; @@ -23,13 +23,15 @@ pub trait RootNodes: Send + Sync { /// the key is the node name. impl RootNodes for T where - T: Deref> + Send + Sync, + T: AsRef> + Send + Sync, { async fn get_by_basename(&self, name: &[u8]) -> Result, Error> { - Ok(self.get(name).cloned()) + Ok(self.as_ref().get(name).cloned()) } fn list(&self) -> Pin> + Send + '_>> { - Box::pin(tokio_stream::iter(self.iter().map(|(_, v)| Ok(v.clone())))) + Box::pin(tokio_stream::iter( + self.as_ref().iter().map(|(_, v)| Ok(v.clone())), + )) } } diff --git a/tvix/castore/src/fs/tests.rs b/tvix/castore/src/fs/tests.rs index 5f2916abd44f..2f27c3c1c8e4 100644 --- a/tvix/castore/src/fs/tests.rs +++ b/tvix/castore/src/fs/tests.rs @@ -1,7 +1,6 @@ use std::{ collections::BTreeMap, io::{self, Cursor}, - ops::Deref, os::unix::fs::MetadataExt, path::Path, sync::Arc, @@ -43,8 +42,8 @@ fn do_mount, BS, DS>( list_root: bool, ) -> io::Result where - BS: Deref + Send + Sync + Clone + 'static, - DS: Deref + Send + Sync + Clone + 'static, + BS: AsRef + Send + Sync + Clone + 'static, + DS: AsRef + Send + Sync + Clone + 'static, { let fs = TvixStoreFs::new( blob_service, diff --git a/tvix/castore/src/import.rs b/tvix/castore/src/import.rs index c4d410497235..cf4b42fb6826 100644 --- a/tvix/castore/src/import.rs +++ b/tvix/castore/src/import.rs @@ -7,7 +7,6 @@ use crate::proto::DirectoryNode; use crate::proto::FileNode; use crate::proto::SymlinkNode; use crate::Error as CastoreError; -use std::ops::Deref; use std::os::unix::ffi::OsStrExt; use std::{ collections::HashMap, @@ -68,7 +67,7 @@ async fn process_entry<'a, BS>( maybe_directory: Option, ) -> Result where - BS: Deref + Clone, + BS: AsRef + Clone, { let file_type = entry.file_type(); @@ -114,7 +113,7 @@ where .await .map_err(|e| Error::UnableToOpen(entry.path().to_path_buf(), e))?; - let mut writer = blob_service.open_write().await; + let mut writer = blob_service.as_ref().open_write().await; if let Err(e) = tokio::io::copy(&mut file, &mut writer).await { return Err(Error::UnableToRead(entry.path().to_path_buf(), e)); @@ -156,12 +155,12 @@ pub async fn ingest_path<'a, BS, DS, P>( ) -> Result where P: AsRef + Debug, - BS: Deref + Clone, - DS: Deref, + BS: AsRef + Clone, + DS: AsRef, { let mut directories: HashMap = HashMap::default(); - let mut directory_putter = directory_service.put_multiple_start(); + let mut directory_putter = directory_service.as_ref().put_multiple_start(); for entry in WalkDir::new(p.as_ref()) .follow_links(false) diff --git a/tvix/castore/src/tests/import.rs b/tvix/castore/src/tests/import.rs index 333254706d61..99e993f36da3 100644 --- a/tvix/castore/src/tests/import.rs +++ b/tvix/castore/src/tests/import.rs @@ -1,10 +1,8 @@ use crate::blobservice::BlobService; -use crate::directoryservice::DirectoryService; use crate::fixtures::*; use crate::import::ingest_path; use crate::proto; use crate::utils::{gen_blob_service, gen_directory_service}; -use std::ops::Deref; use std::sync::Arc; use tempfile::TempDir; @@ -14,8 +12,8 @@ use std::os::unix::ffi::OsStrExt; #[cfg(target_family = "unix")] #[tokio::test] async fn symlink() { - let blob_service: Arc = gen_blob_service().into(); - let directory_service: Arc = gen_directory_service().into(); + let blob_service = gen_blob_service(); + let directory_service = gen_directory_service(); let tmpdir = TempDir::new().unwrap(); @@ -27,8 +25,8 @@ async fn symlink() { .unwrap(); let root_node = ingest_path( - blob_service, - &directory_service.deref(), + Arc::from(blob_service), + directory_service, tmpdir.path().join("doesntmatter"), ) .await @@ -46,7 +44,7 @@ async fn symlink() { #[tokio::test] async fn single_file() { let blob_service: Arc = gen_blob_service().into(); - let directory_service: Arc = gen_directory_service().into(); + let directory_service = gen_directory_service(); let tmpdir = TempDir::new().unwrap(); @@ -54,7 +52,7 @@ async fn single_file() { let root_node = ingest_path( blob_service.clone(), - &directory_service.deref(), + directory_service, tmpdir.path().join("root"), ) .await @@ -78,7 +76,7 @@ async fn single_file() { #[tokio::test] async fn complicated() { let blob_service: Arc = gen_blob_service().into(); - let directory_service: Arc = gen_directory_service().into(); + let directory_service = gen_directory_service(); let tmpdir = TempDir::new().unwrap(); @@ -91,13 +89,9 @@ async fn complicated() { // File ``keep/.keep` std::fs::write(tmpdir.path().join("keep").join(".keep"), vec![]).unwrap(); - let root_node = ingest_path( - blob_service.clone(), - &directory_service.deref(), - tmpdir.path(), - ) - .await - .expect("must succeed"); + let root_node = ingest_path(blob_service.clone(), &directory_service, tmpdir.path()) + .await + .expect("must succeed"); // ensure root_node matched expectations assert_eq!( diff --git a/tvix/glue/src/tvix_store_io.rs b/tvix/glue/src/tvix_store_io.rs index eba8cee7336f..56e3feb674f3 100644 --- a/tvix/glue/src/tvix_store_io.rs +++ b/tvix/glue/src/tvix_store_io.rs @@ -3,7 +3,6 @@ use nix_compat::store_path::StorePath; use std::{ io, - ops::Deref, path::{Path, PathBuf}, }; use tokio::io::AsyncReadExt; @@ -35,8 +34,8 @@ pub struct TvixStoreIO { impl TvixStoreIO where - DS: Deref, - PS: Deref, + DS: AsRef, + PS: AsRef, { pub fn new( blob_service: BS, @@ -70,7 +69,7 @@ where .tokio_handle .block_on(async { self.path_info_service - .deref() + .as_ref() .get(*store_path.digest()) .await }) @@ -93,7 +92,7 @@ where // with the root_node and sub_path, descend to the node requested. Ok(self.tokio_handle.block_on({ async { - directoryservice::descend_to(self.directory_service.deref(), root_node, sub_path) + directoryservice::descend_to(self.directory_service.as_ref(), root_node, sub_path) .await } })?) @@ -102,9 +101,9 @@ where impl EvalIO for TvixStoreIO where - BS: Deref + Clone, - DS: Deref, - PS: Deref, + BS: AsRef + Clone, + DS: AsRef, + PS: AsRef, { #[instrument(skip(self), ret, err)] fn path_exists(&self, path: &Path) -> io::Result { @@ -154,7 +153,7 @@ where self.tokio_handle.block_on(async { let mut reader = { - let resp = self.blob_service.deref().open_read(&digest).await?; + let resp = self.blob_service.as_ref().open_read(&digest).await?; match resp { Some(blob_reader) => blob_reader, None => { @@ -212,10 +211,9 @@ where ) })?; - if let Some(directory) = self - .tokio_handle - .block_on(async { self.directory_service.deref().get(&digest).await })? - { + if let Some(directory) = self.tokio_handle.block_on(async { + self.directory_service.as_ref().get(&digest).await + })? { let mut children: Vec<(bytes::Bytes, FileType)> = Vec::new(); for node in directory.nodes() { children.push(match node { @@ -263,9 +261,9 @@ where let output_path = self.tokio_handle.block_on(async { tvix_store::utils::import_path( path, - self.blob_service.deref(), - self.directory_service.deref(), - self.path_info_service.deref(), + &self.blob_service, + &self.directory_service, + &self.path_info_service, ) .await })?; diff --git a/tvix/store/src/pathinfoservice/fs/mod.rs b/tvix/store/src/pathinfoservice/fs/mod.rs index df7a42e91f59..b7657d638402 100644 --- a/tvix/store/src/pathinfoservice/fs/mod.rs +++ b/tvix/store/src/pathinfoservice/fs/mod.rs @@ -1,6 +1,5 @@ use futures::Stream; use futures::StreamExt; -use std::ops::Deref; use std::pin::Pin; use tonic::async_trait; use tvix_castore::fs::{RootNodes, TvixStoreFs}; @@ -21,9 +20,9 @@ pub fn make_fs( list_root: bool, ) -> TvixStoreFs> where - BS: Deref + Send + Clone + 'static, - DS: Deref + Send + Clone + 'static, - PS: Deref + Send + Sync + Clone + 'static, + BS: AsRef + Send + Clone + 'static, + DS: AsRef + Send + Clone + 'static, + PS: AsRef + Send + Sync + Clone + 'static, { TvixStoreFs::new( blob_service, @@ -46,7 +45,7 @@ pub struct RootNodesWrapper(pub(crate) T); #[async_trait] impl RootNodes for RootNodesWrapper where - T: Deref + Send + Sync, + T: AsRef + Send + Sync, { async fn get_by_basename(&self, name: &[u8]) -> Result, Error> { let Ok(store_path) = nix_compat::store_path::StorePath::from_bytes(name) else { @@ -55,7 +54,7 @@ where Ok(self .0 - .deref() + .as_ref() .get(*store_path.digest()) .await? .map(|path_info| { @@ -68,7 +67,7 @@ where } fn list(&self) -> Pin> + Send>> { - Box::pin(self.0.deref().list().map(|result| { + Box::pin(self.0.as_ref().list().map(|result| { result.map(|path_info| { path_info .node diff --git a/tvix/store/src/utils.rs b/tvix/store/src/utils.rs index 6edbf94eec33..e7e4b7c79fad 100644 --- a/tvix/store/src/utils.rs +++ b/tvix/store/src/utils.rs @@ -1,4 +1,4 @@ -use std::{ops::Deref, path::Path, sync::Arc}; +use std::{path::Path, sync::Arc}; use data_encoding::BASE64; use nix_compat::store_path::{self, StorePath}; @@ -53,9 +53,9 @@ pub async fn import_path( ) -> Result where P: AsRef + std::fmt::Debug, - BS: Deref + Clone, - DS: Deref, - PS: Deref, + BS: AsRef + Clone, + DS: AsRef, + PS: AsRef, { // calculate the name // TODO: make a path_to_name helper function? @@ -71,15 +71,14 @@ where })?; // Ingest the path into blob and directory service. - let root_node = - tvix_castore::import::ingest_path(blob_service, &directory_service.deref(), &path) - .await - .expect("failed to ingest path"); + let root_node = tvix_castore::import::ingest_path(blob_service, &directory_service, &path) + .await + .expect("failed to ingest path"); debug!(root_node =?root_node, "import successful"); // Ask the PathInfoService for the NAR size and sha256 - let (nar_size, nar_sha256) = path_info_service.calculate_nar(&root_node).await?; + let (nar_size, nar_sha256) = path_info_service.as_ref().calculate_nar(&root_node).await?; // Calculate the output path. This might still fail, as some names are illegal. let output_path = store_path::build_nar_based_store_path(&nar_sha256, name).map_err(|_| { @@ -115,7 +114,7 @@ where // put into [PathInfoService], and return the PathInfo that we get back // from there (it might contain additional signatures). - let _path_info = path_info_service.put(path_info).await?; + let _path_info = path_info_service.as_ref().put(path_info).await?; Ok(output_path.to_owned()) } -- cgit 1.4.1