From 9c223450199b466c535f2b715ad68f1f295fa7dc Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Fri, 18 Oct 2024 14:41:14 +0200 Subject: [PATCH] refactor(tvix/[ca]store): use auto_impl This implements BS, DS, PS for Box'ed or Arc'ed variants of it with less code, and less potential to accidentially forget to proxy default trait methods for blanked impls, as fixed in cl/12658. Change-Id: If2cdbb563a73792038ebe7bff45d6f880214855b Reviewed-on: https://cl.tvl.fyi/c/depot/+/12661 Tested-by: BuildkiteCI Autosubmit: flokli Reviewed-by: edef --- tvix/Cargo.lock | 13 +++++++ tvix/Cargo.nix | 35 +++++++++++++++++++ tvix/build/src/buildservice/from_addr.rs | 4 +-- tvix/build/src/buildservice/oci.rs | 4 +-- tvix/castore/Cargo.toml | 1 + tvix/castore/src/blobservice/mod.rs | 24 ++----------- .../src/directoryservice/combinators.rs | 4 +-- tvix/castore/src/directoryservice/mod.rs | 27 ++------------ tvix/castore/src/fs/fuse/tests.rs | 4 +-- tvix/castore/src/fs/mod.rs | 16 ++++----- tvix/store/Cargo.toml | 1 + tvix/store/src/pathinfoservice/combinators.rs | 4 +-- tvix/store/src/pathinfoservice/fs/mod.rs | 11 +++--- tvix/store/src/pathinfoservice/mod.rs | 24 ++----------- tvix/store/src/pathinfoservice/nix_http.rs | 10 +++--- .../src/pathinfoservice/signing_wrapper.rs | 2 +- 16 files changed, 85 insertions(+), 99 deletions(-) diff --git a/tvix/Cargo.lock b/tvix/Cargo.lock index c0c6f1f24..ae077003b 100644 --- a/tvix/Cargo.lock +++ b/tvix/Cargo.lock @@ -277,6 +277,17 @@ version = "1.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1505bd5d3d116872e7271a6d4e16d81d0c8570876c8de68093a09ac269d8aac0" +[[package]] +name = "auto_impl" +version = "1.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3c87f3f15e7794432337fc718554eaa4dc8f04c9677a950ffe366f20a162ae42" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.79", +] + [[package]] name = "autocfg" version = "1.4.0" @@ -4572,6 +4583,7 @@ dependencies = [ "async-process", "async-stream", "async-tempfile", + "auto_impl", "bigtable_rs", "blake3", "bstr", @@ -4765,6 +4777,7 @@ dependencies = [ "async-compression", "async-process", "async-stream", + "auto_impl", "bigtable_rs", "blake3", "bstr", diff --git a/tvix/Cargo.nix b/tvix/Cargo.nix index 7d25ba51e..684fdaf5c 100644 --- a/tvix/Cargo.nix +++ b/tvix/Cargo.nix @@ -992,6 +992,33 @@ rec { "portable-atomic" = [ "dep:portable-atomic" ]; }; }; + "auto_impl" = rec { + crateName = "auto_impl"; + version = "1.2.0"; + edition = "2021"; + sha256 = "0hmfcahj0vrnzq7rayk7r428zp54x9a8awgw6wil753pbvqz71rw"; + procMacro = true; + authors = [ + "Ashley Mannix " + "Lukas Kalbertodt " + ]; + dependencies = [ + { + name = "proc-macro2"; + packageId = "proc-macro2"; + } + { + name = "quote"; + packageId = "quote"; + } + { + name = "syn"; + packageId = "syn 2.0.79"; + features = [ "full" "visit" "visit-mut" ]; + } + ]; + + }; "autocfg" = rec { crateName = "autocfg"; version = "1.4.0"; @@ -15131,6 +15158,10 @@ rec { name = "async-tempfile"; packageId = "async-tempfile"; } + { + name = "auto_impl"; + packageId = "auto_impl"; + } { name = "bigtable_rs"; packageId = "bigtable_rs"; @@ -15917,6 +15948,10 @@ rec { name = "async-stream"; packageId = "async-stream"; } + { + name = "auto_impl"; + packageId = "auto_impl"; + } { name = "bigtable_rs"; packageId = "bigtable_rs"; diff --git a/tvix/build/src/buildservice/from_addr.rs b/tvix/build/src/buildservice/from_addr.rs index 0f4c190c2..ba185bb25 100644 --- a/tvix/build/src/buildservice/from_addr.rs +++ b/tvix/build/src/buildservice/from_addr.rs @@ -20,8 +20,8 @@ pub async fn from_addr( directory_service: DS, ) -> std::io::Result> where - BS: AsRef + Send + Sync + Clone + 'static, - DS: AsRef + Send + Sync + Clone + 'static, + BS: BlobService + Send + Sync + Clone + 'static, + DS: DirectoryService + Send + Sync + Clone + 'static, { let url = Url::parse(uri) .map_err(|e| std::io::Error::other(format!("unable to parse url: {}", e)))?; diff --git a/tvix/build/src/buildservice/oci.rs b/tvix/build/src/buildservice/oci.rs index 875e1de44..7b88518e9 100644 --- a/tvix/build/src/buildservice/oci.rs +++ b/tvix/build/src/buildservice/oci.rs @@ -91,8 +91,8 @@ impl OCIBuildService { #[async_trait] impl BuildService for OCIBuildService where - BS: AsRef + Send + Sync + Clone + 'static, - DS: AsRef + Send + Sync + Clone + 'static, + BS: BlobService + Clone + 'static, + DS: DirectoryService + Clone + 'static, { #[instrument(skip_all, err)] async fn do_build(&self, request: BuildRequest) -> std::io::Result { diff --git a/tvix/castore/Cargo.toml b/tvix/castore/Cargo.toml index 0799b471c..aa44e2e8e 100644 --- a/tvix/castore/Cargo.toml +++ b/tvix/castore/Cargo.toml @@ -52,6 +52,7 @@ vm-memory = { workspace = true, optional = true } vmm-sys-util = { workspace = true, optional = true } virtio-bindings = { workspace = true, optional = true } wu-manber = { workspace = true } +auto_impl = "1.2.0" [build-dependencies] prost-build = { workspace = true } diff --git a/tvix/castore/src/blobservice/mod.rs b/tvix/castore/src/blobservice/mod.rs index 85292722f..efba927b5 100644 --- a/tvix/castore/src/blobservice/mod.rs +++ b/tvix/castore/src/blobservice/mod.rs @@ -1,5 +1,6 @@ use std::io; +use auto_impl::auto_impl; use tonic::async_trait; use crate::composition::{Registry, ServiceBuilder}; @@ -29,6 +30,7 @@ pub use self::object_store::{ObjectStoreBlobService, ObjectStoreBlobServiceConfi /// which will implement a writer interface, and also provides a close funtion, /// to finalize a blob and get its digest. #[async_trait] +#[auto_impl(&, &mut, Arc, Box)] pub trait BlobService: Send + Sync { /// Check if the service has the blob, by its content hash. /// On implementations returning chunks, this must also work for chunks. @@ -60,28 +62,6 @@ pub trait BlobService: Send + Sync { } } -#[async_trait] -impl BlobService for A -where - A: AsRef + Send + Sync, -{ - async fn has(&self, digest: &B3Digest) -> io::Result { - self.as_ref().has(digest).await - } - - async fn open_read(&self, digest: &B3Digest) -> io::Result>> { - self.as_ref().open_read(digest).await - } - - async fn open_write(&self) -> Box { - self.as_ref().open_write().await - } - - async fn chunks(&self, digest: &B3Digest) -> io::Result>> { - self.as_ref().chunks(digest).await - } -} - /// A [tokio::io::AsyncWrite] that the user needs to close() afterwards for persist. /// On success, it returns the digest of the written blob. #[async_trait] diff --git a/tvix/castore/src/directoryservice/combinators.rs b/tvix/castore/src/directoryservice/combinators.rs index 84216de92..4dfc19540 100644 --- a/tvix/castore/src/directoryservice/combinators.rs +++ b/tvix/castore/src/directoryservice/combinators.rs @@ -170,8 +170,8 @@ impl ServiceBuilder for CacheConfig { context: &CompositionContext, ) -> Result, Box> { let (near, far) = futures::join!( - context.resolve(self.near.clone()), - context.resolve(self.far.clone()) + context.resolve::(self.near.clone()), + context.resolve::(self.far.clone()) ); Ok(Arc::new(Cache { near: near?, diff --git a/tvix/castore/src/directoryservice/mod.rs b/tvix/castore/src/directoryservice/mod.rs index 76c7548d4..b3cb0f4fd 100644 --- a/tvix/castore/src/directoryservice/mod.rs +++ b/tvix/castore/src/directoryservice/mod.rs @@ -1,6 +1,7 @@ use crate::composition::{Registry, ServiceBuilder}; use crate::{B3Digest, Directory, Error}; +use auto_impl::auto_impl; use futures::stream::BoxStream; use tonic::async_trait; mod combinators; @@ -39,6 +40,7 @@ pub use self::bigtable::{BigtableDirectoryService, BigtableParameters}; /// This is a simple get and put of [Directory], returning their /// digest. #[async_trait] +#[auto_impl(&, &mut, Arc, Box)] pub trait DirectoryService: Send + Sync { /// Looks up a single Directory message by its digest. /// The returned Directory message *must* be valid. @@ -80,31 +82,6 @@ pub trait DirectoryService: Send + Sync { fn put_multiple_start(&self) -> Box; } -#[async_trait] -impl DirectoryService for A -where - A: AsRef + Send + Sync, -{ - async fn get(&self, digest: &B3Digest) -> Result, Error> { - self.as_ref().get(digest).await - } - - async fn put(&self, directory: Directory) -> Result { - self.as_ref().put(directory).await - } - - fn get_recursive( - &self, - root_directory_digest: &B3Digest, - ) -> BoxStream<'static, Result> { - self.as_ref().get_recursive(root_directory_digest) - } - - fn put_multiple_start(&self) -> Box { - self.as_ref().put_multiple_start() - } -} - /// Provides a handle to put a closure of connected [Directory] elements. /// /// The consumer can periodically call [DirectoryPutter::put], starting from the diff --git a/tvix/castore/src/fs/fuse/tests.rs b/tvix/castore/src/fs/fuse/tests.rs index 9e01204d5..0d68af090 100644 --- a/tvix/castore/src/fs/fuse/tests.rs +++ b/tvix/castore/src/fs/fuse/tests.rs @@ -45,8 +45,8 @@ fn do_mount, BS, DS>( show_xattr: bool, ) -> io::Result where - BS: AsRef + Send + Sync + Clone + 'static, - DS: AsRef + Send + Sync + Clone + 'static, + BS: BlobService + Send + Sync + Clone + 'static, + DS: DirectoryService + Send + Sync + Clone + 'static, { let fs = TvixStoreFs::new( blob_service, diff --git a/tvix/castore/src/fs/mod.rs b/tvix/castore/src/fs/mod.rs index d124c5629..58e01355e 100644 --- a/tvix/castore/src/fs/mod.rs +++ b/tvix/castore/src/fs/mod.rs @@ -121,8 +121,8 @@ pub struct TvixStoreFs { impl TvixStoreFs where - BS: AsRef + Clone + Send, - DS: AsRef + Clone + Send + 'static, + BS: BlobService + Clone + Send, + DS: DirectoryService + Clone + Send + 'static, RN: RootNodes + Clone + 'static, { pub fn new( @@ -186,7 +186,7 @@ where .block_on({ let directory_service = self.directory_service.clone(); let parent_digest = parent_digest.to_owned(); - async move { directory_service.as_ref().get(&parent_digest).await } + async move { directory_service.get(&parent_digest).await } })? .ok_or_else(|| { warn!(directory.digest=%parent_digest, "directory not found"); @@ -302,8 +302,8 @@ const XATTR_NAME_BLOB_DIGEST: &[u8] = b"user.tvix.castore.blob.digest"; impl Layer for TvixStoreFs where - BS: AsRef + Clone + Send + 'static, - DS: AsRef + Send + Clone + 'static, + BS: BlobService + Clone + Send + 'static, + DS: DirectoryService + Send + Clone + 'static, RN: RootNodes + Clone + 'static, { fn root_inode(&self) -> Self::Inode { @@ -313,8 +313,8 @@ where impl FileSystem for TvixStoreFs where - BS: AsRef + Clone + Send + 'static, - DS: AsRef + Send + Clone + 'static, + BS: BlobService + Clone + Send + 'static, + DS: DirectoryService + Send + Clone + 'static, RN: RootNodes + Clone + 'static, { type Handle = u64; @@ -674,7 +674,7 @@ where match self.tokio_handle.block_on({ let blob_service = self.blob_service.clone(); let blob_digest = blob_digest.clone(); - async move { blob_service.as_ref().open_read(&blob_digest).await } + async move { blob_service.open_read(&blob_digest).await } }) { Ok(None) => { warn!("blob not found"); diff --git a/tvix/store/Cargo.toml b/tvix/store/Cargo.toml index b913aed3b..865b1f4f6 100644 --- a/tvix/store/Cargo.toml +++ b/tvix/store/Cargo.toml @@ -49,6 +49,7 @@ redb = { workspace = true, features = ["logging"] } mimalloc = { workspace = true } tonic-reflection = { workspace = true, optional = true } bigtable_rs = { workspace = true, optional = true } +auto_impl = "1.2.0" [build-dependencies] prost-build = { workspace = true } diff --git a/tvix/store/src/pathinfoservice/combinators.rs b/tvix/store/src/pathinfoservice/combinators.rs index 1f413cf31..f386fd52d 100644 --- a/tvix/store/src/pathinfoservice/combinators.rs +++ b/tvix/store/src/pathinfoservice/combinators.rs @@ -88,8 +88,8 @@ impl ServiceBuilder for CacheConfig { context: &CompositionContext, ) -> Result, Box> { let (near, far) = futures::join!( - context.resolve(self.near.clone()), - context.resolve(self.far.clone()) + context.resolve::(self.near.clone()), + context.resolve::(self.far.clone()) ); Ok(Arc::new(Cache { near: near?, diff --git a/tvix/store/src/pathinfoservice/fs/mod.rs b/tvix/store/src/pathinfoservice/fs/mod.rs index d996ec9f6..ea30a2f64 100644 --- a/tvix/store/src/pathinfoservice/fs/mod.rs +++ b/tvix/store/src/pathinfoservice/fs/mod.rs @@ -20,9 +20,9 @@ pub fn make_fs( show_xattr: bool, ) -> TvixStoreFs> where - BS: AsRef + Send + Clone + 'static, - DS: AsRef + Send + Clone + 'static, - PS: AsRef + Send + Sync + Clone + 'static, + BS: BlobService + Send + Clone + 'static, + DS: DirectoryService + Send + Clone + 'static, + PS: PathInfoService + Send + Sync + Clone + 'static, { TvixStoreFs::new( blob_service, @@ -46,7 +46,7 @@ pub struct RootNodesWrapper(pub(crate) T); #[async_trait] impl RootNodes for RootNodesWrapper where - T: AsRef + Send + Sync, + T: PathInfoService + Send + Sync, { async fn get_by_basename(&self, name: &PathComponent) -> Result, Error> { let Ok(store_path) = StorePathRef::from_bytes(name.as_ref()) else { @@ -55,14 +55,13 @@ where Ok(self .0 - .as_ref() .get(*store_path.digest()) .await? .map(|path_info| path_info.node)) } fn list(&self) -> BoxStream> { - Box::pin(self.0.as_ref().list().map(|result| { + Box::pin(self.0.list().map(|result| { result.map(|path_info| { let basename = path_info.store_path.to_string(); ( diff --git a/tvix/store/src/pathinfoservice/mod.rs b/tvix/store/src/pathinfoservice/mod.rs index a0c48f5cc..d31a1e652 100644 --- a/tvix/store/src/pathinfoservice/mod.rs +++ b/tvix/store/src/pathinfoservice/mod.rs @@ -13,6 +13,7 @@ mod fs; #[cfg(test)] mod tests; +use auto_impl::auto_impl; use futures::stream::BoxStream; use tonic::async_trait; use tvix_castore::composition::{Registry, ServiceBuilder}; @@ -45,6 +46,7 @@ pub use self::fs::make_fs; /// The base trait all PathInfo services need to implement. #[async_trait] +#[auto_impl(&, &mut, Arc, Box)] pub trait PathInfoService: Send + Sync { /// Retrieve a PathInfo message by the output digest. async fn get(&self, digest: [u8; 20]) -> Result, Error>; @@ -69,28 +71,6 @@ pub trait PathInfoService: Send + Sync { } } -#[async_trait] -impl PathInfoService for A -where - A: AsRef + Send + Sync + 'static, -{ - async fn get(&self, digest: [u8; 20]) -> Result, Error> { - self.as_ref().get(digest).await - } - - async fn put(&self, path_info: PathInfo) -> Result { - self.as_ref().put(path_info).await - } - - fn list(&self) -> BoxStream<'static, Result> { - self.as_ref().list() - } - - fn nar_calculation_service(&self) -> Option> { - self.as_ref().nar_calculation_service() - } -} - /// Registers the builtin PathInfoService implementations with the registry pub(crate) fn register_pathinfo_services(reg: &mut Registry) { reg.register::>, CachePathInfoServiceConfig>("cache"); diff --git a/tvix/store/src/pathinfoservice/nix_http.rs b/tvix/store/src/pathinfoservice/nix_http.rs index ed386f0e9..29e04aa45 100644 --- a/tvix/store/src/pathinfoservice/nix_http.rs +++ b/tvix/store/src/pathinfoservice/nix_http.rs @@ -66,8 +66,8 @@ impl NixHTTPPathInfoService { #[async_trait] impl PathInfoService for NixHTTPPathInfoService where - BS: AsRef + Send + Sync + Clone + 'static, - DS: AsRef + Send + Sync + Clone + 'static, + BS: BlobService + Send + Sync + Clone + 'static, + DS: DirectoryService + Send + Sync + Clone + 'static, { #[instrument(skip_all, err, fields(path.digest=nixbase32::encode(&digest)))] async fn get(&self, digest: [u8; 20]) -> Result, Error> { @@ -316,10 +316,10 @@ impl ServiceBuilder for NixHTTPPathInfoServiceConfig { &'a self, _instance_name: &str, context: &CompositionContext, - ) -> Result, Box> { + ) -> Result, Box> { let (blob_service, directory_service) = futures::join!( - context.resolve(self.blob_service.clone()), - context.resolve(self.directory_service.clone()) + context.resolve::(self.blob_service.clone()), + context.resolve::(self.directory_service.clone()) ); let mut svc = NixHTTPPathInfoService::new( Url::parse(&self.base_url)?, diff --git a/tvix/store/src/pathinfoservice/signing_wrapper.rs b/tvix/store/src/pathinfoservice/signing_wrapper.rs index 4dff23722..5898a5baa 100644 --- a/tvix/store/src/pathinfoservice/signing_wrapper.rs +++ b/tvix/store/src/pathinfoservice/signing_wrapper.rs @@ -96,7 +96,7 @@ impl ServiceBuilder for KeyFileSigningPathInfoServiceConfig { _instance_name: &str, context: &CompositionContext, ) -> Result, Box> { - let inner = context.resolve(self.inner.clone()).await?; + let inner = context.resolve::(self.inner.clone()).await?; let signing_key = Arc::new( parse_keypair(tokio::fs::read_to_string(&self.keyfile).await?.trim()) .map_err(|e| Error::StorageError(e.to_string()))?