Refactor FsRepository's renders

The goal is to make it so it is possible to open repo without creating
the renders directory for the current user, in situations where that
renders directory will not be used. Examples include when using fuse or
when running `spfs clean`.

This introduces different flavors of a "render store" besides
`RenderStore` that either represent a render store that hasn't been
created yet (but can be) or the lack of a render store. Operations that
don't require a render store can access the local repository without the
renders directory being created.

Signed-off-by: J Robert Ray <[email protected]>

Author: jrray

crates/spfs-cli/cmd-render/src/cmd_render.rs

@@ -4,9 +4,10 @@
 
 use clap::builder::TypedValueParser;
 use clap::Parser;
-use miette::{Context, Result};
+use miette::{Context, IntoDiagnostic, Result};
 use spfs::prelude::*;
 use spfs::storage::fallback::FallbackProxy;
+use spfs::storage::fs::{MaybeRenderStore, RenderStore};
 use spfs::{graph, Error, RenderResult};
 use spfs_cli_common as cli;
 use spfs_cli_common::CommandName;
@@ -74,7 +75,11 @@ impl CmdRender {
 
         let rendered = match &self.target {
             Some(target) => self.render_to_dir(fallback, env_spec, target).await?,
-            None => self.render_to_repo(fallback, env_spec).await?,
+            None => {
+                // This path requires a repository that supports renders.
+                let fallback: FallbackProxy<RenderStore> = fallback.try_into().into_diagnostic()?;
+                self.render_to_repo(fallback, env_spec).await?
+            }
         };
 
         tracing::debug!("render(s) completed successfully");
@@ -85,7 +90,7 @@ impl CmdRender {
 
     async fn render_to_dir(
         &self,
-        repo: FallbackProxy,
+        repo: FallbackProxy<MaybeRenderStore>,
         env_spec: spfs::tracking::EnvSpec,
         target: &std::path::Path,
     ) -> Result<RenderResult> {
@@ -134,7 +139,7 @@ impl CmdRender {
 
     async fn render_to_repo(
         &self,
-        repo: FallbackProxy,
+        repo: FallbackProxy<RenderStore>,
         env_spec: spfs::tracking::EnvSpec,
     ) -> Result<RenderResult> {
         let mut stack = graph::Stack::default();

crates/spfs-cli/common/src/args.rs

@@ -11,7 +11,7 @@ use miette::{Error, IntoDiagnostic, Result, WrapErr};
 #[cfg(feature = "sentry")]
 use once_cell::sync::OnceCell;
 use spfs::io::Pluralize;
-use spfs::storage::LocalRepository;
+use spfs::storage::LocalPayloads;
 use tracing_subscriber::prelude::*;
 
 const SPFS_LOG: &str = "SPFS_LOG";
@@ -138,7 +138,7 @@ impl Render {
         reporter: Reporter,
     ) -> spfs::storage::fs::Renderer<'repo, Repo, Reporter>
     where
-        Repo: spfs::storage::Repository + LocalRepository,
+        Repo: spfs::storage::Repository + LocalPayloads,
         Reporter: spfs::storage::fs::RenderReporter,
     {
         spfs::storage::fs::Renderer::new(repo)

crates/spfs-cli/main/src/cmd_init.rs

@@ -6,6 +6,7 @@ use std::path::PathBuf;
 
 use clap::{Args, Subcommand};
 use miette::Result;
+use spfs::storage::fs::NoRenderStore;
 
 /// Create an empty filesystem repository
 #[derive(Debug, Args)]
@@ -36,7 +37,7 @@ impl InitSubcommand {
     pub async fn run(&self, _config: &spfs::Config) -> Result<i32> {
         match self {
             Self::Repo { path } => {
-                spfs::storage::fs::MaybeOpenFsRepository::create(&path).await?;
+                spfs::storage::fs::MaybeOpenFsRepository::<NoRenderStore>::create(&path).await?;
                 Ok(0)
             }
         }

crates/spfs-cli/main/src/cmd_search.rs

@@ -5,6 +5,7 @@
 use clap::Args;
 use miette::Result;
 use spfs::prelude::*;
+use spfs::storage::fs::NoRenderStore;
 use tokio_stream::StreamExt;
 
 /// Search for available tags by substring
@@ -29,7 +30,10 @@ impl CmdSearch {
             };
             repos.push(remote);
         }
-        repos.insert(0, config.get_local_repository().await?.into());
+        repos.insert(
+            0,
+            config.get_local_repository::<NoRenderStore>().await?.into(),
+        );
         for repo in repos.into_iter() {
             let mut tag_streams = repo.iter_tags();
             while let Some(tag) = tag_streams.next().await {

crates/spfs-vfs/src/fuse.rs

@@ -29,7 +29,7 @@ use fuser::{
     Request,
 };
 use spfs::prelude::*;
-use spfs::storage::LocalRepository;
+use spfs::storage::LocalPayloads;
 #[cfg(feature = "fuse-backend-abi-7-31")]
 use spfs::tracking::BlobRead;
 use spfs::tracking::{Entry, EntryKind, EnvSpec, Manifest};
@@ -376,9 +376,13 @@ impl Filesystem {
         #[allow(unused_mut)]
         let mut flags = FOPEN_KEEP_CACHE;
         for repo in self.repos.iter() {
-            match &**repo {
-                spfs::storage::RepositoryHandle::FS(fs_repo) => {
-                    let Ok(fs_repo) = fs_repo.opened().await else {
+            // XXX: Using a macro here for an easy fix but it would be nicer
+            // if there was a way to borrow the RepositoryHandle as a
+            // `&MaybeOpenFsRepository<NoRenderStore>` since this code
+            // doesn't need to access renders.
+            macro_rules! read_fs {
+                ($fs_repo:ident) => {
+                    let Ok(fs_repo) = $fs_repo.opened().await else {
                         reply.error(libc::ENOENT);
                         return;
                     };
@@ -393,6 +397,19 @@ impl Filesystem {
                         }
                         Err(err) => err!(reply, err),
                     }
+                };
+            }
+            match &**repo {
+                spfs::storage::RepositoryHandle::FSWithMaybeRenders(fs_repo) => {
+                    // XXX: Will this cause the renders directory to get
+                    // created?
+                    read_fs!(fs_repo);
+                }
+                spfs::storage::RepositoryHandle::FSWithRenders(fs_repo) => {
+                    read_fs!(fs_repo);
+                }
+                spfs::storage::RepositoryHandle::FSWithoutRenders(fs_repo) => {
+                    read_fs!(fs_repo);
                 }
                 #[cfg(feature = "fuse-backend-abi-7-31")]
                 repo => match repo.open_payload(*digest).await {

crates/spfs-vfs/src/winfsp/mount.rs

@@ -9,7 +9,7 @@ use std::sync::Arc;
 use dashmap::DashMap;
 use libc::c_void;
 use spfs::prelude::*;
-use spfs::storage::LocalRepository;
+use spfs::storage::LocalPayloads;
 use spfs::tracking::{Entry, EntryKind};
 use spfs::OsError;
 use tokio::io::AsyncReadExt;
@@ -281,9 +281,13 @@ impl winfsp::filesystem::FileSystemContext for Mount {
         let digest = entry.object;
         self.rt.spawn(async move {
             for repo in repos.into_iter() {
-                match &*repo {
-                    spfs::storage::RepositoryHandle::FS(fs_repo) => {
-                        let Ok(fs_repo) = fs_repo.opened().await else {
+                // XXX: Using a macro here for an easy fix but it would be nicer
+                // if there was a way to borrow the RepositoryHandle as a
+                // `&MaybeOpenFsRepository<NoRenderStore>` since this code
+                // doesn't need to access renders.
+                macro_rules! read_fs {
+                    ($fs_repo:ident) => {
+                        let Ok(fs_repo) = $fs_repo.opened().await else {
                             let _ =
                                 send.send(Err(winfsp::FspError::IO(std::io::ErrorKind::NotFound)));
                             return;
@@ -299,6 +303,17 @@ impl winfsp::filesystem::FileSystemContext for Mount {
                             }
                             Err(err) => err!(send, err),
                         }
+                    };
+                }
+                match &*repo {
+                    spfs::storage::RepositoryHandle::FSWithMaybeRenders(fs_repo) => {
+                        read_fs!(fs_repo);
+                    }
+                    spfs::storage::RepositoryHandle::FSWithRenders(fs_repo) => {
+                        read_fs!(fs_repo);
+                    }
+                    spfs::storage::RepositoryHandle::FSWithoutRenders(fs_repo) => {
+                        read_fs!(fs_repo);
                     }
                     repo => match repo.open_payload(digest).await {
                         Ok((stream, _)) => {

crates/spfs/benches/spfs_bench.rs

@@ -9,6 +9,7 @@ use std::time::Duration;
 
 use criterion::{criterion_group, criterion_main, Criterion, Throughput};
 use spfs::prelude::*;
+use spfs::storage::fs::NoRenderStore;
 
 pub fn commit_benchmark(c: &mut Criterion) {
     const NUM_FILES: usize = 1024;
@@ -44,9 +45,11 @@ pub fn commit_benchmark(c: &mut Criterion) {
         .expect("create a temp directory for spfs repo");
     let repo: Arc<RepositoryHandle> = Arc::new(
         tokio_runtime
-            .block_on(spfs::storage::fs::MaybeOpenFsRepository::create(
-                repo_path.path().join("repo"),
-            ))
+            .block_on(
+                spfs::storage::fs::MaybeOpenFsRepository::<NoRenderStore>::create(
+                    repo_path.path().join("repo"),
+                ),
+            )
             .expect("create spfs repo")
             .into(),
     );

crates/spfs/src/bootstrap_test.rs

@@ -11,6 +11,7 @@ use super::build_shell_initialized_command;
 use crate::fixtures::*;
 use crate::resolve::which;
 use crate::runtime;
+use crate::storage::fs::RenderStore;
 
 #[rstest(
     shell,
@@ -37,7 +38,7 @@ async fn test_shell_initialization_startup_scripts(
     };
     let root = tmpdir.path().to_string_lossy().to_string();
     let repo = crate::storage::RepositoryHandle::from(
-        crate::storage::fs::MaybeOpenFsRepository::create(&root)
+        crate::storage::fs::MaybeOpenFsRepository::<RenderStore>::create(&root)
             .await
             .unwrap(),
     );
@@ -113,7 +114,7 @@ async fn test_shell_initialization_no_startup_scripts(shell: &str, tmpdir: tempf
     };
     let root = tmpdir.path().to_string_lossy().to_string();
     let repo = crate::storage::RepositoryHandle::from(
-        crate::storage::fs::MaybeOpenFsRepository::create(&root)
+        crate::storage::fs::MaybeOpenFsRepository::<RenderStore>::create(&root)
             .await
             .unwrap(),
     );

crates/spfs/src/clean.rs

@@ -570,7 +570,7 @@ where
     /// remove data that is still being used
     async unsafe fn remove_unvisited_renders_and_proxies(&self) -> Result<CleanResult> {
         let mut result = CleanResult::default();
-        let storage::RepositoryHandle::FS(repo) = self.repo else {
+        let storage::RepositoryHandle::FSWithRenders(repo) = self.repo else {
             return Ok(result);
         };
         let repo = repo.opened().await?;
@@ -715,8 +715,10 @@ where
                 let future = async move {
                     if !self.dry_run {
                         tracing::trace!(?path, "removing proxy render");
-                        storage::fs::OpenFsRepository::remove_dir_atomically(&path, &workdir)
-                            .await?;
+                        storage::fs::OpenFsRepository::<storage::fs::RenderStore>::remove_dir_atomically(
+                            &path, &workdir,
+                        )
+                        .await?;
                     }
                     Ok(digest)
                 };

crates/spfs/src/clean_test.rs

@@ -13,6 +13,7 @@ use tokio::time::sleep;
 use super::{Cleaner, TracingCleanReporter};
 use crate::encoding::prelude::*;
 use crate::fixtures::*;
+use crate::storage::fs::RenderStore;
 use crate::{storage, tracking, Error};
 
 #[rstest]
@@ -225,7 +226,7 @@ async fn test_clean_untagged_objects_layers_platforms(#[future] tmprepo: TempRep
 async fn test_clean_manifest_renders(tmpdir: tempfile::TempDir) {
     init_logging();
     let tmprepo = Arc::new(
-        storage::fs::MaybeOpenFsRepository::create(tmpdir.path())
+        storage::fs::MaybeOpenFsRepository::<RenderStore>::create(tmpdir.path())
             .await
             .unwrap()
             .into(),
@@ -249,7 +250,7 @@ async fn test_clean_manifest_renders(tmpdir: tempfile::TempDir) {
         .unwrap();
 
     let fs_repo = match &*tmprepo {
-        RepositoryHandle::FS(fs) => fs,
+        RepositoryHandle::FSWithRenders(fs) => fs,
         _ => panic!("Unexpected tmprepo type!"),
     };
     let fs_repo = fs_repo.opened().await.unwrap();
@@ -269,7 +270,7 @@ async fn test_clean_manifest_renders(tmpdir: tempfile::TempDir) {
         .expect("failed to clean repo");
     println!("{result:#?}");
 
-    let files = list_files(fs_repo.fs_impl.renders.as_ref().unwrap().renders.root());
+    let files = list_files(fs_repo.fs_impl.rs_impl.renders.root());
     assert_eq!(
         files,
         Vec::<String>::new(),

crates/spfs/src/commit_test.rs

@@ -6,20 +6,21 @@ use rstest::rstest;
 
 use super::Committer;
 use crate::fixtures::*;
+use crate::storage::fs::RenderStore;
 use crate::Error;
 
 #[rstest]
 #[tokio::test]
 async fn test_commit_empty(tmpdir: tempfile::TempDir) {
     let root = tmpdir.path().to_string_lossy().to_string();
     let repo = crate::storage::RepositoryHandle::from(
-        crate::storage::fs::MaybeOpenFsRepository::create(&root)
+        crate::storage::fs::MaybeOpenFsRepository::<RenderStore>::create(&root)
             .await
             .unwrap(),
     );
     let storage = crate::runtime::Storage::new(repo).unwrap();
     let repo = crate::storage::RepositoryHandle::from(
-        crate::storage::fs::MaybeOpenFsRepository::create(root)
+        crate::storage::fs::MaybeOpenFsRepository::<RenderStore>::create(root)
             .await
             .unwrap(),
     );